From ddb1eb8dd8c9eada9a10ac112c7d3bcd9ad8988c Mon Sep 17 00:00:00 2001 From: lukasgraef Date: Sat, 21 Sep 2024 16:03:36 +0200 Subject: [PATCH 1/4] [java] Fix #5067: CloseResource: False positive for FileSystems.getDefault() --- .../java/rule/errorprone/CloseResourceRule.java | 7 +++++++ .../java/rule/errorprone/xml/CloseResource.xml | 14 ++++++++++++++ 2 files changed, 21 insertions(+) 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 c3aa18a459..28c432a075 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 @@ -204,6 +204,7 @@ public class CloseResourceRule extends AbstractJavaRule { .filter(this::isVariableNotSpecifiedInTryWithResource) .filter(var -> isResourceTypeOrSubtype(var) || isNodeInstanceOfResourceType(getTypeOfVariable(var))) .filterNot(var -> var.isAnnotationPresent("lombok.Cleanup")) + .filterNot(this::isDefaultFileSystem) .toList(); for (ASTVariableId var : vars) { @@ -499,6 +500,12 @@ public class CloseResourceRule extends AbstractJavaRule { return tryStatement == null || !isVariableSpecifiedInTryWithResource(varId, tryStatement); } + private boolean isDefaultFileSystem(ASTVariableId varId) { + @Nullable + ASTExpression initializer = varId.getInitializer(); + return initializer != null && initializer.getText().contentEquals("FileSystems.getDefault()"); + } + private boolean isVariableSpecifiedInTryWithResource(ASTVariableId varId, ASTTryStatement tryWithResource) { // skip own resources - these are definitively closed if (tryWithResource.getResources().descendants(ASTVariableId.class).toList().contains(varId)) { 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 f0e32dbfd4..ac1c35d401 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 @@ -2044,6 +2044,20 @@ public class CastedParameter { // doesn't need to be closed, it's still a paramter… } } +} + ]]> + + + #5067: FileSystems.getDefault() can't be closed + 0 + From 39b8bdf17103b3b01b0501778e48ba6b9554e110 Mon Sep 17 00:00:00 2001 From: lukasgraef Date: Mon, 30 Sep 2024 20:13:01 +0200 Subject: [PATCH 2/4] Review Finding: Check for type java.nio.FileSystems --- .../pmd/lang/java/rule/errorprone/CloseResourceRule.java | 2 +- 1 file changed, 1 insertion(+), 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 28c432a075..440998cc8e 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 @@ -503,7 +503,7 @@ public class CloseResourceRule extends AbstractJavaRule { private boolean isDefaultFileSystem(ASTVariableId varId) { @Nullable ASTExpression initializer = varId.getInitializer(); - return initializer != null && initializer.getText().contentEquals("FileSystems.getDefault()"); + return initializer != null && InvocationMatcher.parse("java.nio.file.FileSystems#getDefault()").matchesCall(initializer); } private boolean isVariableSpecifiedInTryWithResource(ASTVariableId varId, ASTTryStatement tryWithResource) { From 7dcab3f189bb5b446c4ed97667907da2a24991c4 Mon Sep 17 00:00:00 2001 From: lukasgraef Date: Mon, 30 Sep 2024 20:48:20 +0200 Subject: [PATCH 3/4] Fix static analysis findings --- .../pmd/lang/java/rule/errorprone/CloseResourceRule.java | 3 ++- 1 file changed, 2 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 440998cc8e..f7caef91b6 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 @@ -108,6 +108,7 @@ public class CloseResourceRule extends AbstractJavaRule { .desc("Detect if 'close' (or other closeTargets) is called outside of a finally-block").defaultValue(false).build(); private static final InvocationMatcher OBJECTS_NON_NULL = InvocationMatcher.parse("java.util.Objects#nonNull(_)"); + private static final InvocationMatcher FILESYSTEMS_GET_DEFAULT = InvocationMatcher.parse("java.nio.file.FileSystems#getDefault()"); private final Set types = new HashSet<>(); private final Set simpleTypes = new HashSet<>(); @@ -503,7 +504,7 @@ public class CloseResourceRule extends AbstractJavaRule { private boolean isDefaultFileSystem(ASTVariableId varId) { @Nullable ASTExpression initializer = varId.getInitializer(); - return initializer != null && InvocationMatcher.parse("java.nio.file.FileSystems#getDefault()").matchesCall(initializer); + return FILESYSTEMS_GET_DEFAULT.matchesCall(initializer); } private boolean isVariableSpecifiedInTryWithResource(ASTVariableId varId, ASTTryStatement tryWithResource) { From d2c42d24260ef43be9ff7228762c5d2baf04248a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 4 Oct 2024 10:05:48 +0200 Subject: [PATCH 4/4] [doc] Update release notes (#5067, #5225) --- docs/pages/release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 6f89cebd47..08ed81281f 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -32,11 +32,14 @@ The old rule names still work but are deprecated. ### 🐛 Fixed Issues * java * [#4532](https://github.com/pmd/pmd/issues/4532): \[java] Rule misnomer for JUnit* rules +* java-errorprone + * [#5067](https://github.com/pmd/pmd/issues/5067): \[java] CloseResource: False positive for FileSystems.getDefault() ### 🚨 API Changes ### ✨ Merged pull requests * [#4965](https://github.com/pmd/pmd/pull/4965): \[java] Rename JUnit rules with overly restrictive names - [Juan Martín Sotuyo Dodero](https://github.com/jsotuyod) (@jsotuyod) +* [#5225](https://github.com/pmd/pmd/pull/5225): \[java] Fix #5067: CloseResource: False positive for FileSystems.getDefault() - [Lukas Gräf](https://github.com/lukasgraef) (@lukasgraef) * [#5241](https://github.com/pmd/pmd/pull/5241): Ignore javacc code in coverage report - [Juan Martín Sotuyo Dodero](https://github.com/jsotuyod) (@jsotuyod) {% endtocmaker %}