Skip to content

Commit 5899138

Browse files
SONARJAVA-5336 S1068 Fix FP with @Getter and @Setter annotations in automatic analysis (#5022)
1 parent 34326b0 commit 5899138

5 files changed

Lines changed: 44 additions & 5 deletions

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
@@ -195,7 +195,7 @@ public void javaCheckTestSources() throws Exception {
195195
SoftAssertions softly = new SoftAssertions();
196196
softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
197197
softly.assertThat(newTotal).isEqualTo(knownTotal);
198-
softly.assertThat(rulesCausingFPs).hasSize(10);
198+
softly.assertThat(rulesCausingFPs).hasSize(9);
199199
softly.assertThat(rulesNotReporting).hasSize(11);
200200

201201
/**

its/autoscan/src/test/resources/autoscan/diffs/diff_S1068.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
"ruleKey": "S1068",
33
"hasTruePositives": true,
44
"falseNegatives": 0,
5-
"falsePositives": 4
5+
"falsePositives": 0
66
}

java-checks-test-sources/default/src/main/java/checks/unused/UnusedPrivateFieldCheck.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
import java.io.IOException;
77
import java.util.Optional;
88

9+
import lombok.Getter;
10+
import lombok.Setter;
11+
912
class UnusedPrivateFieldCheck {
1013

1114
private int unusedField; // Noncompliant {{Remove this unused "unusedField" private field.}}
@@ -94,6 +97,20 @@ Long transform(String number) {
9497
}
9598
}
9699

100+
class UnusedPrivateFieldCheckUnqualifiedAnnotations {
101+
@Getter
102+
@Setter
103+
static class MyData {
104+
private String name;
105+
private int dob;
106+
}
107+
108+
static class MoreData {
109+
private String airline; // Noncompliant
110+
private int price; // Noncompliant
111+
}
112+
}
113+
97114
class UnusedPrivateFieldCheckQuickfix {
98115

99116
private String unusedField; // Noncompliant [[quickfixes=qf0]]

java-checks/src/main/java/org/sonar/java/checks/unused/UnusedPrivateFieldCheck.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
import org.sonar.plugins.java.api.tree.TypeTree;
5656
import org.sonar.plugins.java.api.tree.VariableTree;
5757

58+
import static org.sonar.java.checks.helpers.AnnotationsHelper.annotationTypeIdentifier;
59+
5860
@Rule(key = "S1068")
5961
public class UnusedPrivateFieldCheck extends IssuableSubscriptionVisitor {
6062

@@ -193,9 +195,15 @@ && onlyUsedInVariableAssignment(symbol)
193195

194196
private static boolean hasOwnerClassAllowedAnnotations(VariableTree variableTree) {
195197
var ownerClass = (ClassTree) variableTree.parent();
196-
return ownerClass.modifiers().annotations().stream().anyMatch(
197-
annotation -> OWNER_CLASS_ALLOWED_ANNOTATIONS.contains(annotation.annotationType().symbolType().fullyQualifiedName())
198-
);
198+
var metadata = ownerClass.symbol().metadata();
199+
for (String name: OWNER_CLASS_ALLOWED_ANNOTATIONS) {
200+
// If the annotation does not use a fully qualified name e.g. `@Getter`,
201+
// then only the identifier portion will be available in automatic analysis.
202+
if (metadata.isAnnotatedWith(name) || metadata.isAnnotatedWith(annotationTypeIdentifier(name))) {
203+
return true;
204+
}
205+
}
206+
return false;
199207
}
200208

201209
private boolean onlyUsedInVariableAssignment(Symbol symbol) {

java-checks/src/test/java/org/sonar/java/checks/unused/UnusedPrivateFieldCheckTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,20 @@ void test() {
3232
.verifyIssues();
3333
}
3434

35+
/**
36+
* Unused private fields can be detected without semantics. The test verifies
37+
* that we correctly process annotations which suppress warnings and
38+
* do not produce FPs when semantics is missing.
39+
*/
40+
@Test
41+
void test_without_semantic() {
42+
CheckVerifier.newVerifier()
43+
.onFile(mainCodeSourcesPath("checks/unused/UnusedPrivateFieldCheck.java"))
44+
.withCheck(new UnusedPrivateFieldCheck())
45+
.withoutSemantic()
46+
.verifyIssues();
47+
}
48+
3549
@Test
3650
void test_non_compiling() {
3751
CheckVerifier.newVerifier()

0 commit comments

Comments
 (0)