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

94.41

/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.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.Collections;
import java.util.HashSet;
import java.util.Set;

import net.sourceforge.pmd.lang.ast.NodeStream;
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.ast.ASTCastExpression;
import net.sourceforge.pmd.lang.java.ast.ASTCatchClause;
import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression;
import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
import net.sourceforge.pmd.lang.java.ast.ASTList;
import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.ast.ASTPatternExpression;
import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
import net.sourceforge.pmd.lang.java.ast.ASTTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTTypePattern;
import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
import net.sourceforge.pmd.lang.java.ast.ASTVariableId;
import net.sourceforge.pmd.lang.java.ast.BinaryOp;
import net.sourceforge.pmd.lang.java.ast.InvocationNode;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.internal.JavaAstUtils;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher;
import net.sourceforge.pmd.reporting.RuleContext;

public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
    // todo dfa

    private static final InvocationMatcher INIT_CAUSE = InvocationMatcher.parse("java.lang.Throwable#initCause(_)");
    private static final CompoundInvocationMatcher ALLOWED_GETTERS = InvocationMatcher.parseAll(
        "java.lang.Throwable#fillInStackTrace()", // returns this
        "java.lang.reflect.InvocationTargetException#getTargetException()", // allowed, to unwrap reflection frames
        "java.lang.reflect.InvocationTargetException#getCause()", // this is equivalent to getTargetException, see javadoc
        // same rationale as for InvocationTargetException
        "java.security.PrivilegedActionException#getException()",
        "java.security.PrivilegedActionException#getCause()"
    );

    private final Set<ASTVariableId> recursingOnVars = new HashSet<>();

    public PreserveStackTraceRule() {
        super(ASTCatchClause.class);
    }

    @Override
    public Object visit(ASTCatchClause catchStmt, Object data) {
        RuleContext ctx = (RuleContext) data;

        ASTVariableId exceptionParam = catchStmt.getParameter().getVarId();
        if (JavaRuleUtil.isExplicitUnusedVarName(exceptionParam.getName())) {
            // ignore those
            return null;
        }

        // Inspect all the throw stmt inside the catch stmt
        for (ASTThrowStatement throwStatement : catchStmt.getBody().descendants(ASTThrowStatement.class)) {
            ASTExpression thrownExpr = throwStatement.getExpr();

            if (!exprConsumesException(Collections.singleton(exceptionParam), thrownExpr, true)) {
                ctx.addViolation(thrownExpr, exceptionParam.getName());
            }
        }
        recursingOnVars.clear();
        return null;
    }

    private boolean exprConsumesException(Set<ASTVariableId> exceptionParams, ASTExpression expr, boolean mayBeSelf) {
        if (expr instanceof ASTConstructorCall) {
            // new Exception(e)
            return ctorConsumesException(exceptionParams, (ASTConstructorCall) expr);

        } else if (expr instanceof ASTMethodCall) {

            return methodConsumesException(exceptionParams, (ASTMethodCall) expr);

        } else if (expr instanceof ASTCastExpression) {

            ASTExpression innermost = JavaAstUtils.peelCasts(expr);
            return exprConsumesException(exceptionParams, innermost, mayBeSelf);

        } else if (expr instanceof ASTConditionalExpression) {

            ASTConditionalExpression ternary = (ASTConditionalExpression) expr;
            Set<ASTVariableId> possibleExceptionParams = new HashSet<>(exceptionParams);

            // Peel out a type pattern variable in case this conditional is an instanceof pattern
            NodeStream.of(ternary.getCondition())
                    .filterIs(ASTInfixExpression.class)
                    .filterMatching(ASTInfixExpression::getOperator, BinaryOp.INSTANCEOF)
                    .map(ASTInfixExpression::getRightOperand)
                    .filterIs(ASTPatternExpression.class)
                    .map(ASTPatternExpression::getPattern)
                    .filterIs(ASTTypePattern.class)
                    .map(ASTTypePattern::getVarId)
                    .firstOpt()
                    .ifPresent(possibleExceptionParams::add);

            return exprConsumesException(possibleExceptionParams, ternary.getThenBranch(), mayBeSelf)
                && exprConsumesException(possibleExceptionParams, ternary.getElseBranch(), mayBeSelf);

        } else if (expr instanceof ASTVariableAccess) {
            JVariableSymbol referencedSym = ((ASTVariableAccess) expr).getReferencedSym();
            if (referencedSym == null) {
                return true; // invalid code, avoid FP
            }
            ASTVariableId decl = referencedSym.tryGetNode();

            if (exceptionParams.contains(decl)) {
                return mayBeSelf;
            } else if (decl == null || decl.isFormalParameter() || decl.isField()) {
                return false;
            }

            if (!this.recursingOnVars.add(decl)) {
                // already recursing on this variable, avoid stackoverflow
                return false;
            }

            // if any of the initializer and usages consumes the variable,
            // answer true.

            if (exprConsumesException(exceptionParams, decl.getInitializer(), mayBeSelf)) {
                return true;
            }

            for (ASTNamedReferenceExpr usage : decl.getLocalUsages()) {
                if (assignmentRhsConsumesException(exceptionParams, decl, usage)) {
                    return true;
                }

                if (JavaAstUtils.followingCallChain(usage).any(it -> consumesExceptionNonRecursive(exceptionParams, it))) {
                    return true;
                }
            }

            return false;
        } else {
            // assume it doesn't
            return false;
        }
    }

    private boolean assignmentRhsConsumesException(Set<ASTVariableId> exceptionParams, ASTVariableId lhsVariable, ASTNamedReferenceExpr usage) {
        if (usage.getIndexInParent() == 0) {
            ASTExpression assignmentRhs = JavaAstUtils.getOtherOperandIfInAssignmentExpr(usage);
            boolean rhsIsSelfReferential =
                NodeStream.of(assignmentRhs)
                          .descendantsOrSelf()
                          .filterIs(ASTVariableAccess.class)
                          .any(it -> JavaAstUtils.isReferenceToVar(it, lhsVariable.getSymbol()));
            return !rhsIsSelfReferential && exprConsumesException(exceptionParams, assignmentRhs, true);
        }
        return false;
    }

    private boolean ctorConsumesException(Set<ASTVariableId> exceptionParams, ASTConstructorCall ctorCall) {
        return ctorCall.isAnonymousClass() && callsInitCauseInAnonInitializer(exceptionParams, ctorCall)
            || anArgumentConsumesException(exceptionParams, ctorCall);
    }

    private boolean consumesExceptionNonRecursive(Set<ASTVariableId> exceptionParam, ASTExpression expr) {
        if (expr instanceof ASTConstructorCall) {
            return ctorConsumesException(exceptionParam, (ASTConstructorCall) expr);
        }
        return expr instanceof InvocationNode && anArgumentConsumesException(exceptionParam, (InvocationNode) expr);
    }

    private boolean methodConsumesException(Set<ASTVariableId> exceptionParams, ASTMethodCall call) {
        if (anArgumentConsumesException(exceptionParams, call)) {
            return true;
        }
        ASTExpression qualifier = call.getQualifier();
        if (qualifier == null) {
            return false;
        }
        boolean mayBeSelf = ALLOWED_GETTERS.anyMatch(call);
        return exprConsumesException(exceptionParams, qualifier, mayBeSelf);
    }

    private boolean callsInitCauseInAnonInitializer(Set<ASTVariableId> exceptionParams, ASTConstructorCall ctorCall) {
        return NodeStream.of(ctorCall.getAnonymousClassDeclaration())
                         .flatMap(ASTTypeDeclaration::getDeclarations)
                         .map(NodeStream.asInstanceOf(ASTFieldDeclaration.class, ASTInitializer.class))
                         .descendants().filterIs(ASTMethodCall.class)
                         .any(it -> isInitCauseWithTargetInArg(exceptionParams, it));
    }

    private boolean isInitCauseWithTargetInArg(Set<ASTVariableId> exceptionParams, JavaNode expr) {
        return INIT_CAUSE.matchesCall(expr) && anArgumentConsumesException(exceptionParams, (ASTMethodCall) expr);
    }

    private boolean anArgumentConsumesException(Set<ASTVariableId> exceptionParams, InvocationNode thrownExpr) {
        for (ASTExpression arg : ASTList.orEmptyStream(thrownExpr.getArguments())) {
            if (exprConsumesException(exceptionParams, arg, true)) {
                return true;
            }
        }
        return false;
    }

}

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	import java.util.Collections;
8	import java.util.HashSet;
9	import java.util.Set;
10
11	import net.sourceforge.pmd.lang.ast.NodeStream;
12	import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
13	import net.sourceforge.pmd.lang.java.ast.ASTCastExpression;
14	import net.sourceforge.pmd.lang.java.ast.ASTCatchClause;
15	import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
16	import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
17	import net.sourceforge.pmd.lang.java.ast.ASTExpression;
18	import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
19	import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression;
20	import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
21	import net.sourceforge.pmd.lang.java.ast.ASTList;
22	import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
23	import net.sourceforge.pmd.lang.java.ast.ASTPatternExpression;
24	import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
25	import net.sourceforge.pmd.lang.java.ast.ASTTypeDeclaration;
26	import net.sourceforge.pmd.lang.java.ast.ASTTypePattern;
27	import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
28	import net.sourceforge.pmd.lang.java.ast.ASTVariableId;
29	import net.sourceforge.pmd.lang.java.ast.BinaryOp;
30	import net.sourceforge.pmd.lang.java.ast.InvocationNode;
31	import net.sourceforge.pmd.lang.java.ast.JavaNode;
32	import net.sourceforge.pmd.lang.java.ast.internal.JavaAstUtils;
33	import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
34	import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
35	import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
36	import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
37	import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher;
38	import net.sourceforge.pmd.reporting.RuleContext;
39
40	public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
41	// todo dfa
42
43	private static final InvocationMatcher INIT_CAUSE = InvocationMatcher.parse("java.lang.Throwable#initCause(_)");	1✔
44	private static final CompoundInvocationMatcher ALLOWED_GETTERS = InvocationMatcher.parseAll(	1✔
45	"java.lang.Throwable#fillInStackTrace()", // returns this
46	"java.lang.reflect.InvocationTargetException#getTargetException()", // allowed, to unwrap reflection frames
47	"java.lang.reflect.InvocationTargetException#getCause()", // this is equivalent to getTargetException, see javadoc
48	// same rationale as for InvocationTargetException
49	"java.security.PrivilegedActionException#getException()",
50	"java.security.PrivilegedActionException#getCause()"
51	);
52
53	private final Set<ASTVariableId> recursingOnVars = new HashSet<>();	1✔
54
55	public PreserveStackTraceRule() {
56	super(ASTCatchClause.class);	1✔
57	}	1✔
58
59	@Override
60	public Object visit(ASTCatchClause catchStmt, Object data) {
61	RuleContext ctx = (RuleContext) data;	1✔
62
63	ASTVariableId exceptionParam = catchStmt.getParameter().getVarId();	1✔
64	if (JavaRuleUtil.isExplicitUnusedVarName(exceptionParam.getName())) {	1✔
65	// ignore those
66	return null;	1✔
67	}
68
69	// Inspect all the throw stmt inside the catch stmt
70	for (ASTThrowStatement throwStatement : catchStmt.getBody().descendants(ASTThrowStatement.class)) {	1✔
71	ASTExpression thrownExpr = throwStatement.getExpr();	1✔
72
73	if (!exprConsumesException(Collections.singleton(exceptionParam), thrownExpr, true)) {	1✔
74	ctx.addViolation(thrownExpr, exceptionParam.getName());	1✔
75	}
76	}	1✔
77	recursingOnVars.clear();	1✔
78	return null;	1✔
79	}
80
81	private boolean exprConsumesException(Set<ASTVariableId> exceptionParams, ASTExpression expr, boolean mayBeSelf) {
82	if (expr instanceof ASTConstructorCall) {	1✔
83	// new Exception(e)
84	return ctorConsumesException(exceptionParams, (ASTConstructorCall) expr);	1✔
85
86	} else if (expr instanceof ASTMethodCall) {	1✔
87
88	return methodConsumesException(exceptionParams, (ASTMethodCall) expr);	1✔
89
90	} else if (expr instanceof ASTCastExpression) {	1✔
91
92	ASTExpression innermost = JavaAstUtils.peelCasts(expr);	1✔
93	return exprConsumesException(exceptionParams, innermost, mayBeSelf);	1✔
94
95	} else if (expr instanceof ASTConditionalExpression) {	1✔
96
97	ASTConditionalExpression ternary = (ASTConditionalExpression) expr;	1✔
98	Set<ASTVariableId> possibleExceptionParams = new HashSet<>(exceptionParams);	1✔
99
100	// Peel out a type pattern variable in case this conditional is an instanceof pattern
101	NodeStream.of(ternary.getCondition())	1✔
102	.filterIs(ASTInfixExpression.class)	1✔
103	.filterMatching(ASTInfixExpression::getOperator, BinaryOp.INSTANCEOF)	1✔
104	.map(ASTInfixExpression::getRightOperand)	1✔
105	.filterIs(ASTPatternExpression.class)	1✔
106	.map(ASTPatternExpression::getPattern)	1✔
107	.filterIs(ASTTypePattern.class)	1✔
108	.map(ASTTypePattern::getVarId)	1✔
109	.firstOpt()	1✔
110	.ifPresent(possibleExceptionParams::add);	1✔
111
112	return exprConsumesException(possibleExceptionParams, ternary.getThenBranch(), mayBeSelf)	1✔
113	&& exprConsumesException(possibleExceptionParams, ternary.getElseBranch(), mayBeSelf);	1!
114
115	} else if (expr instanceof ASTVariableAccess) {	1✔
116	JVariableSymbol referencedSym = ((ASTVariableAccess) expr).getReferencedSym();	1✔
117	if (referencedSym == null) {	1✔
118	return true; // invalid code, avoid FP	1✔
119	}
120	ASTVariableId decl = referencedSym.tryGetNode();	1✔
121
122	if (exceptionParams.contains(decl)) {	1✔
123	return mayBeSelf;	1✔
124	} else if (decl == null \|\| decl.isFormalParameter() \|\| decl.isField()) {	1!
125	return false;	1✔
126	}
127
128	if (!this.recursingOnVars.add(decl)) {	1✔
129	// already recursing on this variable, avoid stackoverflow
130	return false;	1✔
131	}
132
133	// if any of the initializer and usages consumes the variable,
134	// answer true.
135
136	if (exprConsumesException(exceptionParams, decl.getInitializer(), mayBeSelf)) {	1✔
137	return true;	1✔
138	}
139
140	for (ASTNamedReferenceExpr usage : decl.getLocalUsages()) {	1✔
141	if (assignmentRhsConsumesException(exceptionParams, decl, usage)) {	1✔
142	return true;	1✔
143	}
144
145	if (JavaAstUtils.followingCallChain(usage).any(it -> consumesExceptionNonRecursive(exceptionParams, it))) {	1✔
146	return true;	1✔
147	}
148	}	1✔
149
150	return false;	1✔
151	} else {
152	// assume it doesn't
153	return false;	1✔
154	}
155	}
156
157	private boolean assignmentRhsConsumesException(Set<ASTVariableId> exceptionParams, ASTVariableId lhsVariable, ASTNamedReferenceExpr usage) {
158	if (usage.getIndexInParent() == 0) {	1✔
159	ASTExpression assignmentRhs = JavaAstUtils.getOtherOperandIfInAssignmentExpr(usage);	1✔
160	boolean rhsIsSelfReferential =	1✔
161	NodeStream.of(assignmentRhs)	1✔
162	.descendantsOrSelf()	1✔
163	.filterIs(ASTVariableAccess.class)	1✔
164	.any(it -> JavaAstUtils.isReferenceToVar(it, lhsVariable.getSymbol()));	1✔
165	return !rhsIsSelfReferential && exprConsumesException(exceptionParams, assignmentRhs, true);	1✔
166	}
167	return false;	1✔
168	}
169
170	private boolean ctorConsumesException(Set<ASTVariableId> exceptionParams, ASTConstructorCall ctorCall) {
171	return ctorCall.isAnonymousClass() && callsInitCauseInAnonInitializer(exceptionParams, ctorCall)	1!
172	\|\| anArgumentConsumesException(exceptionParams, ctorCall);	1✔
173	}
174
175	private boolean consumesExceptionNonRecursive(Set<ASTVariableId> exceptionParam, ASTExpression expr) {
176	if (expr instanceof ASTConstructorCall) {	1!
UNCOV 177	return ctorConsumesException(exceptionParam, (ASTConstructorCall) expr);	×
178	}
179	return expr instanceof InvocationNode && anArgumentConsumesException(exceptionParam, (InvocationNode) expr);	1!
180	}
181
182	private boolean methodConsumesException(Set<ASTVariableId> exceptionParams, ASTMethodCall call) {
183	if (anArgumentConsumesException(exceptionParams, call)) {	1✔
184	return true;	1✔
185	}
186	ASTExpression qualifier = call.getQualifier();	1✔
187	if (qualifier == null) {	1✔
188	return false;	1✔
189	}
190	boolean mayBeSelf = ALLOWED_GETTERS.anyMatch(call);	1✔
191	return exprConsumesException(exceptionParams, qualifier, mayBeSelf);	1✔
192	}
193
194	private boolean callsInitCauseInAnonInitializer(Set<ASTVariableId> exceptionParams, ASTConstructorCall ctorCall) {
195	return NodeStream.of(ctorCall.getAnonymousClassDeclaration())	1✔
196	.flatMap(ASTTypeDeclaration::getDeclarations)	1✔
197	.map(NodeStream.asInstanceOf(ASTFieldDeclaration.class, ASTInitializer.class))	1✔
198	.descendants().filterIs(ASTMethodCall.class)	1✔
199	.any(it -> isInitCauseWithTargetInArg(exceptionParams, it));	1✔
200	}
201
202	private boolean isInitCauseWithTargetInArg(Set<ASTVariableId> exceptionParams, JavaNode expr) {
203	return INIT_CAUSE.matchesCall(expr) && anArgumentConsumesException(exceptionParams, (ASTMethodCall) expr);	1!
204	}
205
206	private boolean anArgumentConsumesException(Set<ASTVariableId> exceptionParams, InvocationNode thrownExpr) {
207	for (ASTExpression arg : ASTList.orEmptyStream(thrownExpr.getArguments())) {	1✔
208	if (exprConsumesException(exceptionParams, arg, true)) {	1✔
209	return true;	1✔
210	}
211	}	1✔
212	return false;	1✔
213	}
214
215	}

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