[core] Render config errors#519
Conversation
- All renderers preivously including processing errors now also include configuration errors. - I take the chance to fix a couple of issues with `VBHTMLRenderer`, that was rendering processing errors using `toString()` (which has a default implementation) - Notice that renderers that didn't previously include processing errors are unmodified
adangel
left a comment
There was a problem hiding this comment.
Looks good. Maybe we should for now just mention in the changelog, that the XML renderer has now a new element. WDYT?
| // config errors | ||
| for (final Report.ConfigurationError ce : configErrors) { | ||
| buf.setLength(0); | ||
| buf.append("<configerror ").append("rule=\""); |
There was a problem hiding this comment.
I'm wondering whether we have documented the XML report format somewhere.... We are adding now a new element, which might break other tools. I think maven-pmd-plugin is safe, since it doesn't rely on that renderer, but maybe the Jenkins plugin does?
There was a problem hiding this comment.
yes, tools such as jenkins are possibly going to break, but that's why this change is ok for 6.0.0.
We should probably create an XML Schema for this report and expose it...
There was a problem hiding this comment.
Yes, I'm fine with breaking compatibility. We just need to be aware of that.
Creating a schema probably makes sense.
There was a problem hiding this comment.
I did include a notice in the release changes, but feel free to edit it if you don't consider it explicit enough: https://github.com/pmd/pmd/pull/519/files#diff-b7709f07439de5204938d714fe93ed84
I'll get to the XML Schema on a separate PR / task
|
Merged! Thanks again! |
Under multi-thread runs, each thread parsed it's own copy of the ruleset, and removed dysfunctional rules. This meant 1 `ConfigError` per thread was being generated. Since pmd#519, we now log `ConfigErrors`, meaning multiple identical logs in reports and noise for the user. Therefore, we change our approach. RuleSets are parsed only once. Dysfunctional rules are filtered and reported early on, before dispathing actual file analysis. Each analysis thread can then produce their own copy of the ruleset by taking advantage of the `deepCopy` support for rules, ruleset and rulesets introduced in pmd#464. The result is not only more concise reports, but the analysis thread actually perform less work. Rulesets XMLs are parsed exactly once, instead of 1 + 1 per thread as before, and rule configuration is validated only once. During this process an issue with how property descriptors where copied has arised and was fixed. A new unit test to validate this scenario has been introduced.
Before submitting a PR, please check that:
master. The PMD team will merge back to support branches as needed.mvn testpasses.mvn checkstyle:checkpasses. Check this for more infoPR Description:
VBHTMLRenderer, that was rendering processing errors usingtoString()(which has a default implementation)