Some Drupal CS inspections ignored in IDE [#2867541]

As a team we are working on D8 project and every team member is using PHP Storm. We have very strict rules regarding coding standards and if there is one report from Drupal CS detected then build completely fails. But the developers having currently problems with some CS rules, which they are not able to detect in IDE and then they usually push the code which will going to fail and thus we have so many fail builds, which could be fixed beforehand if the IDE would inform them.

For example Drupal_Sniffs_Array_DisallowLongArraySyntaxSniff. The sniff is trying to detect the version of Drupal when inspecting particular file. But if you are in IDE like PHPStorm (latest) and you open single file, IDE will autoinspect file, but it basically creates clone of the file (with directory structure) in the /tmp/phpcs_temp.tmp. The problem here is that there is only one single file (the one you are inspecting) and no other files, so \DrupalPractice_Project::getInfoFile() has no chance to find info file and find out it's a D8 version and thus inspect long array syntax. He just traverse directories like this:

/tmp/phpcs_temp.tmp/src/modules/search_efs/src/Plugin/rest/resource/SearchResource.php
/tmp/phpcs_temp.tmp/src/modules/search_efs/src/Plugin/rest/resource
/tmp/phpcs_temp.tmp/src/modules/search_efs/src/Plugin/rest
/tmp/phpcs_temp.tmp/src/modules/search_efs/src/Plugin
/tmp/phpcs_temp.tmp/src/modules/search_efs/src
/tmp/phpcs_temp.tmp/src/modules/search_efs
/tmp/phpcs_temp.tmp/src/modules
/tmp/phpcs_temp.tmp/src
/tmp/phpcs_temp.tmp
/tmp
/

with no success. It would be cool if we could somehow inform Drupal CS that version of project is really 8.x without trying to find info files, which obviously does not exist in /tmp when working on one particular file. Currently I have to dirty-patch the \Drupal_Sniffs_Array_DisallowLongArraySyntaxSniff::process on first line and set

$drupalVersion = '8.x';

Comment	File	Size	Author
#3	coder-default_drupalVersion-2867541-3.patch	586 bytes	Boobaa

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

6 April 2017 at 20:57

hideaway created an issue. See original summary.

Comment #2

Boobaa

he/him

CreditAttribution: Boobaa at Pronovix commented 20 November 2017 at 20:45

I ran into the same problem, but have a different workaround: my \DrupalPractice_Project::getCoreVersion() returns '8.x' instead of false when no info file is found.

Rationale:

First, when one is working on Drupal 8 core (outside of its modules/themes/profiles directories having those .info.yml files), there is no easy way to retrieve the core version.
Second, 8.x is the latest released core version, thus it's the most meaningful default for this.

Side note: I tried to smuggle in an additional phpcs parameter from PhpStorm, but it's not possible with the latest version: adding some -d drupalVersion=8.x to either the path to phpcs or the path to the standard to use gives errors only.

Comment #3

Boobaa

he/him

CreditAttribution: Boobaa at Pronovix commented 23 November 2017 at 09:03

Status:

Active

» Needs review

File	Size
coder-default_drupalVersion-2867541-3.patch	586 bytes

Well, there are multiple problems here.

The way PhpStorm calls phpcs prevent drupal/coder from "inventing" the core version from the info files.
To get around this, drupal/coder would need a configuration variable which should be used as the core version if present.
There is no way to pass a configuration variable from PhpStorm to phpcs (see my previous comment).
To get around that, a wrapper script is needed which should be called by PhpStorm (instead of phpcs directly) and which passes the configuration variable to phpcs and drupal/coder.

Attaching the drupal/coder patch I'm using for this. My wrapper script is this simple:

#!/bin/bash
phpcs=/var/www/drupalvm/bin/phpcs
$phpcs $@ --runtime-set drupalVersion 8.x

Comment #4

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi at jobiqo - job board technology commented 22 April 2019 at 10:39

Version:	8.x-2.12	» 8.x-3.x-dev
Status:	Needs review	» Needs work

I think this makes sense, thank you!

Some minor things to improve:

1) Please document in your patch how this can be used from the command line and from a phpcs.xml file. See also https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-Ruleset with

2) Please port this to Coder 8.x-3.x and file a pull request at https://github.com/pfrenssen/coder

3) I'm thinking about how we can test this. We could add a line to our .travis.yml config to make an invocation with that new config option. We could also just test the global variable with PHPUnit, if that is possible. Could you try some testing options so that we don't break this in the future?