diff --git a/.all-contributorsrc b/.all-contributorsrc index 0af52570b4..72a0c129d0 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -7866,6 +7866,33 @@ "contributions": [ "code" ] + }, + { + "login": "chenguangqi", + "name": "天热吃西瓜", + "avatar_url": "https://avatars.githubusercontent.com/u/6231010?v=4", + "profile": "http://chenguangqi.github.io/", + "contributions": [ + "bug" + ] + }, + { + "login": "wahajenius", + "name": "Willem A. Hajenius", + "avatar_url": "https://avatars.githubusercontent.com/u/7836322?v=4", + "profile": "https://github.com/wahajenius", + "contributions": [ + "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, diff --git a/Gemfile.lock b/Gemfile.lock index 1f7805ea50..41dfc5878a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -75,7 +75,7 @@ GEM racc (1.8.1) rchardet (1.8.0) rexml (3.3.9) - rouge (4.4.0) + rouge (4.5.0) rufus-scheduler (3.9.2) fugit (~> 1.1, >= 1.11.1) safe_yaml (1.0.5) diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock index 85d32da6ff..c65178abcd 100644 --- a/docs/Gemfile.lock +++ b/docs/Gemfile.lock @@ -266,7 +266,7 @@ GEM concurrent-ruby (~> 1.0) unicode-display_width (1.8.0) uri (0.13.1) - webrick (1.8.2) + webrick (1.9.0) PLATFORMS x86_64-linux diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 3c14674217..496f8a26cb 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -17,6 +17,21 @@ This is a {{ site.pmd.release_type }} release. ### 🐛 Fixed Issues * ant * [#1860](https://github.com/pmd/pmd/issues/1860): \[ant] Reflective access warnings on java > 9 and java < 17 +* apex + * [#5333](https://github.com/pmd/pmd/issues/5333): \[apex] Token recognition errors for string containing unicode escape sequence +* html + * [#5322](https://github.com/pmd/pmd/issues/5322): \[html] CPD throws exception on when HTML file is missing closing tag +* 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 + * [#5329](https://github.com/pmd/pmd/issues/5329): \[java] Type inference issue with unknown method ref in call chain +* java-bestpractices + * [#5083](https://github.com/pmd/pmd/issues/5083): \[java] UnusedPrivateMethod false positive when method reference has no target type + * [#5097](https://github.com/pmd/pmd/issues/5097): \[java] UnusedPrivateMethod FP with raw type missing from the classpath + * [#5318](https://github.com/pmd/pmd/issues/5318): \[java] PreserveStackTraceRule: false-positive on Pattern Matching with instanceof +* java-performance + * [#5287](https://github.com/pmd/pmd/issues/5287): \[java] TooFewBranchesForSwitch false-positive with switch using list of case constants + * [#5314](https://github.com/pmd/pmd/issues/5314): \[java] InsufficientStringBufferDeclarationRule: Lack of handling for char type parameters ### 🚨 API Changes @@ -26,6 +41,7 @@ This is a {{ site.pmd.release_type }} release. instead (note different package `ast` instead of `antlr4`). ### ✨ External Contributions +* [#5284](https://github.com/pmd/pmd/pull/5284): \[apex] Use case-insensitive input stream to avoid choking on Unicode escape sequences - [Willem A. Hajenius](https://github.com/wahajenius) (@wahajenius) {% endtocmaker %} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentBuilder.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentBuilder.java index a135ca4603..7cc9e23f8d 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentBuilder.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentBuilder.java @@ -14,14 +14,19 @@ import java.util.List; import java.util.Map; import java.util.RandomAccess; +import org.antlr.v4.runtime.BaseErrorListener; import org.antlr.v4.runtime.CharStreams; +import org.antlr.v4.runtime.RecognitionException; +import org.antlr.v4.runtime.Recognizer; import org.antlr.v4.runtime.Token; import net.sourceforge.pmd.annotation.InternalApi; +import net.sourceforge.pmd.lang.ast.LexException; import net.sourceforge.pmd.lang.document.TextDocument; import net.sourceforge.pmd.lang.document.TextRegion; import io.github.apexdevtools.apexparser.ApexLexer; +import io.github.apexdevtools.apexparser.CaseInsensitiveInputStream; @InternalApi final class ApexCommentBuilder { @@ -103,7 +108,15 @@ final class ApexCommentBuilder { } private static CommentInformation extractInformationFromComments(TextDocument sourceCode, String suppressMarker) { - ApexLexer lexer = new ApexLexer(CharStreams.fromString(sourceCode.getText().toString())); + String source = sourceCode.getText().toString(); + ApexLexer lexer = new ApexLexer(new CaseInsensitiveInputStream(CharStreams.fromString(source))); + lexer.removeErrorListeners(); + lexer.addErrorListener(new BaseErrorListener() { + @Override + public void syntaxError(Recognizer recognizer, Object offendingSymbol, int line, int charPositionInLine, String msg, RecognitionException e) { + throw new LexException(line, charPositionInLine, sourceCode.getFileId(), msg, e); + } + }); List allCommentTokens = new ArrayList<>(); Map suppressMap = new HashMap<>(); diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentTest.java index 728cce6253..6e847170bf 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexCommentTest.java @@ -66,4 +66,12 @@ class ApexCommentTest extends ApexParserTestBase { ASTFormalComment comment = file.descendants(ASTUserClass.class).children(ASTFormalComment.class).first(); assertEquals(FORMAL_COMMENT_CONTENT, comment.getImage()); } + + @Test + void fileWithUnicodeEscapes() { + ASTApexFile file = apex.parse(FORMAL_COMMENT_CONTENT + "\n" + + "class MyClass { String s = 'Fran\\u00E7ois'; }"); + ASTFormalComment comment = file.descendants(ASTUserClass.class).children(ASTFormalComment.class).first(); + assertEquals(FORMAL_COMMENT_CONTENT, comment.getImage()); + } } diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexLexerTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexLexerTest.java index 67c6706f29..22104ac401 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexLexerTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexLexerTest.java @@ -8,14 +8,18 @@ package net.sourceforge.pmd.lang.apex.ast; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import org.antlr.v4.runtime.BaseErrorListener; import org.antlr.v4.runtime.CharStream; import org.antlr.v4.runtime.CharStreams; import org.antlr.v4.runtime.CommonTokenStream; +import org.antlr.v4.runtime.RecognitionException; +import org.antlr.v4.runtime.Recognizer; import org.antlr.v4.runtime.Token; import org.junit.jupiter.api.Test; import io.github.apexdevtools.apexparser.ApexLexer; import io.github.apexdevtools.apexparser.ApexParser; +import io.github.apexdevtools.apexparser.CaseInsensitiveInputStream; /** * This is an exploration test for {@link ApexLexer}. @@ -49,4 +53,36 @@ class ApexLexerTest { ApexParser.CompilationUnitContext compilationUnit = parser.compilationUnit(); assertNotNull(compilationUnit); } + + @Test + void testLexerUnicodeEscapes() { + String s = "'Fran\\u00E7ois'"; + // note: with apex-parser 4.3.1, no errors are reported anymore + assertEquals(2, getLexingErrors(CharStreams.fromString(s))); + assertEquals(0, getLexingErrors(new CaseInsensitiveInputStream(CharStreams.fromString(s)))); + } + + private int getLexingErrors(CharStream stream) { + ApexLexer lexer = new ApexLexer(stream); + ErrorListener errorListener = new ErrorListener(); + lexer.removeErrorListeners(); // Avoid distracting "token recognition error" stderr output + lexer.addErrorListener(errorListener); + CommonTokenStream tokens = new CommonTokenStream(lexer); + tokens.fill(); + return errorListener.getErrorCount(); + } + + private static class ErrorListener extends BaseErrorListener { + private int errorCount = 0; + + @Override + public void syntaxError(Recognizer recognizer, Object offendingSymbol, int line, + int charPositionInLine, String msg, RecognitionException e) { + ++errorCount; + } + + public int getErrorCount() { + return errorCount; + } + } } diff --git a/pmd-dist/pom.xml b/pmd-dist/pom.xml index 7bfaf114e5..5a40e514e2 100644 --- a/pmd-dist/pom.xml +++ b/pmd-dist/pom.xml @@ -178,7 +178,7 @@ org.apache.commons commons-compress - 1.26.0 + 1.27.1 test diff --git a/pmd-html/src/main/java/net/sourceforge/pmd/lang/html/ast/LineNumbers.java b/pmd-html/src/main/java/net/sourceforge/pmd/lang/html/ast/LineNumbers.java index 423b2772fb..5419b30e3a 100644 --- a/pmd-html/src/main/java/net/sourceforge/pmd/lang/html/ast/LineNumbers.java +++ b/pmd-html/src/main/java/net/sourceforge/pmd/lang/html/ast/LineNumbers.java @@ -50,15 +50,14 @@ class LineNumbers { nextIndex = determineLocation((AbstractHtmlNode) child, nextIndex); } - // autoclosing element, eg - boolean isAutoClose = n.getNumChildren() == 0 - && n instanceof ASTHtmlElement - // nextIndex is up to the closing > at this point - && htmlString.startsWith("/>", nextIndex - 2); + // explicitly closing element, eg. + boolean hasCloseElement = n instanceof ASTHtmlElement + // nextIndex is up to the closing tag at this point + && htmlString.startsWith("", nextIndex); if (n instanceof ASTHtmlDocument) { nextIndex = htmlString.length(); - } else if (n instanceof ASTHtmlElement && !isAutoClose) { + } else if (n instanceof ASTHtmlElement && hasCloseElement) { nextIndex += 2 + n.getXPathNodeName().length() + 1; // } else if (n instanceof ASTHtmlComment) { nextIndex += 4 + 3; // diff --git a/pmd-html/src/test/java/net/sourceforge/pmd/lang/html/cpd/HtmlCpdLexerTest.java b/pmd-html/src/test/java/net/sourceforge/pmd/lang/html/cpd/HtmlCpdLexerTest.java index 04db5c6152..088837d2ce 100644 --- a/pmd-html/src/test/java/net/sourceforge/pmd/lang/html/cpd/HtmlCpdLexerTest.java +++ b/pmd-html/src/test/java/net/sourceforge/pmd/lang/html/cpd/HtmlCpdLexerTest.java @@ -21,4 +21,13 @@ class HtmlCpdLexerTest extends CpdTextComparisonTest { doTest("SimpleHtmlFile"); } + @Test + void invalidHtml() { + doTest("InvalidHtml"); + } + + @Test + void metaTag() { + doTest("MetaTag"); + } } diff --git a/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/InvalidHtml.html b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/InvalidHtml.html new file mode 100644 index 0000000000..73acc68e1d --- /dev/null +++ b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/InvalidHtml.html @@ -0,0 +1,7 @@ + + + + +
+ + diff --git a/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/InvalidHtml.txt b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/InvalidHtml.txt new file mode 100644 index 0000000000..2cc097ffd2 --- /dev/null +++ b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/InvalidHtml.txt @@ -0,0 +1,22 @@ + [Image] or [Truncated image[ Bcol Ecol +L1 + [#document] 1 8 + [#doctype] 1 15 + [\n] 16 16 +L2 + [html] 1 7 + [\n] 17 17 +L3 + [body] 1 7 + [\n] 7 7 +L4 + [#comment] 1 36 + [\n] 37 37 +L5 + [div] 1 22 + [\n] 22 22 +L6 + [\n] 8 8 +L7 + [\n] 8 8 +EOF diff --git a/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/MetaTag.html b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/MetaTag.html new file mode 100644 index 0000000000..d8a96810a8 --- /dev/null +++ b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/MetaTag.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/MetaTag.txt b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/MetaTag.txt new file mode 100644 index 0000000000..0547e117b8 --- /dev/null +++ b/pmd-html/src/test/resources/net/sourceforge/pmd/lang/html/cpd/testdata/MetaTag.txt @@ -0,0 +1,27 @@ + [Image] or [Truncated image[ Bcol Ecol +L1 + [#document] 1 8 + [#doctype] 1 15 + [\n] 16 16 +L2 + [html] 1 7 + [\n] 17 17 +L3 + [head] 1 7 + [\n ] 7 4 +L4 + [#comment] 5 66 + [\n ] 67 4 +L5 + [meta] 5 27 + [\n] 27 27 +L6 + [\n] 8 8 +L7 + [body] 1 7 + [\n] 7 7 +L8 + [\n] 8 8 +L9 + [\n] 8 8 +EOF diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java index 03d0d9faea..229e8a5da0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java @@ -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 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 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 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 exceptionParams, ASTConstructorCall ctorCall) { + return ctorCall.isAnonymousClass() && callsInitCauseInAnonInitializer(exceptionParams, ctorCall) + || anArgumentConsumesException(exceptionParams, ctorCall); } - private boolean consumesExceptionNonRecursive(ASTVariableId exceptionParam, ASTExpression expr) { + private boolean consumesExceptionNonRecursive(Set 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 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 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 exceptionParams, JavaNode expr) { + return INIT_CAUSE.matchesCall(expr) && anArgumentConsumesException(exceptionParams, (ASTMethodCall) expr); } - private boolean anArgumentConsumesException(@NonNull ASTVariableId exceptionParam, InvocationNode thrownExpr) { + private boolean anArgumentConsumesException(Set exceptionParams, InvocationNode thrownExpr) { for (ASTExpression arg : ASTList.orEmptyStream(thrownExpr.getArguments())) { - if (exprConsumesException(exceptionParam, arg, true)) { + if (exprConsumesException(exceptionParams, arg, true)) { return true; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java index a6fabf91a2..5acd063c15 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java @@ -240,6 +240,12 @@ public class InsufficientStringBufferDeclarationRule extends AbstractJavaRulecha private int calculateExpression(ASTExpression expression) { Object value = expression.getConstValue(); - return value == null ? State.UNKNOWN_CAPACITY : (Integer) value; + if (value == null) { + return State.UNKNOWN_CAPACITY; + } + if (value instanceof Character) { + return (Character) value; + } + return (Integer) value; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java index 6a050ca5b3..15ae7b0535 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java @@ -84,12 +84,7 @@ final class ClassStub implements JClassSymbol, AsmStub, AnnotationOwner { this.resolver = resolver; this.names = new Names(internalName); - this.parseLock = new ParseLock() { - // note to devs: to debug the parsing logic you might have - // to replace the implementation of toString temporarily, - // otherwise an IDE could call toString just to show the item - // in the debugger view (which could cause parsing of the class file). - + this.parseLock = new ParseLock("ClassStub:" + internalName) { @Override protected boolean doParse() throws IOException { try (InputStream instream = loader.getInputStream()) { @@ -315,9 +310,9 @@ final class ClassStub implements JClassSymbol, AsmStub, AnnotationOwner { } @Override - public boolean isGeneric() { + public int getTypeParameterCount() { parseLock.ensureParsed(); - return signature.isGeneric(); + return signature.getTypeParameterCount(); } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/GenericSigBase.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/GenericSigBase.java index 6784468646..eabeb13c5a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/GenericSigBase.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/GenericSigBase.java @@ -46,9 +46,9 @@ abstract class GenericSigBase { protected List typeParameters; private final ParseLock lock; - protected GenericSigBase(T ctx) { + protected GenericSigBase(T ctx, String parseLockName) { this.ctx = ctx; - this.lock = new ParseLock() { + this.lock = new ParseLock(parseLockName) { @Override protected boolean doParse() { GenericSigBase.this.doParse(); @@ -81,7 +81,11 @@ abstract class GenericSigBase { protected abstract boolean postCondition(); - protected abstract boolean isGeneric(); + protected abstract int getTypeParameterCount(); + + protected boolean isGeneric() { + return getTypeParameterCount() > 0; + } public void setTypeParams(List tvars) { assert this.typeParameters == null : "Type params were already parsed for " + this; @@ -105,6 +109,7 @@ abstract class GenericSigBase { private static final String OBJECT_BOUND = ":" + OBJECT_SIG; private final @Nullable String signature; + private final int typeParameterCount; private @Nullable JClassType superType; private List superItfs; @@ -116,8 +121,9 @@ abstract class GenericSigBase { @Nullable String signature, // null if doesn't use generics in header @Nullable String superInternalName, // null if this is the Object class String[] interfaces) { - super(ctx); + super(ctx, "LazyClassSignature:" + ctx.getInternalName() + "[" + signature + "]"); this.signature = signature; + this.typeParameterCount = GenericTypeParameterCounter.determineTypeParameterCount(this.signature); this.rawItfs = CollectionUtil.map(interfaces, ctx.getResolver()::resolveFromInternalNameCannotFail); this.rawSuper = ctx.getResolver().resolveFromInternalNameCannotFail(superInternalName); @@ -157,8 +163,9 @@ abstract class GenericSigBase { } @Override - protected boolean isGeneric() { - return signature != null && TypeParamsParser.hasTypeParams(signature); + protected int getTypeParameterCount() { + // note: no ensureParsed() needed, the type parameters are counted eagerly + return typeParameterCount; } @Override @@ -206,6 +213,7 @@ abstract class GenericSigBase { static class LazyMethodType extends GenericSigBase implements TypeAnnotationReceiver { private final @NonNull String signature; + private final int typeParameterCount; private @Nullable TypeAnnotationSet receiverAnnotations; private List parameterTypes; @@ -233,8 +241,9 @@ abstract class GenericSigBase { @Nullable String genericSig, @Nullable String[] exceptions, boolean skipFirstParam) { - super(ctx); + super(ctx, "LazyMethodType:" + (genericSig != null ? genericSig : descriptor)); this.signature = genericSig != null ? genericSig : descriptor; + this.typeParameterCount = GenericTypeParameterCounter.determineTypeParameterCount(genericSig); // generic signatures already omit the synthetic param this.skipFirstParam = skipFirstParam && genericSig == null; this.rawExceptions = exceptions; @@ -288,8 +297,9 @@ abstract class GenericSigBase { @Override - protected boolean isGeneric() { - return TypeParamsParser.hasTypeParams(signature); + protected int getTypeParameterCount() { + // note: no ensureParsed() needed, the type parameters are counted eagerly + return typeParameterCount; } void setParameterTypes(List params) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/GenericTypeParameterCounter.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/GenericTypeParameterCounter.java new file mode 100644 index 0000000000..28f309c43b --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/GenericTypeParameterCounter.java @@ -0,0 +1,36 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.symbols.internal.asm; + +import org.objectweb.asm.signature.SignatureReader; +import org.objectweb.asm.signature.SignatureVisitor; + +class GenericTypeParameterCounter extends SignatureVisitor { + private int count; + + GenericTypeParameterCounter() { + super(AsmSymbolResolver.ASM_API_V); + } + + @Override + public void visitFormalTypeParameter(String name) { + count++; + } + + public int getCount() { + return count; + } + + static int determineTypeParameterCount(String signature) { + if (signature == null) { + return 0; + } + + SignatureReader signatureReader = new SignatureReader(signature); + GenericTypeParameterCounter counter = new GenericTypeParameterCounter(); + signatureReader.accept(counter); + return counter.getCount(); + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ModuleStub.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ModuleStub.java index 3db93664d3..1c0b33b0d2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ModuleStub.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ModuleStub.java @@ -35,7 +35,7 @@ class ModuleStub implements JModuleSymbol, AsmStub, AnnotationOwner { this.resolver = resolver; this.moduleName = moduleName; - this.parseLock = new ParseLock() { + this.parseLock = new ParseLock("ModuleStub:" + moduleName) { @Override protected boolean doParse() throws IOException { try (InputStream instream = loader.getInputStream()) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ParseLock.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ParseLock.java index a7bfada5ea..a0150e00fe 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ParseLock.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ParseLock.java @@ -17,15 +17,30 @@ abstract class ParseLock { private static final Logger LOG = LoggerFactory.getLogger(ParseLock.class); private volatile ParseStatus status = ParseStatus.NOT_PARSED; + private final String name; + + protected ParseLock(String name) { + this.name = name; + } public void ensureParsed() { getFinalStatus(); } + private void logParseLockTrace(String prefix) { + if (LOG.isTraceEnabled()) { + LOG.trace("{} {}: {}", Thread.currentThread().getName(), String.format("%-15s", prefix), this); + } + } + private ParseStatus getFinalStatus() { ParseStatus status = this.status; if (!status.isFinished) { + logParseLockTrace("waiting on"); + synchronized (this) { + logParseLockTrace("locked"); + status = this.status; if (status == ParseStatus.NOT_PARSED) { this.status = ParseStatus.BEING_PARSED; @@ -54,6 +69,7 @@ abstract class ParseLock { throw new IllegalStateException("Thread is reentering the parse lock"); } } + logParseLockTrace("released"); } return status; } @@ -85,7 +101,7 @@ abstract class ParseLock { @Override public String toString() { - return "ParseLock{status=" + status + '}'; + return "ParseLock{name=" + name + ",status=" + status + '}'; } private enum ParseStatus { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java index a6ac882819..7f976967d8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java @@ -717,6 +717,12 @@ public final class TypeOps { // no unchecked warning. return allArgsAreUnboundedWildcards(sargs) ? Convertibility.UNCHECKED_NO_WARNING : Convertibility.UNCHECKED_WARNING; + } else if (sargs.isEmpty()) { + // C <: |C| + // JLS 4.10.2 + // unchecked conversion converts a raw type to a generic type + // subtyping converts a generic type to its raw type + return Convertibility.SUBTYPING; } if (targs.size() != sargs.size()) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprCheckHelper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprCheckHelper.java index c5c20caec9..f6521426e2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprCheckHelper.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprCheckHelper.java @@ -335,6 +335,13 @@ final class ExprCheckHelper { checker.checkExprConstraint(infCtx, capture(r2), r); } completeMethodRefInference(mref, nonWildcard, fun, exactMethod, true); + } else if (TypeOps.isUnresolved(mref.getTypeToSearch())) { + // Then this is neither an exact nor inexact method ref, + // we just don't know what it is. + + // The return values of the mref are assimilated to an (*unknown*) type. + checker.checkExprConstraint(infCtx, ts.UNKNOWN, fun.getReturnType()); + completeMethodRefInference(mref, nonWildcard, fun, ts.UNRESOLVED_METHOD, false); } else { // Otherwise, the method reference is inexact, and: @@ -552,8 +559,15 @@ final class ExprCheckHelper { // finally, add bounds if (result != ts.NO_TYPE) { + Set inputIvars = infCtx.freeVarsIn(groundFun.getFormalParameters()); + // The free vars of the return type depend on the free vars of the parameters. + // This explicit dependency is there to prevent solving the variables in the + // return type before solving those of the parameters. That is because the variables + // mentioned in the return type may be further constrained by adding the return constraints + // below (in the listener), which is only triggered when the input ivars have been instantiated. + infCtx.addInstantiationDependencies(infCtx.freeVarsIn(groundFun.getReturnType()), inputIvars); infCtx.addInstantiationListener( - infCtx.freeVarsIn(groundFun.getFormalParameters()), + inputIvars, solvedCtx -> { if (mayMutateExpr()) { lambda.setInferredType(solvedCtx.ground(groundTargetType)); @@ -562,8 +576,15 @@ final class ExprCheckHelper { lambda.updateTypingContext(solvedGroundFun); } JTypeMirror groundResult = solvedCtx.ground(result); + // We need to build another checker that uses the solved context. + // This is because the free vars may have been adopted by a parent + // context, so the solvedCtx may be that parent context. The checks + // must use that context so that constraints and listeners are added + // to the parent context, since that one is responsible for solving + // the variables. + ExprCheckHelper newChecker = new ExprCheckHelper(solvedCtx, phase, this.checker, site, infer); for (ExprMirror expr : lambda.getResultExpressions()) { - if (!isCompatible(groundResult, expr)) { + if (!newChecker.isCompatible(groundResult, expr)) { return; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprOps.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprOps.java index 8ea1cee3da..a04c91c55f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprOps.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/ExprOps.java @@ -227,7 +227,6 @@ final class ExprOps { } } else { JClassType enclosing = mref.getEnclosingType(); - accessible = mref.getTypeToSearch() .streamMethods(TypeOps.accessibleMethodFilter(mref.getMethodName(), enclosing.getSymbol())) .collect(OverloadSet.collectMostSpecific(enclosing)); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/Infer.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/Infer.java index 02f16611d5..e51ef905f2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/Infer.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/Infer.java @@ -36,7 +36,6 @@ import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.CtorInvocat import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.FunctionalExprMirror; import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.InvocationMirror; import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.InvocationMirror.MethodCtDecl; -import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.LambdaExprMirror; import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.MethodRefMirror; import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.PolyExprMirror; import net.sourceforge.pmd.lang.java.types.internal.infer.InferenceVar.BoundKind; @@ -145,13 +144,19 @@ public final class Infer { LOG.logResolutionFail(rfe.getFailure()); // here we set expected if not null, the lambda will have the target type expr.setInferredType(expected == null ? ts.UNKNOWN : expected); + expr.setFunctionalMethod(ts.UNRESOLVED_METHOD); if (expr instanceof MethodRefMirror) { MethodRefMirror mref = (MethodRefMirror) expr; - mref.setFunctionalMethod(ts.UNRESOLVED_METHOD); + if (!TypeOps.isUnresolved(mref.getTypeToSearch())) { + JMethodSig exactMethod = ExprOps.getExactMethod(mref); + if (exactMethod != null) { + // as a fallback, if the method reference is exact, + // we populate the compile time decl anyway. + mref.setCompileTimeDecl(exactMethod); + return; + } + } mref.setCompileTimeDecl(ts.UNRESOLVED_METHOD); - } else { - LambdaExprMirror lambda = (LambdaExprMirror) expr; - lambda.setFunctionalMethod(ts.UNRESOLVED_METHOD); } } } @@ -602,17 +607,20 @@ public final class Infer { // see: https://docs.oracle.com/javase/specs/jls/se9/html/jls-18.html#jls-18.5.1 // as per https://docs.oracle.com/javase/specs/jls/se9/html/jls-18.html#jls-18.5.2 // we only test it can reduce, we don't commit inferred types at this stage - InferenceContext ctxCopy = infCtx.copy(); - LOG.applicabilityTest(ctxCopy, m); - ctxCopy.solve(/*onlyBoundedVars:*/isPreJava8()); - + InferenceContext ctxCopy = infCtx.shallowCopy(); + LOG.applicabilityTest(ctxCopy); + try { + ctxCopy.solve(/*onlyBoundedVars:*/isPreJava8()); + } finally { + LOG.finishApplicabilityTest(); + } // if unchecked conversion was needed, update the site for invocation pass if (ctxCopy.needsUncheckedConversion()) { site.setNeedsUncheckedConversion(); } // don't commit any types - return m; + return infCtx.mapToIVars(m); } } finally { // Note that even if solve succeeded, listeners checking deferred diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceContext.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceContext.java index ba3a78640e..8cc251fa0e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceContext.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceContext.java @@ -13,11 +13,13 @@ import java.util.Collection; import java.util.Collections; import java.util.Deque; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.function.Supplier; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -38,6 +40,7 @@ import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.Pr import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.SubstituteInst; import net.sourceforge.pmd.lang.java.types.internal.infer.InferenceVar.BoundKind; import net.sourceforge.pmd.lang.java.types.internal.infer.VarWalkStrategy.GraphWalk; +import net.sourceforge.pmd.util.CollectionUtil; /** * Context of a type inference process. This object maintains a set of @@ -51,6 +54,13 @@ final class InferenceContext { private static int ctxId = 0; private final Map> instantiationListeners = new HashMap<>(); + // explicit dependencies between variables for graph building + private final Map> instantiationConstraints = new HashMap<>(); + // This flag is set to true when the explicit dependencies are changed, + // or when this context adopted new ivars. This means we should interrupt + // resolution and recompute the dependency graph between ivars, because + // the new variables may have dependencies on existing variables, and vice versa. + private boolean graphWasChanged = false; private final Set freeVars = new LinkedHashSet<>(); private final Set inferenceVars = new LinkedHashSet<>(); @@ -127,18 +137,19 @@ final class InferenceContext { } } - public InferenceContext copy() { + /** + * Performs a shallow copy of this context, which would allow solving + * the variables without executing listeners. Instantiation listeners + * are not copied, and parent contexts are not copied. + */ + public InferenceContext shallowCopy() { final InferenceContext copy = new InferenceContext(ts, supertypeCheckCache, Collections.emptyList(), logger); copy.freeVars.addAll(this.freeVars); copy.inferenceVars.addAll(this.inferenceVars); copy.incorporationActions.addAll(this.incorporationActions); + copy.instantiationConstraints.putAll(this.instantiationConstraints); copy.mapping = mapping; // mapping is immutable, so we can share it safely - // recursively copy parents… - if (this.parent != null) { - copy.parent = this.parent.copy(); - } - return copy; } @@ -310,10 +321,20 @@ final class InferenceContext { * Copy variable in this inference context to the given context */ void duplicateInto(final InferenceContext that) { + boolean changedGraph = !that.freeVars.containsAll(this.freeVars) + || !this.instantiationConstraints.isEmpty(); + that.graphWasChanged |= changedGraph; that.inferenceVars.addAll(this.inferenceVars); that.freeVars.addAll(this.freeVars); that.incorporationActions.addAll(this.incorporationActions); that.instantiationListeners.putAll(this.instantiationListeners); + CollectionUtil.mergeMaps( + that.instantiationConstraints, + this.instantiationConstraints, + (set1, set2) -> { + set1.addAll(set2); + return set1; + }); this.parent = that; @@ -324,6 +345,30 @@ final class InferenceContext { } + // The `from` ivars depend on the `dependencies` ivars for resolution. + void addInstantiationDependencies(Set from, Set dependencies) { + if (from.isEmpty()) { + return; + } + Set outputVars = new HashSet<>(dependencies); + outputVars.removeAll(from); + if (outputVars.isEmpty()) { + return; + } + for (InferenceVar inputVar : from) { + logger.ivarDependencyRegistered(this, inputVar, outputVars); + instantiationConstraints.merge(inputVar, outputVars, (o1, o2) -> { + o2 = new LinkedHashSet<>(o2); + o2.addAll(o1); + return o2; + }); + } + } + + Map> getInstantiationDependencies() { + return instantiationConstraints; + } + void addInstantiationListener(Set relevantTypes, InstantiationListener listener) { Set free = freeVarsIn(relevantTypes); if (free.isEmpty()) { @@ -448,7 +493,7 @@ final class InferenceContext { } boolean solve(boolean onlyBoundedVars) { - return solve(new GraphWalk(this, onlyBoundedVars)); + return solve(() -> new GraphWalk(this, onlyBoundedVars)); } /** @@ -459,7 +504,28 @@ final class InferenceContext { solve(new GraphWalk(var)); } + + private boolean solve(Supplier newWalker) { + VarWalkStrategy strategy = newWalker.get(); + while (strategy != null) { + if (solve(strategy)) { + break; + } + strategy = newWalker.get(); + } + return freeVars.isEmpty(); + } + + + /** + * This returns true if solving the VarWalkStrategy succeeded entirely. + * Resolution can be interrupted early to account for new ivars and dependencies, + * which may change the graph dependencies. In this case this method returns + * false, we recompute the graph with the new ivars and dependencies, and + * we try again to make progress. + */ private boolean solve(VarWalkStrategy walker) { + graphWasChanged = false; incorporate(); while (walker.hasNext()) { @@ -470,6 +536,12 @@ final class InferenceContext { //repeat until all variables are solved outer: while (!intersect(freeVars, varsToSolve).isEmpty() && progress) { + if (graphWasChanged) { + graphWasChanged = false; + logger.contextDependenciesChanged(this); + return false; + } + progress = false; for (List wave : ReductionStep.WAVES) { if (solveBatchProgressed(varsToSolve, wave)) { @@ -481,7 +553,7 @@ final class InferenceContext { } } } - return freeVars.isEmpty(); + return true; } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/TypeInferenceLogger.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/TypeInferenceLogger.java index 023e422a49..cb7fe3e6d3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/TypeInferenceLogger.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/TypeInferenceLogger.java @@ -12,6 +12,7 @@ import java.util.ArrayDeque; import java.util.Deque; import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -21,6 +22,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.internal.JavaLanguageProperties; import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol; import net.sourceforge.pmd.lang.java.types.JMethodSig; import net.sourceforge.pmd.lang.java.types.JTypeMirror; @@ -33,6 +35,12 @@ import net.sourceforge.pmd.util.StringUtil; /** * A strategy to log the execution traces of {@link Infer}. + * The default does nothing, so the logger calls can be optimized out + * at runtime, while not having to check that logging is enabled at the + * call sites. + * + *

To enable logging for the CLI, use the language property ({@link JavaLanguageProperties}) + * {@code xTypeInferenceLogging}. From tests, see {@code JavaParsingHelper#logTypeInferenceVerbose()}. */ @SuppressWarnings("PMD.UncommentedEmptyMethodBody") public interface TypeInferenceLogger { @@ -61,7 +69,9 @@ public interface TypeInferenceLogger { default void ctxInitialization(InferenceContext ctx, JMethodSig sig) { } - default void applicabilityTest(InferenceContext ctx, JMethodSig sig) { } + default void applicabilityTest(InferenceContext ctx) { } + + default void finishApplicabilityTest() { } default void startArgsChecks() { } @@ -81,6 +91,8 @@ public interface TypeInferenceLogger { default void propagateAndAbort(InferenceContext context, InferenceContext parent) { } + default void contextDependenciesChanged(InferenceContext ctx) { } + // ivar events @@ -90,6 +102,8 @@ public interface TypeInferenceLogger { default void ivarInstantiated(InferenceContext ctx, InferenceVar var, JTypeMirror inst) { } + default void ivarDependencyRegistered(InferenceContext ctx, InferenceVar var, Set deps) { } + /** * Log that the instantiation of the method type m for the given @@ -136,9 +150,11 @@ public interface TypeInferenceLogger { protected final PrintStream out; - protected static final int LEVEL_INCREMENT = 4; - private int level; private String indent; + /** + * Four spaces. + */ + protected static final String BASE_INDENT = " "; protected static final String ANSI_RESET = "\u001B[0m"; protected static final String ANSI_BLUE = "\u001B[34m"; @@ -177,16 +193,24 @@ public interface TypeInferenceLogger { public SimpleLogger(PrintStream out) { this.out = out; - updateLevel(0); + this.indent = ""; } - protected int getLevel() { - return level; + protected void addIndentSegment(String segment) { + indent += segment; } - protected void updateLevel(int increment) { - level += increment; - indent = StringUtils.repeat(' ', level); + protected void removeIndentSegment(String segment) { + assert indent.endsWith(segment) : "mismatched end section!"; + indent = StringUtils.removeEnd(indent, segment); + } + + protected void setIndent(String indent) { + this.indent = indent; + } + + protected String getIndent() { + return indent; } protected void println(String str) { @@ -196,13 +220,13 @@ public interface TypeInferenceLogger { protected void endSection(String footer) { - updateLevel(-LEVEL_INCREMENT); + removeIndentSegment(BASE_INDENT); println(footer); } protected void startSection(String header) { println(header); - updateLevel(+LEVEL_INCREMENT); + addIndentSegment(BASE_INDENT); } @Override @@ -335,7 +359,7 @@ public interface TypeInferenceLogger { class VerboseLogger extends SimpleLogger { - private final Deque marks = new ArrayDeque<>(); + private final Deque marks = new ArrayDeque<>(); public VerboseLogger(PrintStream out) { super(out); @@ -343,16 +367,16 @@ public interface TypeInferenceLogger { } void mark() { - marks.push(getLevel()); + marks.push(getIndent()); } void rollback(String lastWords) { - int pop = marks.pop(); - updateLevel(pop - getLevel()); // back to normal + final String savedIndent = marks.pop(); + setIndent(savedIndent); // back to normal if (!lastWords.isEmpty()) { - updateLevel(+LEVEL_INCREMENT); + addIndentSegment(BASE_INDENT); println(lastWords); - updateLevel(-LEVEL_INCREMENT); + setIndent(savedIndent); } } @@ -369,8 +393,14 @@ public interface TypeInferenceLogger { } @Override - public void applicabilityTest(InferenceContext ctx, JMethodSig sig) { - println(String.format("Applicability testing with Context %-11d%s", ctx.getId(), ppHighlight(ctx.mapToIVars(sig)))); + public void applicabilityTest(InferenceContext ctx) { + println(String.format("Solving with context %d for applicability testing", ctx.getId())); + addIndentSegment("| "); + } + + @Override + public void finishApplicabilityTest() { + removeIndentSegment("| "); } @Override @@ -404,7 +434,7 @@ public interface TypeInferenceLogger { @Override public void startArg(int i, ExprMirror expr, JTypeMirror formalType) { - startSection("Checking arg " + i + " against " + formalType); + startSection("Checking arg " + i + " against " + colorIvars(formalType)); printExpr(expr); } @@ -452,6 +482,16 @@ public interface TypeInferenceLogger { println(addCtxInfo(ctx, "Ivar instantiated") + color(var + " := ", ANSI_BLUE) + colorIvars(inst)); } + @Override + public void ivarDependencyRegistered(InferenceContext ctx, InferenceVar var, Set deps) { + println(addCtxInfo(ctx, "Ivar dependency registered: ") + color(var + " -> ", ANSI_BLUE) + colorIvars(deps)); + } + + @Override + public void contextDependenciesChanged(InferenceContext ctx) { + println("Recomputing dependency graph (ctx " + ctx.getId() + ")"); + } + private @NonNull String addCtxInfo(InferenceContext ctx, String event) { return String.format("%-20s(ctx %d): ", event, ctx.getId()); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/VarWalkStrategy.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/VarWalkStrategy.java index 2177a1cf92..19774ca49e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/VarWalkStrategy.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/VarWalkStrategy.java @@ -90,6 +90,14 @@ interface VarWalkStrategy extends Iterator> { } } + ctx.getInstantiationDependencies().forEach((ivar, deps) -> { + Vertex vertex = graph.addLeaf(ivar); + for (InferenceVar dep : deps) { + Vertex target = graph.addLeaf(dep); + graph.addEdge(vertex, target); + } + }); + // Here, "α depends on β" is modelled by an edge α -> β // Merge strongly connected components into a "super node". diff --git a/pmd-java/src/main/resources/category/java/performance.xml b/pmd-java/src/main/resources/category/java/performance.xml index a8e4c37142..8b2d4e20c5 100644 --- a/pmd-java/src/main/resources/category/java/performance.xml +++ b/pmd-java/src/main/resources/category/java/performance.xml @@ -629,7 +629,7 @@ Note: This rule was named TooFewBranchesForASwitchStatement before PMD 7.7.0. diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symbols/DeadlockTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symbols/DeadlockTest.java new file mode 100644 index 0000000000..15358be481 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symbols/DeadlockTest.java @@ -0,0 +1,157 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.symbols; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import net.sourceforge.pmd.lang.java.JavaParsingHelper; +import net.sourceforge.pmd.lang.java.ast.ASTClassType; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; + +/** + * Tests to help analyze [java] Deadlock when executing PMD in multiple threads #5293. + * + * @see [java] Deadlock when executing PMD in multiple threads #5293 + */ +class DeadlockTest { + + abstract static class Outer implements GenericInterface, GenericClass> { + // must be a nested class, that is reusing the type param T of the outer class + abstract static class Inner { + Inner(Outer grid) { } + } + } + + static class GenericBaseClass { } + + interface GenericInterface { } + + abstract static class GenericClass extends GenericBaseClass> { } + + @Test + @Timeout(2) + void parseWithoutDeadlock() throws InterruptedException { + /* + Deadlock: + t1 -> locks parse for Outer.Inner and waits for parse lock for Outer + ├─ t1 waiting on : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer$Inner[Ljava/lang/Object;],status=NOT_PARSED} + └─ t1 locked : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer$Inner[Ljava/lang/Object;],status=NOT_PARSED} + └─ t1 waiting on : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer[Ljava/lang/Object;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericInterface;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass;>;],status=BEING_PARSED} + t2 -> locks parse for Outer, locks parse for GenericInterface and then waits for parse lock for Outer.Inner + ├─ t2 waiting on : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer[Ljava/lang/Object;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericInterface;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass;>;],status=NOT_PARSED} + └─ t2 locked : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer[Ljava/lang/Object;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericInterface;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass;>;],status=NOT_PARSED} + ├─ t2 waiting on : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest[null],status=NOT_PARSED} + ├─ t2 locked : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest[null],status=NOT_PARSED} + ├─ t2 released : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest[null],status=FULL} + ├─ t2 waiting on : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer[Ljava/lang/Object;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericInterface;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass;>;],status=BEING_PARSED} + ├─ t2 locked : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer[Ljava/lang/Object;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericInterface;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass;>;],status=BEING_PARSED} + ├─ t2 released : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer[Ljava/lang/Object;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericInterface;Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass;>;],status=BEING_PARSED} + └─ t2 waiting on : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass[Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericBaseClass;>;],status=NOT_PARSED} + ├─ t2 locked : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericClass[Lnet/sourceforge/pmd/lang/java/symbols/DeadlockTest$GenericBaseClass;>;],status=NOT_PARSED} + └─ t2 waiting on : ParseLock{name=LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer$Inner[Ljava/lang/Object;],status=NOT_PARSED} + + + In order to reproduce the deadlock reliably, add the following piece into ParseLock, just at the beginning + of the synchronized block (line 42): + + // t1 needs to wait after having the lock, so that t2 can go on and wait on the same lock + if (Thread.currentThread().getName().equals("t1") && this.toString().contains("LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer$Inner[ exceptions = new ArrayList<>(); + Thread.UncaughtExceptionHandler exceptionHandler = (t, e) -> { + exceptions.add(e); + e.printStackTrace(); + }; + + Thread t1 = new Thread(() -> { + ASTCompilationUnit class1 = JavaParsingHelper.DEFAULT.parse( + "package net.sourceforge.pmd.lang.java.symbols;\n" + + "import net.sourceforge.pmd.lang.java.symbols.DeadlockTest.Outer;\n" + + " class Class1 {\n" + + " public static Outer.Inner newInner(Outer grid) {\n" + + " return null;\n" + + " }\n" + + " }\n" + ); + assertNotNull(class1); + + // Outer.Inner = return type of method "newInner" + List classTypes = class1.descendants(ASTClassType.class).toList(); + ASTClassType outerInner = classTypes.get(0); + assertGenericClassType(outerInner, "Inner", "X", "T"); + + // Outer = qualifier of Outer.Inner + ASTClassType outer = classTypes.get(1); + assertEquals("Outer", outer.getSimpleName()); + assertNull(outer.getTypeArguments()); + + // Outer = formal parameter type of method newInner + ASTClassType outerFormalParam = classTypes.get(3); + assertGenericClassType(outerFormalParam, "Outer", "X", "T"); + }, "t1"); + t1.setUncaughtExceptionHandler(exceptionHandler); + + Thread t2 = new Thread(() -> { + ASTCompilationUnit class2 = JavaParsingHelper.DEFAULT.parse( + "package net.sourceforge.pmd.lang.java.symbols;\n" + + "import net.sourceforge.pmd.lang.java.symbols.DeadlockTest.Outer;\n" + + " class Class2 {\n" + + " protected Outer theOuter;\n" + + " }\n" + ); + assertNotNull(class2); + + // Outer = type of field "theOuter" + ASTClassType firstClassType = class2.descendants(ASTClassType.class).first(); + assertNotNull(firstClassType); + assertGenericClassType(firstClassType, "Outer", "M", "T"); + }, "t2"); + t2.setUncaughtExceptionHandler(exceptionHandler); + + t1.start(); + t2.start(); + + t1.join(); + t2.join(); + + assertAll(exceptions.stream() + .map(e -> () -> { + throw e; + })); + } + + private static void assertGenericClassType(ASTClassType classType, String simpleName, String actualTypeParamName, String originalTypeParamName) { + assertEquals(simpleName, classType.getSimpleName()); + assertEquals(1, classType.getTypeArguments().size()); + assertEquals(actualTypeParamName, ((ASTClassType) classType.getTypeArguments().get(0)).getSimpleName()); + JTypeParameterOwnerSymbol symbol = (JTypeParameterOwnerSymbol) classType.getTypeMirror().getSymbol(); + assertEquals(1, symbol.getTypeParameterCount()); + assertEquals(originalTypeParamName, symbol.getTypeParameters().get(0).getName()); + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypesTreeDumpTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypesTreeDumpTest.java index f69b79479c..50d22c663d 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypesTreeDumpTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypesTreeDumpTest.java @@ -50,6 +50,16 @@ class TypesTreeDumpTest extends BaseTreeDumpTest { doTest("UnnamedPatterns"); } + @Test + void testNestedLambdasAndMethodCalls() { + doTest("NestedLambdasAndMethodCalls"); + } + + @Test + void testUnresolvedThings() { + doTest("UnresolvedThings"); + } + @Override protected @NonNull String normalize(@NonNull String str) { return super.normalize(str) diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceCtxUnitTests.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceCtxUnitTests.java index 5e33c6dd01..ba1d834261 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceCtxUnitTests.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/internal/infer/InferenceCtxUnitTests.java @@ -8,7 +8,10 @@ import static net.sourceforge.pmd.lang.java.types.TestUtilitiesForTypesKt.captur import static net.sourceforge.pmd.lang.java.types.internal.infer.BaseTypeInferenceUnitTest.Bound.eqBound; import static net.sourceforge.pmd.lang.java.types.internal.infer.BaseTypeInferenceUnitTest.Bound.lower; import static net.sourceforge.pmd.lang.java.types.internal.infer.BaseTypeInferenceUnitTest.Bound.upper; +import static net.sourceforge.pmd.util.CollectionUtil.setOf; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -19,11 +22,17 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import java.util.List; +import java.util.Set; + +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import net.sourceforge.pmd.lang.java.types.JTypeMirror; import net.sourceforge.pmd.lang.java.types.TypeOps; import net.sourceforge.pmd.lang.java.types.internal.infer.InferenceVar.BoundKind; +import net.sourceforge.pmd.lang.java.types.internal.infer.VarWalkStrategy.GraphWalk; +import net.sourceforge.pmd.util.IteratorUtil; /** * @@ -331,4 +340,88 @@ class InferenceCtxUnitTests extends BaseTypeInferenceUnitTest { assertThat(a, hasBoundsExactly(upper(ts.BOOLEAN.box()))); } + + private static @NotNull List> createBatchSetsFromGraph(InferenceContext ctx) { + GraphWalk graphWalk = new GraphWalk(ctx, false); + List> batches = IteratorUtil.toList(graphWalk); + return batches; + } + + @Test + void testGraphBuilding() { + InferenceContext ctx = emptyCtx(); + InferenceVar a = newIvar(ctx); + InferenceVar b = newIvar(ctx); + + List> batches = createBatchSetsFromGraph(ctx); + // no dependency: unordered + assertThat(batches, containsInAnyOrder(setOf(a), setOf(b))); + } + + @Test + void testGraphBuildingWithDependency() { + InferenceContext ctx = emptyCtx(); + InferenceVar a = newIvar(ctx); + InferenceVar b = newIvar(ctx); + + // a -> b + addSubtypeConstraint(ctx, a, ts.arrayType(b)); + + List> batches = createBatchSetsFromGraph(ctx); + + assertThat(batches, contains(setOf(b), setOf(a))); + } + + @Test + void testGraphBuildingWithDependency2() { + InferenceContext ctx = emptyCtx(); + InferenceVar a = newIvar(ctx); + InferenceVar b = newIvar(ctx); + + // a -> b + // b -> a (because of propagation) + addSubtypeConstraint(ctx, a, b); + + List> batches = createBatchSetsFromGraph(ctx); + + assertThat(batches, contains(setOf(b, a))); + } + + + + + @Test + void testGraphBuildingWithExtraDependency() { + InferenceContext ctx = emptyCtx(); + InferenceVar a = newIvar(ctx); + InferenceVar b = newIvar(ctx); + + // b -> a + ctx.addInstantiationDependencies(setOf(b), setOf(a)); + + List> batches = createBatchSetsFromGraph(ctx); + + assertThat(batches, contains(setOf(a), setOf(b))); + } + + @Test + void testGraphBuildingWithDependencyCycle() { + InferenceContext ctx = emptyCtx(); + InferenceVar a = newIvar(ctx); + InferenceVar b = newIvar(ctx); + InferenceVar c = newIvar(ctx); + + // a -> b, b -> a, + // a -> c, b -> c + a.addBound(BoundKind.UPPER, b); + a.addBound(BoundKind.EQ, listType(c)); + b.addBound(BoundKind.LOWER, a); + b.addBound(BoundKind.LOWER, listType(c)); + + + List> batches = createBatchSetsFromGraph(ctx); + + assertThat(batches, contains(setOf(c), setOf(b, a))); + } + } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/AstTestUtil.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/AstTestUtil.kt index 1545c93f7e..e5c04d1ac5 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/AstTestUtil.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/AstTestUtil.kt @@ -21,6 +21,7 @@ fun JavaNode.declaredMethodSignatures(): List = methodDeclarations() fun JavaNode.methodCalls(): DescendantNodeStream = descendants(ASTMethodCall::class.java) fun JavaNode.firstMethodCall() = methodCalls().crossFindBoundaries().firstOrThrow() +fun JavaNode.firstMethodCall(name: String) = methodCalls().crossFindBoundaries().filter { it.methodName == name }.firstOrThrow() fun JavaNode.ctorCalls(): DescendantNodeStream = descendants(ASTConstructorCall::class.java) fun JavaNode.firstCtorCall() = ctorCalls().crossFindBoundaries().firstOrThrow() diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/SubtypingTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/SubtypingTest.kt index 03cfcddb4a..d3b3ba18c6 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/SubtypingTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/SubtypingTest.kt @@ -13,14 +13,14 @@ import io.kotest.property.Exhaustive import io.kotest.property.checkAll import io.kotest.property.exhaustive.ints import io.kotest.property.forAll -import net.sourceforge.pmd.lang.test.ast.shouldBeA import net.sourceforge.pmd.lang.java.ast.ParserTestCtx import net.sourceforge.pmd.lang.java.symbols.internal.UnresolvedClassStore import net.sourceforge.pmd.lang.java.symbols.internal.asm.createUnresolvedAsmSymbol -import net.sourceforge.pmd.lang.java.types.TypeConversion.* -import net.sourceforge.pmd.lang.java.types.TypeOps.Convertibility.* +import net.sourceforge.pmd.lang.java.types.TypeConversion.capture +import net.sourceforge.pmd.lang.java.types.TypeOps.Convertibility.UNCHECKED_NO_WARNING import net.sourceforge.pmd.lang.java.types.testdata.ComparableList import net.sourceforge.pmd.lang.java.types.testdata.SomeEnum +import net.sourceforge.pmd.lang.test.ast.shouldBeA import kotlin.test.assertTrue /** @@ -187,6 +187,9 @@ class SubtypingTest : FunSpec({ Class shouldBeUncheckedSubtypeOf `Class{String}` ts.STRING shouldBeSubtypeOf `Comparable{Wildcard}` + + val unresolvedT = ts.createUnresolvedAsmSymbol("foo") + unresolvedT[`?`] shouldBeSubtypeOf ts.rawType(unresolvedT) } test("Test wildcard subtyping") { diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/MethodRefInferenceTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/MethodRefInferenceTest.kt index e928dda72b..ef10cf77df 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/MethodRefInferenceTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/MethodRefInferenceTest.kt @@ -5,12 +5,12 @@ package net.sourceforge.pmd.lang.java.types.internal.infer import io.kotest.matchers.shouldBe +import net.sourceforge.pmd.lang.java.ast.* +import net.sourceforge.pmd.lang.java.types.* import net.sourceforge.pmd.lang.test.ast.component6 import net.sourceforge.pmd.lang.test.ast.component7 import net.sourceforge.pmd.lang.test.ast.shouldBe import net.sourceforge.pmd.lang.test.ast.shouldMatchN -import net.sourceforge.pmd.lang.java.ast.* -import net.sourceforge.pmd.lang.java.types.* import java.util.* import java.util.function.* import java.util.stream.Collector @@ -331,12 +331,13 @@ class MethodRefInferenceTest : ProcessorTestSpec({ val t_Archive = acu.firstTypeSignature() val mref = acu.descendants(ASTMethodReference::class.java).firstOrThrow() + val (getName) = acu.declaredMethodSignatures().toList() val call = acu.firstMethodCall() spy.shouldHaveMissingCtDecl(call) acu.withTypeDsl { - mref.referencedMethod shouldBe ts.UNRESOLVED_METHOD + mref.referencedMethod shouldBe getName mref shouldHaveType ts.UNKNOWN call.methodType shouldBe ts.UNRESOLVED_METHOD call.overloadSelectionInfo.apply { @@ -1093,4 +1094,38 @@ class Scratch { mref shouldHaveType t_Additioner } } + + parserTest("Method ref without target type still populates CTDecl if exact") { + val (acu, spy) = parser.parseWithTypeInferenceSpy( + """ + class GenerationType { + static { + foo(GenerationType::isAndroidType); + } + { + foo(this::instance); + } + + private boolean instance(String data) {} + private static boolean isAndroidType(String data) { + return "android".equals(data); + } + } + """.trimIndent() + ) + + val (instance, static) = acu.declaredMethodSignatures() + + val mrefs = acu.descendants(ASTMethodReference::class.java).toList() + val (staticMref, instanceMref) = mrefs + + spy.shouldBeOk { + mrefs.forEach { + it shouldHaveType ts.UNKNOWN + it.functionalMethod shouldBe ts.UNRESOLVED_METHOD + } + instanceMref.referencedMethod shouldBe instance + staticMref.referencedMethod shouldBe static + } + } }) diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/UnresolvedTypesRecoveryTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/UnresolvedTypesRecoveryTest.kt index 24d7ec3c04..7831441b3a 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/UnresolvedTypesRecoveryTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/UnresolvedTypesRecoveryTest.kt @@ -530,6 +530,7 @@ class C { val (mref) = acu.descendants(ASTMethodReference::class.java).toList() val (lambdaCall, mrefCall) = acu.descendants(ASTMethodCall::class.java).toList() + val (fooDecl) = acu.declaredMethodSignatures().toList() spy.shouldHaveNoApplicableMethods(lambdaCall) spy.shouldHaveNoApplicableMethods(mrefCall) @@ -540,7 +541,7 @@ class C { mref shouldHaveType ts.UNKNOWN mref.functionalMethod shouldBe ts.UNRESOLVED_METHOD - mref.referencedMethod shouldBe ts.UNRESOLVED_METHOD + mref.referencedMethod shouldBe fooDecl // still populated because unambiguous } } @@ -562,6 +563,7 @@ class C { val (lambda) = acu.descendants(ASTLambdaExpression::class.java).toList() val (mref) = acu.descendants(ASTMethodReference::class.java).toList() + val (fooDecl) = acu.declaredMethodSignatures().toList() spy.shouldHaveNoLambdaCtx(lambda) spy.shouldHaveNoLambdaCtx(mref) @@ -572,7 +574,7 @@ class C { mref shouldHaveType ts.UNKNOWN mref.functionalMethod shouldBe ts.UNRESOLVED_METHOD - mref.referencedMethod shouldBe ts.UNRESOLVED_METHOD + mref.referencedMethod shouldBe fooDecl // still populated because unambiguous } } @@ -664,4 +666,77 @@ class C { fooToInt.referencedMethod.symbol shouldBe toIntFun } } + + parserTest("Type inference should not resolve UNKNOWN bounded types to Object #5329") { + + val (acu, _) = parser.parseWithTypeInferenceSpy( + """ + import java.util.ArrayList; + import java.util.List; + import java.util.stream.Stream; + import java.util.stream.Collectors; + + class Foo { + public Item methodA(List loads) { + List items = new ArrayList<>(); + loads.stream() + // Here this collect call should have type + // Map<(*unknown*), List<*Item>> + // ie, key is unknown, not Object. + .collect(Collectors.groupingBy(Item::getValue)) + .forEach((a, b) -> items.add(buildItem(a, b))); + } + + private SummaryDto.ItemDto buildItem(BigDecimal a, List b) { + return SummaryDto.ItemDto.builder().build(); + } + } + """ + ) + + val collect = acu.firstMethodCall("collect") + val buildItem = acu.firstMethodCall("buildItem") + val (_, buildItemDecl) = acu.methodDeclarations().toList { it.symbol } + val (itemT) = acu.descendants(ASTClassType::class.java).toList { it.typeMirror } + + acu.withTypeDsl { + collect shouldHaveType java.util.Map::class[ts.UNKNOWN, java.util.List::class[itemT]] + buildItem.methodType.symbol shouldBe buildItemDecl + } + } + + parserTest("Unresolved type should also allow unchecked conversion") { + // The problem here is that ConstraintViolation is not convertible to ConstraintViolation, + // because ConstraintViolation is not on the classpath. + + val (acu, _) = parser.parseWithTypeInferenceSpy( + """ + import java.util.Set; + class Foo { + private void foo(ConstraintViolation constraintViolation) { + constraintViolation.toString(); + } + + public void foos(Set> constraintViolations) { + constraintViolations.forEach(this::foo); + } + + } + """ + ) + +// val (foreach) = acu.methodCalls().toList() + val constraintViolationT = acu.descendants(ASTClassType::class.java) + .filter { it.simpleName == "ConstraintViolation" } + .firstOrThrow().typeMirror.symbol as JClassSymbol + val (fooDecl) = acu.methodDeclarations().toList { it.symbol } + val (mref) = acu.descendants(ASTMethodReference::class.java).toList() + + acu.withTypeDsl { + mref.referencedMethod.symbol shouldBe fooDecl + + mref shouldHaveType java.util.function.Consumer::class[constraintViolationT[`?`]] + + } + } }) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml index 5292c61d64..637b6dffa6 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml @@ -1168,4 +1168,35 @@ public class Foo { ]]> + + #5318 [java] PreserveStackTraceRule: false-positive on Pattern Matching with instanceof + 0 + formatExceptionHandler = e -> { e.printStackTrace(); }; +} +]]> + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml index e81d963fcb..f0146762a8 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml @@ -2134,4 +2134,198 @@ public class ObtainViaTest { } ]]> + + #5324 UnusedPrivateMethod with method reference + 0 + > map = new Main().run(library); + System.out.println(map); + } + + private Map> run(Library library) { + return library + .books() + .stream() + .map(book -> book.lenders().stream().collect(Collectors.toMap(Lender::name, lender -> Map.of(book.title(), lender.status())))) + .reduce(this::reduceBooksAndLenderStatusByLender) + .orElse(null); + } + + private Map> reduceBooksAndLenderStatusByLender( + Map> previousMap, + Map> nextMap + ) { + previousMap.putAll(nextMap); + return previousMap; + } + } + + + record Lender(String name, String status) {} + record Book(String title, Collection lenders) {} + record Library(Collection books) {} + ]]> + + + #5324 UnusedPrivateMethod with unresolved types + 0 + { + try { + return registerUser(email, firstName, lastName); + } catch (Exception e) { + throw new IllegalStateException("Failed to register user for " + email, e); + } + }); + // ... + return user; + } + + private User registerUser(String email, String firstName, String lastName) throws Exception { + // register user logic here... + } + } + ]]> + + + #5329 UnusedPrivateMethod with unresolved types + 0 + items = new ArrayList<>(); + loads.stream() + .collect(Collectors.groupingBy(Item::getValue)) + .forEach((a, b) -> items.add(buildItem(a, b))); + } + + private SummaryDto.ItemDto buildItem(BigDecimal a, List b) { + return SummaryDto.ItemDto.builder().build(); + } + } + ]]> + + + #5097 UnusedPrivateMethod with unresolved target for method reference + 0 + > listOfLists) { + listOfLists.forEach(this::doWork); + } + + //BUT this does??? + //UnusedPrivateMethod - this as a false positive - but what is different? + private void addValidationError(ConstraintViolation constraintViolation) { + constraintViolation.toString(); + } + + public void addValidationErrors(Set> constraintViolations) { + constraintViolations.forEach(this::addValidationError); + } + + } + ]]> + + + + UnusedPrivateMethod #5113 + 0 + foo(int param) { + var optional = param == 0 ? Optional.of(true) : Optional.of(false); + return optional.flatMap(this::dummy); + } + + private Optional dummy(boolean foo) { + return Optional.of(foo); + } + } + ]]> + + + UnusedPrivateMethod #5083 - method reference without target type + 0 + predicate; + + private static boolean isAppleType(String data) { + return "apple".equals(data); + } + + private static boolean isRokuType(String data) { + return "roku".equals(data); + } + + private static boolean isSamsungType(String data) { + return "samsung".equals(data); + } + + private static boolean isAmazonType(String data) { + return "amazon".equals(data); + } + + private static boolean isAndroidType(String data) { + return "android".equals(data); + } + } + ]]> + + + + UnusedPrivateMethod #5083 - method reference without target type (2) + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml index ec267fcfcc..bd5d364eb1 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml @@ -1419,4 +1419,23 @@ public class LiteralExpression { } ]]> + + + #5314 [java] InsufficientStringBufferDeclarationRule: Lack of handling for char type parameters + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForSwitch.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForSwitch.xml index e603390ec6..4cbc9893c7 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForSwitch.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForSwitch.xml @@ -136,6 +136,68 @@ public class SwitchWithRecordPattern { } } } +]]> + + + + #5287 [java] TooFewBranchesForSwitch false-positive with switch using list of case constants + 3 + 0 + 1; + default -> 0; + }; + } + + int checkSwitchStatement(SomeEnum someEnumValue) { + int result; + switch(someEnumValue) { + case A, B, C -> { result = 1; } + default -> { result = 0; } + } + return result; + } + + int checkSwitchExpressionGroup(SomeEnum someEnumValue) { + return switch(someEnumValue) { + case A, B, C: yield 1; + default: yield 0; + }; + } + + int checkSwitchStatementGroup(SomeEnum someEnumValue) { + int result; + switch(someEnumValue) { + case A, B, C: result = 1; break; + default: result = 0; break; + } + return result; + } +} +]]> + + + + From #5311: Another example for list of case constants + 0 + "Hello"; + case E, F, G -> "World"; + }; + } + + enum Bar { + A, B, C, D, E, F, G + } +} ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/NestedLambdasAndMethodCalls.java b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/NestedLambdasAndMethodCalls.java new file mode 100644 index 0000000000..33914e0a68 --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/NestedLambdasAndMethodCalls.java @@ -0,0 +1,38 @@ +package org.example.unusedPrivateMethod; + +import static java.util.Collections.emptySet; + +import java.util.Collection; +import java.util.Map; +import java.util.stream.Collectors; + +public class NestedLambdasAndMethodCalls { + + public static void main(String[] args) { + Library library = new Library(emptySet()); + Map> map = new Main().run(library); + System.out.println(map); + } + + private Map> run(Library library) { + return library + .books() + .stream() + .map(book -> book.lenders().stream().collect(Collectors.toMap(Lender::name, lender -> Map.of(book.title(), lender.status())))) + .reduce(this::reduceBooksAndLenderStatusByLender) + .orElse(null); + } + + private Map> reduceBooksAndLenderStatusByLender( + Map> previousMap, + Map> nextMap + ) { + previousMap.putAll(nextMap); + return previousMap; + } +} + + +record Lender(String name, String status) {} +record Book(String title, Collection lenders) {} +record Library(Collection books) {} \ No newline at end of file diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/NestedLambdasAndMethodCalls.txt b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/NestedLambdasAndMethodCalls.txt new file mode 100644 index 0000000000..9099296ff8 --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/NestedLambdasAndMethodCalls.txt @@ -0,0 +1,194 @@ ++- CompilationUnit[] + +- PackageDeclaration[] + | +- ModifierList[] + +- ImportDeclaration[] + +- ImportDeclaration[] + +- ImportDeclaration[] + +- ImportDeclaration[] + +- ClassDeclaration[@TypeMirror = "org.example.unusedPrivateMethod.NestedLambdasAndMethodCalls"] + | +- ModifierList[] + | +- ClassBody[] + | +- MethodDeclaration[@Name = "main"] + | | +- ModifierList[] + | | +- VoidType[@TypeMirror = "void"] + | | +- FormalParameters[] + | | | +- FormalParameter[@TypeMirror = "java.lang.String[]"] + | | | +- ModifierList[] + | | | +- ArrayType[@TypeMirror = "java.lang.String[]"] + | | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | | +- ArrayDimensions[] + | | | | +- ArrayTypeDim[] + | | | +- VariableId[@Name = "args", @TypeMirror = "java.lang.String[]"] + | | +- Block[] + | | +- LocalVariableDeclaration[] + | | | +- ModifierList[] + | | | +- ClassType[@TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | | +- VariableDeclarator[] + | | | +- VariableId[@Name = "library", @TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | | +- ConstructorCall[@Failed = false, @Function = "org.example.unusedPrivateMethod.Library.new(java.util.Collection) -> org.example.unusedPrivateMethod.Library", @MethodName = "new", @TypeMirror = "org.example.unusedPrivateMethod.Library", @Unchecked = false, @VarargsCall = false] + | | | +- ClassType[@TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | | +- ArgumentList[] + | | | +- MethodCall[@Failed = false, @Function = "java.util.Collections. emptySet() -> java.util.Set", @MethodName = "emptySet", @TypeMirror = "java.util.Set", @Unchecked = false, @VarargsCall = false] + | | | +- ArgumentList[] + | | +- LocalVariableDeclaration[] + | | | +- ModifierList[] + | | | +- ClassType[@TypeMirror = "java.util.Map>"] + | | | | +- TypeArguments[] + | | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | | +- ClassType[@TypeMirror = "java.util.Map"] + | | | | +- TypeArguments[] + | | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- VariableDeclarator[] + | | | +- VariableId[@Name = "map", @TypeMirror = "java.util.Map>"] + | | | +- MethodCall[@Failed = true, @Function = "(*unknown*).(*unknown method*)() -> (*unknown*)", @MethodName = "run", @TypeMirror = "(*unknown*)", @Unchecked = false, @VarargsCall = false] + | | | +- ConstructorCall[@Failed = true, @Function = "(*unknown*).(*unknown method*)() -> (*unknown*)", @MethodName = "new", @TypeMirror = "*Main", @Unchecked = false, @VarargsCall = false] + | | | | +- ClassType[@TypeMirror = "*Main"] + | | | | +- ArgumentList[] + | | | +- ArgumentList[] + | | | +- VariableAccess[@Name = "library", @TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | +- ExpressionStatement[] + | | +- MethodCall[@Failed = false, @Function = "java.io.PrintStream.println(java.lang.Object) -> void", @MethodName = "println", @TypeMirror = "void", @Unchecked = false, @VarargsCall = false] + | | +- FieldAccess[@Name = "out", @TypeMirror = "java.io.PrintStream"] + | | | +- TypeExpression[@TypeMirror = "java.lang.System"] + | | | +- ClassType[@TypeMirror = "java.lang.System"] + | | +- ArgumentList[] + | | +- VariableAccess[@Name = "map", @TypeMirror = "java.util.Map>"] + | +- MethodDeclaration[@Name = "run"] + | | +- ModifierList[] + | | +- ClassType[@TypeMirror = "java.util.Map>"] + | | | +- TypeArguments[] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- ClassType[@TypeMirror = "java.util.Map"] + | | | +- TypeArguments[] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | +- FormalParameters[] + | | | +- FormalParameter[@TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | | +- ModifierList[] + | | | +- ClassType[@TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | | +- VariableId[@Name = "library", @TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | +- Block[] + | | +- ReturnStatement[] + | | +- MethodCall[@Failed = false, @Function = "java.util.Optional>>.orElse(java.util.Map>) -> java.util.Map>", @MethodName = "orElse", @TypeMirror = "java.util.Map>", @Unchecked = false, @VarargsCall = false] + | | +- MethodCall[@Failed = false, @Function = "java.util.stream.Stream>>.reduce(java.util.function.BinaryOperator>>) -> java.util.Optional>>", @MethodName = "reduce", @TypeMirror = "java.util.Optional>>", @Unchecked = false, @VarargsCall = false] + | | | +- MethodCall[@Failed = false, @Function = "java.util.stream.Stream. map(java.util.function.Function>>) -> java.util.stream.Stream>>", @MethodName = "map", @TypeMirror = "java.util.stream.Stream>>", @Unchecked = false, @VarargsCall = false] + | | | | +- MethodCall[@Failed = false, @Function = "java.util.Collection.stream() -> java.util.stream.Stream", @MethodName = "stream", @TypeMirror = "java.util.stream.Stream", @Unchecked = false, @VarargsCall = false] + | | | | | +- MethodCall[@Failed = false, @Function = "org.example.unusedPrivateMethod.Library.books() -> java.util.Collection", @MethodName = "books", @TypeMirror = "java.util.Collection", @Unchecked = false, @VarargsCall = false] + | | | | | | +- VariableAccess[@Name = "library", @TypeMirror = "org.example.unusedPrivateMethod.Library"] + | | | | | | +- ArgumentList[] + | | | | | +- ArgumentList[] + | | | | +- ArgumentList[] + | | | | +- LambdaExpression[@TypeMirror = "java.util.function.Function>>"] + | | | | +- LambdaParameterList[] + | | | | | +- LambdaParameter[@TypeMirror = "org.example.unusedPrivateMethod.Book"] + | | | | | +- ModifierList[] + | | | | | +- VariableId[@Name = "book", @TypeMirror = "org.example.unusedPrivateMethod.Book"] + | | | | +- MethodCall[@Failed = false, @Function = "java.util.stream.Stream. collect(java.util.stream.Collector>>) -> java.util.Map>", @MethodName = "collect", @TypeMirror = "java.util.Map>", @Unchecked = false, @VarargsCall = false] + | | | | +- MethodCall[@Failed = false, @Function = "java.util.Collection.stream() -> java.util.stream.Stream", @MethodName = "stream", @TypeMirror = "java.util.stream.Stream", @Unchecked = false, @VarargsCall = false] + | | | | | +- MethodCall[@Failed = false, @Function = "org.example.unusedPrivateMethod.Book.lenders() -> java.util.Collection", @MethodName = "lenders", @TypeMirror = "java.util.Collection", @Unchecked = false, @VarargsCall = false] + | | | | | | +- VariableAccess[@Name = "book", @TypeMirror = "org.example.unusedPrivateMethod.Book"] + | | | | | | +- ArgumentList[] + | | | | | +- ArgumentList[] + | | | | +- ArgumentList[] + | | | | +- MethodCall[@Failed = false, @Function = "java.util.stream.Collectors. toMap(java.util.function.Function, java.util.function.Function>) -> java.util.stream.Collector>>", @MethodName = "toMap", @TypeMirror = "java.util.stream.Collector>>", @Unchecked = false, @VarargsCall = false] + | | | | +- TypeExpression[@TypeMirror = "java.util.stream.Collectors"] + | | | | | +- ClassType[@TypeMirror = "java.util.stream.Collectors"] + | | | | +- ArgumentList[] + | | | | +- MethodReference[@TypeMirror = "java.util.function.Function"] + | | | | | +- TypeExpression[@TypeMirror = "org.example.unusedPrivateMethod.Lender"] + | | | | | +- ClassType[@TypeMirror = "org.example.unusedPrivateMethod.Lender"] + | | | | +- LambdaExpression[@TypeMirror = "java.util.function.Function>"] + | | | | +- LambdaParameterList[] + | | | | | +- LambdaParameter[@TypeMirror = "org.example.unusedPrivateMethod.Lender"] + | | | | | +- ModifierList[] + | | | | | +- VariableId[@Name = "lender", @TypeMirror = "org.example.unusedPrivateMethod.Lender"] + | | | | +- MethodCall[@Failed = false, @Function = "java.util.Map. of(java.lang.String, java.lang.String) -> java.util.Map", @MethodName = "of", @TypeMirror = "java.util.Map", @Unchecked = false, @VarargsCall = false] + | | | | +- TypeExpression[@TypeMirror = "java.util.Map"] + | | | | | +- ClassType[@TypeMirror = "java.util.Map"] + | | | | +- ArgumentList[] + | | | | +- MethodCall[@Failed = false, @Function = "org.example.unusedPrivateMethod.Book.title() -> java.lang.String", @MethodName = "title", @TypeMirror = "java.lang.String", @Unchecked = false, @VarargsCall = false] + | | | | | +- VariableAccess[@Name = "book", @TypeMirror = "org.example.unusedPrivateMethod.Book"] + | | | | | +- ArgumentList[] + | | | | +- MethodCall[@Failed = false, @Function = "org.example.unusedPrivateMethod.Lender.status() -> java.lang.String", @MethodName = "status", @TypeMirror = "java.lang.String", @Unchecked = false, @VarargsCall = false] + | | | | +- VariableAccess[@Name = "lender", @TypeMirror = "org.example.unusedPrivateMethod.Lender"] + | | | | +- ArgumentList[] + | | | +- ArgumentList[] + | | | +- MethodReference[@TypeMirror = "java.util.function.BinaryOperator>>"] + | | | +- ThisExpression[@TypeMirror = "org.example.unusedPrivateMethod.NestedLambdasAndMethodCalls"] + | | +- ArgumentList[] + | | +- NullLiteral[@TypeMirror = "null"] + | +- MethodDeclaration[@Name = "reduceBooksAndLenderStatusByLender"] + | +- ModifierList[] + | +- ClassType[@TypeMirror = "java.util.Map>"] + | | +- TypeArguments[] + | | +- ClassType[@TypeMirror = "java.lang.String"] + | | +- ClassType[@TypeMirror = "java.util.Map"] + | | +- TypeArguments[] + | | +- ClassType[@TypeMirror = "java.lang.String"] + | | +- ClassType[@TypeMirror = "java.lang.String"] + | +- FormalParameters[] + | | +- FormalParameter[@TypeMirror = "java.util.Map>"] + | | | +- ModifierList[] + | | | +- ClassType[@TypeMirror = "java.util.Map>"] + | | | | +- TypeArguments[] + | | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | | +- ClassType[@TypeMirror = "java.util.Map"] + | | | | +- TypeArguments[] + | | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- VariableId[@Name = "previousMap", @TypeMirror = "java.util.Map>"] + | | +- FormalParameter[@TypeMirror = "java.util.Map>"] + | | +- ModifierList[] + | | +- ClassType[@TypeMirror = "java.util.Map>"] + | | | +- TypeArguments[] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- ClassType[@TypeMirror = "java.util.Map"] + | | | +- TypeArguments[] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | +- VariableId[@Name = "nextMap", @TypeMirror = "java.util.Map>"] + | +- Block[] + | +- ExpressionStatement[] + | | +- MethodCall[@Failed = false, @Function = "java.util.Map>.putAll(java.util.Map>) -> void", @MethodName = "putAll", @TypeMirror = "void", @Unchecked = false, @VarargsCall = false] + | | +- VariableAccess[@Name = "previousMap", @TypeMirror = "java.util.Map>"] + | | +- ArgumentList[] + | | +- VariableAccess[@Name = "nextMap", @TypeMirror = "java.util.Map>"] + | +- ReturnStatement[] + | +- VariableAccess[@Name = "previousMap", @TypeMirror = "java.util.Map>"] + +- RecordDeclaration[@TypeMirror = "org.example.unusedPrivateMethod.Lender"] + | +- ModifierList[] + | +- RecordComponentList[] + | | +- RecordComponent[@TypeMirror = "java.lang.String"] + | | | +- ModifierList[] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- VariableId[@Name = "name", @TypeMirror = "java.lang.String"] + | | +- RecordComponent[@TypeMirror = "java.lang.String"] + | | +- ModifierList[] + | | +- ClassType[@TypeMirror = "java.lang.String"] + | | +- VariableId[@Name = "status", @TypeMirror = "java.lang.String"] + | +- RecordBody[] + +- RecordDeclaration[@TypeMirror = "org.example.unusedPrivateMethod.Book"] + | +- ModifierList[] + | +- RecordComponentList[] + | | +- RecordComponent[@TypeMirror = "java.lang.String"] + | | | +- ModifierList[] + | | | +- ClassType[@TypeMirror = "java.lang.String"] + | | | +- VariableId[@Name = "title", @TypeMirror = "java.lang.String"] + | | +- RecordComponent[@TypeMirror = "java.util.Collection"] + | | +- ModifierList[] + | | +- ClassType[@TypeMirror = "java.util.Collection"] + | | | +- TypeArguments[] + | | | +- ClassType[@TypeMirror = "org.example.unusedPrivateMethod.Lender"] + | | +- VariableId[@Name = "lenders", @TypeMirror = "java.util.Collection"] + | +- RecordBody[] + +- RecordDeclaration[@TypeMirror = "org.example.unusedPrivateMethod.Library"] + +- ModifierList[] + +- RecordComponentList[] + | +- RecordComponent[@TypeMirror = "java.util.Collection"] + | +- ModifierList[] + | +- ClassType[@TypeMirror = "java.util.Collection"] + | | +- TypeArguments[] + | | +- ClassType[@TypeMirror = "org.example.unusedPrivateMethod.Book"] + | +- VariableId[@Name = "books", @TypeMirror = "java.util.Collection"] + +- RecordBody[] diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnnamedPatterns.txt b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnnamedPatterns.txt index 25eb7dd0d7..e1fa78ded1 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnnamedPatterns.txt +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnnamedPatterns.txt @@ -583,7 +583,7 @@ | +- ArgumentList[] | +- StringLiteral[@TypeMirror = "java.lang.String"] +- ExpressionStatement[] - +- MethodCall[@Failed = false, @Function = "java.util.stream.Stream. collect(java.util.stream.Collector>) -> java.util.Map", @MethodName = "collect", @TypeMirror = "java.util.Map", @Unchecked = false, @VarargsCall = false] + +- MethodCall[@Failed = false, @Function = "java.util.stream.Stream. collect(java.util.stream.Collector>) -> java.util.Map", @MethodName = "collect", @TypeMirror = "java.util.Map", @Unchecked = false, @VarargsCall = false] +- MethodCall[@Failed = false, @Function = "java.util.Collection.stream() -> java.util.stream.Stream", @MethodName = "stream", @TypeMirror = "java.util.stream.Stream", @Unchecked = false, @VarargsCall = false] | +- MethodCall[@Failed = false, @Function = "java.util.List. of(java.lang.String, java.lang.String) -> java.util.List", @MethodName = "of", @TypeMirror = "java.util.List", @Unchecked = false, @VarargsCall = false] | | +- TypeExpression[@TypeMirror = "java.util.List"] @@ -593,14 +593,14 @@ | | +- StringLiteral[@TypeMirror = "java.lang.String"] | +- ArgumentList[] +- ArgumentList[] - +- MethodCall[@Failed = false, @Function = "java.util.stream.Collectors. toMap(java.util.function.Function, java.util.function.Function) -> java.util.stream.Collector>", @MethodName = "toMap", @TypeMirror = "java.util.stream.Collector>", @Unchecked = false, @VarargsCall = false] + +- MethodCall[@Failed = false, @Function = "java.util.stream.Collectors. toMap(java.util.function.Function, java.util.function.Function) -> java.util.stream.Collector>", @MethodName = "toMap", @TypeMirror = "java.util.stream.Collector>", @Unchecked = false, @VarargsCall = false] +- TypeExpression[@TypeMirror = "java.util.stream.Collectors"] | +- ClassType[@TypeMirror = "java.util.stream.Collectors"] +- ArgumentList[] +- MethodReference[@TypeMirror = "java.util.function.Function"] | +- TypeExpression[@TypeMirror = "java.lang.String"] | +- ClassType[@TypeMirror = "java.lang.String"] - +- LambdaExpression[@TypeMirror = "java.util.function.Function"] + +- LambdaExpression[@TypeMirror = "java.util.function.Function"] +- LambdaParameterList[] | +- LambdaParameter[@TypeMirror = "java.lang.String"] | +- ModifierList[] diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnresolvedThings.java b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnresolvedThings.java new file mode 100644 index 0000000000..83eb9d62b4 --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnresolvedThings.java @@ -0,0 +1,16 @@ +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; +import java.util.stream.Collectors; +class Foo { + public User methodA(List loads) { + List items = new ArrayList<>(); + loads.stream() + .collect(Collectors.groupingBy(Item::getValue)) + .forEach((a, b) -> items.add(buildItem(a, b))); + } + + private SummaryDto.ItemDto buildItem(BigDecimal a, List b) { + return SummaryDto.ItemDto.builder().build(); + } +} \ No newline at end of file diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnresolvedThings.txt b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnresolvedThings.txt new file mode 100644 index 0000000000..32f1f2643e --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/types/dumptests/UnresolvedThings.txt @@ -0,0 +1,80 @@ ++- CompilationUnit[] + +- ImportDeclaration[] + +- ImportDeclaration[] + +- ImportDeclaration[] + +- ImportDeclaration[] + +- ClassDeclaration[@TypeMirror = "Foo"] + +- ModifierList[] + +- ClassBody[] + +- MethodDeclaration[@Name = "methodA"] + | +- ModifierList[] + | +- ClassType[@TypeMirror = "*User"] + | +- FormalParameters[] + | | +- FormalParameter[@TypeMirror = "java.util.List<*Item>"] + | | +- ModifierList[] + | | +- ClassType[@TypeMirror = "java.util.List<*Item>"] + | | | +- TypeArguments[] + | | | +- ClassType[@TypeMirror = "*Item"] + | | +- VariableId[@Name = "loads", @TypeMirror = "java.util.List<*Item>"] + | +- Block[] + | +- LocalVariableDeclaration[] + | | +- ModifierList[] + | | +- ClassType[@TypeMirror = "java.util.List<*SummaryDto.ItemDto>"] + | | | +- TypeArguments[] + | | | +- ClassType[@TypeMirror = "*SummaryDto.ItemDto"] + | | +- VariableDeclarator[] + | | +- VariableId[@Name = "items", @TypeMirror = "java.util.List<*SummaryDto.ItemDto>"] + | | +- ConstructorCall[@Failed = false, @Function = "java.util.ArrayList<*SummaryDto.ItemDto>.new() -> java.util.ArrayList<*SummaryDto.ItemDto>", @MethodName = "new", @TypeMirror = "java.util.ArrayList<*SummaryDto.ItemDto>", @Unchecked = false, @VarargsCall = false] + | | +- ClassType[@TypeMirror = "java.util.ArrayList"] + | | | +- TypeArguments[] + | | +- ArgumentList[] + | +- ExpressionStatement[] + | +- MethodCall[@Failed = false, @Function = "java.util.Map<(*unknown*), java.util.List<*Item>>.forEach(java.util.function.BiConsumer>) -> void", @MethodName = "forEach", @TypeMirror = "void", @Unchecked = false, @VarargsCall = false] + | +- MethodCall[@Failed = false, @Function = "java.util.stream.Stream<*Item>. collect(java.util.stream.Collector>>) -> java.util.Map<(*unknown*), java.util.List<*Item>>", @MethodName = "collect", @TypeMirror = "java.util.Map<(*unknown*), java.util.List<*Item>>", @Unchecked = false, @VarargsCall = false] + | | +- MethodCall[@Failed = false, @Function = "java.util.Collection<*Item>.stream() -> java.util.stream.Stream<*Item>", @MethodName = "stream", @TypeMirror = "java.util.stream.Stream<*Item>", @Unchecked = false, @VarargsCall = false] + | | | +- VariableAccess[@Name = "loads", @TypeMirror = "java.util.List<*Item>"] + | | | +- ArgumentList[] + | | +- ArgumentList[] + | | +- MethodCall[@Failed = false, @Function = "java.util.stream.Collectors. groupingBy(java.util.function.Function) -> java.util.stream.Collector<*Item, java.lang.Object, java.util.Map<(*unknown*), java.util.List<*Item>>>", @MethodName = "groupingBy", @TypeMirror = "java.util.stream.Collector<*Item, java.lang.Object, java.util.Map<(*unknown*), java.util.List<*Item>>>", @Unchecked = false, @VarargsCall = false] + | | +- TypeExpression[@TypeMirror = "java.util.stream.Collectors"] + | | | +- ClassType[@TypeMirror = "java.util.stream.Collectors"] + | | +- ArgumentList[] + | | +- MethodReference[@TypeMirror = "java.util.function.Function<*Item, (*unknown*)>"] + | | +- AmbiguousName[@TypeMirror = "(*unknown*)"] + | +- ArgumentList[] + | +- LambdaExpression[@TypeMirror = "java.util.function.BiConsumer<(*unknown*), java.util.List<*Item>>"] + | +- LambdaParameterList[] + | | +- LambdaParameter[@TypeMirror = "(*unknown*)"] + | | | +- ModifierList[] + | | | +- VariableId[@Name = "a", @TypeMirror = "(*unknown*)"] + | | +- LambdaParameter[@TypeMirror = "java.util.List<*Item>"] + | | +- ModifierList[] + | | +- VariableId[@Name = "b", @TypeMirror = "java.util.List<*Item>"] + | +- MethodCall[@Failed = false, @Function = "java.util.List<*SummaryDto.ItemDto>.add(*SummaryDto.ItemDto) -> boolean", @MethodName = "add", @TypeMirror = "boolean", @Unchecked = false, @VarargsCall = false] + | +- VariableAccess[@Name = "items", @TypeMirror = "java.util.List<*SummaryDto.ItemDto>"] + | +- ArgumentList[] + | +- MethodCall[@Failed = false, @Function = "Foo.buildItem(*BigDecimal, java.util.List<*Item>) -> *SummaryDto.ItemDto", @MethodName = "buildItem", @TypeMirror = "*SummaryDto.ItemDto", @Unchecked = false, @VarargsCall = false] + | +- ArgumentList[] + | +- VariableAccess[@Name = "a", @TypeMirror = "(*unknown*)"] + | +- VariableAccess[@Name = "b", @TypeMirror = "java.util.List<*Item>"] + +- MethodDeclaration[@Name = "buildItem"] + +- ModifierList[] + +- ClassType[@TypeMirror = "*SummaryDto.ItemDto"] + +- FormalParameters[] + | +- FormalParameter[@TypeMirror = "*BigDecimal"] + | | +- ModifierList[] + | | +- ClassType[@TypeMirror = "*BigDecimal"] + | | +- VariableId[@Name = "a", @TypeMirror = "*BigDecimal"] + | +- FormalParameter[@TypeMirror = "java.util.List<*Item>"] + | +- ModifierList[] + | +- ClassType[@TypeMirror = "java.util.List<*Item>"] + | | +- TypeArguments[] + | | +- ClassType[@TypeMirror = "*Item"] + | +- VariableId[@Name = "b", @TypeMirror = "java.util.List<*Item>"] + +- Block[] + +- ReturnStatement[] + +- MethodCall[@Failed = true, @Function = "(*unknown*).(*unknown method*)() -> (*unknown*)", @MethodName = "build", @TypeMirror = "(*unknown*)", @Unchecked = false, @VarargsCall = false] + +- MethodCall[@Failed = true, @Function = "(*unknown*).(*unknown method*)() -> (*unknown*)", @MethodName = "builder", @TypeMirror = "(*unknown*)", @Unchecked = false, @VarargsCall = false] + | +- AmbiguousName[@TypeMirror = "(*unknown*)"] + | +- ArgumentList[] + +- ArgumentList[] diff --git a/pmd-java/src/test/resources/simplelogger.properties b/pmd-java/src/test/resources/simplelogger.properties new file mode 100644 index 0000000000..8a4eafe283 --- /dev/null +++ b/pmd-java/src/test/resources/simplelogger.properties @@ -0,0 +1,5 @@ +# +# BSD-style license; for more info see http://pmd.sourceforge.net/license.html +# + +#org.slf4j.simpleLogger.log.net.sourceforge.pmd.lang.java.symbols.internal.asm.ParseLock=trace diff --git a/pom.xml b/pom.xml index d09220f92b..dec91f16ef 100644 --- a/pom.xml +++ b/pom.xml @@ -101,7 +101,7 @@ 3.2.5 10.18.1 3.5.0 - 3.24.0 + 3.26.0 1.10.14 3.6.3 4.9.3 @@ -202,7 +202,7 @@ org.apache.maven.plugins maven-clean-plugin - 3.3.2 + 3.4.0