Many tools are able to use the code sniffer rules in the coder_sniffer module, e.g. command line phpcs, IDE's, etc. and they don't need a Drupal module or the rest of the coder modules in order to do that.

We should move the sniffer rules to an external library (hosted on packagist?) to allow it to be used in different ways.

Comments

thursday_bw’s picture

+1 I was making great use of the drupalcs project.

We have our own in house standards for our private and custom drupal modules and regularly merged in upstream changes
from the drupalcs project.

This has now become problematic as we need to track the entire coder module just for the few files in the coder_sniffer folder.

The other advantage that was lost in the merge was the ability to checkout the standards directly into the phpcs's sniffs directory, avoiding having to symlink to it.

I understand there was much debate about merging that into coder, now that it's been done, in hindsight it's a pain in the proverbial.

dkh’s picture

I agree that the php_cs Drupal standard checker should be unbound from the coder module.

With php_cs installed from pear (under MAMP), the Drupal standard lib should live here:

/Applications/MAMP/bin/php/php5.3.13/lib/php/PHP/CodeSniffer/Standards/Drupal

But the coder module (where the php_cs Drupal standard lives) is intended to be installed local to a Drupal project, creating potential Drupal library conflicts if the coder module is found in say .drush AND a Drupal project install.

It seems the ideal solution would be to have the php_cs Drupal standard live in a central location and have the Coder library have a dynamic way to find it -- or even just use a symlink.

Anecdotally, I use Drupal's code sniffer standard in emacs to check the syntax of all my code dynamically. I know a lot of other people do something similar with the Sublime IDE as part of a growing trend, which basically does this:

phpcs --standard=Drupal

douggreen’s picture

We merged phpcs into coder after DrupalCon Munich because it was thought that more people used coder than phpcs, and this would give phpcs wider use. I haven't used coder as a module for years (although many people do). For those of you that don't want a module, can't use you just install it as a drush plugin?

$ cd ~/.drush
$ drush dl coder

I also don't run phpcs directly, but use the drush command line to coder, which has the benefit of (a) uses the coder ignore system, so certain warnings can be ignored and (b) what I think is nicer formatting.

jtsnow’s picture

I agree, having a separate package that would facilitate install the Drupal coding standard under PHP_CodeSniffer would be very useful. I'm currently trying to set up a chef recipe that automatically installs the Drupal code sniffer in the proper place for use with arcanist. I fear that this is prone to breakage in the future without a definitive package for the Drupal code sniffer.

mile23’s picture

I'm working on a Composer-based workflow type tool and I ran up against the problem of the standard inside Coder.

I came up with this workaround.

Within composer.json:

    "repositories": [
        {
            "type": "package",
            "package": {
                "name": "drupalmodule/coder",
                "version": "7.2.1",
                "dist": {
                    "url": "http://ftp.drupal.org/files/projects/coder-7.x-2.1.zip",
                    "type": "zip"
                }
            }
        }
    ],

    "require": {
    	"squizlabs/php_codesniffer": "1.5.1",
    	"drupalmodule/coder": "~7.2"
    }

Now you know that the standard is at ./vendor/drupalmodule/coder/coder_sniffer/Drupal. Of course this will break if the Coder maintainers move it. :-)

As for sustainable solutions: The CS standard could even just be a sandbox project on d.o as long as someone kept it in sync with the Coder one (or vice versa). Its composer.json could just call it a library, or use one of the cs-standards Composer plugins you can find on Packagist.

My project here: https://github.com/paul-m/pareviewer

kim.pepper’s picture

We could just git subtree split and push to a read-only sandbox? Or adding composer.json packaging info and push to github.

klausi’s picture

Issue summary: View changes

Not sure we need to do that. We could just create a composer.json file in the coder repository root directory that describes all the coder sniffer stuff.

@dkh: you can specify "installed_paths" in your PHPCS config (CodeSniffer.conf) then you don't need to symlink anything and you can checkout coder to any location.

So if people use coder sniffer as a library they can just ignore that other coder_review and coder_upgrade module stuff, and we don't have to split up this project again.

I experimented with PHPUnit tests for DrupalPractice and I think we will want to do the same for Coder sniffer. So we will need composer.json anyway.

mile23’s picture

I made a repo here to experiment with:

https://github.com/paul-m/drupal_php_cs

You can pull it into your composer project this way:

  "repositories": [
    {
      "type": "vcs",
      "url": "https://github.com/paul-m/drupal_php_cs"
    }
  ],

...

  "require": {
    "drupal/drupal_php_cs": "dev-master"
  },

It wouldn't be hard to make a packagist entry for it, eliminating the need for the repositories field. But the issue of maintenance is very real. :-)

One day Drupal will be all-Composer-all-the-time, but until then this kind of thing is awkward.

klausi’s picture

Status: Active » Needs review

Ok, so I started experimenting with a composer.json file on a github coder fork: https://github.com/klausi/coder

And also created a test package on packagist: https://packagist.org/packages/klausi/coder

Unfortunately we need to use semantic versioning git tags, so I used 7.2.2 (which would be 7.x-2.2 on drupal.org).

So in order to use the Drupal PHPCS coding standard with a project you would use this composer.json file:

{
    "require": {
        "klausi/coder": "7.2.2"
    }
}

Then you would install composer and the dependencies:

curl -sS https://getcomposer.org/installer | php
php composer.phar install

And then you can start using phpcs on some example.php file that you want to test:

./vendor/bin/phpcs --standard=./vendor/klausi/coder/coder_sniffer/Drupal example.php

Is that sufficient for everyone? Of course we would rename "klausi/coder" to "drupal/coder".

klausi’s picture

Title: Move Drupal code sniffer rules to an external library » Create composer.json to provide Drupal code sniffer rules as external library
mile23’s picture

StatusFileSize
new741 bytes

If we're going to keep the sniffer standard buried in the Coder module packaging, then you can just add composer.json and then point Packagist to the git.drupal.org repo. Sign in to Packagist, create a new package, give it http://git.drupal.org/project/coder.git as the repo, and it will do its thing. No need to have a github repo, unless you just want to.... (Although, it's unclear to me whether the Drupal Association wants all the network traffic.)

Also I'd like to draw your attention to a sandbox project I made called NetBeansDrupalComposed: https://drupal.org/sandbox/mile23/2197899 It pulls down the requirements for doing code standard reviews with NetBeans, and then tells you the configuration paths.

Here's a patch to composer.json for Coder. I changed the name to drupalmodule/coder and added some support info.

klausi’s picture

Status: Needs review » Needs work

EDIT: of course we would point packagist to the drupal.org repository, my Github fork was just a demo and proof of concept.

  1. --- /dev/null
    +++ b/.gitignore
    

    I would rather not add a .gitignore file, people can do that locally if they want.

  2. +++ b/composer.json
    @@ -0,0 +1,15 @@
    +  "name": "drupalmodule/coder",
    

    why "drupalmodule" and not simply "drupal"? This project is more than a module, so that name would be misleading.

  3. +++ b/composer.json
    @@ -0,0 +1,15 @@
    +  "support": {
    +    "issues": "https://drupal.org/project/issues/coder",
    +    "source": "https://drupal.org/project/coder"
    +  },
    

    Nice! So we should also add a homepage entry pointing to https://drupal.org/project/coder as well.

  4. +++ b/composer.json
    @@ -0,0 +1,15 @@
    +    "require": {
    

    indentation error?

Any other opinions on using this approach?

mile23’s picture

StatusFileSize
new1.85 KB

I chose 'drupalmodule' for the vendor because this composer.json file puts a drupal module in your vendor directory. :-) The vendor wouldn't be 'drupal', because it's not Drupal. Maybe 'drupalproject' would be better. Really doesn't matter that much however.

So here's a patch with the changes from #12, and also a README-composer.txt file. Here's what the readme says:

Installation with Composer
--------------------------

The Coder module has a top-level composer.json file. This allows your project
to declare it as a requirement using Composer.

The most obvious use for this would be to use its PHP_CodeSniffer standard
independent of Drupal.

There are a few caveats, however.

The most important is that, due to the non-semantic way in which Drupal modules
are versioned, you must require a dev branch.

So for instance, you'd need something like this:

"require": {
"drupalproject/coder": "dev-7.x-2.x"
}

This also means that your root project must have a minimum-stability of dev.

"minimum-stability": "dev"

The second caveat is that if you want to use the coding standard, it lives at
coder/code_sniffer/Drupal. So you'd need to run phpcs this way:

phpcs --standard=path/to/coder/code_sniffer/Drupal code/to/review

Coder declares a dependency on phpcs for convenience, so you can download it,
install using Composer, and then do a code sniff this way:

./vendor/bin/phpcs --standard=code_sniffer/Drupal code/to/review

klausi’s picture

Status: Needs work » Fixed

I think "drupal" as vendor is reasonable, same as "symfony" is used as vendor. The vendor really is the Drupal community, so I think this is appropriate.

I committed something similar now: http://drupalcode.org/project/coder.git/commit/fa8e809

Downloads on packagist: https://packagist.org/packages/drupal/coder

I created a Git tag "7.2.2" besides "7.x-2.2" so no need for the composer dev hacks above.

mile23’s picture

Very cool. Thanks. :-)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.