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

Custom rule must end in "Sniff"?? #2481

Closed
joshbruce opened this issue Apr 20, 2019 · 3 comments
Closed

Custom rule must end in "Sniff"?? #2481

joshbruce opened this issue Apr 20, 2019 · 3 comments

Comments

@joshbruce
Copy link

Never used this library before; so, sorry for the bumps.

Created ruleset:

<?xml version="1.0"?>
<ruleset name="8fold">
<rule ref="PSR12">
    <exclude name="PSR2.Methods.MethodDeclaration"/>
</rule>
</ruleset>

Worked like a charm. Created custom rule per documentation, I think:

AbstractFinalAndStatic

Updated ruleset:

<?xml version="1.0"?>
<ruleset name="8fold">
<rule ref="PSR12">
    <exclude name="PSR2.Methods.MethodDeclaration"/>
</rule>
<rule ref="./UtilPhpCS/src/AbstractFinalAndStatic.php"/>
</ruleset>

Received error:

ERROR: Ruleset AbstractFinalAndStatic.php is not valid
- On line 108, column 1: ParsePI: PI php never end ...
- On line 108, column 1: Start tag expected, '<' not found

Tried just folder in ruleset:

<?xml version="1.0"?>
<ruleset name="8fold">
<rule ref="PSR12">
    <exclude name="PSR2.Methods.MethodDeclaration"/>
</rule>
<rule ref="./UtilPhpCS/src"/>
</ruleset>

Received error:

ERROR: Ruleset AbstractFinalAndStatic.php is not valid
- On line 108, column 1: ParsePI: PI php never end ...
- On line 108, column 1: Start tag expected, '<' not found

Changed file name to include the suffix "Sniff" (b/c maybe??):

<?xml version="1.0"?>
<ruleset name="8fold">
<rule ref="PSR12">
    <exclude name="PSR2.Methods.MethodDeclaration"/>
</rule>
<rule ref="./UtilPhpCS/src/AbstractFinalAndStaticSniff.php"/>
</ruleset>

Running phpcs resulted in no error; however, the custom sniff is ignored.

Very confused on what I'm doing wrong here. Pretty sure I'm following the docs but they're pretty verbose for me there might be a key requirement buried in there I'm missing.

Step 1 (after getting PHPCS): Create .phpcs.xml (see above).
Step 2: Create sniffer class file that implements Sniff interface:

<?php

namespace Eightfold\UtilPhpCS;

use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Files\File;
use PHP_Codesniffer\Util\Tokens;

class AbstractFinalAndStaticSniff implements Sniff
{
    public function register(): array
    {
        return [T_FUNCTION];
    }

    public function process(File $phpcsFile, $stackPtr)
    {
        $phpcsFile->addError("Custom sniff error");
    }
}

Step 3: Point the .phpcs.xml to the newly created class.
Step 4: Profit??

Any assistance would be greatly appreciated.

@jrfnl
Copy link
Contributor

jrfnl commented Apr 20, 2019

You missed the first line of the tutorial:

All sniffs in PHP_CodeSniffer must belong to a coding standard. A coding standard is a directory with a specific sub-directory structure and a ruleset.xml file.

Ref: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Coding-Standard-Tutorial

Basically, even though the format of the XML file is basically the same, there is a big difference between a custom ruleset, i.e. a phpcs.xml.dist file which would normally be placed in the root of a project and an external PHPCS standard.

A custom ruleset can include existing sniffs from registered standards, include other standards, include other ruleset files etc. It's a configuration file.

An (external) standard can do all that + introduce new sniffs. Standards have to be registered with PHPCS though to be recognized.

So, let's take a step back.

  1. In the repo in which you want your standard to live, create a directory called Eightfold.
  2. In that directory, create a ruleset.xml file, make sure the ruleset name is <ruleset name="Eightfold">.
  3. Now create a subdirectory called UtilPhpCS and place your sniff file into that. Make sure the sniff file is called AbstractFinalAndStaticSniff.php (so, yes, the Sniff.php as a suffix is a must).
  4. Register your external standard with PHPCS using phpcs --config-set installed_paths /path/to/reporoot. See: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#setting-the-installed-standard-paths
  5. For the project for which you want to use this standard, create a phpcs.xml.dist file and add the Eightfold standard, i.e. <rule ref="Eightfold">.
  6. Run PHPCS over the project (& profit!)

If you follow the above steps, I expect you'll have more success.

Take note of the following:

  • You don't have to add the Eightfold.UtilPhpCS.AbstractFinalAndStatic sniff in your ruleset.xml file. All sniffs in subdirectories of the standard are automatically included.
  • PHPCS has pretty specific file name and namespace requirements which must be followed for things to work properly.
    • Sniff files must end with Sniff.php.
    • Sniff files must be placed in subdirectories directly under the directory containing the standard. Subdirectories translate to sniff categories.
    • The sniff class name has to be the same as the file name (minus the .php).
    • The standard must be called the same as the directory it lives in, i.e. <ruleset name="Eightfold"> for a standard in the Eightfold directory.
    • The namespace of a sniff must follow the following convention: namespace StandardName\Sniffs\Category, though you can prefix the namespace is so desired.
      For more information about prefixing the namespace: https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/3.0.1

Hope this helps. Let me know if I can clarify further.

@joshbruce
Copy link
Author

Exceptional description. Think this could replace the docs that are there.

Thank you so much!

@jrfnl
Copy link
Contributor

jrfnl commented Apr 27, 2019

Glad it helped!

jrfnl added a commit to jrfnl/phpcs-security-audit that referenced this issue Feb 3, 2020
PHPCS has specific naming and directory layout requirements for external standards which the `Security` standard did not comply with.

While things sort of worked with the symlink hack, the net effect was:
- The PHPCS autoloader did not work.
- None of the PHPCS ruleset configuration options worked as PHPCS could not match sniffs to files.
- Some sniffs would never load.

This PR fixes this by:
1. Setting the base namespace to `PHPCS_SecurityAudit\Security` and annotating this in the `ruleset.xml` file in the correct manner.
2. Fixing all namespaces and uses thereof throughout the codebase.
3. Fixing the `Drupal8/Utils.php` file which was missing the namespace and was still referring to an out-of-date class name to extend.
4. Fixing the namespace names and file names of the CVE sniffs.
    - The namespace of a sniff has to reflect its path in the standard.
    - The file name has to reflect the name of the sniff.
5. Fixing the names of the CVE sniffs in the example rulesets
6. Removing the symlink file and all references to it.
    Instead `require` the [DealerDirect Composer PHPCS plugin](https://github.com/Dealerdirect/phpcodesniffer-composer-installer) which will sort out the `installed_paths` for PHPCS .
7. Setting the minimum PHPCS version to `3.0.2` as prior to that external standards weren't fully supported in the 3.x branch.
8. Removing the `autoload` section in `composer.json`. This is no longer needed and in certain situations can cause conflicts/fatal errors.

References:
* https://github.com/squizlabs/PHP_CodeSniffer/wiki/Coding-Standard-Tutorial
* https://github.com/squizlabs/PHP_CodeSniffer/wiki/Version-3.0-Upgrade-Guide
* squizlabs/PHP_CodeSniffer#2481 (comment)
* squizlabs/PHP_CodeSniffer#2606
* squizlabs/PHP_CodeSniffer#1469
* https://github.com/Dealerdirect/phpcodesniffer-composer-installer

Fixes 47
jrfnl added a commit to jrfnl/phpcs-security-audit that referenced this issue Feb 18, 2020
PHPCS has specific naming and directory layout requirements for external standards which the `Security` standard did not comply with.

While things sort of worked with the symlink hack, the net effect was:
- The PHPCS autoloader did not work.
- None of the PHPCS ruleset configuration options worked as PHPCS could not match sniffs to files.
- Some sniffs would never load.

This PR fixes this by:
1. Setting the base namespace to `PHPCS_SecurityAudit\Security` and annotating this in the `ruleset.xml` file in the correct manner.
2. Fixing all namespaces and uses thereof throughout the codebase.
3. Fixing the `Drupal8/Utils.php` file which was missing the namespace and was still referring to an out-of-date class name to extend.
4. Fixing the namespace names and file names of the CVE sniffs.
    - The namespace of a sniff has to reflect its path in the standard.
    - The file name has to reflect the name of the sniff.
5. Fixing the names of the CVE sniffs in the example rulesets
6. Removing the symlink file and all references to it.
    Instead `require` the [DealerDirect Composer PHPCS plugin](https://github.com/Dealerdirect/phpcodesniffer-composer-installer) which will sort out the `installed_paths` for PHPCS .
7. Setting the minimum PHPCS version to `3.0.2` as prior to that external standards weren't fully supported in the 3.x branch.
8. Removing the `autoload` section in `composer.json`. This is no longer needed and in certain situations can cause conflicts/fatal errors.

References:
* https://github.com/squizlabs/PHP_CodeSniffer/wiki/Coding-Standard-Tutorial
* https://github.com/squizlabs/PHP_CodeSniffer/wiki/Version-3.0-Upgrade-Guide
* squizlabs/PHP_CodeSniffer#2481 (comment)
* squizlabs/PHP_CodeSniffer#2606
* squizlabs/PHP_CodeSniffer#1469
* https://github.com/Dealerdirect/phpcodesniffer-composer-installer

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

No branches or pull requests

2 participants