-
Notifications
You must be signed in to change notification settings - Fork 714
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
base: master
Are you sure you want to change the base?
Changes from all commits
b7c273f
947f918
1d537b2
1a670e7
c494a6a
f6837a3
eb0e8ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "ruleKey": "S1481", | ||
| "hasTruePositives": true, | ||
| "falseNegatives": 10, | ||
| "falseNegatives": 8, | ||
| "falsePositives": 0 | ||
| } |
| 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); | ||
| } | ||
|
|
||
| }*/ | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,7 +171,6 @@ private static void assertCommonProperties(Symbol unknownSymbol) { | |
| assertThat(unknownSymbol.isPackageSymbol()).isFalse(); | ||
| assertThat(unknownSymbol.isTypeSymbol()).isFalse(); | ||
| assertThat(unknownSymbol.isVariableSymbol()).isFalse(); | ||
| assertThat(unknownSymbol.isMethodSymbol()).isFalse(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't read this carefully, but I'm wondering why this is removed instead of reverting the condition.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added it back
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Edit : |
||
|
|
||
| assertThat(unknownSymbol.isStatic()).isFalse(); | ||
| assertThat(unknownSymbol.isFinal()).isFalse(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.