[core] Allow deep copying of RuleSets#464
Conversation
There was a problem hiding this comment.
In AbstractPropertySource there are already some methods, that may help here (or need to be adjusted to do a real deep copy): copyPropertyDescriptors, hasDescriptor.
There was a problem hiding this comment.
I'll check those, good to know. Thanks!
There was a problem hiding this comment.
I'm now using hasDescriptor, but do not use copyPropertyDescriptors (it just creates a new list with the same property descriptor instances), but I'd rather call the definePropertyDescriptor method to allow rules overriding the method applying their logic rather than just assigning the list.
|
Since this is not really required for 5.8.0, I'm moving this to 6.0.0. There would be more to do in Rule - e.g. use correct getter/setter method names (e.g. setUsesDFA(true) instead of usesDFA()).
|
|
Things I'm sharing but treating as immutable:
I may be sharing something else I'm not fully aware of. |
- Copy constructor for RuleSets - Copy constructor for RuleSet - Copy method for Rules - I'm still sharing a few objects that are not strictly immutables, but treated as them
259177c to
19da55f
Compare
|
|
|
The changes to setters / getters for things like DFA should probably be a separate PR to keep things organized. @adangel am I missing anything else here? |
|
@jsotuyod Merged! Sorry it took so long. |
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.
treated as them
This would allow to start resolving the issue commented on #194 (comment)
This change adds new methods, but doesn't modify existing APIs, so it would be safe for 5.8.0.
However, I'd still want a second opinion as to wether the classes that I allow to share are really to be treated / converted to immutables, or I should extend deep copying to them.
master. The PMD team will merge back to support branches as needed.mvn testpasses.mvn checkstyle:checkpasses. Check this for more info