Support --auto-gen-config for rules which start with "Abstract" (#142)

This solves #124 slightly differently: As we know, which rules exist, we
use that information and build up a map of rule java source files to
rule references. E.g. in
pmd-java/src/main/categories/java/bestpractices.xml we see, that the
rule "AbstractClassWithoutAbstractMethod" is implemented in class
"net.sourceforge.pmd.lang.java.rule.bestpractices.AbstractClassWithoutAbstractMethodRule".
For this class name we can guess the corresponding java source
(.../src/main/java/...). So, when we see this java source in the
changes, then we know, we need to regression test that rule.

This means, we don't guess anymore, whether a java source file is a rule
implementation - we have a list of all rule implementations that we can
check now.

This also fixes the integration test
(https://github.com/pmd/pmd-regression-tester/actions/workflows/manual-integration-tests.yml):
In test case 1 we changed the rule AbstractClassWithoutAbstractMethod
and we expect, that we only see differences from this rule. As other
rules won't be executed, we expected that we get less errors - but with
the change from #141 now all rules have been executed again.


Refs
- #124 
- #141

Author: adangel

History.md

@@ -2,7 +2,9 @@
 ## New and Noteworthy
 ## Enhancements
 ## Fixed Issues
+* [#142](https://github.com/pmd/pmd-regression-tester/pull/142): Support `--auto-gen-config` for rules which start with "Abstract"
 ## Merged pull requests
+* [#142](https://github.com/pmd/pmd-regression-tester/pull/142): Support `--auto-gen-config` for rules which start with "Abstract" - [Andreas Dangel](https://github.com/adangel) (@adangel)
 * [#143](https://github.com/pmd/pmd-regression-tester/pull/143): chore(ci): Use java 17 as default - [Andreas Dangel](https://github.com/adangel) (@adangel)
 ## Dependency Updates
 

lib/pmdtester/builders/rule_set_builder.rb

@@ -25,7 +25,8 @@ def initialize(options)
     def build?
       languages = determine_languages
       filenames = diff_filenames(languages)
-      run_required, rule_refs = get_rule_refs(filenames)
+      all_rules_hash = determine_all_rules
+      run_required, rule_refs = get_rule_refs(filenames, all_rules_hash)
       if run_required
         output_filter_set(rule_refs)
         build_config_file(rule_refs)
@@ -71,8 +72,8 @@ def output_filter_set(rule_refs)
     # filtering possible or if no rules are affected.
     # Whether to run the regression test is returned as an additional boolean flag.
     #
-    def get_rule_refs(filenames)
-      run_required, categories, rules = determine_categories_rules(filenames)
+    def get_rule_refs(filenames, all_rules_hash)
+      run_required, categories, rules = determine_categories_rules(filenames, all_rules_hash)
       logger.debug "Regression test required: #{run_required}"
       logger.debug "Categories: #{categories}"
       logger.debug "Rules: #{rules}"
@@ -122,12 +123,12 @@ def write_dynamic_file(rule_refs)
 
     private
 
-    def determine_categories_rules(filenames)
+    def determine_categories_rules(filenames, all_rules_hash)
       regression_test_required = false
       categories = Set[]
       rules = Set[]
       filenames.each do |filename|
-        matched = check_single_filename?(filename, categories, rules)
+        matched = check_single_filename?(filename, all_rules_hash, categories, rules)
         regression_test_required = true if matched
 
         next if matched
@@ -141,14 +142,13 @@ def determine_categories_rules(filenames)
       [regression_test_required, categories, rules]
     end
 
-    def check_single_filename?(filename, categories, rules)
+    def check_single_filename?(filename, all_rules_hash, categories, rules)
       logger.debug "Checking #{filename}"
 
-      # matches Java-based rule implementations
-      match_data = %r{.+/src/main/java/.+/lang/([^/]+)/rule/([^/]+)/([^/]+)Rule.java}.match(filename)
-      unless match_data.nil? || match_data[3].start_with?('Abstract')
-        logger.debug "Matches: #{match_data.inspect}"
-        rules.add("#{match_data[1]}/#{match_data[2]}.xml/#{match_data[3]}")
+      # direct match of a Java-based rule implementation
+      if all_rules_hash.key?(filename)
+        logger.debug "Direct match: #{all_rules_hash[filename]}"
+        rules.add(all_rules_hash[filename])
         return true
       end
 
@@ -196,5 +196,35 @@ def determine_languages
       end
       languages
     end
+
+    # Creates a mapping between java source files and the rule reference
+    # in the ruleset, e.g.:
+    # 'pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/NcssCountRule.java'
+    #   => 'java/design.xml/NcssCount'
+    def determine_all_rules
+      all_rules_hash = {}
+      Dir.chdir(@options.local_git_repo) do
+        Dir.glob('**/src/main/resources/category/*/*.xml').each do |rulesetfile|
+          match_data = %r{.+/src/main/resources/category/([^/]+)/([^/.]+\.xml)}.match(rulesetfile)
+          ref_prefix = "#{match_data[1]}/#{match_data[2]}"
+          java_base_path = rulesetfile.gsub(%r{src/main/resources/category/.+/.+\.xml}, 'src/main/java')
+
+          doc = File.open(rulesetfile) { |f| Nokogiri::XML(f) }
+          rules = doc.css('ruleset rule')
+          rules.each do |r|
+            next if r.attributes['class'].nil? # skip rule references
+
+            ref = "#{ref_prefix}/#{r.attributes['name'].content}"
+            clazz = r.attributes['class'].content
+
+            # guess java file name at standard location src/main/java relative to the category ruleset
+            java_filename = "#{java_base_path}/#{clazz.gsub('.', '/')}.java"
+
+            all_rules_hash[java_filename] = ref
+          end
+        end
+      end
+      all_rules_hash
+    end
   end
 end

test/resources/rule_set_builder/expected-abstractclasswithoutabstractmethod.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd" name="Dynamic PmdTester Ruleset">
+    <description>The ruleset generated by PmdTester dynamically</description>
+    <rule ref="category/java/bestpractices.xml/AbstractClassWithoutAbstractMethod"/>
+</ruleset>

test/resources/rule_set_builder/partial-pmd-repo/pmd-apex/src/main/resources/category/apex/bestpractices.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<ruleset name="Best Practices"
+         xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
+
+    <description>
+Rules which enforce generally accepted best practices.
+    </description>
+
+
+    <rule name="AvoidGlobalModifier"
+          language="java"
+          since="1.0"
+          message="not important for this test"
+          class="net.sourceforge.pmd.lang.apex.rule.bestpractices.AvoidGlobalModifierRule"
+          externalInfoUrl="not important for this test">
+        <description>
+        not important for this test
+        </description>
+        <priority>3</priority>
+        <example>
+        </example>
+    </rule>
+
+</ruleset>

test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/bestpractices.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<ruleset name="Best Practices"
+    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
+
+    <description>
+Rules which enforce generally accepted best practices.
+    </description>
+
+    <rule name="AbstractClassWithoutAbstractMethod"
+          language="java"
+          since="1.0"
+          message="not important for this test"
+          class="net.sourceforge.pmd.lang.java.rule.bestpractices.AbstractClassWithoutAbstractMethodRule"
+          externalInfoUrl="not important for this test">
+        <description>
+        not important for this test
+        </description>
+        <priority>3</priority>
+        <example>
+        </example>
+    </rule>
+
+    <rule name="AReference" ref="AbstractClassWithoutAbstractMethod"/>
+
+</ruleset>

test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/codestyle.xml

@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<ruleset name="Code Style"
+         xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
+
+    <description>
+        Rules which enforce a specific coding style.
+    </description>
+
+    <rule name="UnnecessaryConstructor"
+          language="java"
+          since="1.0"
+          message="not important for this test"
+          class="net.sourceforge.pmd.lang.java.rule.codestyle.UnnecessaryConstructorRule"
+          externalInfoUrl="not important for this test">
+        <description>
+        not important for this test
+        </description>
+        <priority>3</priority>
+        <example>
+        </example>
+    </rule>
+
+    <rule name="UnnecessaryReturnValue"
+          language="java"
+          since="1.0"
+          message="not important for this test"
+          class="net.sourceforge.pmd.lang.java.rule.codestyle.UnnecessaryReturnValueRule"
+          externalInfoUrl="not important for this test">
+        <description>
+        not important for this test
+        </description>
+        <priority>3</priority>
+        <example>
+        </example>
+    </rule>
+</ruleset>

test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/design.xml

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<ruleset name="Design"
+    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
+
+    <description>
+Rules that help you discover design issues.
+    </description>
+
+    <rule name="NcssCount"
+          language="java"
+          since="1.0"
+          message="not important for this test"
+          class="net.sourceforge.pmd.lang.java.rule.design.NcssCountRule"
+          externalInfoUrl="not important for this test">
+        <description>
+        not important for this test
+        </description>
+        <priority>3</priority>
+        <example>
+        </example>
+    </rule>
+
+</ruleset>

test/test_rule_set_builder.rb

@@ -15,7 +15,7 @@ def cleanup
   def mock_build?(diff_filenames, filter_set = nil, patch_config = nil)
     options = mock
     options.expects(:patch_config).returns(Options::DEFAULT_CONFIG_PATH)
-    options.expects(:local_git_repo).returns('.')
+    options.expects(:local_git_repo).returns("#{PATH_TO_TEST_RESOURCES}/partial-pmd-repo").twice
     options.expects(:base_branch).returns('base_branch')
     options.expects(:patch_branch).returns('patch_branch')
     options.expects(:filter_set=).with(filter_set)
@@ -46,7 +46,9 @@ def test_build_design_codestyle_config
     assert_equal(expected, actual)
   end
 
-  def test_build_all_rulesets_config_abstract_rule_changed
+  def test_build_all_rulesets_config_internal_abstract_class_changed
+    # AbstractJavaRulechainRule is not a rule but an abstract base class for real rules. It lives
+    # in an internal package. "internal" is not a valid ruleset category.
     diff_filenames = <<~DOC
       pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/AbstractJavaRulechainRule.java
     DOC
@@ -56,6 +58,18 @@ def test_build_all_rulesets_config_abstract_rule_changed
            "File #{RuleSetBuilder::PATH_TO_DYNAMIC_CONFIG} must not exist")
   end
 
+  def test_build_all_rulesets_config_abstract_class_changed
+    # AbstractNamingConventionRule is not a rule despite its name, it an abstract base class for real rules.
+    # It lives in a normal category package. "codestyle" is a valid ruleset category.
+    diff_filenames = <<~DOC
+      pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AbstractNamingConventionRule.java
+    DOC
+    mock_build?(diff_filenames, nil, "#{PATH_TO_TEST_RESOURCES}/patch-ruleset.xml")
+
+    assert(!File.exist?(RuleSetBuilder::PATH_TO_DYNAMIC_CONFIG),
+           "File #{RuleSetBuilder::PATH_TO_DYNAMIC_CONFIG} must not exist")
+  end
+
   def test_build_all_rulesets_config
     diff_filenames = <<~DOC
       pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/AvoidGlobalModifierRule.java
@@ -93,6 +107,17 @@ def test_filter_ruleset_single_rule
     assert_equal(expected, actual)
   end
 
+  def test_filter_ruleset_single_rule_named_abstract
+    diff_filenames = <<~DOC
+      pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java
+    DOC
+    mock_build?(diff_filenames, Set['java/bestpractices.xml/AbstractClassWithoutAbstractMethod'])
+
+    expected = File.read("#{PATH_TO_TEST_RESOURCES}/expected-abstractclasswithoutabstractmethod.xml")
+    actual = File.read(RuleSetBuilder::PATH_TO_DYNAMIC_CONFIG)
+    assert_equal(expected, actual)
+  end
+
   def test_filter_ruleset_single_rule_and_category
     diff_filenames = <<~DOC
       pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/NcssCountRule.java