Merge branch 'pr-1514'

docs/pages/release_notes.md

@@ -14,10 +14,18 @@ This is a {{ site.pmd.release_type }} release.
 
 ### New and noteworthy
 
+#### Modified Rules
+
+*   The Java rule {% rule "java/codestyle/LocalVariableCouldBeFinal" %} (`java-codestyle`) has a new
+    property `ignoreForEachDecl`, which is by default disabled. The new property allows for ignoring
+    non-final loop variables in a for-each statement.
+
 ### Fixed Issues
 
 *   java-bestpractices
     *   [#658](https://github.com/pmd/pmd/issues/658): \[java] OneDeclarationPerLine: False positive for loops
+*   java-codestyle
+    *   [#1513](https://github.com/pmd/pmd/issues/1513): \[java] LocalVariableCouldBeFinal: allow excluding the variable in a for-each loop
 *   java-errorprone
     *   [#1035](https://github.com/pmd/pmd/issues/1035): \[java] ReturnFromFinallyBlock: False positive on lambda expression in finally block
 
@@ -26,6 +34,7 @@ This is a {{ site.pmd.release_type }} release.
 ### External Contributions
 
 *   [#1503](https://github.com/pmd/pmd/pull/1503): \[java] Fix for ReturnFromFinallyBlock false-positives - [RishabhDeep Singh](https://github.com/rishabhdeepsingh)
+*   [#1514](https://github.com/pmd/pmd/pull/1514): \[java] LocalVariableCouldBeFinal: allow excluding the variable in a for-each loop - [Kris Scheibe](https://github.com/kris-scheibe)
 *   [#1516](https://github.com/pmd/pmd/pull/1516): \[java] OneDeclarationPerLine: Don't report multiple variables in a for statement. - [Kris Scheibe](https://github.com/kris-scheibe)
 *   [#1521](https://github.com/pmd/pmd/pull/1521): \[java] Upgrade to ASM7 for JDK 11 support - [Mark Pritchard](https://github.com/markpritchard)
 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java

@@ -4,22 +4,36 @@
 
 package net.sourceforge.pmd.lang.java.rule.codestyle;
 
+import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
+
 import java.util.List;
 import java.util.Map;
 
+import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
 import net.sourceforge.pmd.lang.java.rule.performance.AbstractOptimizationRule;
 import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
 import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
 import net.sourceforge.pmd.lang.symboltable.Scope;
+import net.sourceforge.pmd.properties.PropertyDescriptor;
 
 public class LocalVariableCouldBeFinalRule extends AbstractOptimizationRule {
 
+    private static final PropertyDescriptor<Boolean> IGNORE_FOR_EACH =
+            booleanProperty("ignoreForEachDecl").defaultValue(false).desc("Ignore non-final loop variables in a for-each statement.").build();
+
+    public LocalVariableCouldBeFinalRule() {
+        definePropertyDescriptor(IGNORE_FOR_EACH);
+    }
+
     @Override
     public Object visit(ASTLocalVariableDeclaration node, Object data) {
         if (node.isFinal()) {
             return data;
         }
+        if (getProperty(IGNORE_FOR_EACH) && node.jjtGetParent() instanceof ASTForStatement) {
+            return data;
+        }
         Scope s = node.getScope();
         Map<VariableNameDeclaration, List<NameOccurrence>> decls = s.getDeclarations(VariableNameDeclaration.class);
         for (Map.Entry<VariableNameDeclaration, List<NameOccurrence>> entry : decls.entrySet()) {

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml

@@ -4,9 +4,7 @@
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
     <test-code>
-        <description><![CDATA[
+        <description>TEST1</description>
-TEST1
-     ]]></description>
         <expected-problems>1</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -17,9 +15,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST2</description>
-TEST2
-     ]]></description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -30,9 +26,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST3</description>
-TEST3
-     ]]></description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -44,9 +38,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST4</description>
-TEST4
-     ]]></description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -58,9 +50,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST5</description>
-TEST5
-     ]]></description>
         <expected-problems>2</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -74,9 +64,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST6</description>
-TEST6
-     ]]></description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -88,9 +76,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST7</description>
-TEST7
-     ]]></description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -102,9 +88,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST8</description>
-TEST8
-     ]]></description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -116,9 +100,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>TEST9</description>
-TEST9
-     ]]></description>
         <expected-problems>1</expected-problems>
         <code><![CDATA[
 public class Foo {
@@ -132,9 +114,7 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description><![CDATA[
+        <description>Bug 2614040 : false + if a += assignment operator is used inside a method call.</description>
-Bug 2614040 : false + if a += assignment operator is used inside a method call.
-     ]]></description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 /**
@@ -172,6 +152,47 @@ public class Test {
       Assert.assertEquals( array.size() - 1, list.size() );
   }
  }
+}
+       ]]></code>
+   </test-code>
+    <test-code>
+        <description>Parameter to ignore non-final variables in for each loops (see #1513).</description>
+        <rule-property name="ignoreForEachDecl">true</rule-property>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ public void bar() {
+  for ( String c : strings ) {
+   System.out.println(c); // use c
+  }
+ }
+}
+        ]]></code>
+    </test-code>
+    <test-code>
+        <description>By default, for-each loops should be flagged (see #1513).</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>3</expected-linenumbers>
+        <code><![CDATA[
+public class Foo {
+ public void bar() {
+  for ( String c : strings ) {
+   System.out.println(c); // use c
+  }
+ }
+}
+        ]]></code>
+    </test-code>
+    <test-code>
+        <description>Normal for-loops should not be flagged</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ public void bar() {
+  for (int i = 0; i < 10; i++ ) {
+   System.out.println(i);
+  }
+ }
 }
         ]]></code>
     </test-code>