4.0 | Remove public `$error` properties

[jrfnl]

Follow up/repost from squizlabs/PHP_CodeSniffer#2823:

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.

---

@VincentLanglet left the following remark:

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.

---

An initial change related to this issue was already made for the 4.0 branch in commit squizlabs/PHP_CodeSniffer@606d876

With @gsherwood remarking:

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

Upon which @jrfnl replied:

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.

[jrfnl]

I've reviewed the above mentioned sniffs now and have decided against making further changes related to this issue.

The remaining $error properties do not just affect the error level, but also the error messages + error codes and even though I think these properties aren't great and feel very outdated, the BC break feels unnecessary and larger than justified.