Ruleset: configs in higher level ruleset(s) overrule configs set in included ruleset(s) #1003
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a completely different solution to issue squizlabs/PHP_CodeSniffer#2197, compared to the original solution which was previously committed to the old 4.0 branch.
The "old' solution was to process rulesets "top to bottom", i.e. every directive is processed when "read" while recursing through the rulesets. I've evaluated that solution carefully and found it counter-intuitive, which means that I expect that it would raise lots of support questions. I also expect that solution to break way more rulesets than is justified to solve issue squizlabs/PHP_CodeSniffer#2197.
The original solution would require ruleset maintainers to have a high awareness of what configs are being set in included rulesets. Included rulesets are often PHPCS native standards or external standards, which are both subject to change in every new release. With ruleset processing being top-to-bottom, any "must have" config directives would have to be set before any include which could possibly also have that config directive set. This also means that an external standard adding a config directive to one of their rulesets, would become a potentially breaking change, as a "consumer" ruleset may not realize they were relying on a default value for a "must have" config and that value being changed in an included ruleset could now break their CS scans.
So... having said that, this commit contains an alternative, far more specific, and far less invasive, solution for the originally posed problem.
This commit changes the processing of
config
directives to be based on the inclusion depth, with the highest level ruleset setting a config value overruling the value for that config as set in lower level (included) rulesets.When two rulesets at the same "level" both set the same
config
directive, the last one included "wins".To illustrate:
In 3.x:
testVersion
would be set to8.1-
In 4.x:
testVersion
will be set to7.2-
(higher level ruleset wins)In 3.x:
testVersion
would be set to5.6-
In 4.x:
testVersion
will be set to5.6-
(CompanyRulesetA and AnotherRuleset are at the same inclusion depth, last one wins)Includes ample tests.
Includes minor tweak to improve the debug information and only display that something was set when it actually has been set.
Suggested changelog entry
Changed:
<config>
directives will be applied based on the nesting level of the ruleset.<config>
directive set in an included ruleset from the "root" ruleset.<config>
directives set in the "root" ruleset will always "win" over directives in included rulesets.Related issues/external references
Related to squizlabs/PHP_CodeSniffer#1821 - point 2
Fixes squizlabs/PHP_CodeSniffer#2197