Skip to content

[core] Make LanguageRegistry not static#4049

Merged
adangel merged 19 commits into
pmd:pmd/7.0.xfrom
oowekyala:pmd7-lang-registry-non-static
Sep 10, 2022
Merged

[core] Make LanguageRegistry not static#4049
adangel merged 19 commits into
pmd:pmd/7.0.xfrom
oowekyala:pmd7-lang-registry-non-static

Conversation

@oowekyala

Copy link
Copy Markdown
Member

Describe the PR

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@oowekyala oowekyala added this to the 7.0.0 milestone Jul 19, 2022
@ghost

ghost commented Jul 19, 2022

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 24 violations,
introduces 24 new violations, 0 new errors and 0 new configuration errors,
removes 15 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 0 violations,
introduces 699668 new violations, 22 new errors and 0 new configuration errors,
removes 826993 violations, 39 errors and 6 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 25 violations,
introduces 16 new violations, 0 new errors and 0 new configuration errors,
removes 20 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 0 violations,
introduces 699652 new violations, 22 new errors and 0 new configuration errors,
removes 827036 violations, 39 errors and 6 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 34 violations,
introduces 33 new violations, 0 new errors and 0 new configuration errors,
removes 23 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 0 violations,
introduces 699666 new violations, 22 new errors and 0 new configuration errors,
removes 827036 violations, 39 errors and 6 configuration errors.
Full report

Generated by 🚫 Danger

@oowekyala oowekyala marked this pull request as ready for review July 21, 2022 17:00
@oowekyala oowekyala mentioned this pull request Jul 22, 2022
8 tasks
@adangel adangel self-requested a review August 30, 2022 19:26
@adangel adangel mentioned this pull request Sep 10, 2022
55 tasks

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good!

I'm going to merge this now... Note: This will create conflicts with #4059

@oowekyala Do you have any idea about the changed violations in the regression report? It's only the rule LawOfDemeter...

private static String supportedVersions() {
return "Languages and version suported:" + PMD.EOL
+ LanguageRegistry.getLanguages().stream().map(Language::getTerseName).collect(Collectors.joining(", "))
+ LanguageRegistry.PMD.commaSeparatedList(Language::getId)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... this method commaSeparatedList sounds more like a utility method that's too detailed. Do we need this flexibility with the parameter?
What we need here IMHO is a method, that just returns a printable list of available languages...


private @NonNull String supportedLanguages() {
return LanguageRegistry.getLanguages().stream().map(Language::getTerseName).map(StringUtil::inSingleQuotes).collect(Collectors.joining(", "));
return languageRegistry.commaSeparatedList(l -> StringUtil.inSingleQuotes(l.getId()));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's better if the languageRegistry has a method like "getSupportedLanguagesAsString" (or sth. like a "toString" method) which always returns the list the same way - for consistency. Here we add single quotes, in the CLI help, we don't.

adangel added a commit that referenced this pull request Sep 10, 2022
@adangel adangel merged commit 29d31ec into pmd:pmd/7.0.x Sep 10, 2022
@oowekyala oowekyala deleted the pmd7-lang-registry-non-static branch September 10, 2022 20:23
@oowekyala

Copy link
Copy Markdown
Member Author

@oowekyala Do you have any idea about the changed violations in the regression report? It's only the rule LawOfDemeter...

LawOfDemeter has been on my radar for a while... I think the problem is it relies accidentally on the order of rulechain visits, which is not necessarily stable across runs...

adangel added a commit to pmd/pmd-designer that referenced this pull request Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants