xwp / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
29 stars 3 forks source link

Integrate Sniffs with WP-CLI #7

Closed westonruter closed 11 years ago

westonruter commented 11 years ago

We could introduce a new WP-CLI subcommand called lint or phpcs which would apply the sniffs in this project to the WordPress install in the current scope.

I'm not sure of the best route to go as for integrating the sniffs into WP-CLI. Perhaps it would mean forking WP-CLI https://github.com/wp-cli/wp-cli and then adding this repo as a submodule dependency.

mrchrisadams commented 11 years ago

Hi Weston,

I looked into this last night.

Adding phpcs to wpcli

It looks like the recommended approach from the wp-cli developers is to create a plugin, rather than fork the code.

Hypothetically, we could wrap the wordpress coding-standards-project repo in a plugin, to make it accessible to wp-cli inside a wordpress project.

Here are a few links below to showing how you would add commands to wp-cli - they tend to rely on creating plugins to wrap existing code:

https://github.com/wp-cli/wp-cli/wiki/Creating-Commands https://github.com/wp-cli/wp-cli/wiki/Commands-Cookbook

Using composer to manage the dependency on phpcs for wp-cli

It looks like Codesnifer can now be called without needing to rely on PEAR, by using composer instead.

Which means we could call phpcs, without jumping through sudo hoops or forcing a user to rely on PEAR, much like how wp-cli uses composer.

How PHP_Codesniffer seems to work

I looked at how PHP_CodeSniffer is called inside their own project - they create an PHP_CodeSniffer object, then pass in various arguments, returning the result, then they seem to pass it to some printing function for STDOUT.

Once we have some sensible defaults for our main command, that we would pass to phpcs, like the default coding standards and so on, we could then build it up, or allow some way to pass the arguments straight through.

I'm not too sure what is best for this yet.

Calling phpcs from inside wp-cli

It looks like to call phpcs from the wp command, we'd need to do this too, inside the plugin, and then from inside there, requiring a file that wraps the command.

Inside the main wordpress plugin, we'd do something like this, to make sure we're inside the context of a wp cli command, then include a file that wrapper our calls to phpcs.


if ( defined('WP_CLI') && WP_CLI ) {
  include __DIR__ . '/code-sniff.php';
}

 what the wp-cli command might look like:

We'd have to do something like run composer's autoloader on the project level, or the plugin level (depending on where we install the code using composer).

I need to stress this is a pseudocode sketch, as a starting point for a convo:


    // this might need to be called elsewhere, especially if composer 
    // was run on the project root, rather than the plugin root
    require 'vendor/autoload.php';

    /**
     * Implements sniff command.
     *
     * @package wp-cli
     */
    class CodeSniff_Command extends WP_CLI_Command {

      /**
       * Call phpcs from inside the project
       *
       * @synopsis <path_to_file> [<another_path_to_file>]  [--directory=<directory_path>] [--ruleset=<path_to_ruleset>
       */
      function phpcs( $args, $assoc_args ) {
        list( $paths_to_files ) = $args;

        // make a PHP CodeSniffer object - I couldn't find a class to call directly
        // from inside code:
        $codesniff = new PHP_CodeSniffer_CLI;

        //  merge in $paths_to_files to sensible defaults for using PHPCS stored somewhere
        $wp_cli_code_sniff_args = some_kind_of_merge($phpcs_default, $paths_to_files);

        // call the method that processes the files, based on line 496 of the php code sniffer project:
        // https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/CLI.php#L496

        $php_cs_result = $codesniff->process($wp_cli_code_sniff_args);

        if ($php_cs_result == 0) {
          // Print a success message
          WP_CLI::success( "Great! Your code doesn't smell!" );
        }
        else{
         WP_CLI::fail( "Oops - Looks like we still have errors:" );
         WP_CLI::fail(print_in_formatted_way($php_cs_result));
        }
      }
    }

    WP_CLI::add_command( 'sniffer', 'WP_PHPCS_Command' );

This would let us call something like:

wp sniff phpcs

And it could call phpcs.

In theory.

BTW - I'm happy to continue this convo on the new org repo for wordpress coding standards if you prefer. Let me know, and I'll update there as I find more.

westonruter commented 11 years ago

/cc @danielbachhuber @tollmanz

danielbachhuber commented 11 years ago

Sorry for the late reply... still catching up from vacation.

Originally, I proposed packaging this as a plugin because it would allow us to more quickly iterate on WordPress Coding Standards and how the command works. When we felt like it was in a good state, we could contribute it to wp-cli.

However, I am obviously the outsider on this project. Thoughts?

cc @scribu

scribu commented 11 years ago

I basically agree with the plan outlined by @mrchrisadams above.

PS: It's very confusing that both x-team/WordPress-Coding-Standards and WordPress-Coding-Standards/WordPress-Coding-Standards have Github Issues enabled.

westonruter commented 11 years ago

@scribu yes, the issues need to be moved from the x-team fork to the WordPress-Coding-Standards repo. Issues are currently enabled on both for historical reasons, and the WordPress-Coding-Standards organization is actually brand new, and we're working on consolidating and centralizing development there.

westonruter commented 11 years ago

(I've created an issue to move the issues: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/15)

westonruter commented 11 years ago

OK, I've moved the issues from the x-team fork over to the main WordPress-Coding-Standards repo. However, I cannot move your issue comments and retain authorship, so please re-comment on the new issue what you want to remain remain: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/37

I am closing the issue here since it is effectively moved to the new repo.