Install sniffs with relative paths in CodeSniffer.conf

[tmountjr]

Problem/Motivation

My workflow involves running a development version of a site in a vagrant environment while editing code on the host in Atom. I have php_codesniffer installed as a composer dependency in vagrant and through linter-phpcs in Atom. Importantly, I installed the packages from within the vagrant environment.

For reference, here's the require section of my composer.json:

{
    "require": {
        "squizlabs/php_codesniffer": "^2.8",
        "dealerdirect/phpcodesniffer-composer-installer": "^0.3.0",
        "frenck/php-compatibility": "^7.1"
    }
}

I created the following phpcs.xml file:

<?xml version="1.0"?>
<ruleset name="PHP 5.3 Compatibility">
    <description>Ruleset for PHP 5.3 Compatibility</description>

    <!-- Run against the PHPCompatibility ruleset -->
    <rule ref="PHPCompatibility" />

    <!-- Run against the PSR2 ruleset -->
    <rule ref="PSR2" />

    <!-- Set additional config parameters -->
    <config name="testVersion" value="5.3" />
</ruleset>

(Yeah, I have to support php 5.3, and after breaking code pushes twice in the last week - thankfully only to the staging environment - I need a little more checking.)

As soon as I saved phpcs.xml in the root of the project, Atom threw an error:

Error: PHP Fatal error: Uncaught UnexpectedValueException: DirectoryIterator::__construct(/var/www/devsite/vendor/frenck/php-compatibility): failed to open dir: No such file or directory in /Users/tomm/Documents/repos/devsite/vendor/squizlabs/php_codesniffer/CodeSniffer.php:2250 

Which is an odd error, because the /var/www/devsite path is the path within my vagrant environment. I tracked it down to vendor/squizlabs/php_codesniffer/CodeSniffer.conf:

<?php
 $phpCodeSnifferConfig = array (
  'installed_paths' => '/var/www/devsite/vendor/frenck/php-compatibility',
)
?>

Proposed changes

In the short term, I manually changed CodeSniffer.conf to this:

<?php
 $phpCodeSnifferConfig = array (
  'installed_paths' => '../../frenck/php-compatibility',
)
?>

I expect this happened because when I installed the composer packages from Atom, this module picked up the absolute path to the frenck/php-compatibility package and dropped it in CodeSniffer.conf; but because that absolute path doesn't exist outside my vagrant environment, when Atom tries to find that path it throws an exception.

I'm not sure what the best approach here would be. On the one hand, relative paths work fine when you're only using this on a per-project basis and all the additional plugins are similarly located within the same vendor folder. But that breaks down if this is being installed globally because, in theory, additional plugins might be in the user's home directory or within a project directory.

Additionally (maybe another bug), running composer update deleted that entry in CodeSniffer.conf when I changed it to the relative path. That may be a separate bug, or something to address depending on what happens here.

[Potherca]

@tmountjr Thanks for taking the time to report this!

In regards to the relative paths, we might be able to add a config setting that can be changed from the composer.json to force relative paths. I'll give it some thought and report back when I have a plan on how to approach this.

In regards to composer update, that might be related to #13. Or not. I agree to first resolve the path issue and see what happens from there.

[frenck]

@Potherca I've looked into ways of implementing this and here is my main concern: What if you want relative paths in your development environment (e.g. Vagrant) and absolute paths in production?

Using a config setting will makes this harder.

My suggestion (but harder to implement) is to use relative paths on "local" repositories and absolute paths when using globals installations.

This should make it work in all use cases.

[Potherca]

TL;DR

I think using relative paths for local installs and absolute paths for global installs is a good solution.
Currently I do not see any downsides to this approach.

More detailed response

Generally speaking, making "a thing" configurable can be a signal of taking the easy way out. (In effect placing the responsibility with the user and not the code).

Having thought about this feature request for a bit, I agree that making this configurable would be less effective.

There does not seem to be any downside to always using relative paths for "local" (project specific) installs. As this might cause problems for "global" (system-wide) installs, it makes sense to always use absolute paths there.

I can not think of any scenario's where this distinction might cause a problem.

Using relative paths for local installs and absolute paths for global installs looks like the way to go.

[frenck]

Implemented a change for this, opened PR #28

[frenck]

Merged with master, will be in the next release.