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 new file mode 100644 index 0000000000..b99a2fac7d --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java @@ -0,0 +1,136 @@ +import static java.lang.ScopedValue.where; + + +static final ScopedValue SCOPED = ScopedValue.newInstance(); +static final ScopedValue SCOPED2 = ScopedValue.newInstance(); + +// Field storage - shouldn't be flagged as we can't track field usage reliably +ScopedValue.Carrier carrierField; +static ScopedValue.Carrier staticCarrierField; + +// ===== CONTROL FLOW ===== + +void conditionalUsage(boolean condition) { + var carrier = ScopedValue.where(SCOPED, "Conditional usage"); // Compliant - used conditionally + if (condition) { + carrier.run(() -> { + }); + } +} + +// ===== VARIABLE SCENARIOS ===== + +void variableReassignedBeforeUse() { + 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, "Single instance"); // Compliant + 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, "Single instance unused"); // Noncompliant + var carrier2 = carrier1; // Carrier2 points to same object but is never used +} + +void carrierStoredInField() { + carrierField = ScopedValue.where(SCOPED, "Stored in instance field"); // Compliant - way of escaping +} + +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) { + 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 standardFunctinsOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "Standard functions"); // Noncompliant + carrier.toString(); + carrier.hashCode(); + carrier.equals(null); +} + +void getOnCarrier() { + var carrier = ScopedValue.where(SCOPED, "Get function"); // Noncompliant + carrier.get(SCOPED); +} + +// ===== LAMBDA/CLOSURE ===== + +void carrierUsedInsideLambda() { + var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used inside lambda + Runnable r = () -> { + carrier.run(() -> { + }); + }; + r.run(); +} + +// ===== STATIC IMPORT ===== + +void staticImportWhere() { + where(SCOPED, "Static import unused"); // Noncompliant +} + +void staticImportWhereUsed() { + where(SCOPED, "Static import used").run(() -> { + }); // Compliant - used immediately +} + +void staticImportWhereVariable() { + var carrier = where(SCOPED, "Static import unused variable"); // Noncompliant +} + +void staticImportWhereVariableUsed() { + var carrier = where(SCOPED, "Static import used variable "); // Compliant - used + carrier.run(() -> { + }); +} + +// ===== CONSTRUCTOR ARGUMENT ===== + +void carrierAsConstructorArgument() { + var carrier = ScopedValue.where(SCOPED, "Carrier in constructor argument"); // Compliant - escapes via constructor + new CarrierHolder(carrier); +} + +// ===== HELPER METHODS AND CLASSES ===== + + +static class CarrierHolder { + CarrierHolder(ScopedValue.Carrier carrier) // Noncompliant + { + } + + CarrierHolder(ScopedValue.Carrier carrier, int i) // Compliant + { + carrier.run(() -> { + }); + } +} 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..95d9477aa7 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java @@ -0,0 +1,49 @@ +static final ScopedValue myScopedValue = ScopedValue.newInstance(); +static final ScopedValue myScopedValue2 = ScopedValue.newInstance(); + +void main() { + 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, "Chained two").where(myScopedValue2, "times with run").run(() -> { + }); // Compliant, the result is used immediately + 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(() -> { + }); +} + +void escapedCarrierFunctionCall() { + var carrier = ScopedValue.where(myScopedValue, "Escape a carrier with a function call"); // compliant - the result escapes + 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(() -> { + }); +} + +void unusedCarrierArgument(ScopedValue.Carrier carrier) { // Noncompliant +} + +ScopedValue.Carrier escapedCarrierReturn() { + var carrier = ScopedValue.where(myScopedValue, "Escape a carrier with a return"); // compliant - the result escapes + 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 new file mode 100644 index 0000000000..9b046de199 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheck.java @@ -0,0 +1,241 @@ +/* + * 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 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.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.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 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()); + + // ===== VISITOR METHODS ===== + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.METHOD_INVOCATION, Tree.Kind.VARIABLE); + } + + @Override + public void visitNode(Tree tree) { + if (tree instanceof MethodInvocationTree methodInvocationTree) { + checkMethodInvocation(methodInvocationTree); + } else if (tree instanceof VariableTree variableTree) { + checkCarrierVariable(variableTree); + } + } + + // ===== CHECK METHOD INVOCATION ===== + 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 is assigned to a field (escapes) + if (parent instanceof AssignmentExpressionTree) { + return; + } + + // Check if result escapes (returned or passed as argument) + if (isEscaping(mit)) { + return; + } + + // If we reach here, the result is discarded + reportIssue(mit); + } + + // ===== CHECK CARRIER VARIABLE ===== + private void checkCarrierVariable(VariableTree variableTree) { + Symbol symbol = variableTree.symbol(); + + // 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; + List variableUsages = variableSymbol.usages(); + + // Check if variable is reassigned before proper use + if (isWhereResult && isVariableReassignedBeforeProperUse(variableUsages)) { + reportIssue(variableTree.simpleName()); + return; + } + + boolean isProperlyUsed = variableUsages.stream().anyMatch(this::isUsageValid); + + if (!isProperlyUsed) { + reportIssue(variableTree.simpleName()); + } + } + + 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 = 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 = variableUsages.stream() + .filter(u -> u != variableUsage) + .filter(u -> isUsageBefore(u, variableUsage)) + .anyMatch(this::isUsageValid); + if (!hasProperUseBefore) { + return true; + } + } + } + return false; + } + + private boolean isUsageValid(IdentifierTree usage) { + Tree parent = usage.parent(); + + // 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) + if (parent instanceof MemberSelectExpressionTree memberSelect && memberSelect.expression() == usage) { + return CONSUMPTION_METHODS.contains(memberSelect.identifier().name()); + } + + // 3. Delegate logic for aliasing (assignment to other variables) + if (parent instanceof VariableTree varTree) { + return varTree.symbol().usages().stream().anyMatch(this::isUsageValid); + } + + // 4. Check if usage escapes (returned or passed as argument) + return isEscaping(usage); + } + + private static boolean isUsageBefore(IdentifierTree mainUsage, IdentifierTree relativeToUsage) { + return mainUsage.identifierToken().range().start().isBefore(relativeToUsage.identifierToken().range().start()); + } + + // ===== HELPER METHODS ===== + 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 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 true; + } + + // Returned from method + if (parent instanceof ReturnStatementTree) { + return true; + } + + // 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); + } + + return false; + } + + private void reportIssue(Tree tree) { + reportIssue(tree, MESSAGE); + } +} + + 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..0b6e7ca5aa --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/UnusedScopedValueWhereResultCheckTest.java @@ -0,0 +1,42 @@ +/* + * 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(); + } + + @Test + void testEdgeCases() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/UnusedScopedValueWhereResultEdgeCasesSample.java")) + .withCheck(new UnusedScopedValueWhereResultCheck()) + .verifyIssues(); + } + +}