You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A number of sniffs contain a public $error property to toggle whether the sniff should throw an error or a warning.
This property has been superseded by the ability to specify <type>error</type> for each sniff in a custom ruleset since PHPCS 1.4.1.
I'd like to suggest removing these public $error properties from any sniffs which have them and to remove the code within the process() method of those sniffs handling the toggle.
A quick scan yields the following sniffs for which this applies:
Generic.ControlStructures.InlineControlStructure
Generic.Formatting.MultipleStatementAlignment
Generic.PHP.ForbiddenFunctions
Generic.PHP.NoSilencedErrors
Generic.Strings.UnnecessaryStringConcat
Squiz.CSS.ForbiddenStyles (sniff will be removed anyway in 4.0.0)
Squiz.PHP.DiscouragedFunctions
Squiz.PHP.ForbiddenFunctions (inherited)
The Customizable Sniff Properties wiki page would also need to be updated for this change.
I'd suggest adding a Removed in column (where relevant) to the properties table containing the version nr in which a property was removed.
I do like this property when writing my own sniff extending for example Generic.PHP.ForbiddenFunctions. It seems easier to modify the $error property than overriding something in the ruleset.
I think some Sniff made to be extendable could be better with a protected $error property.
I only ended up removing the property from 2 of the sniffs. The rest of the sniffs used the property to change messages or set a new default, as @VincentLanglet described in his comment.
The properties were removed from Generic.Strings.UnnecessaryStringConcat and Generic.Formatting.MultipleStatementAlignment
The rest of the sniffs used the property to change messages or set a new default
With the possible exception of the ForbiddenFunctions sniff(s), I don't see why these properties would remain.
Just like people can use <type> in their ruleset to change something from an error to a warning, they can also adjust the <message> in the ruleset from "this is forbidden" to "this is discouraged", so there is no reason for the sniff to contain that logic.
That leaves the following actions to be executed:
Remove the property from the Generic.ControlStructures.InlineControlStructure sniff and review the error message.
Remove the property from the Generic.PHP.NoSilencedErrors sniff and review the error message.
Change the $error property in Generic.PHP.ForbiddenFunctions from public to protected.
Change the property overload in the Squiz.PHP.DiscouragedFunctions (inherited) from public to protected.
Change the property overload in the Squiz.PHP.ForbiddenFunctions (inherited) from public to protected.
No change needed in the Generic.PHP.DeprecatedFunctions, which also extends the Generic.PHP.ForbiddenFunctions sniff, though the fact that that sniff is also impacted should be mentioned in the changelog.
The text was updated successfully, but these errors were encountered:
Follow up/repost from squizlabs/PHP_CodeSniffer#2823:
@VincentLanglet left the following remark:
An initial change related to this issue was already made for the 4.0 branch in commit squizlabs/PHP_CodeSniffer@606d876
With @gsherwood remarking:
Upon which @jrfnl replied:
That leaves the following actions to be executed:
Generic.ControlStructures.InlineControlStructure
sniff and review the error message.Generic.PHP.NoSilencedErrors
sniff and review the error message.$error
property inGeneric.PHP.ForbiddenFunctions
frompublic
toprotected
.Squiz.PHP.DiscouragedFunctions
(inherited) frompublic
toprotected
.Squiz.PHP.ForbiddenFunctions
(inherited) frompublic
toprotected
.Generic.PHP.DeprecatedFunctions
, which also extends theGeneric.PHP.ForbiddenFunctions
sniff, though the fact that that sniff is also impacted should be mentioned in the changelog.The text was updated successfully, but these errors were encountered: