Adding support of passing project-specific configuration to code sniffer #6
Conversation
| $this->setName('phpdebt') | ||
| ->addArgument('path', InputArgument::REQUIRED, 'The path of the file or directory that needs to be scanned.') | ||
| ->addOption('standard', 's', InputOption::VALUE_REQUIRED, 'The path of the phpcs.xml.dist') | ||
| ->setDescription('Enables option of utilizing project based PHPCS Config.'); |
There was a problem hiding this comment.
The description is for the command, not the option.
$ php src/index.php --help
Description:
Enables option of utilizing project based PHPCS Config.
There was a problem hiding this comment.
Oh yes, I totally missed that. It's fixed now.
| * | ||
| * @package PHPDebt\Console\Command | ||
| */ | ||
| class CodeSnifferConfigCommand extends Command { |
There was a problem hiding this comment.
The description says it's the whole phpdebt command, but the class is named like it's for configuring code sniffer.
There was a problem hiding this comment.
Thanks for catching it, I've written it initially as was thinking that I would be dealing with here code related only to the PHPCS, later my scope changed, but forgot to rename the class. This is fixed now.
|
Is there a solution for scanning multiple paths, or does that still need to be discussed? If you use Example 1, the standard configures phpcs to scan two places: Example 2, use two standards, one that just includes modules and one that just includes the theme: I looked at a project that uses a custom phpcs configuration, and it looks like there's a potential additional problem. It sounds like phpcs only uses the paths specified in the standard if there isn't a path given on the command line. Perhaps it'd be more intuitive if the path given to phpdebt would also override the paths in the standard. The CI scripts we'd have to write would be like Example 1, except that the paths scanned would be the path we see in the command line. |
|
@travis-bradbury, I've updated the code and it'll be using now the format of Example 1. I also found it more logical, thanks for the explanation with examples.
|
|
@RishiKulshreshtha cool, looks good to me now. |
|
@RishiKulshreshtha can you give me a quick summary of this PR? It looks like it switches the setup to use symfony console as the base and then moves some of the phpdebt commands into that? |
@smmccabe, as added above:
Yes, that's correct, but the actual reason to implement this is to allow the user/dev to use their existing/custom |
| $faults = $runner->run(); | ||
| print "phpcs Drupal: " . $faults . "\n"; | ||
| $standards_faults += $faults; | ||
|
|
||
| $faults = $runner->run(); | ||
| print "phpcs DrupalPractice: " . $faults . "\n"; | ||
| $standards_faults += $faults; |
There was a problem hiding this comment.
These two are identical. Also the print lines should reflect $options instead of always being drupal.
| if ($option) { | ||
| define('PHP_CODESNIFFER_CBF', FALSE); | ||
| $runner->config = new Config(["--standard=$option"]); |
There was a problem hiding this comment.
Instead of having this if be running phpcs in two contexts, we should move the running until after and have the if just for determining if we need a default for $option.
I mean to say:
if (!$option) {
$option = 'sane default';
}
$runner->config = new Config(["--standard=$option"]);
...
Now
phpdebtcan be allowed to use the PHPCS config via:phpdebt <path-of-dir/file-to-be-scanned> --standard=phpcs.xml.dist