-
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
base: master
Are you sure you want to change the base?
Changes from all commits
fb4e522
3da6e27
b78deef
caaea40
36cc643
ac1179b
b23c2de
d78ed6b
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 |
|---|---|---|
|
|
@@ -99,7 +99,7 @@ public final boolean isTypeSymbol() { | |
| } | ||
|
|
||
| @Override | ||
| public final boolean isMethodSymbol() { | ||
| public boolean isMethodSymbol() { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -340,6 +340,11 @@ public boolean isVarArgsMethod() { | |
| public boolean isNativeMethod() { | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isMethodSymbol() { | ||
| return true; | ||
|
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 flipped it to
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. Adding a test, however flipping it back to false makes |
||
| } | ||
| } | ||
|
|
||
| public static final class UnknownType implements Type { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.