squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

PHPCS cannot be set to PSR2 by default #2338

Closed oxblue-rbollig closed 5 years ago

oxblue-rbollig commented 5 years ago

I'm trying to set up PHPCS to run with PSR2 by default instead of having to specify it on each run.

I set the default with phpcs --config-set standard PSR2 When I check the configuration, it appears to be set:

$ phpcs --config-show
Using config file: /usr/bin/CodeSniffer.conf

standard: PSR2

When I actually run it on files, it runs as PEAR:

$ phpcs -v transarray.php 
Registering sniffs in the PEAR standard... DONE (28 sniffs registered)
Creating file list... DONE (1 files in queue)
Changing into directory /root
Processing transarray.php [PHP => 136 tokens in 26 lines]... DONE in 2ms  (11 errors, 0 warnings)

If I run phpcs --standard=PSR2 ... it works correctly, but I would rather not have to include that every single time I run it.

The code errors reported match the standards in use.

$ phpcs --version PHP_CodeSniffer version 3.4.0 (stable) by Squiz (http://www.squiz.net)

gsherwood commented 5 years ago

This is working fine for me.

Are you sure there isn't a phpcs.xml file in your current directory that might be overriding settings?

Edit: or anywhere higher up in the directory tree

oxblue-rbollig commented 5 years ago

That's a good thought. I checked and don't have anything there.

I also moved my PHP file to / and tried there. Same result.

Checked the entire filesystem and the only phpcs.xml file - none found.

...

Since it works for you and not for me, I'll try to make a Dockerfile or Vagrant file that creates the same situation. Maybe I'll stumble on the solution or maybe I'll be able to share an environment that has the problem.

oxblue-rbollig commented 5 years ago

GitHub doesn't support attaching Dockerfiles, but here's the content of a simple one that demonstrates the problem I'm having.

Looking into it some more, this may not be a supported installation method... but it's the only one I've found that reliably works on the CLI.

FROM ubuntu:latest
DEBIAN_FRONTEND=noninteractive
RUN apt-get update \
  && apt-get --yes upgrade \
  && apt-get --yes install php php-xml curl \
  && curl -OL https://squizlabs.github.io/PHP_CodeSniffer/phpcs.phar \
  && curl -OL https://squizlabs.github.io/PHP_CodeSniffer/phpcbf.phar \
  && chmod +x *.phar \
  && mv phpcs.phar /usr/sbin/phpcs \
  && mv phpcbf.phar /usr/sbin/phpcbf \ 
  && apt-get --yes remove curl \
  && apt-get --yes autoremove \
  && apt-get --yes clean \
  && phpcs --config-set standard PSR2 \
  && phpcs --config-show \
  && echo "<?php phpinfo();" > test.php \ 
  && phpcs -v test.php
RUN mkdir /dist
oxblue-rbollig commented 5 years ago

image

This gets more interesting. I tried using different installation methods.

On the left, I installed by cloning the repo, cd PHP_CodeSniffer, setting the config and running it.
On the right, I installed with wget.

I set PSR2 in the configuration for both and ran them.

In the repo (left), the first line says it is using PHP_CodeSniffer standard, in the one on the right the first line says it is using PEAR standard. The version numbers are also off by 0.0.1, so I think something changed recently with this.

oxblue-rbollig commented 5 years ago

Bisecting, it looks like this feature has never worked correctly.

jrfnl commented 5 years ago

@oxblue-rbollig Well, from what I can see in the screenshots, I can explain the difference: you are running PHPCS from different directories.

PHPCS determines the ruleset to use in this way:

In the screen on the left, you are running from the PHP_Codesniffer directory which contains a phpcs.xml.dist file for the PHPCS native CS checks, so logically, that file is picked up on and the CodeSniffer.conf file is not checked.

So, no, there haven't been any recent changes which cause this difference in behaviour. The difference is purely caused by the directory from which you are calling PHPCS.

oxblue-rbollig commented 5 years ago

I would be super happy if I'm just misunderstanding or using it wrong.

But I'm running phpcs --show-config from the same directory, and it shows PSR2... so why does it show one standard in the configuration but then use another to actually sniff.

Oh, I didn't mention this in the screen shots, but they are two, separate, fresh containers.

oxblue-rbollig commented 5 years ago

image I removed the phpcs.xml.dist file and ran them again. Same results. CodeSniffer.conf shows PSR2 as the standard, which I assume is where --config-show gets it from.

I don't see any of these files traversing up directories. I also don't see them on the container diff.

oxblue-rbollig commented 5 years ago

Continuing to dig into this... It does work if I create a phpcs.xml file such as:

<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="PSR2" xsi:noNamespaceSchemaLocation="../../../phpcs.xsd">
    <description>The PSR-2 coding standard.</description>
    <arg name="tab-width" value="4"/>
    <rule ref="PSR1"/>
    <rule ref="PSR2"/>
</ruleset>

So, though the CodeSniffer.conf doesn't seem to take effect, I'm in business and this seems to be a viable workaround for most cases.

...

As a side note, I did try adding some breaking code to CodeSniffer.conf and it does actually break the program on run... so something is calling that code even if it doesn't appear to be using the settings.

You can also set config variables that don't exist, so it appears to not validate. An example is:

Using config file: //CodeSniffer.conf

Config value "smurf" added successfully
root@bffa229e78ad:/# cat CodeSniffer.conf 
<?php
 $phpCodeSnifferConfig = array (
  'standard' => 'PSR2',
  'smurf' => 'brainy',
)
?>root@bffa229e78ad:/# 

I think I spelled everything as expected and tried some different forms (PSR2, psr2, etc.) but as a note to others who might run into this same issue... make sure you spell everything correctly.

jrfnl commented 5 years ago

You can also set config variables that don't exist, so it appears to not validate.

Not having any particular validation for setting config values is on purpose. This is to allow external standards to support additional arbitrary config settings.

However, PHPCS itself only looks at particular keys and validates those on use. All other keys in the array are just disregarded. If implemented correctly, external standards using config settings do the same.

oxblue-rbollig commented 5 years ago

Okay, that makes sense.

Found the problem (or rather, someone on Stack Overflow pointed it out.)

It should be phpcs --config-set=default_standard according to the documentation. Setting it correctly makes it work correctly.

Sorry for the fire drill. A little embarrassing since the error is in my first post, but I guess it is subtle given how many people have looked at it and didn't notice it.

Hope this helps the next person.

gsherwood commented 5 years ago

Sorry for not catching that. I'm glad you worked it out in the end.