how to disable a single comarisonOperatorUsage

[adminfriso]

Hello,
I am working on a custom sniff for my (shared) projects. I would lik to allow the T_BOOLEAN_NOT as valid operator on the Squiz.Operators.ComparisonOperatorUsage.NotAllowed rule.
right now the only way (I could find) to exclude this is to exclude the complete rule, but then you will also lose the other checks.

<rule ref="Squiz.Operators.ComparisonOperatorUsage">
    <exclude name="Squiz.Operators.ComparisonOperatorUsage.NotAllowed"/>
</rule>

is there a way to only exclude this single rule?

[dingo-d]

You can silence it, that way you won't be including the entire ruleset

<rule ref="Squiz.Operators.ComparisonOperatorUsage.NotAllowed">
  <severity>0</severity>
</rule>

[adminfriso]

But then i do also silence the check for the other valid and invald OPS

[adminfriso]

for example
61 | ERROR | Operator ! prohibited; use === FALSE instead // Do not want to check for this
66 | ERROR | Operator != prohibited; use !== instead // Do want to check for this

[dingo-d]

Oh, I see. You could remove this one, and create your own that is modified to your liking (basically fork the current one and make modifications that fit your use case).

[adminfriso]

how do you mean? and or is there a way to do it in the current package? I love your work

[dingo-d]

I'm assuming that you are just using phpcs.xml.dist in your project and cherrypicking the rules you want to (the usual way).

What I did was I created a standard for the company I work for, and I added a custom sniff. You could do something like it, define your own sniff and it will be included in your ruleset.

Or you could fork the phpcs and change the sniff in question and then load your fork in the composer. This is a bit more work as you need to keep track of the changes in the upstream branch.

[adminfriso]

will work on the second option. Thanks for your help!

[jrfnl]

@adminfriso Just a heads-up: The simplest way to create your own version of this sniff would be to just extend the PHPCS sniff and overload the $invalidOps property. That way you are not infringing on the copyright of the code.

Unfortunately that property appears to be private, but a PR to this repo to change the property from private to protected should fix that.
I'm also wondering why the property is set to static, A search of the repo and tracing through the file history should be able to explain that. If there's no good reason, the static should probably be removed as well, making overloading the $invalidOps property even easier.

For an example of how to extend/overload a PHPCS sniff in your own external standard, you could have a look at this WPCS sniff which does something similar for another sniff: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress/Sniffs/WhiteSpace/OperatorSpacingSniff.php

[gsherwood]

The way I've helped with this sort of thing in the past was by changing the error codes of a sniff to include the token type that the error was reported on. So instead of a single generic error code, you have a lot of individual ones.

For example, this was in 2.1.0:

• Generic EmptyStatementSniff error codes now contain the type of empty statement detected (request Ability to customise $checkedTokens in Generic.CodeAnalysis.EmptyStatement from within a custom rule #314)
  • All messages generated by this sniff are now errors (empty CATCH was previously a warning)
  • Message code Generic.CodeAnalysis.EmptyStatement.NotAllowed has been removed
  • Message code Generic.CodeAnalysis.EmptyStatement.NotAllowedWarning has been removed
  • New message codes have the format Generic.CodeAnalysis.EmptyStatement.Detected[TYPE]
  • Example code is Generic.CodeAnalysis.EmptyStatement.DetectedCATCH
  • You can now use a custom ruleset to change messages to warnings and to exclude them

Not necessarily the right decision for this sniff, but thought it was worth posting.

[jrfnl]

@gsherwood I'd favour more modular errorcodes, but that change would be a BC-break which would have to wait for the 4.0 version, wouldn't it ?

[gsherwood]

but that change would be a BC-break which would have to wait for the 4.0 version, wouldn't it ?

It could technically be in a minor release given the rules I've set, but I don't think I'd push this in a minor given the sniff has a second error code.

If it had a single code, you could still mute the all the new codes by excluding the sniff. But if you are currently muting just Squiz.Operators.ComparisonOperatorUsage.NotAllowed and you want the other code, then you actually have a worse time with this change. You can't mute Squiz.Operators.ComparisonOperatorUsage.*NotAllowed.

[adminfriso]

You could also add idd in recursion so
Squiz.Operators.ComparisonOperatorUsage.NotAllowed.Method
In this way it will be BC as you will still mute (or add) all using
Squiz.Operators.ComparisonOperatorUsage.NotAllowed