-
Notifications
You must be signed in to change notification settings - Fork 716
SONARJAVA-4960 False positive for S1854 #5378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rombirli
wants to merge
7
commits into
master
Choose a base branch
from
rombirli/S1854-false-positive
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+122
−4
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b7c273f
Draft of "False positive for S1854 Solution"
rombirli 947f918
Moved non-compiling java file to non-compiling/checks
rombirli 1d537b2
Fix autoscan
rombirli 1a670e7
Fix broken semantic tests
rombirli c494a6a
Fix QG
rombirli f6837a3
Address Tomasz' review comments, tests refactoring
rombirli eb0e8ef
Rollback incorrect fix in SymbolsTest.java
rombirli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "ruleKey": "S1481", | ||
| "hasTruePositives": true, | ||
| "falseNegatives": 10, | ||
| "falseNegatives": 8, | ||
| "falsePositives": 0 | ||
| } |
105 changes: 105 additions & 0 deletions
105
...ecks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package checks; | ||
|
|
||
|
|
||
| public class UnusedVariablesFPCheck { | ||
| public class DeobfuscatedUpdateManager { | ||
| // @formatter:off | ||
| // uncommenting the following code makes the issue disappear, as the semantic is fully resolved | ||
| // private interface DataContainer { | ||
| // Iterable<ItemElement> getItems(); | ||
| // } | ||
| // | ||
| // private interface ModelObject { | ||
| // void performAction(); | ||
| // } | ||
| // | ||
| // private interface ItemElement { | ||
| // ModelObject getDataModel(); | ||
| // } | ||
| // | ||
| // private static class SystemConfig { | ||
| // static ConfigMode getMode() { | ||
| // return ConfigMode.ENABLED; | ||
| // } | ||
| // } | ||
| // | ||
| // private enum ConfigMode { | ||
| // ENABLED, | ||
| // DISABLED | ||
| // } | ||
| // | ||
| // static class A { | ||
| // interface GenericCallback<T> { } | ||
| // } | ||
| // @formatter:on | ||
|
|
||
| void processUpdates( | ||
| DataContainer container | ||
| // REMARK : the issue arises from the A.GenericCallback<ModelObject> callback that is not even used (indirect type resolution problem) | ||
| , A.GenericCallback<X> callback | ||
| ) { | ||
| for (ItemElement element : container.getItems()) { | ||
| if (SystemConfig.getMode() == ConfigMode.ENABLED) { | ||
| ModelObject dataModel = element.getDataModel(); // Compliant - false positive was raised here, dataModel is used in the next line | ||
| dataModel.performAction(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| static class StringConcatenation { | ||
| // @formatter:off | ||
| // uncommenting the following code makes the issue disappear, as the semantic is fully resolved | ||
| // private class AClass { | ||
| // private class BClass<T> { | ||
| // public T b; | ||
| // } | ||
| // } | ||
| // @formatter:on | ||
|
|
||
| public String doSomething(AClass.BClass<String> instance) { | ||
| String c = "Hi"; // Compliant - false positive was raised here, c is used in the next line | ||
| return instance.b + c; | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| A user reported a FP on enhanced switch statements like the one below. | ||
| However I was not able to reproduce it in a minimal example. | ||
| https://community.sonarsource.com/t/false-positive-for-s1854-unused-assignments-should-be-removed/114110/12 | ||
|
|
||
| static class EnhancedSwitch { | ||
| // private enum DocumentStatus { | ||
| // DOC01, | ||
| // DOC02 | ||
| // } | ||
| // | ||
| // private interface Document { | ||
| // void setStatus(DocumentStatus status); | ||
| // } | ||
| // | ||
| // private interface Event { | ||
| // } | ||
| // | ||
| // private class SimpleStatusChangedEvent implements Event { | ||
| // } | ||
| // | ||
| // private class NeedClientRecheckEvent implements Event { | ||
| // } | ||
| // private interface DocumentRepository { | ||
| // void save(Document document); | ||
| // } | ||
|
|
||
| void ko(Event event, Document document, DocumentRepository documentRepository) { | ||
| final DocumentStatus status = switch (event) { | ||
| case SimpleStatusChangedEvent ignored -> DocumentStatus.DOC01; | ||
| case NeedClientRecheckEvent ignored -> DocumentStatus.DOC02; | ||
| }; | ||
| document.setStatus(status); | ||
| // ... | ||
| documentRepository.save(document); | ||
| } | ||
|
|
||
| }*/ | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.