Skip to content

Commit 61ef9d2

Browse files
authored
SONARJAVA-4415 S1068 ignores private fields on irrelevant annotations (#4637)
1 parent a9ffbd8 commit 61ef9d2

5 files changed

Lines changed: 60 additions & 5 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S6813",
33
"hasTruePositives": true,
4-
"falseNegatives": 55,
4+
"falseNegatives": 57,
55
"falsePositives": 0
66
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package checks.unused;
2+
3+
import javax.inject.Inject;
4+
import javax.annotation.Nullable;
5+
import javax.annotation.CheckForNull;
6+
7+
class UnusedPrivateFieldCheckWithIgnoredAnnotation {
8+
9+
@Inject
10+
private int unusedFieldAnnotationIsIgnored; // Noncompliant [[sc=15;ec=45]] {{Remove this unused "unusedFieldAnnotationIsIgnored" private field.}}
11+
12+
@Nullable
13+
private Object unusedFieldAnnotationIsAlsoIgnored; // Noncompliant [[sc=18;ec=52]] {{Remove this unused "unusedFieldAnnotationIsAlsoIgnored" private field.}}
14+
15+
@CheckForNull
16+
private Object unusedFieldAnnotationIsNotIgnored; // Compliant
17+
18+
@Inject
19+
@CheckForNull
20+
private Object unusedFieldOneAnnotationIsNotIgnored; // Compliant
21+
}

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@
2828
import java.util.Map;
2929
import java.util.Optional;
3030
import java.util.Set;
31+
import java.util.stream.Collectors;
32+
import java.util.stream.Stream;
3133
import org.sonar.check.Rule;
34+
import org.sonar.check.RuleProperty;
3235
import org.sonar.java.checks.helpers.QuickFixHelper;
3336
import org.sonar.java.model.ExpressionUtils;
3437
import org.sonar.java.model.ModifiersUtils;
@@ -58,6 +61,9 @@
5861
@Rule(key = "S1068")
5962
public class UnusedPrivateFieldCheck extends IssuableSubscriptionVisitor {
6063

64+
private static final String DEFAULT_IGNORE_ANNOTATIONS_KEY = "ignoreAnnotations";
65+
private static final String DEFAULT_IGNORE_ANNOTATIONS_DESCRIPTION = "Ignore annotations with next names (fully qualified class names separated with \",\").";
66+
6167
private static final Tree.Kind[] ASSIGNMENT_KINDS = {
6268
Tree.Kind.ASSIGNMENT,
6369
Tree.Kind.MULTIPLY_ASSIGNMENT,
@@ -77,6 +83,12 @@ public class UnusedPrivateFieldCheck extends IssuableSubscriptionVisitor {
7783
private final Set<String> unknownIdentifiers = new HashSet<>();
7884
private boolean hasNativeMethod = false;
7985

86+
@RuleProperty(
87+
key = DEFAULT_IGNORE_ANNOTATIONS_KEY,
88+
description = DEFAULT_IGNORE_ANNOTATIONS_DESCRIPTION)
89+
public String ignoreAnnotations = "";
90+
private Set<String> ignoredAnnotations;
91+
8092
@Override
8193
public List<Tree.Kind> nodesToVisit() {
8294
return Arrays.asList(Tree.Kind.CLASS, Tree.Kind.METHOD, Tree.Kind.EXPRESSION_STATEMENT, Tree.Kind.IDENTIFIER);
@@ -157,7 +169,7 @@ private void checkClassFields(ClassTree classTree) {
157169
}
158170

159171
public void checkIfUnused(VariableTree tree) {
160-
if (hasNoAnnotation(tree)) {
172+
if (hasOnlyIgnoredAnnotations(tree)) {
161173
Symbol symbol = tree.symbol();
162174
String name = symbol.name();
163175
if (symbol.isPrivate()
@@ -178,8 +190,19 @@ private boolean onlyUsedInVariableAssignment(Symbol symbol) {
178190
return symbol.usages().size() == assignments.getOrDefault(symbol, Collections.emptyList()).size();
179191
}
180192

181-
private static boolean hasNoAnnotation(VariableTree tree) {
182-
return tree.modifiers().annotations().isEmpty();
193+
private boolean hasOnlyIgnoredAnnotations(VariableTree tree) {
194+
return tree.modifiers().annotations().stream().allMatch(
195+
it -> getIgnoredAnnotations().contains(it.annotationType().symbolType().fullyQualifiedName()));
196+
}
197+
198+
private Set<String> getIgnoredAnnotations() {
199+
if (ignoredAnnotations == null) {
200+
ignoredAnnotations = Stream.of(ignoreAnnotations.split(","))
201+
.map(String::trim)
202+
.filter(it -> !it.isEmpty())
203+
.collect(Collectors.toSet());
204+
}
205+
return ignoredAnnotations;
183206
}
184207

185208
private void collectAssignment(ExpressionTree expressionTree) {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,14 @@ void test_quick_fixes() {
6161
.withQuickFixes()
6262
.verifyIssues();
6363
}
64+
65+
@Test
66+
void test_ignored_annotation() {
67+
UnusedPrivateFieldCheck check = new UnusedPrivateFieldCheck();
68+
check.ignoreAnnotations = "javax.inject.Inject,,javax.annotation.Nullable";
69+
CheckVerifier.newVerifier()
70+
.onFile(mainCodeSourcesPath("checks/unused/UnusedPrivateFieldCheckWithIgnoredAnnotation.java"))
71+
.withCheck(check)
72+
.verifyIssues();
73+
}
6474
}

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1068.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ <h3>Exceptions</h3>
3232
<ul>
3333
<li> Annotated fields and classes annotated with Lombok annotations </li>
3434
</ul>
35-
<p>The unused field in this class will not be reported by the rule as it is annotated.</p>
35+
<p>The unused field in this class will not be reported by the rule as it is annotated, except if annotation class <code>SomeAnnotation</code> is
36+
listed in the <code>ignoreAnnotations</code> parameter (see Parameters).</p>
3637
<pre>
3738
public class MyClass {
3839
@SomeAnnotation

0 commit comments

Comments
 (0)