SONARJAVA-5819 SONARJAVA-5818 Fix FPs caused by state reset bug in CipherBlockChainingCheck #5331
Conversation
hashicorp-vault-sonar-prod
bot
changed the title
| public void leaveNode(Tree tree) { | ||
| if (tree == outermostClass) { | ||
| ivFactoryFinder.clear(); | ||
| outermostClass = null; |
There was a problem hiding this comment.
This is the actual change fixing the bug that we observed on Peach
anton-haubner-sonarsource
force-pushed
the
ahbnr/fix-S3329-state-reset
branch
3 times, most recently
from
cd6ae3c to
2e96038
Compare
|
|
||
| addComments(verifier, commentLinesVisitor); | ||
|
|
||
| JavaFileScannerContextForTests testJavaFileScannerContext = visitorsBridge.lastCreatedTestContext(); |
There was a problem hiding this comment.
The main change here is that all file scanner contexts are now considered by the JavaCheckVerifier when dealing with multiple files, instead of just the last one.
Otherwise, issues in all but the last file will not be found by the verifier.
anton-haubner-sonarsource
force-pushed
the
ahbnr/fix-S3329-state-reset
branch
2 times, most recently
from
c35962a to
8a1b82a
Compare
|
There is again an auto-formatting commit: I am isolating the auto-formatting into a separate commit to make it clear which parts of the PR are actually changing behaviour and which are just formatting. That being said, from a previous review, I got the impression that you would prefer to always keep auto-formatting in a separate PR. |
anton-haubner-sonarsource
requested a review
from dorian-burihabwa-sonarsource
There was a problem hiding this comment.
The change looks good overall to me. Let's rework the branch so that we can split the work between 2 tickets: SONARJAVA-5819 to fix the check verifier and SONARJAVA-5818 to fix the check itself
...-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifier.java
Show resolved
Hide resolved
...-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifier.java
Show resolved
Hide resolved
| SonarComponents sonarComponents, InputFile inputFile, JavaVersion javaVersion, boolean inAndroidContext, CacheContext cacheContext | ||
| ) { | ||
| testContext = new JavaFileScannerContextForTests(null, inputFile, null, sonarComponents, javaVersion, false, inAndroidContext, cacheContext); | ||
| SonarComponents sonarComponents, InputFile inputFile, JavaVersion javaVersion, boolean inAndroidContext, CacheContext cacheContext) { |
There was a problem hiding this comment.
| SonarComponents sonarComponents, InputFile inputFile, JavaVersion javaVersion, boolean inAndroidContext, CacheContext cacheContext) { | |
| SonarComponents sonarComponents, InputFile inputFile, JavaVersion javaVersion, boolean inAndroidContext, CacheContext cacheContext | |
| ) { |
…en analyzing multiple files
anton-haubner-sonarsource
force-pushed
the
ahbnr/fix-S3329-state-reset
branch
from
8a1b82a to
96c0b7f
Compare
anton-haubner-sonarsource
changed the title
|
anton-haubner-sonarsource
requested a review
from dorian-burihabwa-sonarsource
2 participants
SONARJAVA-5818
Two nullability issues have been accepted for this PR:
DefaultJavaFileScannerContextand related classes which is way out of scope for this PR.Because of these concerns, I have decided to accept these issues for now.
Let me know if you have objections.
Reviewing commit-by-commit is strongly recommended