[core] Better error reporting for the ruleset parser by oowekyala · Pull Request #3922 · pmd/pmd

oowekyala · 2022-04-15T19:39:51Z

Describe the PR

Use the library I wrote to display nice error messages when parsing the ruleset XML. This also uniformizes the error handling, which previously was a mix of IllegalArgumentExceptions and logger calls.

Sample output:

[main] ERROR net.sourceforge.pmd.PMD - Error at sandbox/broken-rset.xml:75:5
 73|     <!--    </rule>-->
 74| 
 75|     <rule ref="category/java/codestyle.xml/UxxxnnecessaryImport" />
         ^^^^^ Unable to find referenced rule UxxxnnecessaryImport; perhaps the rule name is misspelled?

 76| 
 77|     <rule ref="category/java/codestyle.xml/UnnecessaryImport">
[main] ERROR net.sourceforge.pmd.PMD - Error at sandbox/broken-rset.xml:78:12
 76| 
 77|     <rule ref="category/java/codestyle.xml/UnnecessaryImport">
 78|            <priority>0</priority>
                ^^^^^^^^^ Not a valid priority: '0', expected a number in [1,5]

 79|     </rule>
 80| 
[main] ERROR net.sourceforge.pmd.PMD - Cannot load ruleset sandbox/broken-rset.xml: 2 XML validation errors occurred

Note that as per #3816 we will stop using a Logger to report these errors, so that the [main] ERROR net.sourceforge.pmd.PMD headers will disappear

Previously (notice the second error is not reported):

[main] ERROR net.sourceforge.pmd.PMD - Cannot load ruleset sandbox/broken-rset.xml: Unable to find referenced rule UxxxnnecessaryImport; perhaps the rule name is misspelled?
[main] WARN net.sourceforge.pmd.PMD - No rules found. Maybe you misspelled a rule name? (sandbox/broken-rset.xml)

Related issues

Related to [core] Error reporting vs logging #3816
Part of [core] Error reporting implementation #3923
Blocks [core] Make LanguageRegistry non static #3918 (for trivial reasons)
Blocks [core] Implementation plan for property framework changes #1432

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)

…parser

ghost · 2022-06-25T18:02:17Z

	2 Messages
📖	Compared to pmd/7.0.x: This changeset changes 22 violations, introduces 27 new violations, 0 new errors and 0 new configuration errors, removes 22 violations, 0 errors and 0 configuration errors. Full report
📖	Compared to master: This changeset changes 57093 violations, introduces 34427 new violations, 2 new errors and 0 new configuration errors, removes 138135 violations, 25 errors and 6 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 21 violations, introduces 21 new violations, 0 new errors and 0 new configuration errors, removes 21 violations, 0 errors and 0 configuration errors. [Full report](<html> <title>504 Gateway Time-out</title> 504 Gateway Time-out nginx </html>/diff1/index.html)
✅	Compared to master: This changeset changes 57096 violations, introduces 34419 new violations, 2 new errors and 0 new configuration errors, removes 138132 violations, 25 errors and 6 configuration errors. [Full report](<html> <title>504 Gateway Time-out</title> 504 Gateway Time-out nginx </html>/diff2/index.html)
✅	Compared to pmd/7.0.x: This changeset changes 23 violations, introduces 20 new violations, 0 new errors and 0 new configuration errors, removes 27 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 57096 violations, introduces 34417 new violations, 2 new errors and 0 new configuration errors, removes 138132 violations, 25 errors and 6 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 32 violations, introduces 18 new violations, 0 new errors and 0 new configuration errors, removes 16 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 57095 violations, introduces 34427 new violations, 2 new errors and 0 new configuration errors, removes 138133 violations, 25 errors and 6 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 30 violations, introduces 19 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 57095 violations, introduces 34424 new violations, 2 new errors and 0 new configuration errors, removes 138133 violations, 25 errors and 6 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 19 violations, introduces 15 new violations, 0 new errors and 0 new configuration errors, removes 13 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 57095 violations, introduces 34428 new violations, 2 new errors and 0 new configuration errors, removes 138133 violations, 25 errors and 6 configuration errors. Full report

Generated by 🚫 Danger

…n-ruleset-parser

adangel

Many Thanks! 🎉 This will provide much more useful error messages than ever!

I'll update your library to 3.1 and merge this.

adangel · 2022-07-14T16:18:19Z


    <profiles>
+        <profile>
+            <!-- Adds an SLF4J implementation, useful when developing within an IDE -->


Maybe we can add this as test dependency in general?
Hm... for core, we have this already.
In java, it is missing.
Or do you need it, when you run CLI from within an IDE?

I'll keep it in, but would be interested in how you use it...

I think it was to run tests like CliTest (in module pmd-java). Intellij would run them but print a warning that there is no logger implementation, and hid the messages. I'm not sure a profile is the best way, maybe just having the test dependency as you suggest would be better...

@adangel I remember now where I needed this. I have a bunch of run configurations for Intellij which run the main class PMD with different arguments. Most of them use the classpath of pmd-java, so that you can launch them quicker (Intellij only needs to build pmd-java and its dependencies, not all modules). But Intellij doesn't use test dependencies when a run configuration depends on a module, so without the profile there would be no slf4j impl in this setup.

[core] Better error reporting for the ruleset parser #3922

oowekyala · 2022-07-15T12:16:01Z

Thanks for merging!

They were merged on separate branches

oowekyala added 9 commits April 15, 2022 15:43

Checkout ruleset parser changes

fc81d64

Adapt code to work without changes to properties

a09cc05

Move package

59f7bb3

Update to release version of nxml-messages

cb9a092

Fix errors being duplicated

d7d3bb8

More cleanups

6084611

Add ctor for ruleset reference id

3acc740

Fix quoting

bdc116e

Don't accumulate

f1946be

oowekyala added this to the 6.45.0 milestone Apr 15, 2022

oowekyala added 6 commits April 15, 2022 21:45

Fix bugs

559e883

Cleanup xml parser tests

1fd3436

Fix test strings not being MessageFormat

f9a1e84

Fix improper MessageFormat in some messages

a9364e8

More cleanups

9aee9be

Add a test for the full message

56c18f4

oowekyala modified the milestones: 6.45.0, 7.0.0 Apr 16, 2022

oowekyala mentioned this pull request Apr 16, 2022

[core] Error reporting implementation #3923

Open

12 tasks

oowekyala added 2 commits April 16, 2022 19:42

Fix bug

820c210

add test

7773559

adangel self-requested a review April 26, 2022 20:19

oowekyala mentioned this pull request May 7, 2022

[core] Deprecate some syntax for ruleset references #3958

Merged

4 tasks

oowekyala added 2 commits June 25, 2022 18:34

Merge branch '7.0.x' into xml-messages-in-ruleset-parser

7e2a1bf

Merge branch 'abort-on-semantic-errors' into xml-messages-in-ruleset-…

799fa8b

…parser

oowekyala marked this pull request as ready for review June 25, 2022 17:29

oowekyala added 3 commits June 25, 2022 20:23

Merge branch '7.0.x' into xml-messages-in-ruleset-parser

c3d4b3d

Merge remote-tracking branch 'upstream/pmd/7.0.x' into xml-messages-i…

6d6e190

…n-ruleset-parser

Fix code style

535c74b

adangel self-assigned this Jul 14, 2022

adangel approved these changes Jul 14, 2022

View reviewed changes

adangel added a commit that referenced this pull request Jul 14, 2022

Merge pull request #3922 from oowekyala:xml-messages-in-ruleset-parser

77e39e9

[core] Better error reporting for the ruleset parser #3922

adangel merged commit 24e159a into pmd:pmd/7.0.x Jul 14, 2022

oowekyala deleted the xml-messages-in-ruleset-parser branch July 15, 2022 09:19

oowekyala added a commit to adangel/pmd that referenced this pull request Jul 16, 2022

Better integrate pmd#3922 and pmd#3958

e6c3f6f

They were merged on separate branches

adangel added the in:ruleset-xml About the ruleset schema/parser label Jan 13, 2023

adangel mentioned this pull request Jan 23, 2023

PMD 7 Tracking Issue #3898

Closed

55 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[core] Better error reporting for the ruleset parser#3922

[core] Better error reporting for the ruleset parser#3922
adangel merged 22 commits into
pmd:pmd/7.0.xfrom
oowekyala:xml-messages-in-ruleset-parser

oowekyala commented Apr 15, 2022 •

edited

Loading

Uh oh!

ghost commented Jun 25, 2022 •

edited by ghost

Loading

504 Gateway Time-out

504 Gateway Time-out

Uh oh!

adangel left a comment

Uh oh!

Uh oh!

Uh oh!

adangel Jul 14, 2022

Uh oh!

oowekyala Jul 15, 2022

Uh oh!

oowekyala Jul 20, 2022

Uh oh!

oowekyala commented Jul 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

oowekyala commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the PR

Related issues

Ready?

Uh oh!

ghost commented Jun 25, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

504 Gateway Time-out

504 Gateway Time-out

Uh oh!

adangel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adangel Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

oowekyala Jul 15, 2022

Choose a reason for hiding this comment

Uh oh!

oowekyala Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

oowekyala commented Jul 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oowekyala commented Apr 15, 2022 •

edited

Loading

ghost commented Jun 25, 2022 •

edited by ghost

Loading