From 436ee532600165485e2c59f68e73b7f4c79c3fd4 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 13:31:53 +0100 Subject: [PATCH 01/15] Add reproducer test/sample --- .../UnusedScopedValueWhereResultSample.java | 9 +++++ .../UnusedScopedValueWhereResultCheck.java | 35 +++++++++++++++++++ ...UnusedScopedValueWhereResultCheckTest.java | 34 ++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java new file mode 100644 index 0000000000..c06c0b0863 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java @@ -0,0 +1,9 @@ +static final ScopedValue myScopedValue = ScopedValue.newInstance(); + +void main() { + ScopedValue.where(myScopedValue, "hello"); // Noncompliant + var myUnusedCarrier = ScopedValue.where(myScopedValue, "hello"); // Noncompliant + var myUsedCarrier = ScopedValue.where(myScopedValue, "hello"); // Compliant, the result is assigned to a variable and used + myUsedCarrier.run(() -> { + }); +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java new file mode 100644 index 0000000000..2b2e34ecaf --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -0,0 +1,35 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2025 SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S8432") +public class UnusedScopedValueWhereResultCheck extends IssuableSubscriptionVisitor { + @Override + public List nodesToVisit() { + return List.of(); + } + + @Override + public void visitNode(Tree tree) { + // Implementation of the check goes here + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java new file mode 100644 index 0000000000..cc2482e632 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java @@ -0,0 +1,34 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2025 SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; + +class UnusedScopedValueWhereResultCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/UnusedScopedValueWhereResultSample.java")) + .withCheck(new UnusedScopedValueWhereResultCheck()) + .verifyIssues(); + } + +} From fd73fa361fbfc0e80297417c3b766041b5b23f73 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 14:56:29 +0100 Subject: [PATCH 02/15] small refactoring --- .../UnusedScopedValueWhereResultSample.java | 3 ++ .../UnusedScopedValueWhereResultCheck.java | 30 +++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java index c06c0b0863..419738c4f5 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java @@ -1,8 +1,11 @@ static final ScopedValue myScopedValue = ScopedValue.newInstance(); void main() { + ScopedValue.where(myScopedValue, "hello").run(() -> { + }); // Compliant, the result is used immediately ScopedValue.where(myScopedValue, "hello"); // Noncompliant var myUnusedCarrier = ScopedValue.where(myScopedValue, "hello"); // Noncompliant + var myUnused2LevelCarrier = ScopedValue.where(myScopedValue, "hello").where(ScopedValue.newInstance(), "hello"); // Noncompliant var myUsedCarrier = ScopedValue.where(myScopedValue, "hello"); // Compliant, the result is assigned to a variable and used myUsedCarrier.run(() -> { }); diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index 2b2e34ecaf..b9d0116cdd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -17,19 +17,45 @@ package org.sonar.java.checks; import java.util.List; +import java.util.Set; import org.sonar.check.Rule; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.VariableTree; @Rule(key = "S8432") public class UnusedScopedValueWhereResultCheck extends IssuableSubscriptionVisitor { + private final List immediateUsageMethods = List.of("run", "call"); + @Override public List nodesToVisit() { - return List.of(); + return List.of(Tree.Kind.METHOD_INVOCATION); } @Override public void visitNode(Tree tree) { - // Implementation of the check goes here + if (tree instanceof MethodInvocationTree methodInvocationTree) { + visitMethodInvocationTree(methodInvocationTree); + } + } + + + private void visitMethodInvocationTree(MethodInvocationTree tree) { + if (tree.methodSelect() instanceof MemberSelectExpressionTree memberSelectExpressionTree && + "where".equals(memberSelectExpressionTree.identifier().name())) { + boolean usedImmediately = tree.parent() instanceof MemberSelectExpressionTree parentMemberSelect && + immediateUsageMethods.contains(parentMemberSelect.identifier().name()); + if (usedImmediately) { + return; + } + if (tree.parent() instanceof VariableTree variableTree) { + return; + } + reportIssue(tree, "Hello"); + } } } + + From 71a0a8d8aee48cf224d68d7d0f76ef4be3ff2732 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 15:23:58 +0100 Subject: [PATCH 03/15] Full initial implementation - variable usage, escaping, unused career argument --- .../UnusedScopedValueWhereResultSample.java | 23 +++ .../UnusedScopedValueWhereResultCheck.java | 182 ++++++++++++++++-- 2 files changed, 192 insertions(+), 13 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java index 419738c4f5..d26e85166c 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java @@ -1,6 +1,7 @@ static final ScopedValue myScopedValue = ScopedValue.newInstance(); void main() { + var unusedCarrierGet = ScopedValue.where(myScopedValue, "hello").get(myScopedValue); // Noncompliant ScopedValue.where(myScopedValue, "hello").run(() -> { }); // Compliant, the result is used immediately ScopedValue.where(myScopedValue, "hello"); // Noncompliant @@ -10,3 +11,25 @@ void main() { myUsedCarrier.run(() -> { }); } + +void escapedCarrierFunctionCall() { + var carrier = ScopedValue.where(myScopedValue, "hello"); // ccompliant - the result escapes + usedCarrierArgument(carrier); +} + +void usedCarrierArgument(ScopedValue.Carrier carrier) { // compliant - the carrier is used in the function + carrier.run(() -> { + }); +} + +void unusedCarrierArgument(ScopedValue.Carrier carrier) { // Noncompliant +} + +ScopedValue.Carrier escapedCarrierReturn() { + var carrier = ScopedValue.where(myScopedValue, "hello"); // compliant - the result escapes + return carrier; +} + +void escapedCarrierArgument(ScopedValue.Carrier carrier) { // compliant - the carrier is used in the function + usedCarrierArgument(carrier); +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index b9d0116cdd..e46ba89a12 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -20,41 +20,197 @@ import java.util.Set; import org.sonar.check.Rule; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.Arguments; +import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.ReturnStatementTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.VariableTree; @Rule(key = "S8432") public class UnusedScopedValueWhereResultCheck extends IssuableSubscriptionVisitor { - private final List immediateUsageMethods = List.of("run", "call"); + + private static final String WHERE_METHOD_NAME = "where"; + + private static final String MESSAGE = "Use this ScopedValue.Carrier by calling run() or call(), or remove this useless call to where()."; + + private static final Set CONSUMPTION_METHODS = Set.of("run", "call"); + + private static final MethodMatchers WHERE_MATCHER = MethodMatchers.or( + MethodMatchers.create() + .ofTypes("java.lang.ScopedValue") + .names(WHERE_METHOD_NAME) + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("java.lang.ScopedValue$Carrier") + .names(WHERE_METHOD_NAME) + .withAnyParameters() + .build()); @Override public List nodesToVisit() { - return List.of(Tree.Kind.METHOD_INVOCATION); + return List.of(Tree.Kind.METHOD_INVOCATION, Tree.Kind.VARIABLE); } @Override public void visitNode(Tree tree) { if (tree instanceof MethodInvocationTree methodInvocationTree) { - visitMethodInvocationTree(methodInvocationTree); + checkMethodInvocation(methodInvocationTree); + } else if (tree instanceof VariableTree variableTree) { + checkCarrierVariable(variableTree); + } + } + + private void checkMethodInvocation(MethodInvocationTree mit) { + if (!WHERE_MATCHER.matches(mit)) { + return; + } + + Tree parent = mit.parent(); + + // Special case: if there is no parent, consider it consumed to avoid false positives + if (parent == null) { + return; + } + + // Check if result is immediately consumed (chained .run() or .call()) + if (isImmediatelyConsumed(parent)) { + return; + } + + // Check if result is chained with another .where() - don't report, the final result will be checked + if (isChainedWhere(parent)) { + return; + } + + // Check if result is assigned to a variable - the variable check will handle this + if (parent instanceof VariableTree) { + return; + } + + // Check if result escapes (returned or passed as argument) + if (isEscaping(mit)) { + return; } + + // If we reach here, the result is discarded + reportIssue(mit, MESSAGE); } + private void checkCarrierVariable(VariableTree variableTree) { + Symbol symbol = variableTree.symbol(); + if (!symbol.isVariableSymbol()) { + return; + } + + // Check if this variable is initialized with a where() call + var initializer = variableTree.initializer(); + boolean isWhereResult = initializer instanceof MethodInvocationTree mit && WHERE_MATCHER.matches(mit); + + // Check if this variable is a Carrier type parameter + boolean isCarrierParameter = symbol.isParameter() && isCarrierType(symbol); + + if (!isWhereResult && !isCarrierParameter) { + return; + } + + // Check all usages of the variable + Symbol.VariableSymbol variableSymbol = (Symbol.VariableSymbol) symbol; + boolean isProperlyUsed = variableSymbol.usages().stream().anyMatch(this::isUsageValid); + + if (!isProperlyUsed) { + reportIssue(variableTree.simpleName(), MESSAGE); + } + } + + private static boolean isCarrierType(Symbol symbol) { + return symbol.type().is("java.lang.ScopedValue$Carrier"); + } - private void visitMethodInvocationTree(MethodInvocationTree tree) { - if (tree.methodSelect() instanceof MemberSelectExpressionTree memberSelectExpressionTree && - "where".equals(memberSelectExpressionTree.identifier().name())) { - boolean usedImmediately = tree.parent() instanceof MemberSelectExpressionTree parentMemberSelect && - immediateUsageMethods.contains(parentMemberSelect.identifier().name()); - if (usedImmediately) { - return; + private boolean isUsageValid(IdentifierTree usage) { + Tree parent = usage.parent(); + + // Check if usage is consumed via .run() or .call() + if (parent instanceof MemberSelectExpressionTree memberSelect && memberSelect.expression() == usage) { + String methodName = memberSelect.identifier().name(); + if (CONSUMPTION_METHODS.contains(methodName)) { + return true; } - if (tree.parent() instanceof VariableTree variableTree) { - return; + // Check if it's a chained .where() that eventually gets consumed + if (WHERE_METHOD_NAME.equals(methodName)) { + Tree grandParent = memberSelect.parent(); + if (grandParent instanceof MethodInvocationTree chainedMit) { + return isChainEventuallyConsumed(chainedMit); + } } - reportIssue(tree, "Hello"); } + + // Check if usage escapes (returned or passed as argument) + return isEscaping(usage); + } + + private static boolean isImmediatelyConsumed(Tree parent) { + if (parent instanceof MemberSelectExpressionTree memberSelect) { + return CONSUMPTION_METHODS.contains(memberSelect.identifier().name()); + } + return false; + } + + private static boolean isChainedWhere(Tree parent) { + if (parent instanceof MemberSelectExpressionTree memberSelect) { + return WHERE_METHOD_NAME.equals(memberSelect.identifier().name()); + } + return false; + } + + private boolean isChainEventuallyConsumed(MethodInvocationTree mit) { + Tree parent = mit.parent(); + + // Special case: if there is no parent, consider it consumed to avoid false positives + if (parent == null) { + return true; + } + + if (isImmediatelyConsumed(parent)) { + return true; + } + + if (isChainedWhere(parent) && parent.parent() instanceof MethodInvocationTree nextMit) { + return isChainEventuallyConsumed(nextMit); + } + + if (isEscaping(mit)) { + return true; + } + + if (parent instanceof VariableTree varTree) { + Symbol symbol = varTree.symbol(); + if (symbol.isVariableSymbol()) { + return symbol.usages().stream().anyMatch(this::isUsageValid); + } + } + + return false; + } + + private static boolean isEscaping(Tree tree) { + Tree parent = tree.parent(); + + // Returned from method + if (parent instanceof ReturnStatementTree) { + return true; + } + + // Passed as argument to another method + if (parent instanceof Arguments) { + return true; + } + + return false; } } From d4e600ef8e8720a50401a3eb266112d5765c1cb8 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 15:28:44 +0100 Subject: [PATCH 04/15] Add edge cases "multi-level carrier used" to test sample --- .../checks/UnusedScopedValueWhereResultSample.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java index d26e85166c..914c4af5c5 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java @@ -1,15 +1,20 @@ static final ScopedValue myScopedValue = ScopedValue.newInstance(); void main() { - var unusedCarrierGet = ScopedValue.where(myScopedValue, "hello").get(myScopedValue); // Noncompliant + ScopedValue.where(myScopedValue, "hello").get(myScopedValue); // Noncompliant ScopedValue.where(myScopedValue, "hello").run(() -> { }); // Compliant, the result is used immediately + ScopedValue.where(myScopedValue, "hello").where(ScopedValue.newInstance(), "hello").run(() -> { + }); // Compliant, the result is used immediately ScopedValue.where(myScopedValue, "hello"); // Noncompliant var myUnusedCarrier = ScopedValue.where(myScopedValue, "hello"); // Noncompliant var myUnused2LevelCarrier = ScopedValue.where(myScopedValue, "hello").where(ScopedValue.newInstance(), "hello"); // Noncompliant var myUsedCarrier = ScopedValue.where(myScopedValue, "hello"); // Compliant, the result is assigned to a variable and used + var myUsed2LevelCarrier = ScopedValue.where(myScopedValue, "hello").where(ScopedValue.newInstance(), "hello"); // Compliant, the result is assigned to a variable and used myUsedCarrier.run(() -> { }); + myUsed2LevelCarrier.run(() -> { + }); } void escapedCarrierFunctionCall() { From cd5eba753633bd4b69f113cd77aec163a18d9f87 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 16:16:09 +0100 Subject: [PATCH 05/15] Add support for many edge cases : escaping in variable, control flow. lambdas, variable reassignment, multiple var same carrier... --- ...ScopedValueWhereResultSampleEdgeCases.java | 194 ++++++++++++++++++ .../UnusedScopedValueWhereResultCheck.java | 67 +++++- ...UnusedScopedValueWhereResultCheckTest.java | 8 + 3 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java new file mode 100644 index 0000000000..c7d4aa865a --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java @@ -0,0 +1,194 @@ +import static java.lang.ScopedValue.where; + +class UnusedScopedValueWhereResultSampleEdgeCases { + + static final ScopedValue SCOPED = ScopedValue.newInstance(); + static final ScopedValue SCOPED2 = ScopedValue.newInstance(); + + // Field storage - should be flagged as we can't track field usage reliably + ScopedValue.Carrier carrierField; + static ScopedValue.Carrier staticCarrierField; + + + + void chainedWhereBeforeConsumption() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .run() + carrier.where(SCOPED2, "world").run(() -> {}); + } + + void chainedWhereBeforeCallConsumption() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .call() + carrier.where(SCOPED2, "world").call(() -> "result"); + } + + // ===== CONTROL FLOW ===== + + void conditionalUsage(boolean condition) { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used conditionally + if (condition) { + carrier.run(() -> {}); + } + } + + void conditionalUsageNotAllPaths(boolean condition) { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in one branch (we don't do path-sensitive analysis) + if (condition) { + carrier.run(() -> {}); + } + // else: carrier not used - but we can't detect this without CFG analysis + } + + void carrierInTryFinally() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in finally + try { + doSomething(); + } finally { + carrier.run(() -> {}); + } + } + + void carrierInLoop() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in loop + for (int i = 0; i < 3; i++) { + carrier.run(() -> {}); + } + } + + // ===== VARIABLE SCENARIOS ===== + + void variableReassignedBeforeUse() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier = ScopedValue.where(SCOPED, "world"); + carrier.run(() -> {}); + } + + void multipleVariablesSameCarrier() { + var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - used via carrier2 reference... actually no, different objects + var carrier2 = carrier1; // Carrier2 points to same object + carrier2.run(() -> {}); + } + + void multipleVariablesOneUnused() { + var usedCarrier = ScopedValue.where(SCOPED, "hello"); // Compliant + var unusedCarrier = ScopedValue.where(SCOPED, "world"); // Noncompliant + usedCarrier.run(() -> {}); + } + + void carrierStoredInField() { + carrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping + } + + void carrierStoredInStaticField() { + staticCarrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping + } + + // ===== RETURN VARIATIONS ===== + + ScopedValue.Carrier directReturnWithoutVariable() { + return ScopedValue.where(SCOPED, "hello"); // Compliant - returned directly + } + + ScopedValue.Carrier ternaryReturn(boolean condition) { + var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - potentially returned + var carrier2 = ScopedValue.where(SCOPED, "world"); // Compliant - potentially returned + return condition ? carrier1 : carrier2; + } + + // ===== NON-CONSUMING METHOD CALLS ===== + + void toStringOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.toString(); + } + + void hashCodeOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.hashCode(); + } + + void equalsOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.equals(null); + } + + void getOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.get(SCOPED); + } + + // ===== LAMBDA/CLOSURE ===== + + void carrierUsedInsideLambda() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used inside lambda + Runnable r = () -> { + carrier.run(() -> {}); + }; + r.run(); + } + + void carrierPassedToLambdaConsumer() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes to lambda consumer + consumeCarrier(c -> c.run(() -> {}), carrier); + } + + // ===== NESTED ESCAPING ===== + + void carrierPassedToMethodThatReturnsIt() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call + var returned = identity(carrier); + returned.run(() -> {}); + } + + void carrierPassedToMethodThatReturnsItButUnused() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call (we can't track further) + var returned = identity(carrier); + // returned is not used - but carrier escaped so we don't flag + } + + // ===== STATIC IMPORT ===== + + void staticImportWhere() { + where(SCOPED, "hello"); // Noncompliant + } + + void staticImportWhereUsed() { + where(SCOPED, "hello").run(() -> {}); // Compliant - used immediately + } + + void staticImportWhereVariable() { + var carrier = where(SCOPED, "hello"); // Noncompliant + } + + void staticImportWhereVariableUsed() { + var carrier = where(SCOPED, "hello"); // Compliant - used + carrier.run(() -> {}); + } + + // ===== CONSTRUCTOR ARGUMENT ===== + + void carrierAsConstructorArgument() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via constructor + new CarrierHolder(carrier); + } + + void carrierAsConstructorArgumentDirect() { + new CarrierHolder(ScopedValue.where(SCOPED, "hello")); // Compliant - escapes via constructor + } + + // ===== HELPER METHODS AND CLASSES ===== + + void doSomething() {} + + ScopedValue.Carrier identity(ScopedValue.Carrier carrier) { + return carrier; + } + + void consumeCarrier(java.util.function.Consumer consumer, ScopedValue.Carrier carrier) { + consumer.accept(carrier); + } + + static class CarrierHolder { + CarrierHolder(ScopedValue.Carrier carrier) // Noncompliant + {} + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index e46ba89a12..dff6336b42 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -23,9 +23,12 @@ import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.tree.Arguments; +import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; +import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.NewClassTree; import org.sonar.plugins.java.api.tree.ReturnStatementTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.VariableTree; @@ -92,6 +95,11 @@ private void checkMethodInvocation(MethodInvocationTree mit) { return; } + // Check if result is assigned to a field (escapes) + if (parent instanceof AssignmentExpressionTree) { + return; + } + // Check if result escapes (returned or passed as argument) if (isEscaping(mit)) { return; @@ -120,13 +128,44 @@ private void checkCarrierVariable(VariableTree variableTree) { // Check all usages of the variable Symbol.VariableSymbol variableSymbol = (Symbol.VariableSymbol) symbol; - boolean isProperlyUsed = variableSymbol.usages().stream().anyMatch(this::isUsageValid); + List usages = variableSymbol.usages(); + + // Check if variable is reassigned before proper use + if (isWhereResult && isReassignedBeforeProperUse(usages)) { + reportIssue(variableTree.simpleName(), MESSAGE); + return; + } + + boolean isProperlyUsed = usages.stream().anyMatch(this::isUsageValid); if (!isProperlyUsed) { reportIssue(variableTree.simpleName(), MESSAGE); } } + private boolean isReassignedBeforeProperUse(List usages) { + for (IdentifierTree usage : usages) { + // Check if this usage is a reassignment (left side of assignment) + Tree parent = usage.parent(); + if (parent instanceof AssignmentExpressionTree assignment && assignment.variable() == usage) { + // This is a reassignment - check if there's any proper use before this reassignment + // For simplicity, if we see a reassignment and no proper use, consider it unused + boolean hasProperUseBefore = usages.stream() + .filter(u -> u != usage) + .filter(u -> isBefore(u, usage)) + .anyMatch(this::isUsageValid); + if (!hasProperUseBefore) { + return true; + } + } + } + return false; + } + + private static boolean isBefore(IdentifierTree first, IdentifierTree second) { + return first.identifierToken().range().start().isBefore(second.identifierToken().range().start()); + } + private static boolean isCarrierType(Symbol symbol) { return symbol.type().is("java.lang.ScopedValue$Carrier"); } @@ -134,6 +173,11 @@ private static boolean isCarrierType(Symbol symbol) { private boolean isUsageValid(IdentifierTree usage) { Tree parent = usage.parent(); + // Check if usage is a reassignment (left side of assignment) - not a valid use + if (parent instanceof AssignmentExpressionTree assignment && assignment.variable() == usage) { + return false; + } + // Check if usage is consumed via .run() or .call() if (parent instanceof MemberSelectExpressionTree memberSelect && memberSelect.expression() == usage) { String methodName = memberSelect.identifier().name(); @@ -149,6 +193,11 @@ private boolean isUsageValid(IdentifierTree usage) { } } + // Check if usage is assigned to another variable (aliasing) - counts as valid + if (parent instanceof VariableTree) { + return true; + } + // Check if usage escapes (returned or passed as argument) return isEscaping(usage); } @@ -200,16 +249,30 @@ private boolean isChainEventuallyConsumed(MethodInvocationTree mit) { private static boolean isEscaping(Tree tree) { Tree parent = tree.parent(); + if (parent == null) { + return false; + } + // Returned from method if (parent instanceof ReturnStatementTree) { return true; } - // Passed as argument to another method + // Passed as argument to another method or constructor if (parent instanceof Arguments) { return true; } + // Used in ternary expression - check if the ternary escapes + if (parent instanceof ConditionalExpressionTree) { + return isEscaping(parent); + } + + // Passed to constructor + if (parent instanceof NewClassTree newClass) { + return newClass.arguments().contains(tree); + } + return false; } } diff --git a/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java index cc2482e632..896c686cc9 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java @@ -31,4 +31,12 @@ void test() { .verifyIssues(); } + @Test + void testEdgeCases() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/UnusedScopedValueWhereResultSampleEdgeCases.java")) + .withCheck(new UnusedScopedValueWhereResultCheck()) + .verifyIssues(); + } + } From f0d75d285399889e53c34f3e0a1ac6977a2ab27a Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 16:26:13 +0100 Subject: [PATCH 06/15] Convert UnusedScopedValueWhereResultSampleEdgeCases.java to compact source file --- ...ScopedValueWhereResultSampleEdgeCases.java | 298 +++++++++--------- 1 file changed, 155 insertions(+), 143 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java index c7d4aa865a..5a8194dd9e 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java @@ -1,194 +1,206 @@ import static java.lang.ScopedValue.where; -class UnusedScopedValueWhereResultSampleEdgeCases { - static final ScopedValue SCOPED = ScopedValue.newInstance(); - static final ScopedValue SCOPED2 = ScopedValue.newInstance(); +static final ScopedValue SCOPED = ScopedValue.newInstance(); +static final ScopedValue SCOPED2 = ScopedValue.newInstance(); - // Field storage - should be flagged as we can't track field usage reliably - ScopedValue.Carrier carrierField; - static ScopedValue.Carrier staticCarrierField; +// Field storage - should be flagged as we can't track field usage reliably +ScopedValue.Carrier carrierField; +static ScopedValue.Carrier staticCarrierField; +void chainedWhereBeforeConsumption() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .run() + carrier.where(SCOPED2, "world").run(() -> { + }); +} - void chainedWhereBeforeConsumption() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .run() - carrier.where(SCOPED2, "world").run(() -> {}); - } - - void chainedWhereBeforeCallConsumption() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .call() - carrier.where(SCOPED2, "world").call(() -> "result"); - } +void chainedWhereBeforeCallConsumption() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .call() + carrier.where(SCOPED2, "world").call(() -> "result"); +} - // ===== CONTROL FLOW ===== +// ===== CONTROL FLOW ===== - void conditionalUsage(boolean condition) { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used conditionally - if (condition) { - carrier.run(() -> {}); - } +void conditionalUsage(boolean condition) { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used conditionally + if (condition) { + carrier.run(() -> { + }); } +} - void conditionalUsageNotAllPaths(boolean condition) { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in one branch (we don't do path-sensitive analysis) - if (condition) { - carrier.run(() -> {}); - } - // else: carrier not used - but we can't detect this without CFG analysis +void conditionalUsageNotAllPaths(boolean condition) { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in one branch (we don't do path-sensitive analysis) + if (condition) { + carrier.run(() -> { + }); } + // else: carrier not used - but we can't detect this without CFG analysis +} - void carrierInTryFinally() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in finally - try { - doSomething(); - } finally { - carrier.run(() -> {}); - } +void carrierInTryFinally() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in finally + try { + doSomething(); + } finally { + carrier.run(() -> { + }); } +} - void carrierInLoop() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in loop - for (int i = 0; i < 3; i++) { - carrier.run(() -> {}); - } +void carrierInLoop() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in loop + for (int i = 0; i < 3; i++) { + carrier.run(() -> { + }); } +} - // ===== VARIABLE SCENARIOS ===== +// ===== VARIABLE SCENARIOS ===== - void variableReassignedBeforeUse() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant - carrier = ScopedValue.where(SCOPED, "world"); - carrier.run(() -> {}); - } +void variableReassignedBeforeUse() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier = ScopedValue.where(SCOPED, "world"); + carrier.run(() -> { + }); +} - void multipleVariablesSameCarrier() { - var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - used via carrier2 reference... actually no, different objects - var carrier2 = carrier1; // Carrier2 points to same object - carrier2.run(() -> {}); - } +void multipleVariablesSameCarrier() { + var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - used via carrier2 reference... actually no, different objects + var carrier2 = carrier1; // Carrier2 points to same object + carrier2.run(() -> { + }); +} - void multipleVariablesOneUnused() { - var usedCarrier = ScopedValue.where(SCOPED, "hello"); // Compliant - var unusedCarrier = ScopedValue.where(SCOPED, "world"); // Noncompliant - usedCarrier.run(() -> {}); - } +void multipleVariablesOneUnused() { + var usedCarrier = ScopedValue.where(SCOPED, "hello"); // Compliant + var unusedCarrier = ScopedValue.where(SCOPED, "world"); // Noncompliant + usedCarrier.run(() -> { + }); +} - void carrierStoredInField() { - carrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping - } +void carrierStoredInField() { + carrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping +} - void carrierStoredInStaticField() { - staticCarrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping - } +void carrierStoredInStaticField() { + staticCarrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping +} - // ===== RETURN VARIATIONS ===== +// ===== RETURN VARIATIONS ===== - ScopedValue.Carrier directReturnWithoutVariable() { - return ScopedValue.where(SCOPED, "hello"); // Compliant - returned directly - } +ScopedValue.Carrier directReturnWithoutVariable() { + return ScopedValue.where(SCOPED, "hello"); // Compliant - returned directly +} - ScopedValue.Carrier ternaryReturn(boolean condition) { - var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - potentially returned - var carrier2 = ScopedValue.where(SCOPED, "world"); // Compliant - potentially returned - return condition ? carrier1 : carrier2; - } +ScopedValue.Carrier ternaryReturn(boolean condition) { + var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - potentially returned + var carrier2 = ScopedValue.where(SCOPED, "world"); // Compliant - potentially returned + return condition ? carrier1 : carrier2; +} - // ===== NON-CONSUMING METHOD CALLS ===== +// ===== NON-CONSUMING METHOD CALLS ===== - void toStringOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant - carrier.toString(); - } +void toStringOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.toString(); +} - void hashCodeOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant - carrier.hashCode(); - } +void hashCodeOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.hashCode(); +} - void equalsOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant - carrier.equals(null); - } +void equalsOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.equals(null); +} - void getOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant - carrier.get(SCOPED); - } +void getOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + carrier.get(SCOPED); +} - // ===== LAMBDA/CLOSURE ===== +// ===== LAMBDA/CLOSURE ===== - void carrierUsedInsideLambda() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used inside lambda - Runnable r = () -> { - carrier.run(() -> {}); - }; - r.run(); - } +void carrierUsedInsideLambda() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used inside lambda + Runnable r = () -> { + carrier.run(() -> { + }); + }; + r.run(); +} - void carrierPassedToLambdaConsumer() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes to lambda consumer - consumeCarrier(c -> c.run(() -> {}), carrier); - } +void carrierPassedToLambdaConsumer() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes to lambda consumer + consumeCarrier(c -> c.run(() -> { + }), carrier); +} - // ===== NESTED ESCAPING ===== +// ===== NESTED ESCAPING ===== - void carrierPassedToMethodThatReturnsIt() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call - var returned = identity(carrier); - returned.run(() -> {}); - } +void carrierPassedToMethodThatReturnsIt() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call + var returned = identity(carrier); + returned.run(() -> { + }); +} - void carrierPassedToMethodThatReturnsItButUnused() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call (we can't track further) - var returned = identity(carrier); - // returned is not used - but carrier escaped so we don't flag - } +void carrierPassedToMethodThatReturnsItButUnused() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call (we can't track further) + var returned = identity(carrier); + // returned is not used - but carrier escaped so we don't flag +} - // ===== STATIC IMPORT ===== +// ===== STATIC IMPORT ===== - void staticImportWhere() { - where(SCOPED, "hello"); // Noncompliant - } +void staticImportWhere() { + where(SCOPED, "hello"); // Noncompliant +} - void staticImportWhereUsed() { - where(SCOPED, "hello").run(() -> {}); // Compliant - used immediately - } +void staticImportWhereUsed() { + where(SCOPED, "hello").run(() -> { + }); // Compliant - used immediately +} - void staticImportWhereVariable() { - var carrier = where(SCOPED, "hello"); // Noncompliant - } +void staticImportWhereVariable() { + var carrier = where(SCOPED, "hello"); // Noncompliant +} - void staticImportWhereVariableUsed() { - var carrier = where(SCOPED, "hello"); // Compliant - used - carrier.run(() -> {}); - } +void staticImportWhereVariableUsed() { + var carrier = where(SCOPED, "hello"); // Compliant - used + carrier.run(() -> { + }); +} - // ===== CONSTRUCTOR ARGUMENT ===== +// ===== CONSTRUCTOR ARGUMENT ===== - void carrierAsConstructorArgument() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via constructor - new CarrierHolder(carrier); - } +void carrierAsConstructorArgument() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via constructor + new CarrierHolder(carrier); +} - void carrierAsConstructorArgumentDirect() { - new CarrierHolder(ScopedValue.where(SCOPED, "hello")); // Compliant - escapes via constructor - } +void carrierAsConstructorArgumentDirect() { + new CarrierHolder(ScopedValue.where(SCOPED, "hello")); // Compliant - escapes via constructor +} - // ===== HELPER METHODS AND CLASSES ===== +// ===== HELPER METHODS AND CLASSES ===== - void doSomething() {} +void doSomething() { +} - ScopedValue.Carrier identity(ScopedValue.Carrier carrier) { - return carrier; - } +ScopedValue.Carrier identity(ScopedValue.Carrier carrier) { + return carrier; +} - void consumeCarrier(java.util.function.Consumer consumer, ScopedValue.Carrier carrier) { - consumer.accept(carrier); - } +void consumeCarrier(java.util.function.Consumer consumer, ScopedValue.Carrier carrier) { + consumer.accept(carrier); +} - static class CarrierHolder { - CarrierHolder(ScopedValue.Carrier carrier) // Noncompliant - {} +static class CarrierHolder { + CarrierHolder(ScopedValue.Carrier carrier) // Noncompliant + { } } From bc7009923447d2375c7797e2d3420aa3e2353ee4 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 16:27:59 +0100 Subject: [PATCH 07/15] Rename UnusedScopedValueWhereResultSampleEdgeCases.java to UnusedScopedValueWhereResultEdgeCasesSample.java --- ...ases.java => UnusedScopedValueWhereResultEdgeCasesSample.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename java-checks-test-sources/default/src/main/files/non-compiling/checks/{UnusedScopedValueWhereResultSampleEdgeCases.java => UnusedScopedValueWhereResultEdgeCasesSample.java} (100%) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java similarity index 100% rename from java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSampleEdgeCases.java rename to java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java From e1c658e5a35778f2c95851f19377a3b50d82d683 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 16:33:39 +0100 Subject: [PATCH 08/15] Fix path --- .../java/checks/UnusedScopedValueWhereResultCheckTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java index 896c686cc9..0b6e7ca5aa 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java @@ -34,7 +34,7 @@ void test() { @Test void testEdgeCases() { CheckVerifier.newVerifier() - .onFile(nonCompilingTestSourcesPath("checks/UnusedScopedValueWhereResultSampleEdgeCases.java")) + .onFile(nonCompilingTestSourcesPath("checks/UnusedScopedValueWhereResultEdgeCasesSample.java")) .withCheck(new UnusedScopedValueWhereResultCheck()) .verifyIssues(); } From d57958cafada1e70423f2cf47f84c2198b26d7cb Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Thu, 5 Feb 2026 16:38:19 +0100 Subject: [PATCH 09/15] Fix small edge case --- ...nusedScopedValueWhereResultEdgeCasesSample.java | 8 +++++++- .../checks/UnusedScopedValueWhereResultCheck.java | 14 +++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java index 5a8194dd9e..3a5775aacf 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java @@ -66,13 +66,19 @@ void variableReassignedBeforeUse() { }); } -void multipleVariablesSameCarrier() { +void multipleVariablesSameCarrierUsed() { var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - used via carrier2 reference... actually no, different objects var carrier2 = carrier1; // Carrier2 points to same object carrier2.run(() -> { }); } + +void multipleVariablesSameCarrierUnused() { + var carrier1 = ScopedValue.where(SCOPED, "hello"); // Noncompliant + var carrier2 = carrier1; // Carrier2 points to same object but is never used +} + void multipleVariablesOneUnused() { var usedCarrier = ScopedValue.where(SCOPED, "hello"); // Compliant var unusedCarrier = ScopedValue.where(SCOPED, "world"); // Noncompliant diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index dff6336b42..0223e86a0b 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -193,9 +193,17 @@ private boolean isUsageValid(IdentifierTree usage) { } } - // Check if usage is assigned to another variable (aliasing) - counts as valid - if (parent instanceof VariableTree) { - return true; + // Check if usage is assigned to another variable (aliasing) + if (parent instanceof VariableTree varTree) { + Symbol targetSymbol = varTree.symbol(); + // If assigned to a field, it escapes + if (!targetSymbol.isLocalVariable()) { + return true; + } + // If assigned to a local variable, check if that variable is properly used + if (targetSymbol.isVariableSymbol()) { + return targetSymbol.usages().stream().anyMatch(this::isUsageValid); + } } // Check if usage escapes (returned or passed as argument) From 50e5da1b7460501b47e99ec0c239c368fcbc9bdf Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Fri, 6 Feb 2026 08:45:45 +0100 Subject: [PATCH 10/15] Refine edge cases integration tests --- ...ScopedValueWhereResultEdgeCasesSample.java | 138 ++++-------------- 1 file changed, 32 insertions(+), 106 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java index 3a5775aacf..6380cf3049 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java @@ -4,127 +4,82 @@ static final ScopedValue SCOPED = ScopedValue.newInstance(); static final ScopedValue SCOPED2 = ScopedValue.newInstance(); -// Field storage - should be flagged as we can't track field usage reliably +// Field storage - shouldn't be flagged as we can't track field usage reliably ScopedValue.Carrier carrierField; static ScopedValue.Carrier staticCarrierField; - -void chainedWhereBeforeConsumption() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .run() - carrier.where(SCOPED2, "world").run(() -> { - }); -} - -void chainedWhereBeforeCallConsumption() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - chained .where() then .call() - carrier.where(SCOPED2, "world").call(() -> "result"); -} - // ===== CONTROL FLOW ===== void conditionalUsage(boolean condition) { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used conditionally + var carrier = ScopedValue.where(SCOPED, "Conditional usage"); // Compliant - used conditionally if (condition) { carrier.run(() -> { }); } } -void conditionalUsageNotAllPaths(boolean condition) { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in one branch (we don't do path-sensitive analysis) - if (condition) { - carrier.run(() -> { - }); - } - // else: carrier not used - but we can't detect this without CFG analysis -} - -void carrierInTryFinally() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in finally - try { - doSomething(); - } finally { - carrier.run(() -> { - }); - } -} - -void carrierInLoop() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used in loop - for (int i = 0; i < 3; i++) { - carrier.run(() -> { - }); - } -} - // ===== VARIABLE SCENARIOS ===== void variableReassignedBeforeUse() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant - carrier = ScopedValue.where(SCOPED, "world"); + var carrier = ScopedValue.where(SCOPED, "Initial instance"); // Noncompliant + carrier = ScopedValue.where(SCOPED, "Second instance"); // Compliant - this instance is used carrier.run(() -> { }); } void multipleVariablesSameCarrierUsed() { - var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - used via carrier2 reference... actually no, different objects + var carrier1 = ScopedValue.where(SCOPED, "Single instance"); // Compliant - used via carrier2 reference... actually no, different objects var carrier2 = carrier1; // Carrier2 points to same object carrier2.run(() -> { }); } +void multipleVariablesSameCarrierUsed2() { + var carrier1 = ScopedValue.where(SCOPED, "Single instance"); // Compliant + var carrier2 = carrier1; // Carrier2 points to same object + carrier1.run(() -> { + }); +} void multipleVariablesSameCarrierUnused() { - var carrier1 = ScopedValue.where(SCOPED, "hello"); // Noncompliant + var carrier1 = ScopedValue.where(SCOPED, "Single instance unused"); // Noncompliant var carrier2 = carrier1; // Carrier2 points to same object but is never used } void multipleVariablesOneUnused() { - var usedCarrier = ScopedValue.where(SCOPED, "hello"); // Compliant - var unusedCarrier = ScopedValue.where(SCOPED, "world"); // Noncompliant + var usedCarrier = ScopedValue.where(SCOPED, "Used instance"); // Compliant + var unusedCarrier = usedCarrier.where(SCOPED, "Unused instance"); // Noncompliant usedCarrier.run(() -> { }); } void carrierStoredInField() { - carrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping + carrierField = ScopedValue.where(SCOPED, "Stored in instance field"); // Compliant - way of escaping } void carrierStoredInStaticField() { - staticCarrierField = ScopedValue.where(SCOPED, "hello"); // Compliant - way of escaping + staticCarrierField = ScopedValue.where(SCOPED, "Stored in static field"); // Compliant - way of escaping } // ===== RETURN VARIATIONS ===== -ScopedValue.Carrier directReturnWithoutVariable() { - return ScopedValue.where(SCOPED, "hello"); // Compliant - returned directly -} - ScopedValue.Carrier ternaryReturn(boolean condition) { - var carrier1 = ScopedValue.where(SCOPED, "hello"); // Compliant - potentially returned - var carrier2 = ScopedValue.where(SCOPED, "world"); // Compliant - potentially returned + var carrier1 = ScopedValue.where(SCOPED, "Ternary return - true"); // Compliant - potentially returned + var carrier2 = ScopedValue.where(SCOPED, "Ternary return - false"); // Compliant - potentially returned return condition ? carrier1 : carrier2; } // ===== NON-CONSUMING METHOD CALLS ===== -void toStringOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant +void standardFunctinsOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "Standard functions"); // Noncompliant carrier.toString(); -} - -void hashCodeOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant carrier.hashCode(); -} - -void equalsOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant carrier.equals(null); } void getOnCarrier() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Noncompliant + var carrier = ScopedValue.where(SCOPED, "Get function"); // Noncompliant carrier.get(SCOPED); } @@ -139,44 +94,23 @@ void carrierUsedInsideLambda() { r.run(); } -void carrierPassedToLambdaConsumer() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes to lambda consumer - consumeCarrier(c -> c.run(() -> { - }), carrier); -} - -// ===== NESTED ESCAPING ===== - -void carrierPassedToMethodThatReturnsIt() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call - var returned = identity(carrier); - returned.run(() -> { - }); -} - -void carrierPassedToMethodThatReturnsItButUnused() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via method call (we can't track further) - var returned = identity(carrier); - // returned is not used - but carrier escaped so we don't flag -} - // ===== STATIC IMPORT ===== void staticImportWhere() { - where(SCOPED, "hello"); // Noncompliant + where(SCOPED, "Static import unused"); // Noncompliant } void staticImportWhereUsed() { - where(SCOPED, "hello").run(() -> { + where(SCOPED, "Static import used").run(() -> { }); // Compliant - used immediately } void staticImportWhereVariable() { - var carrier = where(SCOPED, "hello"); // Noncompliant + var carrier = where(SCOPED, "Static import unused variable"); // Noncompliant } void staticImportWhereVariableUsed() { - var carrier = where(SCOPED, "hello"); // Compliant - used + var carrier = where(SCOPED, "Static import used variable "); // Compliant - used carrier.run(() -> { }); } @@ -184,29 +118,21 @@ void staticImportWhereVariableUsed() { // ===== CONSTRUCTOR ARGUMENT ===== void carrierAsConstructorArgument() { - var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - escapes via constructor + var carrier = ScopedValue.where(SCOPED, "Carrier in constructor argument"); // Compliant - escapes via constructor new CarrierHolder(carrier); } -void carrierAsConstructorArgumentDirect() { - new CarrierHolder(ScopedValue.where(SCOPED, "hello")); // Compliant - escapes via constructor -} - // ===== HELPER METHODS AND CLASSES ===== -void doSomething() { -} - -ScopedValue.Carrier identity(ScopedValue.Carrier carrier) { - return carrier; -} - -void consumeCarrier(java.util.function.Consumer consumer, ScopedValue.Carrier carrier) { - consumer.accept(carrier); -} static class CarrierHolder { CarrierHolder(ScopedValue.Carrier carrier) // Noncompliant { } + + CarrierHolder(ScopedValue.Carrier carrier, int i) // Compliant + { + carrier.run(() -> { + }); + } } From a8f9b228a16305fdf511451dd1d7b92e5dab6e17 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Fri, 6 Feb 2026 08:58:03 +0100 Subject: [PATCH 11/15] Refine basic integration tests --- .../UnusedScopedValueWhereResultSample.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java index 914c4af5c5..87effeb77c 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java @@ -1,16 +1,17 @@ static final ScopedValue myScopedValue = ScopedValue.newInstance(); +static final ScopedValue myScopedValue2 = ScopedValue.newInstance(); void main() { - ScopedValue.where(myScopedValue, "hello").get(myScopedValue); // Noncompliant - ScopedValue.where(myScopedValue, "hello").run(() -> { + ScopedValue.where(myScopedValue, "Simple chained with get").get(myScopedValue); // Noncompliant + ScopedValue.where(myScopedValue, "Simple chained with run").run(() -> { }); // Compliant, the result is used immediately - ScopedValue.where(myScopedValue, "hello").where(ScopedValue.newInstance(), "hello").run(() -> { + ScopedValue.where(myScopedValue, "Chained two").where(myScopedValue2, "times with run").run(() -> { }); // Compliant, the result is used immediately - ScopedValue.where(myScopedValue, "hello"); // Noncompliant - var myUnusedCarrier = ScopedValue.where(myScopedValue, "hello"); // Noncompliant - var myUnused2LevelCarrier = ScopedValue.where(myScopedValue, "hello").where(ScopedValue.newInstance(), "hello"); // Noncompliant - var myUsedCarrier = ScopedValue.where(myScopedValue, "hello"); // Compliant, the result is assigned to a variable and used - var myUsed2LevelCarrier = ScopedValue.where(myScopedValue, "hello").where(ScopedValue.newInstance(), "hello"); // Compliant, the result is assigned to a variable and used + ScopedValue.where(myScopedValue, "Simple carrier creation"); // Noncompliant + var myUnusedCarrier = ScopedValue.where(myScopedValue, "Unused carrier in variable"); // Noncompliant + var myUnused2LevelCarrier = ScopedValue.where(myScopedValue, "Unused carrrier in variable").where(myScopedValue2, "2 levels of where"); // Noncompliant + var myUsedCarrier = ScopedValue.where(myScopedValue, "Used carrier in variable"); // Compliant, the result is assigned to a variable and used + var myUsed2LevelCarrier = ScopedValue.where(myScopedValue, "Used carrier in variable").where(myScopedValue2, "2 levels of where"); // Compliant, the result is assigned to a variable and used myUsedCarrier.run(() -> { }); myUsed2LevelCarrier.run(() -> { @@ -18,7 +19,7 @@ void main() { } void escapedCarrierFunctionCall() { - var carrier = ScopedValue.where(myScopedValue, "hello"); // ccompliant - the result escapes + var carrier = ScopedValue.where(myScopedValue, "Escape a carrier with a function call"); // compliant - the result escapes usedCarrierArgument(carrier); } @@ -31,7 +32,7 @@ void unusedCarrierArgument(ScopedValue.Carrier carrier) { // Noncompliant } ScopedValue.Carrier escapedCarrierReturn() { - var carrier = ScopedValue.where(myScopedValue, "hello"); // compliant - the result escapes + var carrier = ScopedValue.where(myScopedValue, "Escape a carrier with a return"); // compliant - the result escapes return carrier; } From 960ca27c92fb753828b12eb3cb23335aa2cc20d4 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Fri, 6 Feb 2026 09:26:02 +0100 Subject: [PATCH 12/15] Refactor UnusedScopedValueWhereResultCheck - rename functions, grouping, decompose isUsageValid --- .../UnusedScopedValueWhereResultCheck.java | 121 ++++++++++-------- 1 file changed, 70 insertions(+), 51 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index 0223e86a0b..ef3e97d927 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -54,6 +54,8 @@ public class UnusedScopedValueWhereResultCheck extends IssuableSubscriptionVisit .withAnyParameters() .build()); + // ===== VISITOR METHODS ===== + @Override public List nodesToVisit() { return List.of(Tree.Kind.METHOD_INVOCATION, Tree.Kind.VARIABLE); @@ -68,6 +70,7 @@ public void visitNode(Tree tree) { } } + // ===== CHECK METHOD INVOCATION ===== private void checkMethodInvocation(MethodInvocationTree mit) { if (!WHERE_MATCHER.matches(mit)) { return; @@ -106,9 +109,12 @@ private void checkMethodInvocation(MethodInvocationTree mit) { } // If we reach here, the result is discarded - reportIssue(mit, MESSAGE); + reportIssue(mit); } + + // ===== CHECK CARRIER VARIABLE ===== + private void checkCarrierVariable(VariableTree variableTree) { Symbol symbol = variableTree.symbol(); if (!symbol.isVariableSymbol()) { @@ -128,31 +134,35 @@ private void checkCarrierVariable(VariableTree variableTree) { // Check all usages of the variable Symbol.VariableSymbol variableSymbol = (Symbol.VariableSymbol) symbol; - List usages = variableSymbol.usages(); + List variableUsages = variableSymbol.usages(); // Check if variable is reassigned before proper use - if (isWhereResult && isReassignedBeforeProperUse(usages)) { - reportIssue(variableTree.simpleName(), MESSAGE); + if (isWhereResult && isVariableReassignedBeforeProperUse(variableUsages)) { + reportIssue(variableTree.simpleName()); return; } - boolean isProperlyUsed = usages.stream().anyMatch(this::isUsageValid); + boolean isProperlyUsed = variableUsages.stream().anyMatch(this::isUsageValid); if (!isProperlyUsed) { - reportIssue(variableTree.simpleName(), MESSAGE); + reportIssue(variableTree.simpleName()); } } - private boolean isReassignedBeforeProperUse(List usages) { - for (IdentifierTree usage : usages) { + private static boolean isCarrierType(Symbol symbol) { + return symbol.type().is("java.lang.ScopedValue$Carrier"); + } + + private boolean isVariableReassignedBeforeProperUse(List variableUsages) { + for (IdentifierTree variableUsage : variableUsages) { // Check if this usage is a reassignment (left side of assignment) - Tree parent = usage.parent(); - if (parent instanceof AssignmentExpressionTree assignment && assignment.variable() == usage) { + Tree parent = variableUsage.parent(); + if (parent instanceof AssignmentExpressionTree assignment && assignment.variable() == variableUsage) { // This is a reassignment - check if there's any proper use before this reassignment // For simplicity, if we see a reassignment and no proper use, consider it unused - boolean hasProperUseBefore = usages.stream() - .filter(u -> u != usage) - .filter(u -> isBefore(u, usage)) + boolean hasProperUseBefore = variableUsages.stream() + .filter(u -> u != variableUsage) + .filter(u -> isUsageBefore(u, variableUsage)) .anyMatch(this::isUsageValid); if (!hasProperUseBefore) { return true; @@ -162,64 +172,62 @@ private boolean isReassignedBeforeProperUse(List usages) { return false; } - private static boolean isBefore(IdentifierTree first, IdentifierTree second) { - return first.identifierToken().range().start().isBefore(second.identifierToken().range().start()); - } - - private static boolean isCarrierType(Symbol symbol) { - return symbol.type().is("java.lang.ScopedValue$Carrier"); - } - private boolean isUsageValid(IdentifierTree usage) { Tree parent = usage.parent(); - // Check if usage is a reassignment (left side of assignment) - not a valid use - if (parent instanceof AssignmentExpressionTree assignment && assignment.variable() == usage) { + // 1. Filter out reassignments immediately + if (isReassignment(usage, parent)) { return false; } - // Check if usage is consumed via .run() or .call() + // 2. Delegate logic for method calls (.run, .call, .where) if (parent instanceof MemberSelectExpressionTree memberSelect && memberSelect.expression() == usage) { - String methodName = memberSelect.identifier().name(); - if (CONSUMPTION_METHODS.contains(methodName)) { - return true; - } - // Check if it's a chained .where() that eventually gets consumed - if (WHERE_METHOD_NAME.equals(methodName)) { - Tree grandParent = memberSelect.parent(); - if (grandParent instanceof MethodInvocationTree chainedMit) { - return isChainEventuallyConsumed(chainedMit); - } - } + return isMethodUsageValid(memberSelect); } - // Check if usage is assigned to another variable (aliasing) + // 3. Delegate logic for aliasing (assignment to other variables) if (parent instanceof VariableTree varTree) { - Symbol targetSymbol = varTree.symbol(); - // If assigned to a field, it escapes - if (!targetSymbol.isLocalVariable()) { - return true; - } - // If assigned to a local variable, check if that variable is properly used - if (targetSymbol.isVariableSymbol()) { - return targetSymbol.usages().stream().anyMatch(this::isUsageValid); - } + return isAliasingValid(varTree); } - // Check if usage escapes (returned or passed as argument) + // 4. Check if usage escapes (returned or passed as argument) return isEscaping(usage); } - private static boolean isImmediatelyConsumed(Tree parent) { - if (parent instanceof MemberSelectExpressionTree memberSelect) { - return CONSUMPTION_METHODS.contains(memberSelect.identifier().name()); + private boolean isMethodUsageValid(MemberSelectExpressionTree memberSelect) { + String methodName = memberSelect.identifier().name(); + if (CONSUMPTION_METHODS.contains(methodName)) { + return true; + } + if (WHERE_METHOD_NAME.equals(methodName) && memberSelect.parent() instanceof MethodInvocationTree chainedMit) { + return isChainEventuallyConsumed(chainedMit); } return false; } - private static boolean isChainedWhere(Tree parent) { + private boolean isAliasingValid(VariableTree varTree) { + Symbol targetSymbol = varTree.symbol(); + if (!targetSymbol.isLocalVariable()) { + return true; + } + return targetSymbol.isVariableSymbol() && + targetSymbol.usages().stream().anyMatch(this::isUsageValid); + } + + private static boolean isUsageBefore(IdentifierTree mainUsage, IdentifierTree relativeToUsage) { + return mainUsage.identifierToken().range().start().isBefore(relativeToUsage.identifierToken().range().start()); + } + + private static boolean isReassignment(IdentifierTree usage, Tree parent) { + return parent instanceof AssignmentExpressionTree assignment && assignment.variable() == usage; + } + + + // ===== HELPER METHODS ===== + + private static boolean isImmediatelyConsumed(Tree parent) { if (parent instanceof MemberSelectExpressionTree memberSelect) { - return WHERE_METHOD_NAME.equals(memberSelect.identifier().name()); + return CONSUMPTION_METHODS.contains(memberSelect.identifier().name()); } return false; } @@ -254,6 +262,13 @@ private boolean isChainEventuallyConsumed(MethodInvocationTree mit) { return false; } + private static boolean isChainedWhere(Tree parent) { + if (parent instanceof MemberSelectExpressionTree memberSelect) { + return WHERE_METHOD_NAME.equals(memberSelect.identifier().name()); + } + return false; + } + private static boolean isEscaping(Tree tree) { Tree parent = tree.parent(); @@ -283,6 +298,10 @@ private static boolean isEscaping(Tree tree) { return false; } + + private void reportIssue(Tree tree) { + reportIssue(tree, MESSAGE); + } } From 7a19152a4aa0b5e1107db7df93739bd22a48fb51 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Fri, 6 Feb 2026 10:04:23 +0100 Subject: [PATCH 13/15] Add missing tests, remove useless code --- ...ScopedValueWhereResultEdgeCasesSample.java | 14 ++-- .../UnusedScopedValueWhereResultSample.java | 8 +++ .../UnusedScopedValueWhereResultCheck.java | 64 ++----------------- 3 files changed, 21 insertions(+), 65 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java index 6380cf3049..b99a2fac7d 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java @@ -28,7 +28,7 @@ void variableReassignedBeforeUse() { } void multipleVariablesSameCarrierUsed() { - var carrier1 = ScopedValue.where(SCOPED, "Single instance"); // Compliant - used via carrier2 reference... actually no, different objects + var carrier1 = ScopedValue.where(SCOPED, "Single instance"); // Compliant var carrier2 = carrier1; // Carrier2 points to same object carrier2.run(() -> { }); @@ -46,13 +46,6 @@ void multipleVariablesSameCarrierUnused() { var carrier2 = carrier1; // Carrier2 points to same object but is never used } -void multipleVariablesOneUnused() { - var usedCarrier = ScopedValue.where(SCOPED, "Used instance"); // Compliant - var unusedCarrier = usedCarrier.where(SCOPED, "Unused instance"); // Noncompliant - usedCarrier.run(() -> { - }); -} - void carrierStoredInField() { carrierField = ScopedValue.where(SCOPED, "Stored in instance field"); // Compliant - way of escaping } @@ -61,6 +54,11 @@ void carrierStoredInStaticField() { staticCarrierField = ScopedValue.where(SCOPED, "Stored in static field"); // Compliant - way of escaping } +void escapingInFieldTwice() { + carrierField = ScopedValue.where(SCOPED, "Instance 1"); // Compliant - escapes, we can't track field usage + carrierField = ScopedValue.where(SCOPED, "Instance 2"); // Compliant - reassigned but still escapes +} + // ===== RETURN VARIATIONS ===== ScopedValue.Carrier ternaryReturn(boolean condition) { diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java index 87effeb77c..95d9477aa7 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java @@ -23,6 +23,10 @@ void escapedCarrierFunctionCall() { usedCarrierArgument(carrier); } +void escapedCarrierFunctionCall2() { + usedCarrierArgument(ScopedValue.where(myScopedValue, "Escape a carrier with a function call")); // compliant - the result escapes +} + void usedCarrierArgument(ScopedValue.Carrier carrier) { // compliant - the carrier is used in the function carrier.run(() -> { }); @@ -36,6 +40,10 @@ ScopedValue.Carrier escapedCarrierReturn() { return carrier; } +ScopedValue.Carrier escapedCarrierReturn2() { + return ScopedValue.where(myScopedValue, "Escape a carrier with a return"); // compliant - the result escapes +} + void escapedCarrierArgument(ScopedValue.Carrier carrier) { // compliant - the carrier is used in the function usedCarrierArgument(carrier); } diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index ef3e97d927..761ff118fe 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -28,7 +28,6 @@ import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; -import org.sonar.plugins.java.api.tree.NewClassTree; import org.sonar.plugins.java.api.tree.ReturnStatementTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.VariableTree; @@ -117,9 +116,6 @@ private void checkMethodInvocation(MethodInvocationTree mit) { private void checkCarrierVariable(VariableTree variableTree) { Symbol symbol = variableTree.symbol(); - if (!symbol.isVariableSymbol()) { - return; - } // Check if this variable is initialized with a where() call var initializer = variableTree.initializer(); @@ -175,9 +171,9 @@ private boolean isVariableReassignedBeforeProperUse(List variabl private boolean isUsageValid(IdentifierTree usage) { Tree parent = usage.parent(); - // 1. Filter out reassignments immediately - if (isReassignment(usage, parent)) { - return false; + // Special case: if there is no parent, consider it valid to avoid false positives + if (parent == null) { + return true; } // 2. Delegate logic for method calls (.run, .call, .where) @@ -199,30 +195,18 @@ private boolean isMethodUsageValid(MemberSelectExpressionTree memberSelect) { if (CONSUMPTION_METHODS.contains(methodName)) { return true; } - if (WHERE_METHOD_NAME.equals(methodName) && memberSelect.parent() instanceof MethodInvocationTree chainedMit) { - return isChainEventuallyConsumed(chainedMit); - } + return false; } private boolean isAliasingValid(VariableTree varTree) { - Symbol targetSymbol = varTree.symbol(); - if (!targetSymbol.isLocalVariable()) { - return true; - } - return targetSymbol.isVariableSymbol() && - targetSymbol.usages().stream().anyMatch(this::isUsageValid); + return varTree.symbol().usages().stream().anyMatch(this::isUsageValid); } private static boolean isUsageBefore(IdentifierTree mainUsage, IdentifierTree relativeToUsage) { return mainUsage.identifierToken().range().start().isBefore(relativeToUsage.identifierToken().range().start()); } - private static boolean isReassignment(IdentifierTree usage, Tree parent) { - return parent instanceof AssignmentExpressionTree assignment && assignment.variable() == usage; - } - - // ===== HELPER METHODS ===== private static boolean isImmediatelyConsumed(Tree parent) { @@ -232,36 +216,6 @@ private static boolean isImmediatelyConsumed(Tree parent) { return false; } - private boolean isChainEventuallyConsumed(MethodInvocationTree mit) { - Tree parent = mit.parent(); - - // Special case: if there is no parent, consider it consumed to avoid false positives - if (parent == null) { - return true; - } - - if (isImmediatelyConsumed(parent)) { - return true; - } - - if (isChainedWhere(parent) && parent.parent() instanceof MethodInvocationTree nextMit) { - return isChainEventuallyConsumed(nextMit); - } - - if (isEscaping(mit)) { - return true; - } - - if (parent instanceof VariableTree varTree) { - Symbol symbol = varTree.symbol(); - if (symbol.isVariableSymbol()) { - return symbol.usages().stream().anyMatch(this::isUsageValid); - } - } - - return false; - } - private static boolean isChainedWhere(Tree parent) { if (parent instanceof MemberSelectExpressionTree memberSelect) { return WHERE_METHOD_NAME.equals(memberSelect.identifier().name()); @@ -272,8 +226,9 @@ private static boolean isChainedWhere(Tree parent) { private static boolean isEscaping(Tree tree) { Tree parent = tree.parent(); + // Special case: if there is no parent, consider it escaping to avoid false positives if (parent == null) { - return false; + return true; } // Returned from method @@ -291,11 +246,6 @@ private static boolean isEscaping(Tree tree) { return isEscaping(parent); } - // Passed to constructor - if (parent instanceof NewClassTree newClass) { - return newClass.arguments().contains(tree); - } - return false; } From 25f94b6b4c229a21fb49ce6c471477b43507dc40 Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Fri, 6 Feb 2026 10:10:01 +0100 Subject: [PATCH 14/15] Remove useless methods in UnusedScopedValueWhereResultCheck --- .../UnusedScopedValueWhereResultCheck.java | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index 761ff118fe..7491102fef 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -178,31 +178,18 @@ private boolean isUsageValid(IdentifierTree usage) { // 2. Delegate logic for method calls (.run, .call, .where) if (parent instanceof MemberSelectExpressionTree memberSelect && memberSelect.expression() == usage) { - return isMethodUsageValid(memberSelect); + return CONSUMPTION_METHODS.contains(memberSelect.identifier().name()); } // 3. Delegate logic for aliasing (assignment to other variables) if (parent instanceof VariableTree varTree) { - return isAliasingValid(varTree); + return varTree.symbol().usages().stream().anyMatch(this::isUsageValid); } // 4. Check if usage escapes (returned or passed as argument) return isEscaping(usage); } - private boolean isMethodUsageValid(MemberSelectExpressionTree memberSelect) { - String methodName = memberSelect.identifier().name(); - if (CONSUMPTION_METHODS.contains(methodName)) { - return true; - } - - return false; - } - - private boolean isAliasingValid(VariableTree varTree) { - return varTree.symbol().usages().stream().anyMatch(this::isUsageValid); - } - private static boolean isUsageBefore(IdentifierTree mainUsage, IdentifierTree relativeToUsage) { return mainUsage.identifierToken().range().start().isBefore(relativeToUsage.identifierToken().range().start()); } From 87729b6cd75aa6a88e7537544f339efa7b450dce Mon Sep 17 00:00:00 2001 From: "romain.birling" Date: Fri, 6 Feb 2026 10:41:00 +0100 Subject: [PATCH 15/15] Fix code style : line breaks --- .../sonar/java/checks/UnusedScopedValueWhereResultCheck.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java index 7491102fef..9b046de199 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -111,9 +111,7 @@ private void checkMethodInvocation(MethodInvocationTree mit) { reportIssue(mit); } - // ===== CHECK CARRIER VARIABLE ===== - private void checkCarrierVariable(VariableTree variableTree) { Symbol symbol = variableTree.symbol(); @@ -195,7 +193,6 @@ private static boolean isUsageBefore(IdentifierTree mainUsage, IdentifierTree re } // ===== HELPER METHODS ===== - private static boolean isImmediatelyConsumed(Tree parent) { if (parent instanceof MemberSelectExpressionTree memberSelect) { return CONSUMPTION_METHODS.contains(memberSelect.identifier().name());