phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[java] PreserveStackTrace - consider instance type patterns

Browse Source 
Fixes #5318
This commit is contained in:
Andreas Dangel
2024-11-14 17:39:07 +01:00
parent bb2782241e
commit 61eb116833
3 changed files with 78 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/pages/release_notes.md
View File

@ -20,6 +20,8 @@ This is a {{ site.pmd.release_type }} release.
* java
* [#5293](https://github.com/pmd/pmd/issues/5293): \[java] Deadlock when executing PMD in multiple threads
* [#5324](https://github.com/pmd/pmd/issues/5324): \[java] Issue with type inference of nested lambdas
* java-bestpractices
* [#5318](https://github.com/pmd/pmd/issues/5318): \[java] PreserveStackTraceRule: false-positive on Pattern Matching with instanceof
### 🚨 API Changes

73
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java
View File

@ -4,11 +4,10 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.ast.NodeStream;
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.ast.ASTCastExpression;
@ -17,13 +16,17 @@ 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;
@ -64,7 +67,7 @@ public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
for (ASTThrowStatement throwStatement : catchStmt.getBody().descendants(ASTThrowStatement.class)) {
ASTExpression thrownExpr = throwStatement.getExpr();
if (!exprConsumesException(exceptionParam, thrownExpr, true)) {
if (!exprConsumesException(Collections.singleton(exceptionParam), thrownExpr, true)) {
asCtx(data).addViolation(thrownExpr, exceptionParam.getName());
}
}
@ -72,25 +75,39 @@ public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
return null;
}
private boolean exprConsumesException(ASTVariableId exceptionParam, ASTExpression expr, boolean mayBeSelf) {
private boolean exprConsumesException(Set<ASTVariableId> exceptionParams, ASTExpression expr, boolean mayBeSelf) {
if (expr instanceof ASTConstructorCall) {
// new Exception(e)
return ctorConsumesException(exceptionParam, (ASTConstructorCall) expr);
return ctorConsumesException(exceptionParams, (ASTConstructorCall) expr);
} else if (expr instanceof ASTMethodCall) {
return methodConsumesException(exceptionParam, (ASTMethodCall) expr);
return methodConsumesException(exceptionParams, (ASTMethodCall) expr);
} else if (expr instanceof ASTCastExpression) {
ASTExpression innermost = JavaAstUtils.peelCasts(expr);
return exprConsumesException(exceptionParam, innermost, mayBeSelf);
return exprConsumesException(exceptionParams, innermost, mayBeSelf);
} else if (expr instanceof ASTConditionalExpression) {
ASTConditionalExpression ternary = (ASTConditionalExpression) expr;
return exprConsumesException(exceptionParam, ternary.getThenBranch(), mayBeSelf)
&& exprConsumesException(exceptionParam, ternary.getElseBranch(), mayBeSelf);
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();
@ -99,7 +116,7 @@ public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
}
ASTVariableId decl = referencedSym.tryGetNode();
if (decl == exceptionParam) {
if (exceptionParams.contains(decl)) {
return mayBeSelf;
} else if (decl == null || decl.isFormalParameter() || decl.isField()) {
return false;
@ -113,16 +130,16 @@ public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
// if any of the initializer and usages consumes the variable,
// answer true.
if (exprConsumesException(exceptionParam, decl.getInitializer(), mayBeSelf)) {
if (exprConsumesException(exceptionParams, decl.getInitializer(), mayBeSelf)) {
return true;
}
for (ASTNamedReferenceExpr usage : decl.getLocalUsages()) {
if (assignmentRhsConsumesException(exceptionParam, decl, usage)) {
if (assignmentRhsConsumesException(exceptionParams, decl, usage)) {
return true;
}
if (JavaAstUtils.followingCallChain(usage).any(it -> consumesExceptionNonRecursive(exceptionParam, it))) {
if (JavaAstUtils.followingCallChain(usage).any(it -> consumesExceptionNonRecursive(exceptionParams, it))) {
return true;
}
}
@ -134,7 +151,7 @@ public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
}
}
private boolean assignmentRhsConsumesException(ASTVariableId exceptionParam, ASTVariableId lhsVariable, ASTNamedReferenceExpr usage) {
private boolean assignmentRhsConsumesException(Set<ASTVariableId> exceptionParams, ASTVariableId lhsVariable, ASTNamedReferenceExpr usage) {
if (usage.getIndexInParent() == 0) {
ASTExpression assignmentRhs = JavaAstUtils.getOtherOperandIfInAssignmentExpr(usage);
boolean rhsIsSelfReferential =
@ -142,25 +159,25 @@ public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
.descendantsOrSelf()
.filterIs(ASTVariableAccess.class)
.any(it -> JavaAstUtils.isReferenceToVar(it, lhsVariable.getSymbol()));
return !rhsIsSelfReferential && exprConsumesException(exceptionParam, assignmentRhs, true);
return !rhsIsSelfReferential && exprConsumesException(exceptionParams, assignmentRhs, true);
}
return false;
}
private boolean ctorConsumesException(ASTVariableId exceptionParam, ASTConstructorCall ctorCall) {
return ctorCall.isAnonymousClass() && callsInitCauseInAnonInitializer(exceptionParam, ctorCall)
|| anArgumentConsumesException(exceptionParam, ctorCall);
private boolean ctorConsumesException(Set<ASTVariableId> exceptionParams, ASTConstructorCall ctorCall) {
return ctorCall.isAnonymousClass() && callsInitCauseInAnonInitializer(exceptionParams, ctorCall)
|| anArgumentConsumesException(exceptionParams, ctorCall);
}
private boolean consumesExceptionNonRecursive(ASTVariableId exceptionParam, ASTExpression expr) {
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(ASTVariableId exceptionParam, ASTMethodCall call) {
if (anArgumentConsumesException(exceptionParam, call)) {
private boolean methodConsumesException(Set<ASTVariableId> exceptionParams, ASTMethodCall call) {
if (anArgumentConsumesException(exceptionParams, call)) {
return true;
}
ASTExpression qualifier = call.getQualifier();
@ -168,24 +185,24 @@ public class PreserveStackTraceRule extends AbstractJavaRulechainRule {
return false;
}
boolean mayBeSelf = ALLOWED_GETTERS.anyMatch(call);
return exprConsumesException(exceptionParam, qualifier, mayBeSelf);
return exprConsumesException(exceptionParams, qualifier, mayBeSelf);
}
private boolean callsInitCauseInAnonInitializer(ASTVariableId exceptionParam, ASTConstructorCall ctorCall) {
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(exceptionParam, it));
.any(it -> isInitCauseWithTargetInArg(exceptionParams, it));
}
private boolean isInitCauseWithTargetInArg(ASTVariableId exceptionSym, JavaNode expr) {
return INIT_CAUSE.matchesCall(expr) && anArgumentConsumesException(exceptionSym, (ASTMethodCall) expr);
private boolean isInitCauseWithTargetInArg(Set<ASTVariableId> exceptionParams, JavaNode expr) {
return INIT_CAUSE.matchesCall(expr) && anArgumentConsumesException(exceptionParams, (ASTMethodCall) expr);
}
private boolean anArgumentConsumesException(@NonNull ASTVariableId exceptionParam, InvocationNode thrownExpr) {
private boolean anArgumentConsumesException(Set<ASTVariableId> exceptionParams, InvocationNode thrownExpr) {
for (ASTExpression arg : ASTList.orEmptyStream(thrownExpr.getArguments())) {
if (exprConsumesException(exceptionParam, arg, true)) {
if (exprConsumesException(exceptionParams, arg, true)) {
return true;
}
}

31
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml
View File

@ -1168,4 +1168,35 @@ public class Foo {
]]></code>
</test-code>
<test-code>
<description>#5318 [java] PreserveStackTraceRule: false-positive on Pattern Matching with instanceof</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.function.Consumer;
public class Foo {
public void withPatternMatching(String a) {
try {
int i = Integer.parseInt(a);
} catch (RuntimeException e) {
throw e instanceof NumberFormatException numberFormatException
? formatExceptionHandler.accept(numberFormatException)
: e;
}
}
public void withExplicitCast(String a) {
try {
int i = Integer.parseInt(a);
} catch (RuntimeException e) {
throw e instanceof NumberFormatException
? formatExceptionHandler.accept((NumberFormatException) e)
: e;
}
}
private Consumer<NumberFormatException> formatExceptionHandler = e -> { e.printStackTrace(); };
}
]]></code>
</test-code>
</test-data>