From 3627167efac8e7732ccb31b4abd82717cdb0d2a6 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <andreas.dangel@pmd-code.org>
Date: Fri, 14 Jan 2022 09:13:37 +0100
Subject: [PATCH] [java] Update rules TooManyMethods and TooManyFields

TooManyRules is now a XPath rule as well
---
 .ci/files/all-java.xml                        |  4 +-
 .../java/rule/design/TooManyFieldsRule.java   | 54 -------------------
 .../main/resources/category/java/design.xml   | 38 ++++++-------
 .../java/rule/design/TooManyFieldsTest.java   |  1 -
 .../java/rule/design/TooManyMethodsTest.java  |  1 -
 .../java/rule/design/xml/TooManyFields.xml    |  3 ++
 6 files changed, 21 insertions(+), 80 deletions(-)
 delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsRule.java

diff --git a/.ci/files/all-java.xml b/.ci/files/all-java.xml
index 040fb26ada..1fa50b951b 100644
--- a/.ci/files/all-java.xml
+++ b/.ci/files/all-java.xml
@@ -155,8 +155,8 @@
     <rule ref="category/java/design.xml/SimplifyConditional"/>
     <rule ref="category/java/design.xml/SingularField"/>
     <rule ref="category/java/design.xml/SwitchDensity"/>
-    <!-- <rule ref="category/java/design.xml/TooManyFields"/> -->
-    <!-- <rule ref="category/java/design.xml/TooManyMethods"/> -->
+    <rule ref="category/java/design.xml/TooManyFields"/>
+    <rule ref="category/java/design.xml/TooManyMethods"/>
     <!-- <rule ref="category/java/design.xml/UseObjectForClearerAPI"/> -->
     <rule ref="category/java/design.xml/UseUtilityClass"/>
     <rule ref="category/java/design.xml/UselessOverridingMethod"/>
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsRule.java
deleted file mode 100644
index 10f1ccbbe1..0000000000
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsRule.java
+++ /dev/null
@@ -1,54 +0,0 @@
-/**
- * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
- */
-
-package net.sourceforge.pmd.lang.java.rule.design;
-
-import static net.sourceforge.pmd.properties.constraints.NumericConstraints.positive;
-
-import java.util.List;
-
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.properties.PropertyDescriptor;
-import net.sourceforge.pmd.properties.PropertyFactory;
-
-
-public class TooManyFieldsRule extends AbstractJavaRule {
-
-    private static final int DEFAULT_MAXFIELDS = 15;
-
-    private static final PropertyDescriptor<Integer> MAX_FIELDS_DESCRIPTOR
-            = PropertyFactory.intProperty("maxfields")
-                             .desc("Max allowable fields")
-                             .defaultValue(DEFAULT_MAXFIELDS)
-                             .require(positive())
-                             .build();
-
-    public TooManyFieldsRule() {
-        definePropertyDescriptor(MAX_FIELDS_DESCRIPTOR);
-        addRuleChainVisit(ASTClassOrInterfaceDeclaration.class);
-    }
-
-    @Override
-    public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
-        final int maxFields = getProperty(MAX_FIELDS_DESCRIPTOR);
-        int counter = 0;
-
-        final List<ASTFieldDeclaration> l = node.findDescendantsOfType(ASTFieldDeclaration.class);
-
-        for (ASTFieldDeclaration fd : l) {
-            if (fd.isFinal() && fd.isStatic()) {
-                continue;
-            }
-            counter++;
-        }
-
-        if (counter > maxFields) {
-            addViolation(data, node);
-        }
-
-        return data;
-    }
-}
diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml
index ff4d2f590e..8b9cfb174d 100644
--- a/pmd-java/src/main/resources/category/java/design.xml
+++ b/pmd-java/src/main/resources/category/java/design.xml
@@ -1300,7 +1300,7 @@ public class Foo {
           language="java"
           since="3.0"
           message="Too many fields"
-          class="net.sourceforge.pmd.lang.java.rule.design.TooManyFieldsRule"
+          class="net.sourceforge.pmd.lang.rule.XPathRule"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#toomanyfields">
         <description>
 Classes that have too many fields can become unwieldy and could be redesigned to have fewer fields,
@@ -1308,6 +1308,20 @@ possibly through grouping related fields in new objects.  For example, a class w
 city/state/zip fields could park them within a single Address field.
         </description>
         <priority>3</priority>
+        <properties>
+            <property name="maxfields" type="Integer" description="Max allowable fields" min="1" max="1000" value="15"/>
+            <property name="xpath">
+                <value>
+<![CDATA[
+//ClassOrInterfaceDeclaration/ClassOrInterfaceBody
+  [count(FieldDeclaration
+            [not(pmd-java:modifiers() = 'final')]
+            [not(pmd-java:modifiers() = 'static')]
+        ) > $maxfields]
+]]>
+                </value>
+            </property>
+        </properties>
         <example>
 <![CDATA[
 public class Person {   // too many separate fields
@@ -1341,30 +1355,10 @@ complexity and find a way to have more fine grained objects.
             <property name="maxmethods" type="Integer" description="The method count reporting threshold" min="1" max="1000" value="10"/>
             <property name="xpath">
                 <value>
-                    <!--  FIXME: Refine XPath to discard 'get' and 'set' methods with Block no more than 3 lines,
-                                something like this:
-                                    not (
-                                            (
-                                                starts-with(@Name,'get')
-                                                or
-                                                starts-with(@Name,'set')
-                                                or
-                                                starts-with(@Name,'is')
-                                            )
-                                            and (
-                                                    (
-                                                        (../Block/attribute::endLine)
-                                                         -
-                                                        (../Block/attribute::beginLine)
-                                                    ) <= 3
-                                            )
-                                        )
-                                This will avoid discarding 'real' methods...
-                     -->
 <![CDATA[
  //ClassOrInterfaceDeclaration/ClassOrInterfaceBody
      [
-      count(./ClassOrInterfaceBodyDeclaration/MethodDeclaration[
+      count(MethodDeclaration[
          not (
                 starts-with(@Name,'get')
                 or
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsTest.java
index 8852e9e350..12cad4bac7 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyFieldsTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design;
 
 import net.sourceforge.pmd.testframework.PmdRuleTst;
 
-@org.junit.Ignore("Rule has not been updated yet")
 public class TooManyFieldsTest extends PmdRuleTst {
     // no additional unit tests
 }
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyMethodsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyMethodsTest.java
index b47d8b6623..c81262b265 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyMethodsTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/TooManyMethodsTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design;
 
 import net.sourceforge.pmd.testframework.PmdRuleTst;
 
-@org.junit.Ignore("Rule has not been updated yet")
 public class TooManyMethodsTest extends PmdRuleTst {
     // no additional unit tests
 }
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/TooManyFields.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/TooManyFields.xml
index 9d03088002..2e91d9c903 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/TooManyFields.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/TooManyFields.xml
@@ -22,12 +22,14 @@ public class Foo {
         <description>3 fields, reduced max to 2</description>
         <rule-property name="maxfields">2</rule-property>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>1</expected-linenumbers>
         <code-ref id="id-3-fields"/>
     </test-code>
 
     <test-code>
         <description>16 fields, bad</description>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>1</expected-linenumbers>
         <code><![CDATA[
 public class Foo {
     int a1;
@@ -116,6 +118,7 @@ public interface Foo {
     <test-code>
         <description>2 inner classes, each with > 10 fields</description>
         <expected-problems>2</expected-problems>
+        <expected-linenumbers>2,20</expected-linenumbers>
         <code><![CDATA[
 public class Foo {
     public class Bar1 {