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

"extensions" argument from first included rule clobbers all others #2602

Closed
TravisCarden opened this issue Sep 6, 2019 · 8 comments
Closed

Comments

@TravisCarden
Copy link

I maintain a coding standard (acquia/coding-standards) that needs to sniff the various non-.php extensions that Drupal uses (e.g., .module). I have specified them in my ruleset.xml but with no effect. If I specify them in the phpcs.xml.dist of a project using the standard it has no effect. If I specify them on the command line they do take effect. Is this a bug or am I doing something wrong?

@jrfnl
Copy link
Contributor

jrfnl commented Sep 6, 2019

@TravisCarden Not sure why it is working from the command-line and not from your ruleset, but what I do see is that you're not telling PHPCS as what type of file the new extensions should be interpreted - PHP, JS or CSS.

You can explicitly set this by using newext/type. Could you try that to see if that makes a difference ?

<arg name="extensions" value="inc,install/php,module/php,php,profile/php,test/php,theme/php,yml"/>

I've presumed any "unknown" extension (non-php, inc, js, css) is PHP, but I'm unsure what to do with the yml extension as I doubt PHPCS can do anything with a Yaml file.

TravisCarden pushed a commit to acquia/coding-standards-php that referenced this issue Sep 6, 2019
TravisCarden pushed a commit to acquia/coding-standards-php that referenced this issue Sep 6, 2019
@TravisCarden
Copy link
Author

Thanks, @jrfnl. That had no effect. However, I think I've found the problem: If I include multiple rules, the extensions arg value from the first one seems to take precedence over my own extensions arg and any defined in rules included subsequently. I moved the Drupal rule (which I took the extensions arg from in the first place) to the top of my ruleset and it started working. I'm working around the issue for the time being with this commit (acquia/coding-standards-php@3ea1e57), but it sure seems like a bug that ought to be fixed upstream now. For the maintainers, you can reproduce the issue as follows:

mkdir test
cd test
composer init -n
composer require acquia/coding-standards:v0.4.0 dealerdirect/phpcodesniffer-composer-installer
echo '<?php' > test.module
./vendor/bin/phpcs -vs --standard=AcquiaDrupalStrict test.module

Upon running PHPCS in the last line you should receive output like this:

Registering sniffs in the AcquiaDrupalStrict standard... DONE (289 sniffs registered)
Creating file list... DONE (1 files in queue)
Changing into directory /Users/travis.carden/Projects/acquia/test
Processing test.module [PHP => 1 tokens in 1 lines]... DONE in 5ms (1 errors, 0 warnings)

FILE: /Users/travis.carden/Projects/acquia/test/test.module
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] Missing file doc comment
   |       |     (Drupal.Commenting.FileComment.Missing)
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

Time: 125ms; Memory: 12MB

But you will instead receive this:

Registering sniffs in the AcquiaDrupalStrict standard... DONE (289 sniffs registered)
Creating file list... DONE (0 files in queue)

@TravisCarden TravisCarden changed the title "extensions" argument not taking effect when specified in ruleset.xml "extensions" argument from first included rule clobbers all others Sep 6, 2019
@jrfnl
Copy link
Contributor

jrfnl commented Sep 8, 2019

This may be related to #2197

@gsherwood
Copy link
Member

As @jrfnl said, it's the same issue as #2197. The workaround is to do exactly what you've done. The real fix is detailed in the issue, but wont be made until version 4 due to BC breaks.

I'm going to close this now it has been referenced in the main issue, and a workaround was found.

@sebastienbarre
Copy link

@TravisCarden I tried to reproduce your solution, without success. I have the same issue, with multiple <arg name="extensions" directives in different ruleset.xml. Re-ordering them the way you did makes no difference for me; PHPCS will read everything, set its internal config options first based on what it found across all ruleset.xml files, then process the rules. There is no way to intermingle a change of extensions in between rules. Did I misunderstand your workaround?

@TravisCarden
Copy link
Author

TravisCarden commented Dec 12, 2019

As I recall, @sebastienbarre, the key was that 1) I specified my own extension list in my extensions arg, and 2) that no other rulesets than the one I moved to last specified an extension list. But unless you didn't do number 1, your understanding of the parsing logic wouldn't account for my success.

@sebastienbarre
Copy link

@TravisCarden wrong Sebastien, but thx :)

@TravisCarden
Copy link
Author

Oops! Autocomplete failed me. :p

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

4 participants