[java] CloseResource - avoid duplicated violations

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java

@@ -69,10 +69,6 @@ public class CloseResourceRule extends AbstractJavaRule {
     private static final String WRAPPING_TRY_WITH_RES_VAR_MESSAGE = "it is recommended to wrap resource in try-with-resource declaration directly";
     private static final String CLOSE_IN_FINALLY_BLOCK_MESSAGE = "'' is not closed within a finally block, thus might not be closed at all in case of exceptions";
 
-    private final Set<String> types = new HashSet<>();
-    private final Set<String> simpleTypes = new HashSet<>();
-
-    private final Set<String> closeTargets = new HashSet<>();
     private static final PropertyDescriptor<List<String>> CLOSE_TARGETS_DESCRIPTOR =
             stringListProperty("closeTargets")
                            .desc("Methods which may close this resource")
@@ -97,6 +93,13 @@ public class CloseResourceRule extends AbstractJavaRule {
                     "java.util.stream.DoubleStream")
             .build();
 
+    private final Set<String> types = new HashSet<>();
+    private final Set<String> simpleTypes = new HashSet<>();
+    private final Set<String> closeTargets = new HashSet<>();
+
+    // keeps track of already reported violations to avoid duplicated violations for the same variable
+    private final Set<String> reportedVarNames = new HashSet<>();
+
 
     public CloseResourceRule() {
         definePropertyDescriptor(CLOSE_TARGETS_DESCRIPTOR);
@@ -147,13 +150,16 @@ public class CloseResourceRule extends AbstractJavaRule {
     }
 
     private void checkForResources(ASTMethodOrConstructorDeclaration methodOrConstructor, Object data) {
+        reportedVarNames.clear();
         Map<ASTVariableDeclarator, TypeNode> resVars = getResourceVariables(methodOrConstructor);
         for (Map.Entry<ASTVariableDeclarator, TypeNode> resVarEntry : resVars.entrySet()) {
             ASTVariableDeclarator resVar = resVarEntry.getKey();
             TypeNode resVarType = resVarEntry.getValue();
             if (isWrappingResourceSpecifiedInTry(resVar)) {
+                reportedVarNames.add(resVar.getVariableId().getName());
                 addViolationWithMessage(data, resVar, WRAPPING_TRY_WITH_RES_VAR_MESSAGE);
             } else if (shouldVarOfTypeBeClosedInMethod(resVar, resVarType, methodOrConstructor)) {
+                reportedVarNames.add(resVar.getVariableId().getName());
                 addCloseResourceViolation(resVar.getVariableId(), resVarType, data);
             }
         }
@@ -619,7 +625,7 @@ public class CloseResourceRule extends AbstractJavaRule {
         ASTName methodCall = prefix.getFirstChildOfType(ASTName.class);
         if (methodCall != null) {
             String closedVar = getVariableClosedByMethodCall(methodCall);
-            if (closedVar != null && isNotInFinallyBlock(prefix)) {
+            if (closedVar != null && isNotInFinallyBlock(prefix) && !reportedVarNames.contains(closedVar)) {
                 String violationMsg = closeInFinallyBlockMessageForVar(closedVar);
                 addViolationWithMessage(data, prefix, violationMsg);
             }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml

@@ -377,7 +377,12 @@ public class Test {
 
     <test-code>
         <description>#947 CloseResource rule fails if field is marked with annotation</description>
-        <expected-problems>3</expected-problems>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>8,9</expected-linenumbers>
+        <expected-messages>
+            <message>Ensure that resources like this Connection object are closed after use</message>
+            <message>Ensure that resources like this Statement object are closed after use</message>
+        </expected-messages>
         <code><![CDATA[
 import java.sql.Connection;
 import java.sql.ResultSet;
@@ -543,7 +548,11 @@ public class Bar {
 
     <test-code>
         <description>#1375 CloseResource not detected properly - ok</description>
-        <expected-problems>2</expected-problems>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>6</expected-linenumbers>
+        <expected-messages>
+            <message>Ensure that resources like this ResultSet object are closed after use</message>
+        </expected-messages>
         <code><![CDATA[
 import java.sql.ResultSet;
 import java.sql.Statement;
@@ -567,7 +576,11 @@ public class Foo {
 
     <test-code>
         <description>#1375 CloseResource not detected properly - false negative</description>
-        <expected-problems>2</expected-problems>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>6</expected-linenumbers>
+        <expected-messages>
+            <message>Ensure that resources like this ResultSet object are closed after use</message>
+        </expected-messages>
         <code><![CDATA[
 import java.sql.ResultSet;
 import java.sql.Statement;
@@ -1312,7 +1325,7 @@ public class Foo {
 }
         ]]></code>
     </test-code>
-    
+
     <test-code>
         <description>lambda doesn't close resource false-negative test</description>
         <expected-problems>1</expected-problems>
@@ -1333,7 +1346,7 @@ public class Foo {
 }
         ]]></code>
     </test-code>
-    
+
     <test-code>
         <description>don't wrap try-with-resource variable test</description>
         <expected-problems>1</expected-problems>