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 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