Skip to content

"include-pattern" to file extensions #2486

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

Closed
celorodovalho opened this issue May 1, 2019 · 4 comments
Closed

"include-pattern" to file extensions #2486

celorodovalho opened this issue May 1, 2019 · 4 comments

Comments

@celorodovalho
Copy link

celorodovalho commented May 1, 2019

I'm trying to add an "include-pattern" in a specific rule, like that:

    <rule ref="Company.Files.PhtmlValidation">
        <exclude-pattern>*.php</exclude-pattern>
        <include-pattern>*.phtml</include-pattern>
    </rule>

And running over this command:

vendor/bin/phpcs --standard=/home/me/Workspace/project/vendor/company/module/phpcs/ruleset.xml /home/me/Workspace/project/app/code/Company/Module

And nothings happens.
But if I try with the command (with "--extensions=phtml"):

vendor/bin/phpcs --standard=/home/me/Workspace/project/vendor/company/module/phpcs/ruleset.xml /home/me/Workspace/project/app/code/Company/Module --extensions=phtml

it gives me the result:

FILE: ...orkspace/project/app/code/Company/Module/view/frontend/templates/js/teste-file.phtml
--------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------
 24 | ERROR | [x] Potentially XSS vulnerability. Please verify that output is escaped.
 25 | ERROR | [x] Potentially XSS vulnerability. Please verify that output is escaped.
 25 | ERROR | [x] @escapeNotVerified annotation detected. Please use the correct escape strategy and
    |       |     remove annotation. 1

The "include-pattern" should not include the phtml files like specified in ruleset.xml?

@jrfnl
Copy link
Contributor

jrfnl commented May 1, 2019

I've not run any tests, so what I about to say is unverified, but all the same....

By default PHPCS will scan files with the following extensions: .php, .inc, .js and .css. See: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#specifying-valid-file-extensions

Setting an include/exclude-pattern only has effect on files which are being scanned.

As .phtml files are not scanned by default, the include-pattern used above won't have any effect unless you combine it with adding .phtml to the scanned extensions (like you did on the command line).

Using the below snippet in your ruleset, the phtml extension gets added to the list of scanned file extensions and your include pattern should now work (reminder: untested):

    <arg name="extensions" value="php,inc,phtml,js,css"/>

    <rule ref="Company.Files.PhtmlValidation">
        <exclude-pattern>*.php</exclude-pattern>
        <include-pattern>*.phtml</include-pattern>
    </rule>

Also, please take note that the include/exclude-pattern directives are a special kind of regex. The patterns you are currently using would also match MySpecialPHPFile.html and tophtml.css for instance.
See: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-files-and-folders

If that's not what you want, you need to make the patterns more specific. Example:

    <rule ref="Company.Files.PhtmlValidation">
        <exclude-pattern>*\.php$</exclude-pattern>
        <include-pattern>*\.phtml$</include-pattern>
    </rule>

@gsherwood
Copy link
Member

As .phtml files are not scanned by default, the include-pattern used above won't have any effect unless you combine it with adding .phtml to the scanned extensions (like you did on the command line).

This is correct.

@celorodovalho
Copy link
Author

The option:

<arg name="extensions" value="php,inc,phtml,js,css"/>

works fine for me, thank you!

@gsherwood
Copy link
Member

works fine for me, thank you!

Good news. Thanks for posting your solution.

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

3 participants