diff --git a/moodle/Sniffs/Files/LineLengthSniff.php b/moodle/Sniffs/Files/LineLengthSniff.php index f12e8c9..9e0e607 100644 --- a/moodle/Sniffs/Files/LineLengthSniff.php +++ b/moodle/Sniffs/Files/LineLengthSniff.php @@ -24,8 +24,8 @@ namespace MoodleHQ\MoodleCS\moodle\Sniffs\Files; +use MoodleHQ\MoodleCS\moodle\Util\Docblocks; use PHP_CodeSniffer\Standards\Generic\Sniffs\Files\LineLengthSniff as GenericLineLengthSniff; -use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Files\File; class LineLengthSniff extends GenericLineLengthSniff @@ -40,6 +40,17 @@ public function process(File $file, $stackptr) { if (strpos($file->getFilename(), DIRECTORY_SEPARATOR . 'lang' . DIRECTORY_SEPARATOR) !== false) { return; } + parent::process($file, $stackptr); } + + protected function checkLineLength($phpcsFile, $tokens, $stackPtr) { + // Ignore lines that are part of a docblock. + // We may extend this to only ignore certain tags in the future. + if (Docblocks::getStartOfCurrentDocblock($phpcsFile, $stackPtr) !== null) { + return; + } + + parent::checkLineLength($phpcsFile, $tokens, $stackPtr); + } } diff --git a/moodle/Tests/Sniffs/Files/LineLengthSniffTest.php b/moodle/Tests/Sniffs/Files/LineLengthSniffTest.php new file mode 100644 index 0000000..2cecbbe --- /dev/null +++ b/moodle/Tests/Sniffs/Files/LineLengthSniffTest.php @@ -0,0 +1,75 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\Commenting; + +use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase; + +/** + * Test the MissingDocblockSniff sniff. + * + * @copyright 2024 onwards Andrew Lyons + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\Files\LineLengthSniff + */ +class LineLengthSniffTest extends MoodleCSBaseTestCase +{ + /** + * @dataProvider fixtureProvider + */ + public function testSniffWithFixtures( + string $fixture, + ?string $fixturePath, + array $errors, + array $warnings + ): void { + // xdebug_break(); + $this->setStandard('moodle'); + $this->setSniff('moodle.Files.LineLength'); + $this->setFixture( + sprintf("%s/fixtures/LineLength/%s.php", __DIR__, $fixture), + $fixturePath, + ); + $this->setErrors($errors); + $this->setWarnings($warnings); + + $this->verifyCsResults(); + } + + public static function fixtureProvider(): array { + $cases = [ + [ + 'fixture' => 'langfile', + 'fixturePath' => '/lang/en/assignfeedback_editpdf.php', + 'errors' => [], + 'warnings' => [], + ], + [ + 'fixture' => 'standard', + 'fixturePath' => null, + 'errors' => [ + 13 => 'Line exceeds maximum limit of 180 characters; contains 182 characters', + ], + 'warnings' => [ + + ], + ], + ]; + return $cases; + } +} diff --git a/moodle/Tests/Sniffs/Files/fixtures/LineLength/langfile.php b/moodle/Tests/Sniffs/Files/fixtures/LineLength/langfile.php new file mode 100644 index 0000000..be2a149 --- /dev/null +++ b/moodle/Tests/Sniffs/Files/fixtures/LineLength/langfile.php @@ -0,0 +1,3 @@ +(?:[^"]|\\")*)" activity name to "(?P(?:[^"]|\\")*)"$/ + */ + public function i_change_names(): void { + // This is also a really stupidly long comment but this one is not allowed to be over long. The reason we accept long docblock strings but not long comments string is because + // docblocks are used as code. + } +} diff --git a/moodle/Util/Docblocks.php b/moodle/Util/Docblocks.php index 2dea703..b18de54 100644 --- a/moodle/Util/Docblocks.php +++ b/moodle/Util/Docblocks.php @@ -144,6 +144,15 @@ abstract class Docblocks // Commented out: 'uses' => ['#.*/tests/.*_test.php#'], can also be out from tests (Coding style dixit). ]; + /** @var int[]|string[] A list of tokens which are within a docblock */ + protected static array $midDocBlockTokens = [ + T_DOC_COMMENT, + T_DOC_COMMENT_STAR, + T_DOC_COMMENT_WHITESPACE, + T_DOC_COMMENT_TAG, + T_DOC_COMMENT_STRING, + ]; + /** * Get the docblock for a file, class, interface, trait, or method. * @@ -164,38 +173,60 @@ public static function getDocBlock( } /** - * Get the docblock pointer for a file, class, interface, trait, or method. + * Get the start pointer for a token which is in a docblock. * * @param File $phpcsFile - * @param int $stackPtr - * @return null|int + * @param int $stackPtr The token to check + * @return null|int The docblock start token if the stackPtr is in a docblock, null otherwise. */ - public static function getDocBlockPointer( + public static function getStartOfCurrentDocblock( File $phpcsFile, int $stackPtr ): ?int { $tokens = $phpcsFile->getTokens(); + if (!array_key_exists($stackPtr, $tokens)) { + // This _should_ not happen unless it is called incorrectly, like in the LineLength sniff. + return null; // @codeCoverageIgnore + } $token = $tokens[$stackPtr]; - // Check if the passed pointer was for a doc. - $midDocBlockTokens = [ - T_DOC_COMMENT, - T_DOC_COMMENT_STAR, - T_DOC_COMMENT_WHITESPACE, - T_DOC_COMMENT_TAG, - T_DOC_COMMENT_STRING, - ]; if ($token['code'] === T_DOC_COMMENT_OPEN_TAG) { return $stackPtr; - } elseif ($token['code'] === T_DOC_COMMENT_CLOSE_TAG) { + } + + if ($token['code'] === T_DOC_COMMENT_CLOSE_TAG) { // The pointer was for a close tag. Fetch the corresponding open tag. return $token['comment_opener']; - } elseif (in_array($token['code'], $midDocBlockTokens)) { + } + + if (in_array($token['code'], self::$midDocBlockTokens)) { // The pointer was for a token inside the docblock. Fetch the corresponding open tag. $commentStart = $phpcsFile->findPrevious(T_DOC_COMMENT_OPEN_TAG, $stackPtr); return $commentStart ?: null; } + return null; + } + + /** + * Get the docblock pointer for a file, class, interface, trait, or method. + * + * @param File $phpcsFile + * @param int $stackPtr + * @return null|int + */ + public static function getDocBlockPointer( + File $phpcsFile, + int $stackPtr + ): ?int { + $tokens = $phpcsFile->getTokens(); + $token = $tokens[$stackPtr]; + + // Check if the passed pointer was for a doc. + if ($docblockPtr = self::getStartOfCurrentDocblock($phpcsFile, $stackPtr)) { + return $docblockPtr; + } + // If the pointer was for a file, fetch the doc tag from the open tag. if ($tokens[$stackPtr]['code'] === T_OPEN_TAG) { return self::getDocTagFromOpenTag($phpcsFile, $stackPtr);