-
-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Based on a discussion between @fredden and me.
Why have PHPCS specific sniffs ?
- There are a number of best practices when writing sniffs for which compliance currently needs to be checked manually in PRs by a human reviewer, while this could be automated.
- There are a few "upgrades" which could be made to sniffs once support for PHPCS 3.x has been dropped, for which auto-fixers could be written (and more things like this are expected for the future).
- Etc...
It would be helpful for maintainers of PHPCS standards, including the maintainers of PHPCS itself, to have these kind of things automatically flagged via the CI on PRs.
Why wasn't this done before ?
Well, I did actually think about this before, but as the public for these type of sniffs is very limited, I didn't deem it worth the time of writing and maintaining the sniffs. Aside from that, there always were other things to do in the repos which had higher priority.
This argument still stands, but who knows, someone else may have the time ?
Why should this be part of this repo ?
When thinking through the most logical place for these type of sniffs, there are a number of options:
- PHPCSDevTools
- New, separate repo
- This repo
PHPCSDevTools is not a good option because the PHPCSDebug standard/sniff should be compatible with a wide range of PHPCS versions, while I see no reason for sniffs in a PhpcsQA standard to be compatible with a wide range of PHPCS versions, but as PHPCSDebug is part of DevTools, the minimum supported PHPCS version cannot (and should not) be raised without good reason.
A new, separate repo has the positive side-effect that it might make the sniffs/standard more discoverable as the package/repo will be published as a new, separate package.
On the "con" side, it would mean yet another repo to maintain.
As for this repo....
This repo already provides a coding standard for devs writing PHPCS sniffs, so a significant part of the expected public for these sniffs already uses PHPCSDev anyhow, so it seems like a good fit.
Why set this up as a separate standard instead of adding sniffs to the existing PHPCSDev standard ?
To make it more straight-forward for packages which want to use the PHPCS specific sniffs, but not the PHPCSDev standard, to include just these sniffs.
If the sniffs would be added to the PHPCSDev standard, devs for an external standard would need to keep track of activity in this repo and would need to include each PHPCS-related sniff individually in their own project ruleset.
<rule ref="PHPCSDev.CatA.SniffNameA"/>
<rule ref="PHPCSDev.CatA.SniffNameB"/>
<rule ref="PHPCSDev.CatB.SniffNameA"/>... while, if the sniffs would be added to a new, separate, standard which only contains these sniffs, devs for packages which want to use these sniffs could just do:
<rule ref="PhpcsQA"/>... and will then automatically benefit from every new PhpcsQA sniff being added, while they could also still use selective inclusions as per the previous example if so desired.
Initial sniff idea list
-
PhpcsQA.UpgradeTo4.UseTokensConstants: a sniff to detect use of the deprecated[PHP_CodeSniffer\Util\]Tokens::$....token group variables and auto-fix these to use the PHPCS 4.xTokens::CONST_NAMEtoken group constants. - A sniff to check that calls to
File::findPrevious()andFile::findNext()with$typesset toT_WHITESPACEorTokens::EMPTY_TOKENSand$excludeset totrue, passnullfor the$endparameter. - Along the same lines, a sniff to check that calls to
File::findPrevious()andFile::findNext()with$excludenot passed or set tofalse, do not passnullfor the$endparameter to prevent the "find" from walking to the start/end of the file. - Sniffs to help external standards switch over from using the PHPCS native utilities to using the utilities in PHPCSUtils.
Another idea, though that probably doesn't belong in this standard and might be better to write as a generic code analysis sniff for PHPCSExtra: a sniff to check that most PHP native array_* functions aren't used directly on deeply nested arrays.
I.e. don't do: array_pop($array['key1']['key2']), but assign $array['key1']['key2']to an interim variable ahead of thearray_pop()` function call.
Additional actions needed
- The CI/QA checks for this repo will need to be expanded significantly as the repo currently barely has any (as there is no code).
The checks and configs in PHPCSExtra can be used as inspiration/basis.