Skip to content

Commit 49377ab

Browse files
SONARJAVA-6106 ScopedValue instances should be assigned to a stable reference (#5480)
1 parent 2c189be commit 49377ab

File tree

8 files changed

+277
-1
lines changed

8 files changed

+277
-1
lines changed

its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2896,5 +2896,11 @@
28962896
"hasTruePositives": false,
28972897
"falseNegatives": 0,
28982898
"falsePositives": 0
2899+
},
2900+
{
2901+
"ruleKey": "S8465",
2902+
"hasTruePositives": true,
2903+
"falseNegatives": 0,
2904+
"falsePositives": 0
28992905
}
29002906
]
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S8465",
3+
"hasTruePositives": true,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package checks;
2+
3+
import java.util.Objects;
4+
import org.apache.commons.lang3.tuple.Pair;
5+
6+
class ScopedValueStableReferenceCheckSample {
7+
8+
private static final ScopedValue<String> VALUE = ScopedValue.newInstance();
9+
10+
public void where() {
11+
ScopedValue.where(ScopedValue.newInstance(), "inaccessible").run(() -> { // Noncompliant {{Consider using a stable reference for ScopedValue instances.}}
12+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
13+
// Cannot reference the scoped value here, as it has no name.
14+
});
15+
}
16+
17+
public void chainedWhere() {
18+
ScopedValue<String> scopedValue = ScopedValue.newInstance();
19+
ScopedValue.where(scopedValue, "accessible").where(ScopedValue.newInstance(), "inaccessible").run(() -> { // Noncompliant {{Consider using a stable reference for ScopedValue instances.}}
20+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
21+
});
22+
}
23+
24+
public void nestedArgument() {
25+
ScopedValue.where(Objects.requireNonNull(ScopedValue.newInstance()), "scopedValue").run(() -> {}); // Noncompliant {{Consider using a stable reference for ScopedValue instances.}}
26+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
27+
ScopedValue.where((ScopedValue.newInstance()), "scopedValue").run(() -> {}); // Noncompliant {{Consider using a stable reference for ScopedValue instances.}}
28+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
29+
ScopedValue.where(Pair.of(ScopedValue.newInstance(), 0).getLeft(), "scopedValue").run(() -> {}); // Noncompliant {{Consider using a stable reference for ScopedValue instances.}}
30+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
31+
ScopedValue<String> scopedValue;
32+
ScopedValue.where(Pair.of((scopedValue = ScopedValue.newInstance()), ScopedValue.newInstance()).getLeft(), "scopedValue").run(scopedValue::get); // Noncompliant {{Consider using a stable reference for ScopedValue instances.}}
33+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
34+
}
35+
36+
public String readFieldInWhere() {
37+
return ScopedValue.where(VALUE, "field value").call(VALUE::get); // Compliant
38+
}
39+
40+
public String readLocalVarInWhere() {
41+
ScopedValue<String> value = ScopedValue.newInstance();
42+
return ScopedValue.where(value, "local value").call(value::get); // Compliant
43+
}
44+
45+
public String assignLocalVarInWhere() {
46+
ScopedValue<String> value;
47+
String result = ScopedValue.where((value = ScopedValue.newInstance()), "local value").call(value::get); // Compliant
48+
result = ScopedValue.where((value = ((ScopedValue.newInstance()))), "local value").call(value::get); // Compliant
49+
return result;
50+
}
51+
52+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import org.sonar.check.Rule;
20+
import org.sonar.java.checks.methods.AbstractMethodDetection;
21+
import org.sonar.plugins.java.api.JavaVersion;
22+
import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
23+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
24+
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
25+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
26+
import org.sonar.plugins.java.api.tree.Tree;
27+
28+
@Rule(key = "S8465")
29+
public class ScopedValueStableReferenceCheck extends AbstractMethodDetection implements JavaVersionAwareVisitor {
30+
31+
private static final String MESSAGE = "Consider using a stable reference for ScopedValue instances.";
32+
33+
private static final MethodMatchers WHERE_MATCHER = MethodMatchers.create()
34+
.ofTypes("java.lang.ScopedValue", "java.lang.ScopedValue$Carrier")
35+
.names("where")
36+
.withAnyParameters()
37+
.build();
38+
39+
@Override
40+
public boolean isCompatibleWithJavaVersion(JavaVersion version) {
41+
return version.isJava25Compatible();
42+
}
43+
44+
@Override
45+
protected MethodMatchers getMethodInvocationMatchers() {
46+
return WHERE_MATCHER;
47+
}
48+
49+
@Override
50+
protected void onMethodInvocationFound(MethodInvocationTree methodInvocation) {
51+
var finder = new NewInstanceInvocationFinder();
52+
methodInvocation.arguments()
53+
.get(0)
54+
.accept(finder);
55+
MethodInvocationTree newInstanceInvocation = finder.invocation;
56+
if (newInstanceInvocation != null) {
57+
reportIssue(newInstanceInvocation, MESSAGE);
58+
}
59+
}
60+
61+
private static class NewInstanceInvocationFinder extends BaseTreeVisitor {
62+
63+
private static final MethodMatchers NEW_INSTANCE_MATCHER = MethodMatchers.create()
64+
.ofTypes("java.lang.ScopedValue")
65+
.names("newInstance")
66+
.addWithoutParametersMatcher()
67+
.build();
68+
69+
private MethodInvocationTree invocation = null;
70+
71+
@Override
72+
public void visitMethodInvocation(MethodInvocationTree methodInvocation) {
73+
Tree parent = nearestNonParenthesizedParent(methodInvocation);
74+
if (NEW_INSTANCE_MATCHER.matches(methodInvocation.methodSymbol()) && parent != null && !parent.is(Tree.Kind.ASSIGNMENT)) {
75+
invocation = methodInvocation;
76+
return;
77+
}
78+
super.visitMethodInvocation(methodInvocation);
79+
}
80+
81+
private static Tree nearestNonParenthesizedParent(Tree tree) {
82+
Tree parent = tree.parent();
83+
while (parent != null && parent.is(Tree.Kind.PARENTHESIZED_EXPRESSION)) {
84+
parent = parent.parent();
85+
}
86+
return parent;
87+
}
88+
89+
}
90+
91+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
22+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
23+
24+
class ScopedValueStableReferenceCheckTest {
25+
26+
@Test
27+
void test() {
28+
CheckVerifier.newVerifier()
29+
.onFile(mainCodeSourcesPath("checks/ScopedValueStableReferenceCheckSample.java"))
30+
.withCheck(new ScopedValueStableReferenceCheck())
31+
.withJavaVersion(25)
32+
.verifyIssues();
33+
}
34+
35+
@Test
36+
void test_java_24() {
37+
CheckVerifier.newVerifier()
38+
.onFile(mainCodeSourcesPath("checks/ScopedValueStableReferenceCheckSample.java"))
39+
.withCheck(new ScopedValueStableReferenceCheck())
40+
.withJavaVersion(24)
41+
.verifyNoIssues();
42+
}
43+
44+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<p>A <code>ScopedValue</code> must be accessible within the functional task where it is bound. Creating a new instance directly inside a
2+
<code>ScopedValue.where()</code> call makes the key unreachable, as there is no variable name to reference when calling <code>.get()</code>.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>The primary purpose of a <code>ScopedValue</code> is to provide a way to share data without passing it as method arguments. To retrieve the value
5+
using <code>.get()</code>, you must have access to the <code>ScopedValue</code> instance (the key).</p>
6+
<p>If a <code>ScopedValue</code> is instantiated anonymously within the <code>.where()</code> method, it is "lost" immediately after the binding is
7+
created. The code inside the <code>Runnable</code> or <code>Callable</code> has no way to reference that specific instance to retrieve the associated
8+
value, rendering the scoped value useless.</p>
9+
<h2>How to fix it</h2>
10+
<p>Assign the <code>ScopedValue</code> instance to a stable reference — typically a <code>static final</code> field — before using it in a binding.
11+
This allows different parts of the code to refer to the same key to set and retrieve values.</p>
12+
<h3>Code examples</h3>
13+
<h4>Noncompliant code example</h4>
14+
<pre data-diff-id="1" data-diff-type="noncompliant">
15+
public class Renderer {
16+
17+
public void process() {
18+
// Noncompliant: The ScopedValue instance is anonymous and unreachable inside the run method
19+
ScopedValue.where(ScopedValue.newInstance(), "DARK").run(() -&gt; {
20+
render();
21+
});
22+
}
23+
24+
void render() {
25+
// There is no way to call .get() here because the key is unknown
26+
}
27+
28+
}
29+
</pre>
30+
<h4>Compliant solution</h4>
31+
<pre data-diff-id="1" data-diff-type="compliant">
32+
public class Renderer {
33+
34+
// Compliant: The ScopedValue is assigned to a constant that can be referenced elsewhere
35+
private static final ScopedValue&lt;String&gt; THEME = ScopedValue.newInstance();
36+
37+
public void process() {
38+
ScopedValue.where(THEME, "DARK").run(() -&gt; {
39+
render();
40+
});
41+
}
42+
43+
void render() {
44+
System.out.println("Theme is: " + THEME.get());
45+
}
46+
47+
}
48+
</pre>
49+
<h2>Resources</h2>
50+
<h3>Documentation</h3>
51+
<ul>
52+
<li><a href="https://openjdk.org/jeps/506">JEP 506: Scoped Values</a></li>
53+
</ul>
54+
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"title": "ScopedValue instances should be assigned to a stable reference",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Major",
11+
"ruleSpecification": "RSPEC-8465",
12+
"sqKey": "S8465",
13+
"scope": "All",
14+
"quickfix": "unknown",
15+
"code": {
16+
"impacts": {
17+
"MAINTAINABILITY": "HIGH",
18+
"RELIABILITY": "MEDIUM"
19+
},
20+
"attribute": "CONVENTIONAL"
21+
}
22+
}

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,7 @@
525525
"S8445",
526526
"S8446",
527527
"S8447",
528-
"S8450"
528+
"S8450",
529+
"S8465"
529530
]
530531
}

0 commit comments

Comments
 (0)