From b7c273f800aa6b56871d7a494471cf1c16ff9fa5 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Wed, 17 Dec 2025 10:26:29 +0100 Subject: [PATCH 1/7] Draft of "False positive for S1854 Solution" - https://community.sonarsource.com/t/false-positive-for-s1854-unused-assignments-should-be-removed/114110/10 --- .../java/checks/UnusedVariablesFPCheck.java | 119 ++++++++++++++++++ .../sonar/java/checks/DeadStoreCheckTest.java | 9 ++ .../java/org/sonar/java/model/Symbols.java | 5 +- 3 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/UnusedVariablesFPCheck.java diff --git a/java-checks-test-sources/default/src/main/java/checks/UnusedVariablesFPCheck.java b/java-checks-test-sources/default/src/main/java/checks/UnusedVariablesFPCheck.java new file mode 100644 index 00000000000..f7564e5fffc --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/UnusedVariablesFPCheck.java @@ -0,0 +1,119 @@ +package com; + + +public class UnusedVariablesFPCheck { + public class DeobfuscatedUpdateManager { +// private interface DataContainer { +// Iterable 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 { } +// } + + /** + * Deobfuscated names with AI from : + * ```java + * package com; + *

+ * public class BCid51 { + * void HJid232(EJid229 YHid199, Yid24.RIid217 QJid241) { + * for (XFid148 RJid242 : YHid199.WIid222()) { + * int DHid178; + * if (Fid5.BGid151() == CGid152.DGid153) { + * MBid37 IHid183 = RJid242.OFid139(); + * IHid183.AGid150(); + * } + * } + * } + * } + * ``` + */ + void processUpdates( + DataContainer container + // REMARK : the issue arises from the A.GenericCallback callback that is not even used (indirect type resolution problem) + , A.GenericCallback callback + ) { + for (ItemElement element : container.getItems()) { + if (SystemConfig.getMode() == ConfigMode.ENABLED) { + ModelObject dataModel = element.getDataModel(); + dataModel.performAction(); + } + } + } + + } + + static class StringConcatenation { +// private class AClass { +// private class BClass { +// public T b; +// } +// } + + public String doSomething(AClass.BClass instance) { + String c = "Hi"; // Rule S1854 + return instance.b + c; + } + } + + 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); + } + + } + + class Obvious { +// void obvious() { +// int i = 0; // doesn't raise issue +// i = 1; // raises issue +// } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java index 395d5e9635e..38277516890 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java @@ -51,6 +51,15 @@ void test_non_compiling() { .verifyIssues(); } + @Test + void test_fp() { + CheckVerifier.newVerifier() + .onFile(TestUtils.mainCodeSourcesPath("checks/UnusedVariablesFPCheck.java")) + .withJavaVersion(14) + .withCheck(new DeadStoreCheck()) + .verifyNoIssues(); + } + private static class EraseSymbols extends BaseTreeVisitor { @Override diff --git a/java-frontend/src/main/java/org/sonar/java/model/Symbols.java b/java-frontend/src/main/java/org/sonar/java/model/Symbols.java index 901db702bee..311fc943584 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/Symbols.java +++ b/java-frontend/src/main/java/org/sonar/java/model/Symbols.java @@ -99,7 +99,7 @@ public final boolean isTypeSymbol() { } @Override - public final boolean isMethodSymbol() { + public boolean isMethodSymbol() { return false; } @@ -340,6 +340,9 @@ public boolean isVarArgsMethod() { public boolean isNativeMethod() { return false; } + + @Override + public boolean isMethodSymbol() { return true; } } public static final class UnknownType implements Type { From 947f91867d0980ce5172a0329e7052a79be86f86 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 18 Dec 2025 09:39:05 +0100 Subject: [PATCH 2/7] Moved non-compiling java file to non-compiling/checks --- .../non-compiling}/checks/UnusedVariablesFPCheck.java | 2 +- .../src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename java-checks-test-sources/default/src/main/{java => files/non-compiling}/checks/UnusedVariablesFPCheck.java (99%) diff --git a/java-checks-test-sources/default/src/main/java/checks/UnusedVariablesFPCheck.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java similarity index 99% rename from java-checks-test-sources/default/src/main/java/checks/UnusedVariablesFPCheck.java rename to java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java index f7564e5fffc..5fbca87214d 100644 --- a/java-checks-test-sources/default/src/main/java/checks/UnusedVariablesFPCheck.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java @@ -1,4 +1,4 @@ -package com; +package checks; public class UnusedVariablesFPCheck { diff --git a/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java index 38277516890..fbc8b2b50b2 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java @@ -54,7 +54,7 @@ void test_non_compiling() { @Test void test_fp() { CheckVerifier.newVerifier() - .onFile(TestUtils.mainCodeSourcesPath("checks/UnusedVariablesFPCheck.java")) + .onFile(TestUtils.nonCompilingTestSourcesPath("checks/UnusedVariablesFPCheck.java")) .withJavaVersion(14) .withCheck(new DeadStoreCheck()) .verifyNoIssues(); From 1d537b2ed6960a1d0eb13fef47c286d770e7949f Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 18 Dec 2025 15:29:52 +0100 Subject: [PATCH 3/7] Fix autoscan --- its/autoscan/src/test/resources/autoscan/diffs/diff_S1481.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1481.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1481.json index 9bd4666fbb3..0a9e1c62b2b 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1481.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1481.json @@ -1,6 +1,6 @@ { "ruleKey": "S1481", "hasTruePositives": true, - "falseNegatives": 10, + "falseNegatives": 8, "falsePositives": 0 } From 1a670e74ae2acd4c3a2a67ff31e90c9de46ab146 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Mon, 5 Jan 2026 09:45:24 +0100 Subject: [PATCH 4/7] Fix broken semantic tests --- .../src/test/java/org/sonar/java/model/JParserSemanticTest.java | 2 +- .../src/test/java/org/sonar/java/model/SymbolsTest.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java b/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java index bd7b0a17661..6dd0136518f 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java @@ -1285,7 +1285,7 @@ private java.util.Collection> samples() { assertThat(recovered.isTypeSymbol()).isFalse(); assertThat(recovered.isVariableSymbol()).isFalse(); - assertThat(recovered.isMethodSymbol()).isFalse(); + assertThat(recovered.isMethodSymbol()).isTrue(); assertThat(recovered.isPackageSymbol()).isFalse(); assertThat(recovered.isAbstract()).isFalse(); diff --git a/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java b/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java index 1cb3732ed20..bccfc56a320 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java @@ -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(); assertThat(unknownSymbol.isStatic()).isFalse(); assertThat(unknownSymbol.isFinal()).isFalse(); From c494a6abc764ac4143932ad6718013d8c25eb583 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Mon, 5 Jan 2026 10:51:07 +0100 Subject: [PATCH 5/7] Fix QG --- .../src/main/java/org/sonar/java/model/Symbols.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/java-frontend/src/main/java/org/sonar/java/model/Symbols.java b/java-frontend/src/main/java/org/sonar/java/model/Symbols.java index 311fc943584..15050dfb22f 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/Symbols.java +++ b/java-frontend/src/main/java/org/sonar/java/model/Symbols.java @@ -342,7 +342,9 @@ public boolean isNativeMethod() { } @Override - public boolean isMethodSymbol() { return true; } + public boolean isMethodSymbol() { + return true; + } } public static final class UnknownType implements Type { @@ -470,7 +472,7 @@ public boolean isIntersectionType() { @Override public Type[] getIntersectionTypes() { - return new Type[] { this }; + return new Type[]{this}; } } } From f6837a3d0162c44b277d247501494cf9f069691f Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 8 Jan 2026 08:23:25 +0100 Subject: [PATCH 6/7] Address Tomasz' review comments, tests refactoring --- .../checks/UnusedVariablesFPCheck.java | 42 +++++++------------ .../sonar/java/checks/DeadStoreCheckTest.java | 2 +- .../java/org/sonar/java/model/Symbols.java | 2 +- .../org/sonar/java/model/SymbolsTest.java | 1 + 4 files changed, 17 insertions(+), 30 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java index 5fbca87214d..006e39ff228 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedVariablesFPCheck.java @@ -3,6 +3,8 @@ 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 getItems(); // } @@ -29,25 +31,8 @@ public class DeobfuscatedUpdateManager { // static class A { // interface GenericCallback { } // } + // @formatter:on - /** - * Deobfuscated names with AI from : - * ```java - * package com; - *

- * public class BCid51 { - * void HJid232(EJid229 YHid199, Yid24.RIid217 QJid241) { - * for (XFid148 RJid242 : YHid199.WIid222()) { - * int DHid178; - * if (Fid5.BGid151() == CGid152.DGid153) { - * MBid37 IHid183 = RJid242.OFid139(); - * IHid183.AGid150(); - * } - * } - * } - * } - * ``` - */ void processUpdates( DataContainer container // REMARK : the issue arises from the A.GenericCallback callback that is not even used (indirect type resolution problem) @@ -55,7 +40,7 @@ void processUpdates( ) { for (ItemElement element : container.getItems()) { if (SystemConfig.getMode() == ConfigMode.ENABLED) { - ModelObject dataModel = element.getDataModel(); + ModelObject dataModel = element.getDataModel(); // Compliant - false positive was raised here, dataModel is used in the next line dataModel.performAction(); } } @@ -64,18 +49,26 @@ void processUpdates( } 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 { // public T b; // } // } + // @formatter:on public String doSomething(AClass.BClass instance) { - String c = "Hi"; // Rule S1854 + 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, @@ -108,12 +101,5 @@ void ko(Event event, Document document, DocumentRepository documentRepository) { documentRepository.save(document); } - } - - class Obvious { -// void obvious() { -// int i = 0; // doesn't raise issue -// i = 1; // raises issue -// } - } + }*/ } diff --git a/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java index fbc8b2b50b2..3255bb089c2 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java @@ -52,7 +52,7 @@ void test_non_compiling() { } @Test - void test_fp() { + void test_incomplete_semantic() { CheckVerifier.newVerifier() .onFile(TestUtils.nonCompilingTestSourcesPath("checks/UnusedVariablesFPCheck.java")) .withJavaVersion(14) diff --git a/java-frontend/src/main/java/org/sonar/java/model/Symbols.java b/java-frontend/src/main/java/org/sonar/java/model/Symbols.java index 15050dfb22f..19b5ef3d6bf 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/Symbols.java +++ b/java-frontend/src/main/java/org/sonar/java/model/Symbols.java @@ -472,7 +472,7 @@ public boolean isIntersectionType() { @Override public Type[] getIntersectionTypes() { - return new Type[]{this}; + return new Type[] { this }; } } } diff --git a/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java b/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java index bccfc56a320..0184834cccc 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java @@ -171,6 +171,7 @@ private static void assertCommonProperties(Symbol unknownSymbol) { assertThat(unknownSymbol.isPackageSymbol()).isFalse(); assertThat(unknownSymbol.isTypeSymbol()).isFalse(); assertThat(unknownSymbol.isVariableSymbol()).isFalse(); + assertThat(unknownSymbol.isMethodSymbol()).isTrue(); assertThat(unknownSymbol.isStatic()).isFalse(); assertThat(unknownSymbol.isFinal()).isFalse(); From eb0e8ef134ea89f977bb6d81cfd365866bfa0536 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 8 Jan 2026 11:10:05 +0100 Subject: [PATCH 7/7] Rollback incorrect fix in SymbolsTest.java --- .../src/test/java/org/sonar/java/model/SymbolsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java b/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java index 0184834cccc..bccfc56a320 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/SymbolsTest.java @@ -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()).isTrue(); assertThat(unknownSymbol.isStatic()).isFalse(); assertThat(unknownSymbol.isFinal()).isFalse();