Skip to content

Make exclusion list more specific for Magento template files #11

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

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

fredden
Copy link

@fredden fredden commented Mar 28, 2024

When working with a website that we have inherited from another agency, I noticed that the Generic.PHP.DisallowShortOpenTag rule was applying to *.php files, but not to *.phtml files. After some investigation, I found that the <exclude-pattern> directive was applying too broadly. When a sniff is in both standards, this <exclude-pattern> rule meant that it would only apply to PHP files, not Magento2 template files.

I made a list of all overlapping sniffs (ie, those in both the Youwe and Magento2 standards), and then included only those that made sense to not apply to Magento template files in the exclude list. To see the sniffs included in these standards, one can run vendor/bin/phpcs -e --standard=Youwe and vendor/bin/phpcs -e --standard=Magento2.

Some examples of a sniffs not marked for exclusion here are:

  • GlobalPhpUnit.Coverage.CoversTag - we should not be defining any PHPUnit test classes/methods in template files, so this rule should never match anything anyway.
  • PSR12.Classes.OpeningBraceSpace - no classes should be declared in template files, so this rule should never match anything anyway.
  • Squiz.WhiteSpace.CastSpacing - cast syntax should be uniform in all files scanned, not just .php files.

This is a follow-up to #5 and #10.

Copy link
Collaborator

@leonhelmus leonhelmus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good addition thanks @fredden !

@fredden fredden merged commit 30e96bb into master Apr 19, 2024
@fredden fredden deleted the check-phtml-files-better branch April 19, 2024 10:25
@fredden fredden removed the request for review from igorwulff April 19, 2024 10:25
@fredden fredden mentioned this pull request Apr 30, 2024
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

Successfully merging this pull request may close these issues.

3 participants