pmd / pmd / 722

Committed 14 Aug 2026 11:10AM UTC coverage: 79.301% (+0.008%) from 79.293%

Build # 722

Build Type

push

github

Committed by

web-flow

Commit Message

[java] Fix #6611: UnnecessaryVarargsArrayCreation ignores overload ambiguity (#6949)

* [java] UnnecessaryVarargsArrayCreation: suppress when removing array creates overload ambiguity

When an explicit array creation (e.g. new String[]{...}) is passed to a
varargs parameter, the rule suggested removing it. But when the callee
has another overload that is also applicable to the expanded varargs
argument list, removing the array would make the call ambiguous (or
select a different overload), so the explicit array is required for
overload resolution and the suggested fix does not compile.

Before reporting, check the sibling overloads of the selected executable:
if any of them is applicable (fixed- or variable-arity) to the argument
list obtained by expanding the array initializer into varargs elements,
suppress the violation. Constructors are enumerated from the selected
executable's own class (exhaustive, since constructors are not inherited);
methods are streamed from the declaring type via
JTypeMirror.streamMethods, which includes overloads inherited from
supertypes.

The applicability check is a conservative approximation of JLS
15.12.2.2-4 based on subtyping, treating generic parameter types by
their upper bound.

Fixes #6611

Signed-off-by: 付典 <fudianchn@gmail.com>

* import order

---------

Signed-off-by: 付典 <fudianchn@gmail.com>
Co-authored-by: Sören Glimm <git@uncleowen.de>

Coverage Stats

19503 of 25562 branches covered (76.3%)

Branch coverage included in aggregate %.

48 of 59 new or added lines in 1 file covered. (81.36%)

27 existing lines in 10 files now uncovered.

42260 of 52322 relevant lines covered (80.77%)

0.82 hits per line

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

95.42

/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentRule.java

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

package net.sourceforge.pmd.lang.java.rule.bestpractices;


import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Set;

import org.checkerframework.checker.nullness.qual.Nullable;

import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement;
import net.sourceforge.pmd.lang.java.ast.ASTForeachStatement;
import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTVariableId;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.UnaryOp;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass;
import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.AssignmentEntry;
import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.DataflowResult;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
import net.sourceforge.pmd.reporting.RuleContext;

public class UnusedAssignmentRule extends AbstractJavaRulechainRule {

    /*
        Detects unused assignments. This performs a reaching definition
        analysis. This makes the assumption that there is no dead code.

        Since we have the reaching definitions at each variable usage, we
        could also use that to detect other kinds of bug, eg conditions
        that are always true, or dereferences that will always NPE. In
        the general case though, this is complicated and better left to
        a DFA library, eg google Z3.

        This analysis may be used as-is to detect switch labels that
        fall-through, which could be useful to improve accuracy of other
        rules.

        TODO
           * labels on arbitrary statements (currently only loops)
           * explicit ctor call (hard to impossible without type res,
             or at least proper graph algorithms like toposort)
                -> this is pretty invisible as it causes false negatives, not FPs
           * test ternary expr
           * more precise exception handling: since we have access to
             the overload for method & ctors, we can know where its thrown
             exceptions may end up in enclosing catches.
           * extract the reaching definition analysis, to exploit control
           flow information in rules + symbol table. The following are needed
           to implement scoping of pattern variables, and are already computed
           by this analysis:
             * whether a switch may fall through
             * whether a statement always completes abruptly
             * whether a statement never completes abruptly because of break


        DONE
           * conditionals
           * loops
           * switch
           * loop labels
           * try/catch/finally
           * lambdas
           * constructors + initializers
           * anon class
           * test this.field in ctors
           * foreach var should be reassigned from one iter to another
           * test local class/anonymous class
           * shortcut conditionals have their own control-flow
           * parenthesized expressions
           * conditional exprs in loops
           * ignore variables that start with 'ignore'
           * ignore params of native methods
           * ignore params of abstract methods

     */

    private static final PropertyDescriptor<Boolean> CHECK_PREFIX_INCREMENT =
        PropertyFactory.booleanProperty("checkUnusedPrefixIncrement")
                       .desc("Report expressions like ++i that may be replaced with (i + 1)")
                       .defaultValue(false)
                       .build();

    private static final PropertyDescriptor<Boolean> REPORT_UNUSED_VARS =
        PropertyFactory.booleanProperty("reportUnusedVariables")
                       .desc("Report variables that are only initialized, and never read at all. "
                                 + "The rule UnusedVariable already cares for that, but you can enable it if needed")
                       .defaultValue(false)
                       .build();

    public UnusedAssignmentRule() {
        super(ASTCompilationUnit.class);
        definePropertyDescriptor(CHECK_PREFIX_INCREMENT);
        definePropertyDescriptor(REPORT_UNUSED_VARS);
    }

    @Override
    public Object visit(ASTCompilationUnit node, Object data) {
        RuleContext ctx = (RuleContext) data;

        DataflowResult result = DataflowPass.getDataflowResult(node);
        reportFinished(result, ctx);
        return null;
    }

    private void reportFinished(DataflowResult result, RuleContext ruleCtx) {

        for (AssignmentEntry entry : result.getUnusedAssignments()) {
            if (entry.isUnaryReassign() && isIgnorablePrefixIncrement(entry.getLocation())) {
                continue;
            }

            Set<AssignmentEntry> killers = result.getKillers(entry);
            final String reason;
            if (killers.isEmpty()) {
                // var went out of scope before being used (no assignment kills it, yet it's unused)

                if (entry.isField()) {
                    // assignments to fields don't really go out of scope
                    continue;
                } else if (suppressUnusedVariableRuleOverlap(entry)) {
                    // see REPORT_UNUSED_VARS property
                    continue;
                }
                // This is a "DU" anomaly, the others are "DD"
                reason = null;
            } else if (killers.size() == 1) {
                AssignmentEntry k = killers.iterator().next();
                if (k.getLocation().equals(entry.getLocation())) {
                    // assignment reassigns itself, only possible in a loop
                    if (suppressUnusedVariableRuleOverlap(entry)) {
                        continue;
                    } else if (entry.isForeachVar()) {
                        reason = null;
                    } else {
                        reason = "reassigned every iteration";
                    }
                } else {
                    reason = "overwritten on line " + k.getLine();
                }
            } else {
                reason = joinLines("overwritten on lines ", killers);
            }
            if (reason == null && JavaRuleUtil.isExplicitUnusedVarName(entry.getVarId().getName())) {
                // Then the variable is never used (cf UnusedVariable)
                // We ignore those that start with "ignored", as that is standard
                // practice for exceptions, and may be useful for resources/foreach vars
                continue;
            }
            ruleCtx.addViolationWithMessage(entry.getLocation(), makeMessage(entry, reason, entry.isField()));
        }
    }

    private boolean suppressUnusedVariableRuleOverlap(AssignmentEntry entry) {
        return !getProperty(REPORT_UNUSED_VARS) && (entry.isInitializer() || entry.isBlankDeclaration());
    }

    private static String getKind(ASTVariableId id) {
        if (id.isField()) {
            return "field";
        } else if (id.isResourceDeclaration()) {
            return "resource";
        } else if (id.isExceptionBlockParameter()) {
            return "exception parameter";
        } else if (id.ancestors().get(2) instanceof ASTForeachStatement) {
            return "loop variable";
        } else if (id.isFormalParameter()) {
            return "parameter";
        }
        return "variable";
    }

    private boolean isIgnorablePrefixIncrement(JavaNode assignment) {
        if (assignment instanceof ASTUnaryExpression) {
            // the variable value is used if it was found somewhere else
            // than in statement position
            UnaryOp op = ((ASTUnaryExpression) assignment).getOperator();
            return !getProperty(CHECK_PREFIX_INCREMENT) && !op.isPure() && op.isPrefix()
                && !(assignment.getParent() instanceof ASTExpressionStatement);
        }
        return false;
    }

    private static String makeMessage(AssignmentEntry assignment, @Nullable String reason, boolean isField) {
        // if reason is null, then the variable is unused (at most assigned to)

        StringBuilder result = new StringBuilder(64);
        if (assignment.isInitializer()) {
            result.append(isField ? "the field initializer for"
                                  : "the initializer for variable");
        } else if (assignment.isBlankDeclaration()) {
            if (reason != null) {
                result.append("the initial value of ");
            }
            result.append(getKind(assignment.getVarId()));
        } else { // regular assignment
            if (assignment.isUnaryReassign()) {
                result.append("the updated value of ");
            } else {
                result.append("the value assigned to ");
            }
            result.append(isField ? "field" : "variable");
        }
        result.append(" ''").append(assignment.getVarId().getName()).append("''");
        result.append(" is never used");
        if (reason != null) {
            result.append(" (").append(reason).append(")");
        }
        result.setCharAt(0, Character.toUpperCase(result.charAt(0)));
        return result.toString();
    }

    private static String joinLines(String prefix, Set<AssignmentEntry> killers) {
        StringBuilder sb = new StringBuilder(prefix);
        List<AssignmentEntry> sorted = new ArrayList<>(killers);
        sorted.sort(Comparator.naturalOrder());

        sb.append(sorted.get(0).getLine());
        for (int i = 1; i < sorted.size() - 1; i++) {
            sb.append(", ").append(sorted.get(i).getLine());
        }
        sb.append(" and ").append(sorted.get(sorted.size() - 1).getLine());

        return sb.toString();
    }

}

1	/*
2	* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
3	*/
4
5	package net.sourceforge.pmd.lang.java.rule.bestpractices;
6
7
8	import java.util.ArrayList;
9	import java.util.Comparator;
10	import java.util.List;
11	import java.util.Set;
12
13	import org.checkerframework.checker.nullness.qual.Nullable;
14
15	import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
16	import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement;
17	import net.sourceforge.pmd.lang.java.ast.ASTForeachStatement;
18	import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression;
19	import net.sourceforge.pmd.lang.java.ast.ASTVariableId;
20	import net.sourceforge.pmd.lang.java.ast.JavaNode;
21	import net.sourceforge.pmd.lang.java.ast.UnaryOp;
22	import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
23	import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass;
24	import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.AssignmentEntry;
25	import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.DataflowResult;
26	import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
27	import net.sourceforge.pmd.properties.PropertyDescriptor;
28	import net.sourceforge.pmd.properties.PropertyFactory;
29	import net.sourceforge.pmd.reporting.RuleContext;
30
31	public class UnusedAssignmentRule extends AbstractJavaRulechainRule {
32
33	/*
34	Detects unused assignments. This performs a reaching definition
35	analysis. This makes the assumption that there is no dead code.
36
37	Since we have the reaching definitions at each variable usage, we
38	could also use that to detect other kinds of bug, eg conditions
39	that are always true, or dereferences that will always NPE. In
40	the general case though, this is complicated and better left to
41	a DFA library, eg google Z3.
42
43	This analysis may be used as-is to detect switch labels that
44	fall-through, which could be useful to improve accuracy of other
45	rules.
46
47	TODO
48	* labels on arbitrary statements (currently only loops)
49	* explicit ctor call (hard to impossible without type res,
50	or at least proper graph algorithms like toposort)
51	-> this is pretty invisible as it causes false negatives, not FPs
52	* test ternary expr
53	* more precise exception handling: since we have access to
54	the overload for method & ctors, we can know where its thrown
55	exceptions may end up in enclosing catches.
56	* extract the reaching definition analysis, to exploit control
57	flow information in rules + symbol table. The following are needed
58	to implement scoping of pattern variables, and are already computed
59	by this analysis:
60	* whether a switch may fall through
61	* whether a statement always completes abruptly
62	* whether a statement never completes abruptly because of break
63
64
65	DONE
66	* conditionals
67	* loops
68	* switch
69	* loop labels
70	* try/catch/finally
71	* lambdas
72	* constructors + initializers
73	* anon class
74	* test this.field in ctors
75	* foreach var should be reassigned from one iter to another
76	* test local class/anonymous class
77	* shortcut conditionals have their own control-flow
78	* parenthesized expressions
79	* conditional exprs in loops
80	* ignore variables that start with 'ignore'
81	* ignore params of native methods
82	* ignore params of abstract methods
83
84	*/
85
86	private static final PropertyDescriptor<Boolean> CHECK_PREFIX_INCREMENT =	1✔
87	PropertyFactory.booleanProperty("checkUnusedPrefixIncrement")	1✔
88	.desc("Report expressions like ++i that may be replaced with (i + 1)")	1✔
89	.defaultValue(false)	1✔
90	.build();	1✔
91
92	private static final PropertyDescriptor<Boolean> REPORT_UNUSED_VARS =	1✔
93	PropertyFactory.booleanProperty("reportUnusedVariables")	1✔
94	.desc("Report variables that are only initialized, and never read at all. "	1✔
95	+ "The rule UnusedVariable already cares for that, but you can enable it if needed")
96	.defaultValue(false)	1✔
97	.build();	1✔
98
99	public UnusedAssignmentRule() {
100	super(ASTCompilationUnit.class);	1✔
101	definePropertyDescriptor(CHECK_PREFIX_INCREMENT);	1✔
102	definePropertyDescriptor(REPORT_UNUSED_VARS);	1✔
103	}	1✔
104
105	@Override
106	public Object visit(ASTCompilationUnit node, Object data) {
107	RuleContext ctx = (RuleContext) data;	1✔
108
109	DataflowResult result = DataflowPass.getDataflowResult(node);	1✔
110	reportFinished(result, ctx);	1✔
111	return null;	1✔
112	}
113
114	private void reportFinished(DataflowResult result, RuleContext ruleCtx) {
115
116	for (AssignmentEntry entry : result.getUnusedAssignments()) {	1✔
117	if (entry.isUnaryReassign() && isIgnorablePrefixIncrement(entry.getLocation())) {	1✔
118	continue;	1✔
119	}
120
121	Set<AssignmentEntry> killers = result.getKillers(entry);	1✔
122	final String reason;
123	if (killers.isEmpty()) {	1✔
124	// var went out of scope before being used (no assignment kills it, yet it's unused)
125
126	if (entry.isField()) {	1✔
127	// assignments to fields don't really go out of scope
128	continue;	1✔
129	} else if (suppressUnusedVariableRuleOverlap(entry)) {	1✔
130	// see REPORT_UNUSED_VARS property
131	continue;	1✔
132	}
133	// This is a "DU" anomaly, the others are "DD"
134	reason = null;	1✔
135	} else if (killers.size() == 1) {	1✔
136	AssignmentEntry k = killers.iterator().next();	1✔
137	if (k.getLocation().equals(entry.getLocation())) {	1✔
138	// assignment reassigns itself, only possible in a loop
139	if (suppressUnusedVariableRuleOverlap(entry)) {	1✔
140	continue;	1✔
141	} else if (entry.isForeachVar()) {	1✔
142	reason = null;	1✔
143	} else {
144	reason = "reassigned every iteration";	1✔
145	}
146	} else {
147	reason = "overwritten on line " + k.getLine();	1✔
148	}
149	} else {	1✔
150	reason = joinLines("overwritten on lines ", killers);	1✔
151	}
152	if (reason == null && JavaRuleUtil.isExplicitUnusedVarName(entry.getVarId().getName())) {	1✔
153	// Then the variable is never used (cf UnusedVariable)
154	// We ignore those that start with "ignored", as that is standard
155	// practice for exceptions, and may be useful for resources/foreach vars
156	continue;	1✔
157	}
158	ruleCtx.addViolationWithMessage(entry.getLocation(), makeMessage(entry, reason, entry.isField()));	1✔
159	}	1✔
160	}	1✔
161
162	private boolean suppressUnusedVariableRuleOverlap(AssignmentEntry entry) {
163	return !getProperty(REPORT_UNUSED_VARS) && (entry.isInitializer() \|\| entry.isBlankDeclaration());	1✔
164	}
165
166	private static String getKind(ASTVariableId id) {
167	if (id.isField()) {	1!
168	return "field";	×
169	} else if (id.isResourceDeclaration()) {	1!
UNCOV 170	return "resource";	×
171	} else if (id.isExceptionBlockParameter()) {	1✔
172	return "exception parameter";	1✔
173	} else if (id.ancestors().get(2) instanceof ASTForeachStatement) {	1✔
174	return "loop variable";	1✔
175	} else if (id.isFormalParameter()) {	1✔
176	return "parameter";	1✔
177	}
178	return "variable";	1✔
179	}
180
181	private boolean isIgnorablePrefixIncrement(JavaNode assignment) {
182	if (assignment instanceof ASTUnaryExpression) {	1!
183	// the variable value is used if it was found somewhere else
184	// than in statement position
185	UnaryOp op = ((ASTUnaryExpression) assignment).getOperator();	1✔
186	return !getProperty(CHECK_PREFIX_INCREMENT) && !op.isPure() && op.isPrefix()	1!
187	&& !(assignment.getParent() instanceof ASTExpressionStatement);	1✔
188	}
UNCOV 189	return false;	×
190	}
191
192	private static String makeMessage(AssignmentEntry assignment, @Nullable String reason, boolean isField) {
193	// if reason is null, then the variable is unused (at most assigned to)
194
195	StringBuilder result = new StringBuilder(64);	1✔
196	if (assignment.isInitializer()) {	1✔
197	result.append(isField ? "the field initializer for"	1✔
198	: "the initializer for variable");	1✔
199	} else if (assignment.isBlankDeclaration()) {	1✔
200	if (reason != null) {	1✔
201	result.append("the initial value of ");	1✔
202	}
203	result.append(getKind(assignment.getVarId()));	1✔
204	} else { // regular assignment
205	if (assignment.isUnaryReassign()) {	1✔
206	result.append("the updated value of ");	1✔
207	} else {
208	result.append("the value assigned to ");	1✔
209	}
210	result.append(isField ? "field" : "variable");	1✔
211	}
212	result.append(" ''").append(assignment.getVarId().getName()).append("''");	1✔
213	result.append(" is never used");	1✔
214	if (reason != null) {	1✔
215	result.append(" (").append(reason).append(")");	1✔
216	}
217	result.setCharAt(0, Character.toUpperCase(result.charAt(0)));	1✔
218	return result.toString();	1✔
219	}
220
221	private static String joinLines(String prefix, Set<AssignmentEntry> killers) {
222	StringBuilder sb = new StringBuilder(prefix);	1✔
223	List<AssignmentEntry> sorted = new ArrayList<>(killers);	1✔
224	sorted.sort(Comparator.naturalOrder());	1✔
225
226	sb.append(sorted.get(0).getLine());	1✔
227	for (int i = 1; i < sorted.size() - 1; i++) {	1✔
228	sb.append(", ").append(sorted.get(i).getLine());	1✔
229	}
230	sb.append(" and ").append(sorted.get(sorted.size() - 1).getLine());	1✔
231
232	return sb.toString();	1✔
233	}
234
235	}

pmd / pmd / 722

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