From b73b1e92f3951915cf25b9659e6ba51d4a80c106 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <andreas.dangel@pmd-code.org>
Date: Fri, 21 Aug 2020 09:33:24 +0200
Subject: [PATCH] [java] CloseResource: add new property "closeNotInFinally"

---
 docs/pages/release_notes.md                            | 10 ++++++++++
 .../lang/java/rule/errorprone/CloseResourceRule.java   |  9 +++++++++
 .../lang/java/rule/errorprone/xml/CloseResource.xml    |  3 +++
 3 files changed, 22 insertions(+)

diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
index 7dee579f3b..43c3e9c56e 100644
--- a/docs/pages/release_notes.md
+++ b/docs/pages/release_notes.md
@@ -36,6 +36,16 @@ See also [[all] Ensure PMD/CPD uses tab width of 1 for tabs consistently #2656](
     cases where the variable of the caught exception is reassigned. This practice is surprising and prevents
     further evolution of the code like multi-catch.
 
+#### Modified Rules
+
+*   The Java rule {% rule "java/errorprone/CloseResource" %} (`java-errorprone`) has a new property
+    `closeNotInFinally`. With this property set to `true` the rule will also find calls to close a
+    resource, which are not in a finally-block of a try-statement. If a resource is not closed within a
+    finally block, it might not be closed at all in case of exceptions.
+    
+    As this new detection would yield many new violations, it is disabled by default. It might be
+    enabled in a later version of PMD.
+
 #### Deprecated Rules
 
 *   The Java rule {% rule "java/errorprone/DataflowAnomalyAnalysis" %} (`java-errorprone`)
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 e2f3b97045..25bf71385a 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,6 +93,10 @@ public class CloseResourceRule extends AbstractJavaRule {
                     "java.util.stream.DoubleStream")
             .build();
 
+    private static final PropertyDescriptor<Boolean> DETECT_CLOSE_NOT_IN_FINALLY =
+            booleanProperty("closeNotInFinally")
+                .desc("Detect if 'close' (or other closeTargets) is called outside of a finally-block").defaultValue(false).build();
+
     private final Set<String> types = new HashSet<>();
     private final Set<String> simpleTypes = new HashSet<>();
     private final Set<String> closeTargets = new HashSet<>();
@@ -106,6 +110,7 @@ public class CloseResourceRule extends AbstractJavaRule {
         definePropertyDescriptor(TYPES_DESCRIPTOR);
         definePropertyDescriptor(USE_CLOSE_AS_DEFAULT_TARGET);
         definePropertyDescriptor(ALLOWED_RESOURCE_TYPES);
+        definePropertyDescriptor(DETECT_CLOSE_NOT_IN_FINALLY);
     }
 
     @Override
@@ -622,6 +627,10 @@ public class CloseResourceRule extends AbstractJavaRule {
 
     @Override
     public Object visit(ASTPrimaryPrefix prefix, Object data) {
+        if (!getProperty(DETECT_CLOSE_NOT_IN_FINALLY)) {
+            return super.visit(prefix, data);
+        }
+
         ASTName methodCall = prefix.getFirstChildOfType(ASTName.class);
         if (methodCall != null && isNodeInstanceOfResourceType(methodCall)) {
             String closedVar = getVariableClosedByMethodCall(methodCall);
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 9758e85511..3b3894818f 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
@@ -377,6 +377,7 @@ public class Test {
 
     <test-code>
         <description>#947 CloseResource rule fails if field is marked with annotation</description>
+        <rule-property name="closeNotInFinally">true</rule-property>
         <expected-problems>2</expected-problems>
         <expected-linenumbers>8,9</expected-linenumbers>
         <expected-messages>
@@ -548,6 +549,7 @@ public class Bar {
 
     <test-code>
         <description>#1375 CloseResource not detected properly - ok</description>
+        <rule-property name="closeNotInFinally">true</rule-property>
         <expected-problems>1</expected-problems>
         <expected-linenumbers>6</expected-linenumbers>
         <expected-messages>
@@ -1308,6 +1310,7 @@ public class Foo {
 
     <test-code>
         <description>wrapped ByteArrayInputStream false-negative test</description>
+        <rule-property name="closeNotInFinally">true</rule-property>
         <expected-problems>1</expected-problems>
         <expected-linenumbers>7</expected-linenumbers>
         <expected-messages>