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.

Sign up for GitHub

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

Jump to bottom

Sniff errors/warnings not all named #297

Closed
jrfnl opened this issue Jan 11, 2015 · 5 comments
Closed

Sniff errors/warnings not all named #297

jrfnl opened this issue Jan 11, 2015 · 5 comments

Comments

@jrfnl
Copy link
Member

jrfnl commented Jan 11, 2015

Each error/warning can be named:

$phpcsFile->addError($error, $stackPtr, 'SpacingAfter');

//vs the unnamed variant:

$phpcsFile->addError($error, $stackPtr);

If they are, they can be selectively disabled:

<exclude name="WordPress.VIP.RestrictedFunctions.user_meta" />

A number of the WP specific sniffs at this moment don't have all errors/warnings named which means that if an error/warning is buggy or a project for whatever reason chooses not to use that error/warning, they have to disable the complete sniff as the specific errors/warnings can't be disabled.

I'd suggest that naming all errors/warnings generated would be good practice. Opinions ?
@JDGrimes
Copy link
Contributor

Duplicate: #140

@westonruter
Copy link
Member

👍

jrfnl added a commit to jrfnl/WordPress-Coding-Standards that referenced this issue Jan 12, 2015
@jrfnl
Add granular error/warning names - issue WordPress#297, WordPress#140
0e532eb 
Notes:
* I've made a start at adding granular error/warning names.
* I've mostly ignored sniff files which only throw one specific error as those can be disabled by just disabling the sniff itself.
* For error/warning names, I've used the Squiz and PEAR coding standards for inspiration and stuck to the names used in there for the same error if one existed.
* In 1 instance (VIP/CronIntervalSniff), the existing error name did not comply with the CamelCaps convention for the names. I've changed the name. This will break backward compatibility for any rulesets referencing the 'old' name.
@jrfnl
Copy link
Member Author

jrfnl commented Jan 12, 2015

PR #298 addressed this.

westonruter added a commit that referenced this issue Jan 14, 2015
@westonruter
Merge pull request #298 from jrfnl/Issue-297-sniff-errornames
47f76ee 
Add granular error/warning names - issue #297, #140
@westonruter
Copy link
Member

PR merged. Nice work.

@jrfnl
Copy link
Member Author

jrfnl commented Jan 14, 2015

Thanks ;-)

@JDGrimes JDGrimes mentioned this issue May 14, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@westonruter @jrfnl @JDGrimes