From ae12930b0e5daaaa81843cac0b0fc6d09f5b4c6f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 9 Oct 2025 12:12:30 +0200 Subject: [PATCH 1/4] Generate ruleset for known rules With the previous fix for #124, if a rule named "Abstract..." has been changed, we would create a regression report for all rules. This PR now changes the implementation to create a list of all rules with there java source file name. --- lib/pmdtester/builders/rule_set_builder.rb | 52 +++++++++++++++---- ...ted-abstractclasswithoutabstractmethod.xml | 5 ++ .../resources/category/apex/bestpractices.xml | 27 ++++++++++ .../resources/category/java/bestpractices.xml | 28 ++++++++++ .../resources/category/java/codestyle.xml | 39 ++++++++++++++ .../main/resources/category/java/design.xml | 26 ++++++++++ test/test_rule_set_builder.rb | 29 ++++++++++- 7 files changed, 193 insertions(+), 13 deletions(-) create mode 100644 test/resources/rule_set_builder/expected-abstractclasswithoutabstractmethod.xml create mode 100644 test/resources/rule_set_builder/partial-pmd-repo/pmd-apex/src/main/resources/category/apex/bestpractices.xml create mode 100644 test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/bestpractices.xml create mode 100644 test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/codestyle.xml create mode 100644 test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/design.xml diff --git a/lib/pmdtester/builders/rule_set_builder.rb b/lib/pmdtester/builders/rule_set_builder.rb index 99375022..ad892e22 100644 --- a/lib/pmdtester/builders/rule_set_builder.rb +++ b/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 diff --git a/test/resources/rule_set_builder/expected-abstractclasswithoutabstractmethod.xml b/test/resources/rule_set_builder/expected-abstractclasswithoutabstractmethod.xml new file mode 100644 index 00000000..2b008a36 --- /dev/null +++ b/test/resources/rule_set_builder/expected-abstractclasswithoutabstractmethod.xml @@ -0,0 +1,5 @@ + + + The ruleset generated by PmdTester dynamically + + diff --git a/test/resources/rule_set_builder/partial-pmd-repo/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/test/resources/rule_set_builder/partial-pmd-repo/pmd-apex/src/main/resources/category/apex/bestpractices.xml new file mode 100644 index 00000000..bba5884f --- /dev/null +++ b/test/resources/rule_set_builder/partial-pmd-repo/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -0,0 +1,27 @@ + + + + + +Rules which enforce generally accepted best practices. + + + + + + not important for this test + + 3 + + + + + diff --git a/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/bestpractices.xml b/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/bestpractices.xml new file mode 100644 index 00000000..72c28571 --- /dev/null +++ b/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -0,0 +1,28 @@ + + + + + +Rules which enforce generally accepted best practices. + + + + + not important for this test + + 3 + + + + + + + diff --git a/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/codestyle.xml b/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/codestyle.xml new file mode 100644 index 00000000..5e911058 --- /dev/null +++ b/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/codestyle.xml @@ -0,0 +1,39 @@ + + + + + + Rules which enforce a specific coding style. + + + + + not important for this test + + 3 + + + + + + + not important for this test + + 3 + + + + diff --git a/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/design.xml b/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/design.xml new file mode 100644 index 00000000..ac0ac175 --- /dev/null +++ b/test/resources/rule_set_builder/partial-pmd-repo/pmd-java/src/main/resources/category/java/design.xml @@ -0,0 +1,26 @@ + + + + + +Rules that help you discover design issues. + + + + + not important for this test + + 3 + + + + + diff --git a/test/test_rule_set_builder.rb b/test/test_rule_set_builder.rb index 586872ee..d777b676 100644 --- a/test/test_rule_set_builder.rb +++ b/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 From 9cb700b3d068c45e330d5810007c1404d3daf8b4 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 9 Oct 2025 16:31:44 +0200 Subject: [PATCH 2/4] Skip tests completely while building PMD This should make the build a tiny bit faster. --- lib/pmdtester/builders/pmd_report_builder.rb | 2 +- test/test_pmd_report_builder.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pmdtester/builders/pmd_report_builder.rb b/lib/pmdtester/builders/pmd_report_builder.rb index e126dc14..7858edd3 100644 --- a/lib/pmdtester/builders/pmd_report_builder.rb +++ b/lib/pmdtester/builders/pmd_report_builder.rb @@ -256,7 +256,7 @@ def build_pmd_with_maven # build command since PMD migrated to central portal './mvnw clean package -V ' \ '-PfastSkip ' \ - '-DskipTests ' \ + '-Dmaven.test.skip=true ' \ '-T1C -B' else extra_java_home = "#{Dir.home}/openjdk11" diff --git a/test/test_pmd_report_builder.rb b/test/test_pmd_report_builder.rb index a028905f..892c9c2d 100644 --- a/test/test_pmd_report_builder.rb +++ b/test/test_pmd_report_builder.rb @@ -388,7 +388,7 @@ def stub_pmd_build_maven_new_pmd7_build PmdTester::Cmd.stubs(:execute_successfully).with do |cmd, extra_java_home| if cmd == './mvnw clean package -V ' \ '-PfastSkip ' \ - '-DskipTests ' \ + '-Dmaven.test.skip=true ' \ '-T1C -B' && extra_java_home.nil? FileUtils.mkdir_p 'pmd-dist/target' From b5c89c69f6bd282a66500744f248f96a08fe1e19 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 9 Oct 2025 16:51:42 +0200 Subject: [PATCH 3/4] Update History.md --- History.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/History.md b/History.md index 7ab865ef..e75a8965 100644 --- a/History.md +++ b/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 From 33891a15915a668ef21d6a7f76754c2d05aea4d4 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 10 Oct 2025 15:07:27 +0200 Subject: [PATCH 4/4] Revert "Skip tests completely while building PMD" We need to compile tests, as pmd-ant is depending on the test artifacts from pmd-core... This reverts commit 9cb700b3d068c45e330d5810007c1404d3daf8b4. --- lib/pmdtester/builders/pmd_report_builder.rb | 2 +- test/test_pmd_report_builder.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pmdtester/builders/pmd_report_builder.rb b/lib/pmdtester/builders/pmd_report_builder.rb index 7858edd3..e126dc14 100644 --- a/lib/pmdtester/builders/pmd_report_builder.rb +++ b/lib/pmdtester/builders/pmd_report_builder.rb @@ -256,7 +256,7 @@ def build_pmd_with_maven # build command since PMD migrated to central portal './mvnw clean package -V ' \ '-PfastSkip ' \ - '-Dmaven.test.skip=true ' \ + '-DskipTests ' \ '-T1C -B' else extra_java_home = "#{Dir.home}/openjdk11" diff --git a/test/test_pmd_report_builder.rb b/test/test_pmd_report_builder.rb index 892c9c2d..a028905f 100644 --- a/test/test_pmd_report_builder.rb +++ b/test/test_pmd_report_builder.rb @@ -388,7 +388,7 @@ def stub_pmd_build_maven_new_pmd7_build PmdTester::Cmd.stubs(:execute_successfully).with do |cmd, extra_java_home| if cmd == './mvnw clean package -V ' \ '-PfastSkip ' \ - '-Dmaven.test.skip=true ' \ + '-DskipTests ' \ '-T1C -B' && extra_java_home.nil? FileUtils.mkdir_p 'pmd-dist/target'