pmd / pmd / 23

Committed 30 May 2025 04:25PM UTC coverage: 78.377% (-0.2%) from 78.601%

Build # 23

Build Type

push

github

Committed by

adangel

Commit Message

[core] Add rule to report unnecessary suppression comments/annotations (#5609)

Merge pull request #5609 from oowekyala:new-rule-UnnecessarySuppression

Run Details

17712 of 23434 branches covered (75.58%)

Branch coverage included in aggregate %.

159 of 328 new or added lines in 22 files covered. (48.48%)

36 existing lines in 4 files now uncovered.

38902 of 48799 relevant lines covered (79.72%)

0.81 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

87.59

/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAnnotationSuppressor.java

/*
 * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
 */

package net.sourceforge.pmd.lang.java.internal;

import static net.sourceforge.pmd.util.CollectionUtil.listOf;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;

import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.NodeStream;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr;
import net.sourceforge.pmd.lang.java.ast.ASTClassDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassType;
import net.sourceforge.pmd.lang.java.ast.ASTExecutableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMemberValue;
import net.sourceforge.pmd.lang.java.ast.ASTMemberValuePair;
import net.sourceforge.pmd.lang.java.ast.ASTModifierList;
import net.sourceforge.pmd.lang.java.ast.ASTTypeParameter;
import net.sourceforge.pmd.lang.java.ast.ASTTypeParameters;
import net.sourceforge.pmd.lang.java.ast.ASTVariableId;
import net.sourceforge.pmd.lang.java.ast.Annotatable;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.JavaVisitorBase;
import net.sourceforge.pmd.lang.java.ast.ModifierOwner.Visibility;
import net.sourceforge.pmd.lang.java.rule.errorprone.ImplicitSwitchFallThroughRule;
import net.sourceforge.pmd.lang.java.symbols.JExecutableSymbol;
import net.sourceforge.pmd.lang.java.types.JTypeMirror;
import net.sourceforge.pmd.lang.java.types.JTypeVar;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.lang.rule.Rule;
import net.sourceforge.pmd.reporting.AbstractAnnotationSuppressor;
import net.sourceforge.pmd.reporting.ViolationSuppressor;
import net.sourceforge.pmd.util.OptionalBool;

/**
 * Helper methods to suppress violations based on annotations.
 *
 * An annotation suppresses a rule if the annotation is a {@link SuppressWarnings},
 * and if the set of suppressed warnings ({@link SuppressWarnings#value()})
 * contains at least one of those:
 * <ul>
 * <li>"PMD" (suppresses all rules);
 * <li>"PMD.rulename", where rulename is the name of the given rule;
 * <li>"all" (conventional value to suppress all warnings).
 * </ul>
 *
 * <p>Additionally, the following values suppress a specific set of rules:
 * <ul>
 * <li>{@code "unused"}: suppresses rules like UnusedLocalVariable or UnusedPrivateField;
 * <li>{@code "serial"}: suppresses BeanMembersShouldSerialize, NonSerializableClass and MissingSerialVersionUID;
 * <li>{@code "fallthrough"}: suppresses ImplicitSwitchFallthrough #1899
 * </ul>
 *
 * @see <a href="https://docs.oracle.com/javase/specs/jls/se21/html/jls-9.html#jls-9.6.4.5">JLS 9.6.4.5. @SuppressWarnings</a>
 * @since 7.14.0
 */
final class JavaAnnotationSuppressor extends AbstractAnnotationSuppressor<ASTAnnotation> {

    private static final Set<String> UNUSED_RULES = new HashSet<>(Arrays.asList("UnusedPrivateField", "UnusedLocalVariable", "UnusedPrivateMethod", "UnusedFormalParameter", "UnusedAssignment", "SingularField"));
    private static final Set<String> SERIAL_RULES = new HashSet<>(Arrays.asList("BeanMembersShouldSerialize", "NonSerializableClass", "MissingSerialVersionUID"));


    static final List<ViolationSuppressor> ALL_JAVA_SUPPRESSORS = listOf(new JavaAnnotationSuppressor());

    private JavaAnnotationSuppressor() {
        super(ASTAnnotation.class);
    }


    @Override
    protected NodeStream<ASTAnnotation> getAnnotations(Node n) {
        if (n instanceof Annotatable) {
            return ((Annotatable) n).getDeclaredAnnotations();
        }
        return NodeStream.empty();
    }

    @Override
    protected boolean annotationParamSuppresses(String stringVal, Rule rule) {
        return super.annotationParamSuppresses(stringVal, rule)
            || "serial".equals(stringVal) && SERIAL_RULES.contains(rule.getName())
            || "unused".equals(stringVal) && UNUSED_RULES.contains(rule.getName())
            || "fallthrough".equals(stringVal) && rule instanceof ImplicitSwitchFallThroughRule;
    }

    @Override
    protected boolean walkAnnotation(ASTAnnotation annotation, AnnotationWalkCallbacks callbacks) {
        if (TypeTestUtil.isA(SuppressWarnings.class, annotation)) {
            for (ASTMemberValue value : annotation.getFlatValue(ASTMemberValuePair.VALUE_ATTR)) {
                Object constVal = value.getConstValue();
                if (constVal instanceof String) {
                    if (callbacks.processNode(value, (String) constVal)) {
                        return true;
                    }
                }
            }
        }
        return false;
    }

    JavaNode getAnnotationScope(ASTAnnotation a) {
        if (a.getParent() instanceof ASTModifierList) {
            return a.getParent().getParent();
        }
        return null;
    }

    @SuppressWarnings("unused")
    public static void foo1(int i) {
        i = 2;
        foo2(i);
    }

    @SuppressWarnings("unused")
    private static void foo2(int i) {
        System.out.println("i = " + i);
    }

    private static OptionalBool hasUnusedWarning(JavaNode node) {
        if (node == null) {
            return OptionalBool.UNKNOWN;
        }

        if (hasUnusedVariables(node)) {
            return OptionalBool.YES;
        } else if (hasUnusedTypeParam(node)) {
            return OptionalBool.YES;
        } else if (hasUnusedMethod(node)) {
            return OptionalBool.YES;
        }

        if (node instanceof ASTFieldDeclaration) {
            // for annotated fields, "unused" annotation is unambiguous - it only is used for this field.
            return OptionalBool.NO;
        }

        if (node instanceof ASTClassDeclaration) {
            ASTClassDeclaration classDecl = (ASTClassDeclaration) node;
            if (classDecl.getEffectiveVisibility() == Visibility.V_PRIVATE) {
                // is this private class is used in this compilation unit?
                boolean used = node.getRoot().descendants(ASTClassType.class)
                        .toStream()
                        .map(ASTClassType::getTypeMirror)
                        .map(JTypeMirror::getSymbol)
                        .filter(Objects::nonNull)
                        .anyMatch(s -> s.equals(classDecl.getSymbol()));
                if (used) {
                    return OptionalBool.NO;
                }
            }
        }

        return OptionalBool.UNKNOWN;
    }

    /**
     * Searches for local variables, fields, formal parameters.
     */
    private static boolean hasUnusedVariables(JavaNode node) {
        return node.descendants(ASTVariableId.class)
                .crossFindBoundaries()
                .any(it -> {
                    return it.getLocalUsages().isEmpty() || it.getLocalUsages().stream()
                            .map(ASTAssignableExpr::getAccessType)
                            .noneMatch(a -> a == ASTAssignableExpr.AccessType.READ);
                });

    }

    private static boolean hasUnusedTypeParam(JavaNode node) {
        // we can do this in a single traversal because type params must
        // be declared before they are used (in tree order)
        Set<JTypeVar> unusedTypeParams = new HashSet<>();
        node.acceptVisitor(new JavaVisitorBase<Void, Void>() {
            @Override
            public Void visit(ASTTypeParameters node, Void p) {
                // add all params before visiting bounds
                for (ASTTypeParameter parm : node) {
                    unusedTypeParams.add(parm.getTypeMirror());
                }
                return super.visit(node, p);
            }

            @Override
            public Void visit(ASTClassType node, Void data) {
                JTypeMirror ty = node.getTypeMirror();
                if (ty instanceof JTypeVar) {
                    unusedTypeParams.remove(ty);
                }
                return super.visit(node, data);
            }

            @Override
            public Void visit(ASTModifierList node, Void data) {
                // no need to visit those
                return data;
            }
        }, null);

        return !unusedTypeParams.isEmpty();
    }

    private static boolean hasUnusedMethod(JavaNode node) {
        Set<JExecutableSymbol> privateMethods = new HashSet<>();
        for (ASTExecutableDeclaration decl : node.descendantsOrSelf()
                .crossFindBoundaries()
                .filterIs(ASTExecutableDeclaration.class)) {
            Visibility visibility = decl.getEffectiveVisibility();
            if (visibility.isAtMost(Visibility.V_PRIVATE)) {
                privateMethods.add(decl.getSymbol());
            } else {
                // We cannot know if non-private (package-private, protected, public) method is effectively used
                return false;
            }
        }

        return false;

        // if (!privateMethods.isEmpty()) {
        // node is a private method/ctor or is a class decl that contains only private methods
        // TODO we need to somehow sync this with UnusedPrivateMethod and check, if these private
        // methods are indeed unused or not. See also #5727.
        // for now, we give up and assume, all private methods are used
        // and always return false...
        // }
    }

    @Override
    protected OptionalBool isSuppressingNonPmdWarnings(String stringVal, ASTAnnotation annotation) {
        if ("unused".equals(stringVal)) {
            JavaNode scope = getAnnotationScope(annotation);
            if (hasUnusedWarning(scope) == OptionalBool.NO) {
                return OptionalBool.NO;
            }
        }
        return super.isSuppressingNonPmdWarnings(stringVal, annotation);
    }
}

1	/*
2	* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
3	*/
4
5	package net.sourceforge.pmd.lang.java.internal;
6
7	import static net.sourceforge.pmd.util.CollectionUtil.listOf;
8
9	import java.util.Arrays;
10	import java.util.HashSet;
11	import java.util.List;
12	import java.util.Objects;
13	import java.util.Set;
14
15	import net.sourceforge.pmd.lang.ast.Node;
16	import net.sourceforge.pmd.lang.ast.NodeStream;
17	import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
18	import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr;
19	import net.sourceforge.pmd.lang.java.ast.ASTClassDeclaration;
20	import net.sourceforge.pmd.lang.java.ast.ASTClassType;
21	import net.sourceforge.pmd.lang.java.ast.ASTExecutableDeclaration;
22	import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
23	import net.sourceforge.pmd.lang.java.ast.ASTMemberValue;
24	import net.sourceforge.pmd.lang.java.ast.ASTMemberValuePair;
25	import net.sourceforge.pmd.lang.java.ast.ASTModifierList;
26	import net.sourceforge.pmd.lang.java.ast.ASTTypeParameter;
27	import net.sourceforge.pmd.lang.java.ast.ASTTypeParameters;
28	import net.sourceforge.pmd.lang.java.ast.ASTVariableId;
29	import net.sourceforge.pmd.lang.java.ast.Annotatable;
30	import net.sourceforge.pmd.lang.java.ast.JavaNode;
31	import net.sourceforge.pmd.lang.java.ast.JavaVisitorBase;
32	import net.sourceforge.pmd.lang.java.ast.ModifierOwner.Visibility;
33	import net.sourceforge.pmd.lang.java.rule.errorprone.ImplicitSwitchFallThroughRule;
34	import net.sourceforge.pmd.lang.java.symbols.JExecutableSymbol;
35	import net.sourceforge.pmd.lang.java.types.JTypeMirror;
36	import net.sourceforge.pmd.lang.java.types.JTypeVar;
37	import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
38	import net.sourceforge.pmd.lang.rule.Rule;
39	import net.sourceforge.pmd.reporting.AbstractAnnotationSuppressor;
40	import net.sourceforge.pmd.reporting.ViolationSuppressor;
41	import net.sourceforge.pmd.util.OptionalBool;
42
43	/**
44	* Helper methods to suppress violations based on annotations.
45	*
46	* An annotation suppresses a rule if the annotation is a {@link SuppressWarnings},
47	* and if the set of suppressed warnings ({@link SuppressWarnings#value()})
48	* contains at least one of those:
49	* <ul>
50	* <li>"PMD" (suppresses all rules);
51	* <li>"PMD.rulename", where rulename is the name of the given rule;
52	* <li>"all" (conventional value to suppress all warnings).
53	* </ul>
54	*
55	* <p>Additionally, the following values suppress a specific set of rules:
56	* <ul>
57	* <li>{@code "unused"}: suppresses rules like UnusedLocalVariable or UnusedPrivateField;
58	* <li>{@code "serial"}: suppresses BeanMembersShouldSerialize, NonSerializableClass and MissingSerialVersionUID;
59	* <li>{@code "fallthrough"}: suppresses ImplicitSwitchFallthrough #1899
60	* </ul>
61	*
62	* @see <a href="https://docs.oracle.com/javase/specs/jls/se21/html/jls-9.html#jls-9.6.4.5">JLS 9.6.4.5. @SuppressWarnings</a>
63	* @since 7.14.0
64	*/
65	final class JavaAnnotationSuppressor extends AbstractAnnotationSuppressor<ASTAnnotation> {
66
67	private static final Set<String> UNUSED_RULES = new HashSet<>(Arrays.asList("UnusedPrivateField", "UnusedLocalVariable", "UnusedPrivateMethod", "UnusedFormalParameter", "UnusedAssignment", "SingularField"));	1✔
68	private static final Set<String> SERIAL_RULES = new HashSet<>(Arrays.asList("BeanMembersShouldSerialize", "NonSerializableClass", "MissingSerialVersionUID"));	1✔
69
70
71	static final List<ViolationSuppressor> ALL_JAVA_SUPPRESSORS = listOf(new JavaAnnotationSuppressor());	1✔
72
73	private JavaAnnotationSuppressor() {
74	super(ASTAnnotation.class);	1✔
75	}	1✔
76
77
78	@Override
79	protected NodeStream<ASTAnnotation> getAnnotations(Node n) {
80	if (n instanceof Annotatable) {	1✔
81	return ((Annotatable) n).getDeclaredAnnotations();	1✔
82	}
83	return NodeStream.empty();	1✔
84	}
85
86	@Override
87	protected boolean annotationParamSuppresses(String stringVal, Rule rule) {
88	return super.annotationParamSuppresses(stringVal, rule)	1✔
89	\|\| "serial".equals(stringVal) && SERIAL_RULES.contains(rule.getName())	1!
90	\|\| "unused".equals(stringVal) && UNUSED_RULES.contains(rule.getName())	1✔
91	\|\| "fallthrough".equals(stringVal) && rule instanceof ImplicitSwitchFallThroughRule;	1!
92	}
93
94	@Override
95	protected boolean walkAnnotation(ASTAnnotation annotation, AnnotationWalkCallbacks callbacks) {
96	if (TypeTestUtil.isA(SuppressWarnings.class, annotation)) {	1✔
97	for (ASTMemberValue value : annotation.getFlatValue(ASTMemberValuePair.VALUE_ATTR)) {	1✔
98	Object constVal = value.getConstValue();	1✔
99	if (constVal instanceof String) {	1!
100	if (callbacks.processNode(value, (String) constVal)) {	1!
NEW 101	return true;	×
102	}
103	}
104	}	1✔
105	}
106	return false;	1✔
107	}
108
109	JavaNode getAnnotationScope(ASTAnnotation a) {
110	if (a.getParent() instanceof ASTModifierList) {	1!
111	return a.getParent().getParent();	1✔
112	}
NEW 113	return null;	×
114	}
115
116	@SuppressWarnings("unused")
117	public static void foo1(int i) {
NEW 118	i = 2;	×
NEW 119	foo2(i);	×
NEW 120	}	×
121
122	@SuppressWarnings("unused")
123	private static void foo2(int i) {
NEW 124	System.out.println("i = " + i);	×
NEW 125	}	×
126
127	private static OptionalBool hasUnusedWarning(JavaNode node) {
128	if (node == null) {	1!
NEW 129	return OptionalBool.UNKNOWN;	×
130	}
131
132	if (hasUnusedVariables(node)) {	1✔
133	return OptionalBool.YES;	1✔
134	} else if (hasUnusedTypeParam(node)) {	1✔
135	return OptionalBool.YES;	1✔
136	} else if (hasUnusedMethod(node)) {	1!
NEW 137	return OptionalBool.YES;	×
138	}
139
140	if (node instanceof ASTFieldDeclaration) {	1✔
141	// for annotated fields, "unused" annotation is unambiguous - it only is used for this field.
142	return OptionalBool.NO;	1✔
143	}
144
145	if (node instanceof ASTClassDeclaration) {	1✔
146	ASTClassDeclaration classDecl = (ASTClassDeclaration) node;	1✔
147	if (classDecl.getEffectiveVisibility() == Visibility.V_PRIVATE) {	1✔
148	// is this private class is used in this compilation unit?
149	boolean used = node.getRoot().descendants(ASTClassType.class)	1✔
150	.toStream()	1✔
151	.map(ASTClassType::getTypeMirror)	1✔
152	.map(JTypeMirror::getSymbol)	1✔
153	.filter(Objects::nonNull)	1✔
154	.anyMatch(s -> s.equals(classDecl.getSymbol()));	1✔
155	if (used) {	1✔
156	return OptionalBool.NO;	1✔
157	}
158	}
159	}
160
161	return OptionalBool.UNKNOWN;	1✔
162	}
163
164	/**
165	* Searches for local variables, fields, formal parameters.
166	*/
167	private static boolean hasUnusedVariables(JavaNode node) {
168	return node.descendants(ASTVariableId.class)	1✔
169	.crossFindBoundaries()	1✔
170	.any(it -> {	1✔
171	return it.getLocalUsages().isEmpty() \|\| it.getLocalUsages().stream()	1✔
172	.map(ASTAssignableExpr::getAccessType)	1✔
173	.noneMatch(a -> a == ASTAssignableExpr.AccessType.READ);	1✔
174	});
175
176	}
177
178	private static boolean hasUnusedTypeParam(JavaNode node) {
179	// we can do this in a single traversal because type params must
180	// be declared before they are used (in tree order)
181	Set<JTypeVar> unusedTypeParams = new HashSet<>();	1✔
182	node.acceptVisitor(new JavaVisitorBase<Void, Void>() {	1✔
183	@Override
184	public Void visit(ASTTypeParameters node, Void p) {
185	// add all params before visiting bounds
186	for (ASTTypeParameter parm : node) {	1✔
187	unusedTypeParams.add(parm.getTypeMirror());	1✔
188	}	1✔
189	return super.visit(node, p);	1✔
190	}
191
192	@Override
193	public Void visit(ASTClassType node, Void data) {
194	JTypeMirror ty = node.getTypeMirror();	1✔
195	if (ty instanceof JTypeVar) {	1✔
196	unusedTypeParams.remove(ty);	1✔
197	}
198	return super.visit(node, data);	1✔
199	}
200
201	@Override
202	public Void visit(ASTModifierList node, Void data) {
203	// no need to visit those
204	return data;	1✔
205	}
206	}, null);
207
208	return !unusedTypeParams.isEmpty();	1✔
209	}
210
211	private static boolean hasUnusedMethod(JavaNode node) {
212	Set<JExecutableSymbol> privateMethods = new HashSet<>();	1✔
213	for (ASTExecutableDeclaration decl : node.descendantsOrSelf()	1✔
214	.crossFindBoundaries()	1✔
215	.filterIs(ASTExecutableDeclaration.class)) {	1✔
216	Visibility visibility = decl.getEffectiveVisibility();	1✔
217	if (visibility.isAtMost(Visibility.V_PRIVATE)) {	1!
218	privateMethods.add(decl.getSymbol());	1✔
219	} else {
220	// We cannot know if non-private (package-private, protected, public) method is effectively used
NEW 221	return false;	×
222	}
223	}	1✔
224
225	return false;	1✔
226
227	// if (!privateMethods.isEmpty()) {
228	// node is a private method/ctor or is a class decl that contains only private methods
229	// TODO we need to somehow sync this with UnusedPrivateMethod and check, if these private
230	// methods are indeed unused or not. See also #5727.
231	// for now, we give up and assume, all private methods are used
232	// and always return false...
233	// }
234	}
235
236	@Override
237	protected OptionalBool isSuppressingNonPmdWarnings(String stringVal, ASTAnnotation annotation) {
238	if ("unused".equals(stringVal)) {	1✔
239	JavaNode scope = getAnnotationScope(annotation);	1✔
240	if (hasUnusedWarning(scope) == OptionalBool.NO) {	1✔
241	return OptionalBool.NO;	1✔
242	}
243	}
244	return super.isSuppressingNonPmdWarnings(stringVal, annotation);	1✔
245	}
246	}

pmd / pmd / 23

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous