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

Support PHP CodeSniffer standards in packages installed outside of the vendor directory #63

Closed echernyavskiy closed 6 years ago

echernyavskiy commented 6 years ago

Problem/Motivation

The aim of this is to add proper support for PHP CodeSniffer standards in packages installed outside of the vendor directory. One such example is Drupal's Coder module which is most commonly installed into web/sites/all/modules/contributed/coder or web/modules/contributed/coder where the web directory is on the same level as vendor. The issue is that for such packages composer's getInstallPath() returns an install path relative to the project root directory, and such relative paths are not treated properly later in the body of this plugin's updateInstalledPaths().

Expected behaviour

The coding standard's install path is handled properly by this plugin's updateInstalledPaths() so that the coding standard is properly detected.

Actual behaviour

The coding standard's install path is processed in such a way that the value added to PHPCS's installed_paths is a non-existent directory.

Steps to reproduce

Require both dealerdirect/phpcodesniffer-composer-installer and drupal/coder in composer.json, run composer install. drupal/coder will be installed into the configured Drupal modules directory and will not be properly detected by this plugin.

Proposed changes

The suggestion is to ensure all coding standard install paths are absolute before searching for available rule-sets in updateInstalledPaths(). See #64.

frenck commented 6 years ago

Having Coding Standards in your main project is already supported. The only limitation is the maximum search depth, which is going to be configurable when #46 gets merged in.

Am I missing something?

echernyavskiy commented 6 years ago

I'll try to be a bit more specific here. So given the following pretty typical Drupal project structure:

...
vendor
...
web
  - modules
    - contributed
      - coder
        - coder_sniffer
          - Drupal
            - ruleset.xml
...
composer.json
composer.lock

PHPCodeSniffer composer installer rightfully discovers this standard among others:

array(9) {
  [0]=>
  string(52) "/var/www/project"
  [1]=>
  string(29) "web/modules/contributed/coder"
  [2]=>
  string(98) "/var/www/project/vendor/escapestudios/symfony2-coding-standard"
  ...
  [8]=>
  string(84) "/var/www/project/vendor/slevomat/coding-standard"
}

But later on in updateInstalledPaths() it assumes that the path is absolute and attempts to get a relative path for an already relative one, which results in configuring PHPCodeSniffer with a non-existent standard install path:

string(58) "web/modules/contributed/coder/coder_sniffer/DrupalPractice"
string(77) "../../../../../../../../../../../web/modules/contributed/coder/coder_sniffer/"
frenck commented 6 years ago

I get it, nevertheless, I'm unable to reproduce.

I have set up a full Drupal project to test this out and this is what happens on my machine (note I've added a var_dump of the found $installedPaths array to the output for debugging.)

$ composer require drupal/coder                                                                                                                      
Using version ^8.3 for drupal/coder
./composer.json has been updated
    1/3:    http://repo.packagist.org/p/provider-2018-10$3f3940f9e55c8044a96080611abc07a61ac8963d9525ca5aa8adab8c837af3ec.json
    2/3:    http://repo.packagist.org/p/provider-latest$2d59e62583baac197336eab2a88ee9c54911e3def04e263e9f73fb3fef7553d1.json
    3/3:    http://repo.packagist.org/p/provider-2017$c78961d9a33367c72796bdbcb31d5d64c91da7d524f370a41582e2459b40d30b.json
    Finished: success: 3, skipped: 0, failure: 0, total: 3
> DrupalProject\composer\ScriptHandler::checkComposerVersion
Loading composer repositories with package information
Updating dependencies (including require-dev)
Nothing to install or update
Writing lock file
Generating autoload files
array(2) {
  [0]=>
  string(44) "/Volumes/CODE/dealerdirect/codesniffer-test3"
  [1]=>
  string(64) "/Volumes/CODE/dealerdirect/codesniffer-test3/vendor/drupal/coder"
}
PHP CodeSniffer Config installed_paths set to ../../drupal/coder/coder_sniffer/

Used composer.json for this test: https://gist.github.com/frenck/a03bd5695636d1105aea1895aea774e0

Please note, I've used the latest dev version of the installer in the above example, but I've got the same results with the v0.4.4 release.

frenck commented 6 years ago

@echernyavskiy I've ended up implementing your solution (slightly different / more incorporated with the plugin, since Filesystem is now used more widely by the Plugin).

Since I lack a reproduction scenario, I cannot ensure this fixes your problem.

frenck commented 6 years ago

@echernyavskiy It is in the v0.5.0 release. So if you could test it and let me know? That would be awesome 👍

echernyavskiy commented 6 years ago

@frenck, yup, tested and it works in my setup as well, thanks.

frenck commented 6 years ago

Nice @echernyavskiy! Thanks for letting me know!

I'll go ahead and close up this issue.