PHPCSStandards / composer-installer

Composer installer for PHP_CodeSniffer coding standards
https://packagist.org/packages/dealerdirect/phpcodesniffer-composer-installer
MIT License
559 stars 36 forks source link

phpcs not recognizing `installed_paths` #84

Closed SturmB closed 4 years ago

SturmB commented 5 years ago

Problem/Motivation

phpcs is not seeing the WordPress standards.

Expected behaviour

I expect to see WordPress as one of the installed coding standards when I run phpcs -i.

Actual behaviour

WordPress does not show up in the list of installed coding standards.

Steps to reproduce

  1. Create a new WordPress site with Local by Flywheel.
  2. Install phpcs globally with composer global require "squizlabs/php_codesniffer=*".
  3. Install the WordPress coding standards with composer create-project wp-coding-standards/wpcs --no-dev in the project's app/public directory (the root of the WordPress installation).
  4. Install this package with composer require --dev dealerdirect/phpcodesniffer-composer-installer from the same directory as the previous step. Notice that it says installed_paths set to ../../../wpcs
  5. Run phpcs -i and see that WordPress is not in the list.

Proposed changes

I know so little about this process that I have absolutely no proposals for what to change nor how. All I can do is give you my current setup:

OS: Windows 10 1903. WordPress: 5.2.3 PHP: 7.3.2 MySQL: 5.6.34 Composer: 1.8.6 phpcs: 3.4.2 (stable) wp-coding-standards: 2.1.1 phpcodesniffer-composer-installer: v0.5.0

I'm sure that I'm just doing something wrong, so please don't make any assumptions; ask me for any clarifications and I'll be happy to provide what I can.

Potherca commented 5 years ago

Hi, thank you for reporting this bug!

I'm currently in the process of get the currently open merge-requests into a new (minor) release. Once that is done, I'll come back to this issue.

It could be related to other relative/absolute path issues already reported. I also need to check what the deal was with global/local installs of this plugin and sniffs.

Will let you know when I am available to make progress on this issue.

johndodev commented 5 years ago

+1, with another standard, not detected by phpcs event if it says installed_paths set to...

johndodev commented 5 years ago

Oops, it is working, it was a problem of chmod, when I trying to do it manually by doing : ./vendor/bin/phpcs --config-set installed_paths /mypath/vendor/phpcompatibility/php-compatibility/ I get

PHP Warning:  file_put_contents(/mypath/vendor/squizlabs/php_codesniffer/CodeSniffer.conf):                                  failed to open stream: Permission denied in vendor/squizlabs/php_codesniffer/src/Config.php on line 1613
ERROR: Config file vendor/squizlabs/php_codesniffer/CodeSniffer.conf could not be written

The real problem is that this library do not say when the process has failed, I mean, that error is not reported by the installer, so we thought it should work at the first place.

it may be the same problem @SturmB

SturmB commented 5 years ago

Thanks for the input, @johndodev, but it seems my issue is different.

~/Sites/phpcs/app/public  phpcs --config-set installed_paths ./wpcs
Using config file: C:\Users\<username_hidden>\AppData\Roaming\Composer\vendor\squizlabs\php_codesniffer\CodeSniffer.conf

Config value "installed_paths" added successfully
 ~/Sites/phpcs/app/public  phpcs -i
The installed coding standards are MySource, PEAR, PSR1, PSR12, PSR2, Squiz and Zend

I wonder if the problem is that I have phpcs installed globally, but I have the WordPress standard installed locally. Must I have them both installed in either one context or the other?

johndodev commented 5 years ago

Ok, I don't know, both are installed locally for me. It seems more an issue of how phpcs handle the config "installed_paths" than an issue in that lib. Maybe try with an absolute path.

phpcs --config-set installed_paths /absolute/path/wpcs
Potherca commented 5 years ago

The real problem is that this library do not say when the process has failed, I mean, that error is not reported by the installer, so we thought it should work at the first place.

Yeah. We have another issue that centers around incorrect (or lacking) error reporting. Also, the plugin does not "error out" properly when it encounters problems. It is part of what we are currently looking into with #80

I wonder if the problem is that I have phpcs installed globally, but I have the WordPress standard installed locally. Must I have them both installed in either one context or the other?

There is definitely some quirky/unintuitive behaviour there. This is another thing I want to get more to grips with before releasing potential fixes (as to avoid fixing one scenario but breaking another one).

I would like to thank you both for your further feedback. Please bear with us as we continue to investigate these issues.

SturmB commented 5 years ago

Thanks, @Potherca. It should be noted that I was simply following the instructions in the main phpcs readme. I ran composer global require "squizlabs/php_codesniffer=*", as shown in that readme, before installing this package.

jrfnl commented 4 years ago

FYI: PR #80 has been merged which should improve the error reporting and PR #93 has just been opened as a further iteration on that, so hopefully it will now become easier to know when something is going wrong and what.

Potherca commented 4 years ago

FYI: #93 Has been merged. 👍

jrfnl commented 4 years ago

@SturmB Could you do a phpcs --config-show and paste the output here ?

Potherca commented 4 years ago

Version v0.6.0 has just been released. Besides several fixes, this version also adds improved debug/verbose output.

@SturmB Could you try to reproduce the issue you reported with v0.6.0 and let us know if it still happens? If so, please run the plugin in verbose mode and paste the output of the run here (that should make it possible for us to re-produce the issue ourselves).

SturmB commented 4 years ago

I will do so as soon as I get some time in the next week. It'll be iffy because my dev setup has changed since then. (I'm now using WSL rather than ConEmu, etc.)

SturmB commented 4 years ago

Finally was able to replicate my original setup for this. (Had to disable Hyper-V for a bit.)

The result of following the steps in my original post:

 ~/Sites/phpcs/app/public  composer global require "squizlabs/php_codesniffer=*"
Changed current directory to C:/Users/christopher.mcgee/AppData/Roaming/Composer
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Updating squizlabs/php_codesniffer (3.4.2 => 3.5.4): Downloading (100%)
Writing lock file
Generating autoload files
 ~/Sites/phpcs/app/public  composer create-project wp-coding-standards/wpcs --no-dev
Installing wp-coding-standards/wpcs (2.2.1)

  [InvalidArgumentException]
  Project directory C:/Users/christopher.mcgee/Sites/phpcs/app/public/wpcs/ is not empty.

create-project [-s|--stability STABILITY] [--prefer-source] [--prefer-dist] [--repository REPOSITORY] [--repository-url REPOSITORY-URL] [--dev] [--no-dev] [--no-custom-installers] [--no-scripts] [--no-progress] [--no-secure-http] [--keep-vcs] [--remove-vcs] [--no-install] [--ignore-platform-reqs] [--] [<package>] [<directory>] [<version>]

 ✘  ~/Sites/phpcs/app/public  rm -rf wpcs
 ~/Sites/phpcs/app/public  composer create-project wp-coding-standards/wpcs --no-dev
Installing wp-coding-standards/wpcs (2.2.1)
  - Installing wp-coding-standards/wpcs (2.2.1): Downloading (100%)
Created project in C:\Users\christopher.mcgee\Sites\phpcs\app\public\wpcs
Loading composer repositories with package information
Updating dependencies
Package operations: 1 install, 0 updates, 0 removals
  - Installing squizlabs/php_codesniffer (3.5.4): Loading from cache
Writing lock file
Generating autoload files
 ~/Sites/phpcs/app/public  composer require --dev dealerdirect/phpcodesniffer-composer-installer
Using version ^0.6.2 for dealerdirect/phpcodesniffer-composer-installer
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Updating dealerdirect/phpcodesniffer-composer-installer (v0.5.0 => v0.6.2): Downloading (100%)
Writing lock file
Generating autoload files
PHP CodeSniffer Config installed_paths set to ../../../wpcs
 ~/Sites/phpcs/app/public  phpcs -i
The installed coding standards are MySource, PEAR, PSR1, PSR12, PSR2, Squiz and Zend
 ~/Sites/phpcs/app/public 

As soon as I get a chance, I'll try this same thing with my current WSL setup.

jrfnl commented 4 years ago

I wonder if the problem is that I have phpcs installed globally, but I have the WordPress standard installed locally. Must I have them both installed in either one context or the other?

@SturmB Looking at the output, I think your own hunch was correct. To me this sounds like you're using two distinct installs of PHPCS. One of which has WPCS installed, the other doesn't.

Could you try running the below commands for me from the ~/Sites/phpcs/app/public directory to confirm if that is the issue ?

If my assessment is correct, I'd like to suggest the following which should create a workable situation for you:

  1. Throw away the WPCS install in ~/Sites/phpcs/app/public
  2. Run composer global require wp-coding-standards/wpcs dealerdirect/phpcodesniffer-composer-installer Note: no --dev, as based on your above listing you've installed PHPCS in require not require-dev for your global Composer install.
  3. Test it by running phpcs -i. This should now list both the PHPCS native standards as well as the WordPress standards.

If all is right, you should now be able to use the phpcs command anywhere on your system and it will work with all the standards listed via the phpcs -i command.

If, at a later point in time, you decide to install more PHPCS standards via composer global require, the plugin will automatically take care of registering them with PHPCS and they should be available to you globally straight after doing the require.

And yes, the WPCS install instructions need improvement as they now presume that people only use PHPCS with WPCS and won't have other installs. It's on the never-ending @todo list ;-)

SturmB commented 4 years ago

Yep, that was it, @jrfnl:

 ~/Sites/phpcs/app/public  which phpcs
/cygdrive/c/Users/<username>/AppData/Roaming/Composer/vendor/bin/phpcs
 ~/Sites/phpcs/app/public  vendor/bin/phpcs -i
The installed coding standards are MySource, PEAR, PSR1, PSR12, PSR2, Squiz, Zend, WordPress, WordPress-Core, WordPress-Docs and WordPress-Extra
 ~/Sites/phpcs/app/public  phpcs --config-show
Using config file:

 ~/Sites/phpcs/app/public  vendor/bin/phpcs --config-show
Using config file: C:\Users\<username>\Sites\phpcs\app\public\vendor\squizlabs\php_codesniffer\CodeSniffer.conf

installed_paths: ../../../wpcs
 ~/Sites/phpcs/app/public 

As before, this is with my old setup. Just to confirm while I'm here in a Windows session with Hyper-V disabled (so I can use Local by Flywheel), I'll run through the rest of your steps:

 ~/Sites/phpcs/app/public  rm -rf wpcs
 ~/Sites/phpcs/app/public  composer global require wp-coding-standards/wpcs dealerdirect/phpcodesniffer-composer-installer
Changed current directory to C:/Users/christopher.mcgee/AppData/Roaming/Composer
Using version ^2.2 for wp-coding-standards/wpcs
Using version ^0.6.2 for dealerdirect/phpcodesniffer-composer-installer
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 2 installs, 0 updates, 0 removals
  - Installing dealerdirect/phpcodesniffer-composer-installer (v0.6.2): Loading from cache
  - Installing wp-coding-standards/wpcs (2.2.1): Loading from cache
Writing lock file
Generating autoload files
PHP CodeSniffer Config installed_paths set to ../../wp-coding-standards/wpcs
 ~/Sites/phpcs/app/public  phpcs -i
The installed coding standards are MySource, PEAR, PSR1, PSR12, PSR2, Squiz, Zend, WordPress, WordPress-Core, WordPress-Docs and WordPress-Extra
 ~/Sites/phpcs/app/public 

So it seems that your assessment is, indeed, correct. I still have a great deal to learn about Composer, it seems. As well as to make this happen in WSL, which shouldn't be an issue now that I know the culprit of this problem.

Thanks so much for your help, @jrfnl!

jrfnl commented 4 years ago

@SturmB No problem. I'm just glad we've figured out together. Sorry it took so long.