hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Merge branch 'pr-5331'

Browse Source
This commit is contained in:
Juan Martín Sotuyo Dodero 2024-11-15 16:47:09 -03:00
commit f7de8d3e7e
5 changed files with 120 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
.all-contributorsrc
View File

@ -7883,6 +7883,15 @@
"contributions": [ "contributions": [
"code" "code"
] ]
},
{
"login": "VitaliiIevtushenko",
"name": "Vitalii Yevtushenko",
"avatar_url": "https://avatars.githubusercontent.com/u/11145125?v=4",
"profile": "https://github.com/VitaliiIevtushenko",
"contributions": [
"bug"
]
} }
], ],
"contributorsPerLine": 7, "contributorsPerLine": 7,

67
docs/pages/pmd/projectdocs/credits.md
View File

File diff suppressed because it is too large Load Diff

1
docs/pages/release_notes.md
View File

@ -27,6 +27,7 @@ This is a {{ site.pmd.release_type }} release.
* [#5329](https://github.com/pmd/pmd/issues/5329): \[java] Type inference issue with unknown method ref in call chain * [#5329](https://github.com/pmd/pmd/issues/5329): \[java] Type inference issue with unknown method ref in call chain
* java-bestpractices * java-bestpractices
* [#5083](https://github.com/pmd/pmd/issues/5083): \[java] UnusedPrivateMethod false positive when method reference has no target type * [#5083](https://github.com/pmd/pmd/issues/5083): \[java] UnusedPrivateMethod false positive when method reference has no target type
* [#5318](https://github.com/pmd/pmd/issues/5318): \[java] PreserveStackTraceRule: false-positive on Pattern Matching with instanceof
* java-performance * java-performance
* [#5314](https://github.com/pmd/pmd/issues/5314): \[java] InsufficientStringBufferDeclarationRule: Lack of handling for char type parameters * [#5314](https://github.com/pmd/pmd/issues/5314): \[java] InsufficientStringBufferDeclarationRule: Lack of handling for char type parameters

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