diff --git a/CHANGELOG.md b/CHANGELOG.md index 73754e9..82cd447 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt ## [Unreleased] ### Added - The existing `moodle.PHPUnit.TestCaseCovers` sniff now detects multiple uses of the `@coversDefaultClass` annotation. Only one is allowed by class. +- The existing `moodle.Files.BoilerplateComment` sniff now performs more checks (spacing, placement, blank lines, ...) and is able to fix many of them. ### Changed - Made codes for `moodle.Commenting.MissingDocblock` more specific to the scenario (Fixes #154). diff --git a/moodle/Sniffs/Files/BoilerplateCommentSniff.php b/moodle/Sniffs/Files/BoilerplateCommentSniff.php index 57efe20..eb5f987 100644 --- a/moodle/Sniffs/Files/BoilerplateCommentSniff.php +++ b/moodle/Sniffs/Files/BoilerplateCommentSniff.php @@ -30,7 +30,7 @@ class BoilerplateCommentSniff implements Sniff { - protected static $comment = [ + protected static array $comment = [ "// This file is part of", "//", "// Moodle is free software: you can redistribute it and/or modify", @@ -44,80 +44,231 @@ class BoilerplateCommentSniff implements Sniff "// GNU General Public License for more details.", "//", "// You should have received a copy of the GNU General Public License", - "// along with Moodle. If not, see .", + "// along with Moodle. If not, see .", ]; - public function register() { + + public string $productName = 'Moodle'; + + public string $firstLinePostfix = ' - https://moodle.org/'; + + public function register(): array + { return [T_OPEN_TAG]; } - public function process(File $file, $stackptr) { + public function process(File $phpcsFile, $stackPtr): void + { // We only want to do this once per file. - $prevopentag = $file->findPrevious(T_OPEN_TAG, $stackptr - 1); + $prevopentag = $phpcsFile->findPrevious(T_OPEN_TAG, $stackPtr - 1); if ($prevopentag !== false) { return; // @codeCoverageIgnore } - if ($stackptr > 0) { - $file->addError('The first thing in a PHP file must be the 0) { + $phpcsFile->addError('The first thing in a PHP file must be the getTokens(); + $tokens = $phpcsFile->getTokens(); // Allow T_PHPCS_XXX comment annotations in the first line (skip them). - if ($commentptr = $file->findNext(Tokens::$phpcsCommentTokens, $stackptr + 1, $stackptr + 3)) { - $stackptr = $commentptr; + if ($commentptr = $phpcsFile->findNext(Tokens::$phpcsCommentTokens, $stackPtr + 1, $stackPtr + 3)) { + $stackPtr = $commentptr; } - // Find count the number of newlines after the opening findNext(T_COMMENT, $expectedafter + 1); + + // Check that it appears to be a Moodle boilerplate comment. + $regex = $this->regexForLine(self::$comment[0]); + $boilerplatefound = ($firstcommentptr !== false) && preg_match($regex, $tokens[$firstcommentptr]['content']); - if ($numnewlines > 0) { - $file->addError( - 'The opening addFixableError( + 'Moodle boilerplate not found', + $stackPtr, + 'NoBoilerplateComment' ); + + if ($fix) { + $this->insertBoilerplate($phpcsFile, $expectedafter); + } return; } - $offset = $stackptr + $numnewlines + 1; // Now check the text of the comment. + $textfixed = false; + $tokenptr = $firstcommentptr; foreach (self::$comment as $lineindex => $line) { - $tokenptr = $offset + $lineindex; + // We already checked the first line. + if ($lineindex === 0) { + continue; + } + + $tokenptr = $firstcommentptr + $lineindex; + $iseof = $tokenptr >= $phpcsFile->numTokens; + + if ($iseof || $tokens[$tokenptr]['code'] != T_COMMENT || strpos($tokens[$tokenptr]['content'], '//') !== 0) { + $errorline = $iseof ? $tokenptr - 1 : $tokenptr; + + $fix = $phpcsFile->addFixableError( + 'Comment does not contain full Moodle boilerplate', + $errorline, + 'CommentEndedTooSoon' + ); + + if ($fix) { + $this->completeBoilerplate($phpcsFile, $tokenptr - 1, $lineindex); + return; + } - if (!array_key_exists($tokenptr, $tokens)) { - $file->addError('Reached the end of the file before finding ' . - 'all of the opening comment.', $tokenptr - 1, 'FileTooShort'); + // No point checking whitespace after comment if it is incomplete. return; } - $regex = str_replace( - ['Moodle', 'http\\:'], - ['.*', 'https?\\:'], - '/^' . preg_quote($line, '/') . '/' - ); + $regex = $this->regexForLine($line); - if ( - $tokens[$tokenptr]['code'] != T_COMMENT || - !preg_match($regex, $tokens[$tokenptr]['content']) - ) { - $file->addError( + if (!preg_match($regex, $tokens[$tokenptr]['content'])) { + $fix = $phpcsFile->addFixableError( 'Line %s of the opening comment must start "%s".', $tokenptr, 'WrongLine', [$lineindex + 1, $line] ); + + if ($fix) { + $phpcsFile->fixer->replaceToken($tokenptr, $line . "\n"); + $textfixed = true; + } + } + } + + if ($firstcommentptr !== $expectedafter + 1) { + $fix = $phpcsFile->addFixableError( + 'Moodle boilerplate not found at first line', + $expectedafter + 1, + 'NotAtFirstLine' + ); + + // If the boilerplate comment has been changed we need to commit the fixes before + // moving it. + if ($fix && !$textfixed) { + $this->moveBoilerplate($phpcsFile, $firstcommentptr, $expectedafter); } + + // There's no point in checking the whitespace after the boilerplate + // if it's not in the right place. + return; + } + + if ($tokenptr === $phpcsFile->numTokens - 1) { + return; + } + + $tokenptr++; + + $nextnonwhitespace = $phpcsFile->findNext(T_WHITESPACE, $tokenptr, null, true); + + // Allow indentation. + if ($nextnonwhitespace !== false && strpos($tokens[$nextnonwhitespace - 1]['content'], "\n") === false) { + $nextnonwhitespace--; } + + if ( + ($nextnonwhitespace === false) && array_key_exists($tokenptr + 1, $tokens) || + ($nextnonwhitespace !== false && $nextnonwhitespace !== $tokenptr + 1) + ) { + $fix = $phpcsFile->addFixableError( + 'Boilerplate comment must be followed by a single blank line or end of file', + $tokenptr, + 'SingleTrailingNewLine' + ); + + if ($fix) { + if ($nextnonwhitespace === false) { + while (array_key_exists(++$tokenptr, $tokens)) { + $phpcsFile->fixer->replaceToken($tokenptr, ''); + } + } elseif ($nextnonwhitespace === $tokenptr) { + $phpcsFile->fixer->addContentBefore($tokenptr, "\n"); + } else { + while (++$tokenptr < $nextnonwhitespace) { + if ($tokens[$tokenptr]['content'][-1] === "\n") { + $phpcsFile->fixer->replaceToken($tokenptr, ''); + } + } + } + } + } + } + + private function fullComment(): array + { + $result = []; + foreach (self::$comment as $lineindex => $line) { + if ($lineindex === 0) { + $result[] = $line . ' ' . $this->productName . $this->firstLinePostfix; + } else { + $result[] = str_replace('Moodle', $this->productName, $line); + } + } + return $result; + } + + private function insertBoilerplate(File $file, int $stackptr): void + { + $prefix = substr($file->getTokens()[$stackptr]['content'], -1) === "\n" ? '' : "\n"; + $file->fixer->addContent($stackptr, $prefix . implode("\n", $this->fullComment()) . "\n"); + } + + private function moveBoilerplate(File $file, int $start, int $target): void + { + $tokens = $file->getTokens(); + + $file->fixer->beginChangeset(); + + // If we have only whitespace between expected location and first comment, just remove it. + $nextnonwhitespace = $file->findPrevious(T_WHITESPACE, $start - 1, $target, true); + + if ($nextnonwhitespace === false || $nextnonwhitespace === $target) { + foreach (range($target + 1, $start - 1) as $whitespaceptr) { + $file->fixer->replaceToken($whitespaceptr, ''); + } + $file->fixer->endChangeset(); + return; + } + + // Otherwise shift existing comment to correct place. + $existingboilerplate = []; + foreach (range(0, count(self::$comment)) as $lineindex) { + $tokenptr = $start + $lineindex; + + $existingboilerplate[] = $tokens[$tokenptr]['content']; + + $file->fixer->replaceToken($tokenptr, ''); + } + + $file->fixer->addContent($target, implode("", $existingboilerplate) . "\n"); + + $file->fixer->endChangeset(); + } + + private function completeBoilerplate(File $file, $stackptr, int $lineindex): void + { + $file->fixer->addContent($stackptr, implode("\n", array_slice($this->fullComment(), $lineindex)) . "\n"); + } + + /** + * @param string $line + * @return string + */ + private function regexForLine(string $line): string + { + return str_replace( + ['Moodle', 'https\\:'], + ['.*', 'https?\\:'], + '/^' . preg_quote($line, '/') . '/' + ); } } diff --git a/moodle/Tests/FilesBoilerPlateCommentTest.php b/moodle/Tests/FilesBoilerPlateCommentTest.php index b9fd94b..7405124 100644 --- a/moodle/Tests/FilesBoilerPlateCommentTest.php +++ b/moodle/Tests/FilesBoilerPlateCommentTest.php @@ -69,7 +69,7 @@ public function testMoodleFilesBoilerplateCommentBlank() { $this->setFixture(__DIR__ . '/fixtures/files/boilerplatecomment/blank.php'); $this->setErrors([ - 2 => 'followed by exactly one newline', + 2 => 'not found at first line', ]); $this->setWarnings([]); @@ -82,7 +82,7 @@ public function testMoodleFilesBoilerplateCommentShort() { $this->setFixture(__DIR__ . '/fixtures/files/boilerplatecomment/short.php'); $this->setErrors([ - 14 => 'FileTooShort', + 14 => 'CommentEndedTooSoon', ]); $this->setWarnings([]); @@ -95,7 +95,20 @@ public function testMoodleFilesBoilerplateCommentShortEmpty() { $this->setFixture(__DIR__ . '/fixtures/files/boilerplatecomment/short_empty.php'); $this->setErrors([ - 1 => 'FileTooShort', + 1 => 'NoBoilerplateComment', + ]); + $this->setWarnings([]); + + $this->verifyCsResults(); + } + + public function testMoodleFilesBoilerplateCommentShortNotEof() { + $this->setStandard('moodle'); + $this->setSniff('moodle.Files.BoilerplateComment'); + $this->setFixture(__DIR__ . '/fixtures/files/boilerplatecomment/short_not_eof.php'); + + $this->setErrors([ + 15 => 'CommentEndedTooSoon', ]); $this->setWarnings([]); @@ -142,4 +155,53 @@ public function testMoodleFilesBoilerplateCommentGnuHttps() { $this->verifyCsResults(); } + + /** + * Assert that boilerplate is found if it is not the first thing in the file. + */ + public function testMoodleFilesBoilerplateCommentWrongPlace() { + $this->setStandard('moodle'); + $this->setSniff('moodle.Files.BoilerplateComment'); + $this->setFixture(__DIR__ . '/fixtures/files/boilerplatecomment/wrong_place.php'); + + $this->setErrors([ + 2 => 'not found at first line', + 9 => 'either version 3 of the License', + ]); + $this->setWarnings([]); + + $this->verifyCsResults(); + } + + /** + * Assert that boilerplate is followed by a single newline (detect and remove excessive) + */ + public function testMoodleFilesBoilerplateCommentTrailingWhitespace() { + $this->setStandard('moodle'); + $this->setSniff('moodle.Files.BoilerplateComment'); + $this->setFixture(__DIR__ . '/fixtures/files/boilerplatecomment/trailing_whitespace.php'); + + $this->setErrors([ + 16 => 'SingleTrailingNewLine', + ]); + $this->setWarnings([]); + + $this->verifyCsResults(); + } + + /** + * Assert that boilerplate is followed by a single newline (detect and fix missing) + */ + public function testMoodleFilesBoilerplateCommentTrailingWhitespaceMissing() { + $this->setStandard('moodle'); + $this->setSniff('moodle.Files.BoilerplateComment'); + $this->setFixture(__DIR__ . '/fixtures/files/boilerplatecomment/trailing_whitespace_missing.php'); + + $this->setErrors([ + 16 => 'SingleTrailingNewLine', + ]); + $this->setWarnings([]); + + $this->verifyCsResults(); + } } diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/blank.php.fixed b/moodle/Tests/fixtures/files/boilerplatecomment/blank.php.fixed new file mode 100644 index 0000000..925b3cc --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/blank.php.fixed @@ -0,0 +1,17 @@ +. + +class someclass { } diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/short.php.fixed b/moodle/Tests/fixtures/files/boilerplatecomment/short.php.fixed new file mode 100644 index 0000000..a03b015 --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/short.php.fixed @@ -0,0 +1,15 @@ +. diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/short_empty.php.fixed b/moodle/Tests/fixtures/files/boilerplatecomment/short_empty.php.fixed new file mode 100644 index 0000000..5abeeab --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/short_empty.php.fixed @@ -0,0 +1,15 @@ +. diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/short_not_eof.php b/moodle/Tests/fixtures/files/boilerplatecomment/short_not_eof.php new file mode 100644 index 0000000..1ebf251 --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/short_not_eof.php @@ -0,0 +1,17 @@ +. + +class some_class { +} diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace.php b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace.php new file mode 100644 index 0000000..bcf5873 --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace.php @@ -0,0 +1,17 @@ +. + + diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace.php.fixed b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace.php.fixed new file mode 100644 index 0000000..a7b697d --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace.php.fixed @@ -0,0 +1,16 @@ +. + diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace_missing.php b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace_missing.php new file mode 100644 index 0000000..610f486 --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace_missing.php @@ -0,0 +1,18 @@ +. + // It's not important that these lines are wrongly indented, Here + // we are only confirming that the Boilerplate Sniff fixed respects it. + class someclass { } diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace_missing.php.fixed b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace_missing.php.fixed new file mode 100644 index 0000000..220779f --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/trailing_whitespace_missing.php.fixed @@ -0,0 +1,19 @@ +. + + // It's not important that these lines are wrongly indented, Here + // we are only confirming that the Boilerplate Sniff fixed respects it. + class someclass { } diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/wrong_place.php b/moodle/Tests/fixtures/files/boilerplatecomment/wrong_place.php new file mode 100644 index 0000000..ad4a777 --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/wrong_place.php @@ -0,0 +1,20 @@ +. + +class someclass { } diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/wrong_place.php.fixed b/moodle/Tests/fixtures/files/boilerplatecomment/wrong_place.php.fixed new file mode 100644 index 0000000..9cbbe97 --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/wrong_place.php.fixed @@ -0,0 +1,19 @@ +. + +use core\context\system; + +class someclass { } diff --git a/moodle/Tests/fixtures/files/boilerplatecomment/wrongline.php.fixed b/moodle/Tests/fixtures/files/boilerplatecomment/wrongline.php.fixed new file mode 100644 index 0000000..47da489 --- /dev/null +++ b/moodle/Tests/fixtures/files/boilerplatecomment/wrongline.php.fixed @@ -0,0 +1,17 @@ +. + +class someclass { }