Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I would like to contribute to Coder Sniffer more than the few patches I've submitted, but I feel like the requirements are a little unclear. I'd like to see some contribution guidelines, either in the handbook or in a CONTRIBUTE.txt or something, to help would-be contributors to get started and be maximally helpful. As a casual contributor, (at least) the following things stand out to me as potential points of confusion:
- What is the relationship between Coder and Coder Sniffer? When should a contribution be made to one versus the other? Is there any intention of feature parity between the two? How do we avoid duplication of effort/logic?
- What is Coder Sniffer's scope? Is everything contained Drupal's coding standards in scope? Is anything not contained in our official standards automatically out of scope?
- How do the coding standards for sniff files differ from Drupal's coding standards? Can the sniff files themselves be phpcs-ed to ensure conformity before submitting patches?
- What is the testing strategy? I know there are some good/bad files in the repository. How are tests run against them? What kind of tests should new sniffs contain?
Comments
Comment #1
klausi"Coder Review" is the old regex-based checker, "Coder Sniffer" is the new PHPCS based checker that was merged from the drupalcs project.
All new contributions should go into Coder Sniffer. Coder Review still exists to not break backwards compatibility in 7.x-2.x and because it is way faster than Coder Sniffer.
We cannot fully avoid duplication, as duplication between those two already exists and they are maintained by different people: I only care about Coder Sniffer, while the rest of the Coder maintainers are responsible for Coder Review.
Coder Sniffer's scope is everything that is defined in Drupal's coding standards. But that is not a hard line; we have a sniff for example that throws warnings if people use more than 2 blank lines in a row in a function, which is not defined at all in the coding standards. Still, bigger impact standards should first be approved by the community and be documented before we implement them here.
Coder sniffer uses the PHPCS coding standards to be compatible with PHPCS in order to be easily merged into PHPCS core at some point (which is not realistic because Coder Sniffer is GPLv2+). You can check our sniffs like this:
Please note that we don't use @author and @license tags.
The testing strategy does not really exist, there are only a couple of example PHP files to run the sniffer manually against it. I apologized for that already in an issue that I can't find right now. But yeah, ideally we should have test cases such as PHPCS has, anyone wants to work on that?
Running tests is done as follows:
If that runs through without throwing warnings your change has no regression that we know of.
If that displays the error you wanted to trigger with your sniff then you are good.
New sniffs should have example(s) in the bad* files. Bug fixes should have examples in good* files.
Comment #2
TravisCarden CreditAttribution: TravisCarden commentedThanks, @klausi! Should this go in a handbook page or something?
Comment #3
klausiGood news: I have now converted this hacky test case execution to PHPUnit! Install dependencies with
Execute the tests with
Bad news: The Drupal_BadUnitTest is way too big, any volunteers to convert this to dedicated unit tests per sniff file?
This also helps us now with testing regressions for the PHPCS 2.0 upgrade, see #2189495: Support for PHP_CodeSniffer 2.0.0.
Comment #4
klausiCoder 7.x is frozen now and will not receive updates. Coder 8.x-2.x can be used to check code for any Drupal version, Coder 8.x-2.x also supports the phpcbf command to automatically fix conding standard errors. Please check if this issue is still relevant and reopen against that version if necessary.