Skip to content

Commit 3dea37c

Browse files
rombirlirombirli
authored
SONARJAVA-6108 ScopedValue where result is ignored (#5441)
1 parent 25ed53b commit 3dea37c

10 files changed

Lines changed: 532 additions & 1 deletion

File tree

‎its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public void javaCheckTestSources() throws Exception {
199199
softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
200200
softly.assertThat(newTotal).isEqualTo(knownTotal);
201201
softly.assertThat(rulesCausingFPs).hasSize(10);
202-
softly.assertThat(rulesNotReporting).hasSize(14);
202+
softly.assertThat(rulesNotReporting).hasSize(15);
203203

204204
/**
205205
* 4. Check total number of differences (FPs + FNs)

‎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
@@ -2872,5 +2872,11 @@
28722872
"hasTruePositives": false,
28732873
"falseNegatives": 0,
28742874
"falsePositives": 0
2875+
},
2876+
{
2877+
"ruleKey": "8432",
2878+
"hasTruePositives": false,
2879+
"falseNegatives": 0,
2880+
"falsePositives": 0
28752881
}
28762882
]

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S8432.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S8432",
3+
"hasTruePositives": false,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}

‎java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultEdgeCasesSample.java‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import static java.lang.ScopedValue.where;
2+
3+
4+
static final ScopedValue<String> SCOPED = ScopedValue.newInstance();
5+
static final ScopedValue<String> SCOPED2 = ScopedValue.newInstance();
6+
7+
// Field storage - shouldn't be flagged as we can't track field usage reliably
8+
ScopedValue.Carrier carrierField;
9+
static ScopedValue.Carrier staticCarrierField;
10+
11+
// ===== CONTROL FLOW =====
12+
13+
void conditionalUsage(boolean condition) {
14+
var carrier = ScopedValue.where(SCOPED, "Conditional usage"); // Compliant - used conditionally
15+
if (condition) {
16+
carrier.run(() -> {
17+
});
18+
}
19+
}
20+
21+
// ===== VARIABLE SCENARIOS =====
22+
23+
void variableReassignedBeforeUse() {
24+
var carrier = ScopedValue.where(SCOPED, "Initial instance"); // Noncompliant
25+
carrier = ScopedValue.where(SCOPED, "Second instance"); // Compliant - this instance is used
26+
carrier.run(() -> {
27+
});
28+
}
29+
30+
void multipleVariablesSameCarrierUsed() {
31+
var carrier1 = ScopedValue.where(SCOPED, "Single instance"); // Compliant
32+
var carrier2 = carrier1; // Carrier2 points to same object
33+
carrier2.run(() -> {
34+
});
35+
}
36+
37+
void multipleVariablesSameCarrierUsed2() {
38+
var carrier1 = ScopedValue.where(SCOPED, "Single instance"); // Compliant
39+
var carrier2 = carrier1; // Carrier2 points to same object
40+
carrier1.run(() -> {
41+
});
42+
}
43+
44+
void multipleVariablesSameCarrierUnused() {
45+
var carrier1 = ScopedValue.where(SCOPED, "Single instance unused"); // Noncompliant
46+
var carrier2 = carrier1; // Carrier2 points to same object but is never used
47+
}
48+
49+
void carrierStoredInField() {
50+
carrierField = ScopedValue.where(SCOPED, "Stored in instance field"); // Compliant - way of escaping
51+
}
52+
53+
void carrierStoredInStaticField() {
54+
staticCarrierField = ScopedValue.where(SCOPED, "Stored in static field"); // Compliant - way of escaping
55+
}
56+
57+
void escapingInFieldTwice() {
58+
carrierField = ScopedValue.where(SCOPED, "Instance 1"); // Compliant - escapes, we can't track field usage
59+
carrierField = ScopedValue.where(SCOPED, "Instance 2"); // Compliant - reassigned but still escapes
60+
}
61+
62+
// ===== RETURN VARIATIONS =====
63+
64+
ScopedValue.Carrier ternaryReturn(boolean condition) {
65+
var carrier1 = ScopedValue.where(SCOPED, "Ternary return - true"); // Compliant - potentially returned
66+
var carrier2 = ScopedValue.where(SCOPED, "Ternary return - false"); // Compliant - potentially returned
67+
return condition ? carrier1 : carrier2;
68+
}
69+
70+
// ===== NON-CONSUMING METHOD CALLS =====
71+
72+
void standardFunctinsOnCarrier() {
73+
var carrier = ScopedValue.where(SCOPED, "Standard functions"); // Noncompliant
74+
carrier.toString();
75+
carrier.hashCode();
76+
carrier.equals(null);
77+
}
78+
79+
void getOnCarrier() {
80+
var carrier = ScopedValue.where(SCOPED, "Get function"); // Noncompliant
81+
carrier.get(SCOPED);
82+
}
83+
84+
// ===== LAMBDA/CLOSURE =====
85+
86+
void carrierUsedInsideLambda() {
87+
var carrier = ScopedValue.where(SCOPED, "hello"); // Compliant - used inside lambda
88+
Runnable r = () -> {
89+
carrier.run(() -> {
90+
});
91+
};
92+
r.run();
93+
}
94+
95+
// ===== STATIC IMPORT =====
96+
97+
void staticImportWhere() {
98+
where(SCOPED, "Static import unused"); // Noncompliant
99+
}
100+
101+
void staticImportWhereUsed() {
102+
where(SCOPED, "Static import used").run(() -> {
103+
}); // Compliant - used immediately
104+
}
105+
106+
void staticImportWhereVariable() {
107+
var carrier = where(SCOPED, "Static import unused variable"); // Noncompliant
108+
}
109+
110+
void staticImportWhereVariableUsed() {
111+
var carrier = where(SCOPED, "Static import used variable "); // Compliant - used
112+
carrier.run(() -> {
113+
});
114+
}
115+
116+
// ===== CONSTRUCTOR ARGUMENT =====
117+
118+
void carrierAsConstructorArgument() {
119+
var carrier = ScopedValue.where(SCOPED, "Carrier in constructor argument"); // Compliant - escapes via constructor
120+
new CarrierHolder(carrier);
121+
}
122+
123+
// ===== HELPER METHODS AND CLASSES =====
124+
125+
126+
static class CarrierHolder {
127+
CarrierHolder(ScopedValue.Carrier carrier) // Noncompliant
128+
{
129+
}
130+
131+
CarrierHolder(ScopedValue.Carrier carrier, int i) // Compliant
132+
{
133+
carrier.run(() -> {
134+
});
135+
}
136+
}

‎java-checks-test-sources/default/src/main/files/non-compiling/checks/UnusedScopedValueWhereResultSample.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
static final ScopedValue myScopedValue = ScopedValue.newInstance();
2+
static final ScopedValue myScopedValue2 = ScopedValue.newInstance();
3+
4+
void main() {
5+
ScopedValue.where(myScopedValue, "Simple chained with get").get(myScopedValue); // Noncompliant
6+
ScopedValue.where(myScopedValue, "Simple chained with run").run(() -> {
7+
}); // Compliant, the result is used immediately
8+
ScopedValue.where(myScopedValue, "Chained two").where(myScopedValue2, "times with run").run(() -> {
9+
}); // Compliant, the result is used immediately
10+
ScopedValue.where(myScopedValue, "Simple carrier creation"); // Noncompliant
11+
var myUnusedCarrier = ScopedValue.where(myScopedValue, "Unused carrier in variable"); // Noncompliant
12+
var myUnused2LevelCarrier = ScopedValue.where(myScopedValue, "Unused carrrier in variable").where(myScopedValue2, "2 levels of where"); // Noncompliant
13+
var myUsedCarrier = ScopedValue.where(myScopedValue, "Used carrier in variable"); // Compliant, the result is assigned to a variable and used
14+
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
15+
myUsedCarrier.run(() -> {
16+
});
17+
myUsed2LevelCarrier.run(() -> {
18+
});
19+
}
20+
21+
void escapedCarrierFunctionCall() {
22+
var carrier = ScopedValue.where(myScopedValue, "Escape a carrier with a function call"); // compliant - the result escapes
23+
usedCarrierArgument(carrier);
24+
}
25+
26+
void escapedCarrierFunctionCall2() {
27+
usedCarrierArgument(ScopedValue.where(myScopedValue, "Escape a carrier with a function call")); // compliant - the result escapes
28+
}
29+
30+
void usedCarrierArgument(ScopedValue.Carrier carrier) { // compliant - the carrier is used in the function
31+
carrier.run(() -> {
32+
});
33+
}
34+
35+
void unusedCarrierArgument(ScopedValue.Carrier carrier) { // Noncompliant
36+
}
37+
38+
ScopedValue.Carrier escapedCarrierReturn() {
39+
var carrier = ScopedValue.where(myScopedValue, "Escape a carrier with a return"); // compliant - the result escapes
40+
return carrier;
41+
}
42+
43+
ScopedValue.Carrier escapedCarrierReturn2() {
44+
return ScopedValue.where(myScopedValue, "Escape a carrier with a return"); // compliant - the result escapes
45+
}
46+
47+
void escapedCarrierArgument(ScopedValue.Carrier carrier) { // compliant - the carrier is used in the function
48+
usedCarrierArgument(carrier);
49+
}

0 commit comments

Comments
 (0)