From 9e078366590b5efe2fc336c48fd863ded3e4b521 Mon Sep 17 00:00:00 2001
From: Gwilym Kuiper <gwilym@gearset.com>
Date: Fri, 3 Apr 2020 15:10:30 +0100
Subject: [PATCH] Correctly detect fields that appear after static
 initialization blocks

---
 .../FieldDeclarationsShouldBeAtStartRule.java   | 14 +++++++++++++-
 .../xml/FieldDeclarationsShouldBeAtStart.xml    | 17 +++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java
index 8144dabbbb..9386accdad 100644
--- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java
+++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java
@@ -8,6 +8,8 @@ import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement;
 import net.sourceforge.pmd.lang.apex.ast.ASTField;
@@ -32,7 +34,7 @@ public class FieldDeclarationsShouldBeAtStartRule extends AbstractApexRule {
 
         List<ApexNode<?>> nonFieldDeclarations = new ArrayList<>();
 
-        nonFieldDeclarations.addAll(node.findChildrenOfType(ASTMethod.class));
+        nonFieldDeclarations.addAll(getMethodNodes(node));
         nonFieldDeclarations.addAll(node.findChildrenOfType(ASTUserClass.class));
         nonFieldDeclarations.addAll(node.findChildrenOfType(ASTProperty.class));
         nonFieldDeclarations.addAll(node.findChildrenOfType(ASTBlockStatement.class));
@@ -54,4 +56,14 @@ public class FieldDeclarationsShouldBeAtStartRule extends AbstractApexRule {
 
         return super.visit(node, data);
     }
+
+    private List<ApexNode<?>> getMethodNodes(ASTUserClass node) {
+        // The method <clinit> represents static initializer blocks, of which there can be many. Given that the
+        // <clinit> method doesn't contain location information, only the containing ASTBlockStatements, we fetch
+        // them for that method only.
+        return node.findChildrenOfType(ASTMethod.class).stream()
+            .flatMap(method -> method.getImage().equals("<clinit>") ?
+                method.findChildrenOfType(ASTBlockStatement.class).stream() : Stream.of(method))
+            .collect(Collectors.toList());
+    }
 }
diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml
index 2abc99d575..f8f1b95503 100644
--- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml
+++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml
@@ -184,6 +184,23 @@ class Foo {
 
     public Integer thisIsNotOkay;
 }
+]]>
+        </code>
+    </test-code>
+
+    <test-code>
+        <description>Fields should go before static block statements</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>6</expected-linenumbers>
+        <code>
+            <![CDATA[
+class Foo {
+    static {
+        System.debug('Hello');
+    }
+
+    public Integer thisIsNotOkay;
+}
 ]]>
         </code>
     </test-code>