Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to customise $checkedTokens in Generic.CodeAnalysis.EmptyStatement from within a custom rule #314

Closed
u01jmg3 opened this issue Nov 7, 2014 · 8 comments

Comments

@u01jmg3
Copy link

u01jmg3 commented Nov 7, 2014

e.g.

<rule ref="Generic.CodeAnalysis.EmptyStatement">
    <properties>
        <property name="checkedTokens" type="array" value="T_CATCH,T_DO,T_ELSE,T_ELSEIF,T_FOR,T_FOREACH,T_SWITCH,T_TRY,T_WHILE" />
    </properties>
</rule>
@aik099
Copy link
Contributor

aik099 commented Nov 7, 2014

👍 to make it configurable.

Just of curiosity how empty statements can be useful to you?

@u01jmg3
Copy link
Author

u01jmg3 commented Nov 7, 2014

That's a good question 😄
Apart from having the option to choose to configure, sometimes I come across applications with if statements ready for code to be inserted should say extra validation be required and I don't want PHPCS to complain. The very fact PHPCS has checked tokens within its internals suggests there should be the ability to turn tokens on/off at will.

@aik099
Copy link
Contributor

aik099 commented Nov 7, 2014

Coding Standard is called as such to enforce standard. Fact, that empty IF exists and user must fill content in should trigger an error, so user know that he must fill code in.

If you don't need that empty IF to happen, then just comment it out.

@u01jmg3
Copy link
Author

u01jmg3 commented Nov 7, 2014

Yep, I agree but it would still be useful to be able to configure this rule as you can with others. If my suggestion is not applicable then this issue can be closed.

@aik099
Copy link
Contributor

aik099 commented Nov 7, 2014

Indeed, that's why I also put +1 before.

@u01jmg3
Copy link
Author

u01jmg3 commented Nov 7, 2014

👍

gsherwood added a commit that referenced this issue Dec 16, 2014
@gsherwood
Copy link
Member

I've decided to do this in a different way because the sniff pre-dates ruleset files and is showing its age. Instead, I've removed that member var and changed the message codes to include the type of statement that was detected.

So to turn off empty IF statement errors, use this:

<rule ref="Generic.CodeAnalysis.EmptyStatement.DetectedIF">
   <severity>0</severity>
</rule>

Or to turn empty IF statement errors into warnings, use this:

<rule ref="Generic.CodeAnalysis.EmptyStatement.DetectedIF">
  <type>warning</type>
</rule>

The messages codes are:

Generic.CodeAnalysis.EmptyStatement.DetectedCATCH
Generic.CodeAnalysis.EmptyStatement.DetectedDO
Generic.CodeAnalysis.EmptyStatement.DetectedELSE
Generic.CodeAnalysis.EmptyStatement.DetectedELSEIF
Generic.CodeAnalysis.EmptyStatement.DetectedFOR
Generic.CodeAnalysis.EmptyStatement.DetectedFOREACH
Generic.CodeAnalysis.EmptyStatement.DetectedIF
Generic.CodeAnalysis.EmptyStatement.DetectedSWITCH
Generic.CodeAnalysis.EmptyStatement.DetectedTRY
Generic.CodeAnalysis.EmptyStatement.DetectedWHILE

Hopefully that gives you enough flexibility to do what you need without having to change the core sniffs or write your own.

@u01jmg3
Copy link
Author

u01jmg3 commented Dec 20, 2014

Works perfectly for me - thanks for implementing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants