From 5b63d787c7691c992847d9d9c69b7f0f4c1391e9 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 10:10:47 +0200 Subject: [PATCH 1/9] [java] CloseResource: ignore variables that are initialized from parameters If a closable local variable is initialized from a method/constructor parameter, then it is ignored. In that case, the resource is not created in this method, but somewhere else. Therefore the resources should be closed there. --- .../rule/errorprone/CloseResourceRule.java | 49 +++++++++++++++++-- .../rule/errorprone/xml/CloseResource.xml | 20 ++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java index a9cad8b0ac..ae0ea7925c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java @@ -22,12 +22,15 @@ import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; import net.sourceforge.pmd.lang.java.ast.ASTBlock; import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.java.ast.ASTCastExpression; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters; import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; @@ -40,6 +43,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTTryStatement; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer; +import net.sourceforge.pmd.lang.java.ast.MethodLikeNode.MethodLikeKind; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; @@ -141,7 +145,7 @@ public class CloseResourceRule extends AbstractJavaRule { return super.visit(node, data); } - private void checkForResources(Node node, Object data) { + private void checkForResources(ASTMethodOrConstructorDeclaration node, Object data) { List localVars = node.findDescendantsOfType(ASTLocalVariableDeclaration.class); List vars = new ArrayList<>(); Map ids = new HashMap<>(); @@ -174,13 +178,13 @@ public class CloseResourceRule extends AbstractJavaRule { } } - if (!isAllowedResourceType(type)) { + if (!isAllowedResourceType(type) && !isCastMethodParameter(var, node)) { ids.put(var.getVariableId(), type); } } } - // if there are connections, ensure each is closed. + // if there are closables, ensure each is closed. for (Map.Entry entry : ids.entrySet()) { ASTVariableDeclaratorId variableId = entry.getKey(); ensureClosed((ASTLocalVariableDeclaration) variableId.jjtGetParent().jjtGetParent(), variableId, @@ -188,6 +192,45 @@ public class CloseResourceRule extends AbstractJavaRule { } } + /** + * Checks whether the variable is initialized via a cast expression from a method parameter. + * @param var the variable that is being initialized + * @param methodOrCstor the method or constructor in which the variable is declared + * @return true if the variable is initialized from a method parameter. false + * otherwise. + */ + private boolean isCastMethodParameter(ASTVariableDeclarator var, ASTMethodOrConstructorDeclaration methodOrCstor) { + if (!var.hasInitializer()) { + return false; + } + + boolean result = false; + ASTVariableInitializer initializer = var.getInitializer(); + if (initializer.jjtGetChild(0) instanceof ASTExpression + && initializer.jjtGetChild(0).jjtGetChild(0) instanceof ASTCastExpression) { + ASTCastExpression cast = (ASTCastExpression) initializer.jjtGetChild(0).jjtGetChild(0); + ASTName name = cast.jjtGetChild(1).getFirstDescendantOfType(ASTName.class); + if (name != null) { + ASTFormalParameters formalParameters = null; + if (methodOrCstor.getKind() == MethodLikeKind.METHOD) { + formalParameters = ((ASTMethodDeclaration) methodOrCstor).getFormalParameters(); + } else if (methodOrCstor.getKind() == MethodLikeKind.CONSTRUCTOR) { + formalParameters = ((ASTConstructorDeclaration) methodOrCstor).getFormalParameters(); + } + if (formalParameters != null) { + List ids = formalParameters.findDescendantsOfType(ASTVariableDeclaratorId.class); + for (ASTVariableDeclaratorId id : ids) { + if (id.hasImageEqualTo(name.getImage())) { + result = true; + break; + } + } + } + } + } + return result; + } + private ASTExpression getAllocationFirstArgument(ASTExpression expression) { List allocations = expression.findDescendantsOfType(ASTAllocationExpression.class); ASTExpression firstArgument = null; diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index 008b5521b0..d3af0a927e 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1117,6 +1117,26 @@ public class CloseResourceCase { e.printStackTrace(); } } +} + ]]> + + + + Don't consider streams that are passed in as method arguments + 0 + From 619172560be94054306b8724768e44d962b22eef Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 10:39:56 +0200 Subject: [PATCH 2/9] [java] CloseResource: consider method parameters in general --- .../rule/errorprone/CloseResourceRule.java | 39 ++++++++----------- .../rule/errorprone/xml/CloseResource.xml | 3 ++ 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java index ae0ea7925c..0a5abe6af4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java @@ -22,7 +22,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; import net.sourceforge.pmd.lang.java.ast.ASTBlock; import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; -import net.sourceforge.pmd.lang.java.ast.ASTCastExpression; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTExpression; @@ -178,7 +177,7 @@ public class CloseResourceRule extends AbstractJavaRule { } } - if (!isAllowedResourceType(type) && !isCastMethodParameter(var, node)) { + if (!isAllowedResourceType(type) && !isMethodParameter(var, node)) { ids.put(var.getVariableId(), type); } } @@ -193,37 +192,33 @@ public class CloseResourceRule extends AbstractJavaRule { } /** - * Checks whether the variable is initialized via a cast expression from a method parameter. + * Checks whether the variable is initialized from a method parameter. * @param var the variable that is being initialized * @param methodOrCstor the method or constructor in which the variable is declared * @return true if the variable is initialized from a method parameter. false * otherwise. */ - private boolean isCastMethodParameter(ASTVariableDeclarator var, ASTMethodOrConstructorDeclaration methodOrCstor) { + private boolean isMethodParameter(ASTVariableDeclarator var, ASTMethodOrConstructorDeclaration methodOrCstor) { if (!var.hasInitializer()) { return false; } boolean result = false; ASTVariableInitializer initializer = var.getInitializer(); - if (initializer.jjtGetChild(0) instanceof ASTExpression - && initializer.jjtGetChild(0).jjtGetChild(0) instanceof ASTCastExpression) { - ASTCastExpression cast = (ASTCastExpression) initializer.jjtGetChild(0).jjtGetChild(0); - ASTName name = cast.jjtGetChild(1).getFirstDescendantOfType(ASTName.class); - if (name != null) { - ASTFormalParameters formalParameters = null; - if (methodOrCstor.getKind() == MethodLikeKind.METHOD) { - formalParameters = ((ASTMethodDeclaration) methodOrCstor).getFormalParameters(); - } else if (methodOrCstor.getKind() == MethodLikeKind.CONSTRUCTOR) { - formalParameters = ((ASTConstructorDeclaration) methodOrCstor).getFormalParameters(); - } - if (formalParameters != null) { - List ids = formalParameters.findDescendantsOfType(ASTVariableDeclaratorId.class); - for (ASTVariableDeclaratorId id : ids) { - if (id.hasImageEqualTo(name.getImage())) { - result = true; - break; - } + ASTName name = initializer.getFirstDescendantOfType(ASTName.class); + if (name != null) { + ASTFormalParameters formalParameters = null; + if (methodOrCstor.getKind() == MethodLikeKind.METHOD) { + formalParameters = ((ASTMethodDeclaration) methodOrCstor).getFormalParameters(); + } else if (methodOrCstor.getKind() == MethodLikeKind.CONSTRUCTOR) { + formalParameters = ((ASTConstructorDeclaration) methodOrCstor).getFormalParameters(); + } + if (formalParameters != null) { + List ids = formalParameters.findDescendantsOfType(ASTVariableDeclaratorId.class); + for (ASTVariableDeclaratorId id : ids) { + if (id.hasImageEqualTo(name.getImage())) { + result = true; + break; } } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index d3af0a927e..ef24cd3914 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1135,6 +1135,9 @@ public class CloseResourceFP { } else if (in instanceof ByteArrayInputStream) { ByteArrayInputStream bin = (ByteArrayInputSream) in; doCheck(bin); + } else { + BufferedInputStream buf = new BufferedInputStream(in); + doCheck(buf); } } } From ef8ed08ee0999ba20eb221eb1e6015d7bbb34fc6 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 11:37:52 +0200 Subject: [PATCH 3/9] [java] CloseResource: fix false positive with assignments before try --- .../rule/errorprone/CloseResourceRule.java | 11 +++++--- .../rule/errorprone/xml/CloseResource.xml | 25 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java index 0a5abe6af4..eaa1fe7df6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java @@ -20,6 +20,7 @@ import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTBlock; import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; @@ -345,10 +346,14 @@ public class CloseResourceRule extends AbstractJavaRule { boolean criticalStatements = false; for (int i = parentBlockIndex + 1; i < tryBlockIndex; i++) { - // assume variable declarations are not critical - ASTLocalVariableDeclaration varDecl = blocks.get(i) + // assume variable declarations are not critical and assignments are not critical + ASTBlockStatement block = blocks.get(i); + ASTLocalVariableDeclaration varDecl = block .getFirstDescendantOfType(ASTLocalVariableDeclaration.class); - if (varDecl == null) { + ASTStatementExpression statementExpression = block.getFirstDescendantOfType(ASTStatementExpression.class); + + if (varDecl == null && (statementExpression == null + || statementExpression.getFirstChildOfType(ASTAssignmentOperator.class) == null)) { criticalStatements = true; break; } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index ef24cd3914..d1b30a2271 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1140,6 +1140,31 @@ public class CloseResourceFP { doCheck(buf); } } +} + ]]> + + + + Indirect chaining with assignment before try + 0 + From e0535f4bceb8ec925cf26468ca53c86e74957d7e Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 11:42:18 +0200 Subject: [PATCH 4/9] [java] CloseResource: add additional test case --- .../pmd/lang/java/rule/errorprone/xml/CloseResource.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index d1b30a2271..bf94289319 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1140,6 +1140,11 @@ public class CloseResourceFP { doCheck(buf); } } + + public void dump(final Writer writer) { + final PrintWriter printWriter = writer instanceof PrintWriter ? (PrintWriter) writer : new PrintWriter(writer); + printWriter.println(this); + } } ]]> From 377275edae6799a81b9f2401383d6da48c0cc866 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 11:54:20 +0200 Subject: [PATCH 5/9] [java] CloseResource: possible false positive with Streams By default `java.util.stream.Stream` is now ignored. Fixes #1922 --- .../rule/errorprone/CloseResourceRule.java | 2 +- .../rule/errorprone/xml/CloseResource.xml | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java index eaa1fe7df6..2626b6b3e3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java @@ -93,7 +93,7 @@ public class CloseResourceRule extends AbstractJavaRule { stringListProperty("allowedResourceTypes") .desc("Exact class names that do not need to be closed") .defaultValues("java.io.ByteArrayOutputStream", "java.io.ByteArrayInputStream", "java.io.StringWriter", - "java.io.CharArrayWriter") + "java.io.CharArrayWriter", "java.util.stream.Stream") .build(); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index bf94289319..6e55cc356a 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1170,6 +1170,33 @@ public class CloseResourceFP { renderer.render(cpd.getMatches(), writer); } } +} + ]]> + + + + #1922 [java] CloseResource possible false positive with Streams + 0 + Stream> filterResults(List candidates, Function matchExtractor, String query, MatchSelector limiter) { + if (query.length() < MIN_QUERY_LENGTH) { + return Stream.empty(); + } + + // violation here + Stream> base = candidates.stream() + .map(it -> { + String cand = matchExtractor.apply(it); + return new MatchResult<>(0, it, cand, query, new TextFlow(makeNormalText(cand))); + }); + return limiter.selectBest(base); + } } ]]> From cb63f5f8c417b9f50d4ec68571c8e1099de7b686 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 12:02:34 +0200 Subject: [PATCH 6/9] [java] CloseResource: add test case for #1076 The problem is not reproducible. --- .../rule/errorprone/closeresource/Statement.java | 14 ++++++++++++++ .../java/rule/errorprone/xml/CloseResource.xml | 14 ++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/closeresource/Statement.java diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/closeresource/Statement.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/closeresource/Statement.java new file mode 100644 index 0000000000..74998af9ad --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/closeresource/Statement.java @@ -0,0 +1,14 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.errorprone.closeresource; + + +/** + * This Statement has nothing to do with {@link java.sql.Statement}. So using this, + * should not trigger the rule CloseResource, since this class is not autoclosable. + */ +public class Statement { + +} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index 6e55cc356a..7cb775a985 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1197,6 +1197,20 @@ public class CloseResourceStream { }); return limiter.selectBest(base); } +} + ]]> + + + + #1076 [java] CloseResource false positive on non-SQL classes called Statement + 0 + From c87af46d3f482212cc2157c709c0dfdab65ee23d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 12:25:42 +0200 Subject: [PATCH 7/9] Update release notes * Reference fixed issues in test cases for CloseResources * Fixes #1922 * Fixes #1966 * Fixes #1967 --- docs/pages/release_notes.md | 12 ++++++++++++ .../lang/java/rule/errorprone/xml/CloseResource.xml | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 4c51249b1d..cf77dad19b 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,10 +14,22 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy +#### Modified Rules + +* The Java rule {% rule "java/errorprone/CloseResource" %} (`java-errorprone`) now ignores by default instances + of `java.util.stream.Stream`. These streams are `AutoCloseable`, but most streams are backed by collections, + arrays, or generating functions, which require no special resource management. However, there are some exceptions: + The stream returned by `Files::lines(Path)` is backed by a actual file and needs to be closed. These instances + won't be found by default by the rule anymore. + ### Fixed Issues * java-codestyle * [#1951](https://github.com/pmd/pmd/issues/1951): \[java] UnnecessaryFullyQualifiedName rule triggered when variable name clashes with package name +* java-errorprone + * [#1922](https://github.com/pmd/pmd/issues/1922): \[java] CloseResource possible false positive with Streams + * [#1966](https://github.com/pmd/pmd/issues/1966): \[java] CloseResource false positive if Stream is passed as method parameter + * [#1967](https://github.com/pmd/pmd/issues/1967): \[java] CloseResource false positive with late assignment of variable ### API Changes diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index 7cb775a985..9c296200fa 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1122,7 +1122,7 @@ public class CloseResourceCase { - Don't consider streams that are passed in as method arguments + #1966 [java] CloseResource false positive if Stream is passed as method parameter 0 - Indirect chaining with assignment before try + #1967 [java] CloseResource false positive with late assignment of variable 0 Date: Sat, 10 Aug 2019 18:11:28 +0200 Subject: [PATCH 8/9] [java] CloseResource: fix false-negative when byte array is passed in --- .../rule/errorprone/CloseResourceRule.java | 2 +- .../rule/errorprone/xml/CloseResource.xml | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java index 2626b6b3e3..379ce5711f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java @@ -217,7 +217,7 @@ public class CloseResourceRule extends AbstractJavaRule { if (formalParameters != null) { List ids = formalParameters.findDescendantsOfType(ASTVariableDeclaratorId.class); for (ASTVariableDeclaratorId id : ids) { - if (id.hasImageEqualTo(name.getImage())) { + if (id.hasImageEqualTo(name.getImage()) && isResourceTypeOrSubtype(id)) { result = true; break; } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index 9c296200fa..d73947ab2b 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1211,6 +1211,30 @@ public class CloseResourceStatementFP { public void check() { Statement s = new Statement(); } +} + ]]> + + + + False-negative if only byte array is passed in as method parameter + 1 + 6 + From 7f7213b8e782190c13522101633ef32c4ac99b8d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Aug 2019 19:04:08 +0200 Subject: [PATCH 9/9] [java] CloseResource: Fix NPE if type of method parameter is not known --- .../java/rule/errorprone/CloseResourceRule.java | 2 +- .../lang/java/rule/errorprone/xml/CloseResource.xml | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java index 379ce5711f..0f4907d54b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java @@ -267,7 +267,7 @@ public class CloseResourceRule extends AbstractJavaRule { return true; } } - } else if (refType.jjtGetChild(0) instanceof ASTReferenceType) { + } else if (refType.jjtGetNumChildren() > 0 && refType.jjtGetChild(0) instanceof ASTReferenceType) { // no type information (probably missing auxclasspath) - use simple types ASTReferenceType ref = (ASTReferenceType) refType.jjtGetChild(0); if (ref.jjtGetChild(0) instanceof ASTClassOrInterfaceType) { diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml index d73947ab2b..dbbc83e9d9 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml @@ -1235,6 +1235,19 @@ public class CloseResourceFN { throw new IllegalStateException("Failed to deserialize object type", ex); } } +} + ]]> + + + + NullPointerException if type of method parameter is not known + 1 +