Skip to content

Commit

Permalink
Bump to codechecker/codenisffer 3.0
Browse files Browse the repository at this point in the history
Changes in commands are important because all the old CLI machinery
in phpcs 3.0 has been removed. There were some different solutions
for this:

- start using the bin/phpcs binary (exec) and forget.
- create a custom runner/reporter like we did in local_codechecker.
- use exclusively the new phpcs 3.0 classes to get it done.

While I was really tempted to re-use the local_codechecker custom
runner / reporter, finally I went the pure phpcs classes way to
avoid creating any dependency on local_codechecker (apart from the
standard).

Unit tests have been slightly improved to better assert that all the
expected elements in the output are there.

Finally, we have had to instruct phpcs where the PHPCompatibility
standard is because, when running from PHAR, the CodeSniffer.conf
file bundled is not read and it's expected to be in real filesystem.
This was causing codechecher not to work from PHAR, so we are now
configuring it from the commands (Checker and Fixer).

Closes moodlehq#49
  • Loading branch information
stronk7 committed Jan 27, 2021
1 parent 5ad54af commit 9e577d2
Show file tree
Hide file tree
Showing 9 changed files with 250 additions and 107 deletions.
12 changes: 3 additions & 9 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,11 @@
"type": "package",
"package": {
"name": "moodlehq/moodle-local_codechecker",
"version": "2.9.8",
"version": "3.0.0",
"source": {
"url": "https://github.com/moodlehq/moodle-local_codechecker.git",
"type": "git",
"reference": "v2.9.8"
},
"autoload": {
"classmap": [
"pear/PHP/CodeSniffer.php",
"pear/PHP/CodeSniffer/CLI.php"
]
"reference": "v3.0.0"
}
}
},
Expand Down Expand Up @@ -75,7 +69,7 @@
],
"require": {
"php": ">=7.0.8",
"moodlehq/moodle-local_codechecker": "^2.9.8",
"moodlehq/moodle-local_codechecker": "^3.0.0",
"moodlehq/moodle-local_ci": "^1.0.8",
"moodlehq/moodle-local_moodlecheck": "^1.0.4",
"sebastian/phpcpd": "^3.0",
Expand Down
14 changes: 4 additions & 10 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt
- `moodle-plugin-ci phpunit` when coverage report is included, phpdbg is called with ignore memory limits param
to avoid memory exhaused errors.
- Updated project dependencies to current [moodle-local_moodlecheck](https://github.com/moodlehq/moodle-local_moodlecheck) and [moodle-local_ci](https://github.com/moodlehq/moodle-local_ci) versions.
- Updated version of [moodle-local_codechecker](https://github.com/moodlehq/moodle-local_codechecker) to v3.0.0.

### Added
- Detect existence of legacy php-webdriver, and use a different Firefox image when it is in use
Expand Down
11 changes: 3 additions & 8 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,6 @@
</errorLevel>
</MissingReturnType>

<!-- CodeSniffer has some bad PHPDoc -->
<InvalidPropertyAssignmentValue>
<errorLevel type="suppress">
<file name="src/Bridge/CodeSnifferCLI.php"/>
</errorLevel>
</InvalidPropertyAssignmentValue>

<!-- CodeSniffer has some bad PHPDoc -->
<NullArgument>
<errorLevel type="suppress">
Expand All @@ -102,10 +95,12 @@
</errorLevel>
</NullArgument>

<!-- It is defined in Moodle itself -->
<!-- It is defined in Moodle itself or included by the commands -->
<UndefinedClass>
<errorLevel type="suppress">
<file name="src/Bridge/Moodle.php"/>
<file name="src/Command/CodeCheckerCommand.php"/>
<file name="src/Command/CodeFixerCommand.php"/>
</errorLevel>
</UndefinedClass>
</issueHandlers>
Expand Down
48 changes: 0 additions & 48 deletions src/Bridge/CodeSnifferCLI.php

This file was deleted.

103 changes: 88 additions & 15 deletions src/Command/CodeCheckerCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,24 @@

namespace MoodlePluginCI\Command;

use MoodlePluginCI\Bridge\CodeSnifferCLI;
use MoodlePluginCI\StandardResolver;
use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Reporter;
use PHP_CodeSniffer\Runner;
use PHP_CodeSniffer\Util\Timing;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Finder\Finder;

// Cannot be autoload from composer, because this autoloader is used for both
// phpcs and any standard Sniff, so it must be loaded at the end. For more info, see:
// https://github.com/squizlabs/PHP_CodeSniffer/issues/1463#issuecomment-300637855
//
// The alternative is to, instead of using PHP_CodeSniffer like a library, just
// use the binaries bundled with it (phpcs, phpcbf...) but we want to use as lib for now.
require_once __DIR__.'/../../vendor/moodlehq/moodle-local_codechecker/phpcs/autoload.php';

/**
* Run Moodle Code Checker on a plugin.
*/
Expand Down Expand Up @@ -72,24 +83,86 @@ protected function execute(InputInterface $input, OutputInterface $output)
return $this->outputSkip($output);
}

// Must define this before the sniffer due to odd code inclusion resulting in sniffer being included twice.
$cli = new CodeSnifferCLI([
'reports' => ['full' => null],
'colors' => $output->isDecorated(),
'encoding' => 'utf-8',
'showProgress' => true,
'reportWidth' => 120,
]);
// Needed constant.
if (defined('PHP_CODESNIFFER_CBF') === false) {
define('PHP_CODESNIFFER_CBF', false);
}

\PHP_CodeSniffer::setConfigData('testVersion', PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION, true);
Timing::startTiming();

$runner = new Runner();
$runner->config = new Config(['--parallel=1']); // Pass a param to shortcut params coming from caller CLI.

// This is not needed normally, because phpcs loads the CodeSniffer.conf from its
// root directory. But when this is run from within a .phar file, it expects the
// config file to be out from the phar, in the same directory.
//
// While this approach is logic and enabled to configure phpcs, we don't want that
// in this case, we just want to ensure phpcs knows about our standards,
// so we are going to force the installed_paths config here.
//
// And it needs need to do it BEFORE the runner init! (or it's lost)
//
// Note: the "moodle" one is not really needed, because it's autodetected normally,
// but the PHPCompatibility one is. There are some issues about version PHPCompatibility 10
// to stop requiring to be configured here, but that's future version. Revisit this when
// available.
//
// Note: the paths are relative to the base CodeSniffer directory, aka, the directory
// where "src" sits.
$runner->config->setConfigData('installed_paths', './../PHPCompatibility');
$runner->config->standards = [$this->standard]; // Also BEFORE init() or it's lost.

$runner->init();

$runner->config->parallel = 1;
$runner->config->reports = ['full' => null];
$runner->config->colors = $output->isDecorated();
$runner->config->verbosity = 0;
$runner->config->encoding = 'utf-8';
$runner->config->showProgress = true;
$runner->config->showSources = true;
$runner->config->interactive = false;
$runner->config->cache = false;
$runner->config->extensions = ['php' => 'PHP'];
$runner->config->reportWidth = 132;

$runner->config->files = $files;

$runner->config->setConfigData('testVersion', PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION, true);

// Create the reporter to manage all the reports from the run.
$runner->reporter = new Reporter($runner->config);

// And build the file list to iterate over.
/** @var object[] $todo */
$todo = new \PHP_CodeSniffer\Files\FileList($runner->config, $runner->ruleset);
$numFiles = count($todo);
$numProcessed = 0;

foreach ($todo as $file) {
if ($file->ignored === false) {
try {
$runner->processFile($file);
++$numProcessed;
$runner->printProgress($file, $numFiles, $numProcessed);
} catch (\PHP_CodeSniffer\Exceptions\DeepExitException $e) {
echo $e->getMessage();

return $e->getCode();
} catch (\Exception $e) {
$error = 'Problem during processing; checking has been aborted. The error message was: '.$e->getMessage();
$file->addErrorOnLine($error, 1, 'Internal.Exception');
}
$file->cleanUp();
}
}

$sniffer = new \PHP_CodeSniffer();
$sniffer->setCli($cli);
$sniffer->process($files, $this->standard);
$results = $sniffer->reporting->printReport('full', false, $sniffer->cli->getCommandLineValues(), null, 120);
// Have finished, generate the final reports.
$runner->reporter->printReports();

$maxwarnings = (int) $input->getOption('max-warnings');

return ($results['errors'] > 0 || ($maxwarnings >= 0 && $results['warnings'] > $maxwarnings)) ? 1 : 0;
return ($runner->reporter->totalErrors > 0 || ($maxwarnings >= 0 && $runner->reporter->totalWarnings > $maxwarnings)) ? 1 : 0;
}
}
104 changes: 89 additions & 15 deletions src/Command/CodeFixerCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,22 @@

namespace MoodlePluginCI\Command;

use MoodlePluginCI\Bridge\CodeSnifferCLI;
use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Reporter;
use PHP_CodeSniffer\Runner;
use PHP_CodeSniffer\Util\Timing;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

// Cannot be autoload from composer, because this autoloader is used for both
// phpcs and any standard Sniff, so it must be loaded at the end. For more info, see:
// https://github.com/squizlabs/PHP_CodeSniffer/issues/1463#issuecomment-300637855
//
// The alternative is to, instead of using PHP_CodeSniffer like a library, just
// use the binaries bundled with it (phpcs, phpcbf...) but we want to use as lib for now.
require_once __DIR__.'/../../vendor/moodlehq/moodle-local_codechecker/phpcs/autoload.php';

/**
* Run PHP Code Beautifier and Fixer on a plugin.
*/
Expand All @@ -39,23 +50,86 @@ protected function execute(InputInterface $input, OutputInterface $output)
if (count($files) === 0) {
return $this->outputSkip($output, 'No files found to process.');
}
if (!defined('PHP_CODESNIFFER_CBF')) { // Can be defined in tests.

// Needed constant.
if (defined('PHP_CODESNIFFER_CBF') === false) {
define('PHP_CODESNIFFER_CBF', true);
}

$cli = new CodeSnifferCLI([
'reports' => ['cbf' => null],
'colors' => $output->isDecorated(),
'encoding' => 'utf-8',
'files' => $files,
'reportWidth' => 120,
'phpcbf-suffix' => '',
]);

$sniffer = new \PHP_CodeSniffer();
$sniffer->setCli($cli);
$sniffer->process($files, $this->standard);
$sniffer->reporting->printReport('cbf', false, $sniffer->cli->getCommandLineValues(), null, 120);
Timing::startTiming();

$runner = new Runner();
$runner->config = new Config(['--parallel=1']); // Pass a param to shortcut params coming from caller CLI.

// This is not needed normally, because phpcs loads the CodeSniffer.conf from its
// root directory. But when this is run from within a .phar file, it expects the
// config file to be out from the phar, in the same directory.
//
// While this approach is logic and enabled to configure phpcs, we don't want that
// in this case, we just want to ensure phpcs knows about our standards,
// so we are going to force the installed_paths config here.
//
// And it needs need to do it BEFORE the runner init! (or it's lost)
//
// Note: the "moodle" one is not really needed, because it's autodetected normally,
// but the PHPCompatibility one is. There are some issues about version PHPCompatibility 10
// to stop requiring to be configured here, but that's future version. Revisit this when
// available.
//
// Note: the paths are relative to the base CodeSniffer directory, aka, the directory
// where "src" sits.
$runner->config->setConfigData('installed_paths', './../PHPCompatibility');
$runner->config->standards = [$this->standard]; // Also BEFORE init() or it's lost.

$runner->init();

$runner->config->parallel = 1;
$runner->config->reports = ['cbf' => null];
$runner->config->colors = $output->isDecorated();
$runner->config->verbosity = 0;
$runner->config->encoding = 'utf-8';
$runner->config->showProgress = true;
$runner->config->showSources = true;
$runner->config->interactive = false;
$runner->config->cache = false;
$runner->config->extensions = ['php' => 'PHP'];
$runner->config->reportWidth = 132;

$runner->config->files = $files;

$runner->config->setConfigData('testVersion', PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION, true);

// Create the reporter to manage all the reports from the run.
$runner->reporter = new Reporter($runner->config);

// And build the file list to iterate over.
/** @var object[] $todo */
$todo = new \PHP_CodeSniffer\Files\FileList($runner->config, $runner->ruleset);
$numFiles = count($todo);
$numProcessed = 0;

foreach ($todo as $file) {
if ($file->ignored === false) {
try {
$runner->processFile($file);
++$numProcessed;
$runner->printProgress($file, $numFiles, $numProcessed);
} catch (\PHP_CodeSniffer\Exceptions\DeepExitException $e) {
echo $e->getMessage();

return $e->getCode();
} catch (\Exception $e) {
$error = 'Problem during processing; checking has been aborted. The error message was: '.$e->getMessage();
$file->addErrorOnLine($error, 1, 'Internal.Exception');
}
$file->cleanUp();
}
}

// Have finished, generate the final reports and timing.
$runner->reporter->printReports();
echo PHP_EOL;
Timing::printRunTime();

return 0;
}
Expand Down
Loading

0 comments on commit 9e577d2

Please sign in to comment.