From 0c0f124c40729c436036703ec78b2ae5d95ff9d3 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <andreas.dangel@pmd-code.org>
Date: Thu, 28 Oct 2021 15:58:43 +0200
Subject: [PATCH] [java] Update rule AvoidBranchingStatementAsLastInLoop

---
 .ci/files/all-java.xml                          |  2 +-
 ...AvoidBranchingStatementAsLastInLoopRule.java | 17 +++++------------
 ...AvoidBranchingStatementAsLastInLoopTest.java |  1 -
 .../xml/AvoidBranchingStatementAsLastInLoop.xml | 15 ++++++++++++++-
 4 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/.ci/files/all-java.xml b/.ci/files/all-java.xml
index c6bd180f68..e0e9946b1a 100644
--- a/.ci/files/all-java.xml
+++ b/.ci/files/all-java.xml
@@ -174,7 +174,7 @@
     <!-- <rule ref="category/java/errorprone.xml/AssignmentToNonFinalStatic"/> -->
     <rule ref="category/java/errorprone.xml/AvoidAccessibilityAlteration"/>
     <rule ref="category/java/errorprone.xml/AvoidAssertAsIdentifier"/>
-    <!-- <rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop"/> -->
+    <rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop"/>
     <!-- <rule ref="category/java/errorprone.xml/AvoidCallingFinalize"/> -->
     <!-- <rule ref="category/java/errorprone.xml/AvoidCatchingNPE"/> -->
     <rule ref="category/java/errorprone.xml/AvoidCatchingThrowable"/>
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopRule.java
index b7348aa26f..1d6f66afd1 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopRule.java
@@ -20,12 +20,11 @@ import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
 import net.sourceforge.pmd.properties.PropertyFactory;
-import net.sourceforge.pmd.properties.PropertySource;
 
-public class AvoidBranchingStatementAsLastInLoopRule extends AbstractJavaRule {
+public class AvoidBranchingStatementAsLastInLoopRule extends AbstractJavaRulechainRule {
 
     public static final String CHECK_FOR = "for";
     public static final String CHECK_DO = "do";
@@ -53,20 +52,17 @@ public class AvoidBranchingStatementAsLastInLoopRule extends AbstractJavaRule {
 
 
     public AvoidBranchingStatementAsLastInLoopRule() {
+        super(ASTBreakStatement.class, ASTContinueStatement.class, ASTReturnStatement.class);
         definePropertyDescriptor(CHECK_BREAK_LOOP_TYPES);
         definePropertyDescriptor(CHECK_CONTINUE_LOOP_TYPES);
         definePropertyDescriptor(CHECK_RETURN_LOOP_TYPES);
-
-        addRuleChainVisit(ASTBreakStatement.class);
-        addRuleChainVisit(ASTContinueStatement.class);
-        addRuleChainVisit(ASTReturnStatement.class);
     }
 
 
     @Override
     public Object visit(ASTBreakStatement node, Object data) {
         // skip breaks, that are within a switch statement
-        if (node.getNthParent(3) instanceof ASTSwitchStatement) {
+        if (node.ancestors().get(1) instanceof ASTSwitchStatement) {
             return data;
         }
         return check(CHECK_BREAK_LOOP_TYPES, node, data);
@@ -74,7 +70,7 @@ public class AvoidBranchingStatementAsLastInLoopRule extends AbstractJavaRule {
 
 
     protected Object check(PropertyDescriptor<List<String>> property, Node node, Object data) {
-        Node parent = node.getNthParent(5);
+        Node parent = node.ancestors().get(1);
         if (parent instanceof ASTForStatement) {
             if (hasPropertyValue(property, CHECK_FOR)) {
                 super.addViolation(data, node);
@@ -109,9 +105,6 @@ public class AvoidBranchingStatementAsLastInLoopRule extends AbstractJavaRule {
     }
 
 
-    /**
-     * @see PropertySource#dysfunctionReason()
-     */
     @Override
     public String dysfunctionReason() {
         return checksNothing() ? "All loop types are ignored" : null;
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopTest.java
index 3a266f5964..3628a5de05 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidBranchingStatementAsLastInLoopTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
 
 import net.sourceforge.pmd.testframework.PmdRuleTst;
 
-@org.junit.Ignore("Rule has not been updated yet")
 public class AvoidBranchingStatementAsLastInLoopTest extends PmdRuleTst {
     // no additional unit tests
 }
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidBranchingStatementAsLastInLoop.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidBranchingStatementAsLastInLoop.xml
index 69f5b69865..6f5bbf7650 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidBranchingStatementAsLastInLoop.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidBranchingStatementAsLastInLoop.xml
@@ -101,6 +101,7 @@ public class Foo {
     <test-code>
         <description>violations: break:for/do/while, continue:for/do/while and return:for/do/while</description>
         <expected-problems>9</expected-problems>
+        <expected-linenumbers>5,8,11,14,17,20,23,26,29</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -110,6 +111,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>3</expected-problems>
+        <expected-linenumbers>8,17,26</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -119,6 +121,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes">for|do|while</rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>3</expected-problems>
+        <expected-linenumbers>11,20,29</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -128,6 +131,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes">for|do|while</rule-property>
         <expected-problems>3</expected-problems>
+        <expected-linenumbers>5,14,23</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -137,6 +141,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>8</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -146,6 +151,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>26</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -155,6 +161,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>17</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -164,6 +171,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes">for</rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>11</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -173,6 +181,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes">do</rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>29</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -182,6 +191,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes">while</rule-property>
         <rule-property name="checkReturnLoopTypes"></rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>20</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -191,6 +201,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes">for</rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>5</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -200,6 +211,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes">do</rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>23</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -209,6 +221,7 @@ public class Foo {
         <rule-property name="checkContinueLoopTypes"></rule-property>
         <rule-property name="checkReturnLoopTypes">while</rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>14</expected-linenumbers>
         <code-ref id="violations"/>
     </test-code>
 
@@ -217,7 +230,7 @@ public class Foo {
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
-    public void bar() {
+    public void bar(String str) {
         for (int i=0; i<str.length(); i++)
             switch(str.charAt(i)) {
                 case 'a':