diff --git a/moodle/Sniffs/Commenting/VariableCommentSniff.php b/moodle/Sniffs/Commenting/VariableCommentSniff.php new file mode 100644 index 0000000..ea9ea9d --- /dev/null +++ b/moodle/Sniffs/Commenting/VariableCommentSniff.php @@ -0,0 +1,304 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Sniffs\Commenting; + +use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Sniffs\AbstractVariableSniff; +use PHPCSUtils\Utils\ObjectDeclarations; + +/** + * Parses and verifies the variable doc comment. + * + * The Sniff is based upon the Squiz Labs version, but it has been modified to accept int, rather than integer. + * + * @author Greg Sherwood + * @author Andrew Lyons + * @copyright 2006-2015 Squiz Pty Ltd (ABN 77 084 670 600) + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ +class VariableCommentSniff extends AbstractVariableSniff +{ + /** + * An array of variable types for param/var we will check. + * + * @var string[] + */ + protected static $allowedTypes = [ + 'array', + 'bool', + 'float', + 'int', + 'mixed', + 'object', + 'string', + 'resource', + 'callable', + ]; + + protected static $ignoredTokens = [ + T_PUBLIC => T_PUBLIC, + T_PRIVATE => T_PRIVATE, + T_PROTECTED => T_PROTECTED, + T_VAR => T_VAR, + T_STATIC => T_STATIC, + T_READONLY => T_READONLY, + T_WHITESPACE => T_WHITESPACE, + T_STRING => T_STRING, + T_NS_SEPARATOR => T_NS_SEPARATOR, + T_NAMESPACE => T_NAMESPACE, + T_NULLABLE => T_NULLABLE, + T_TYPE_UNION => T_TYPE_UNION, + T_TYPE_INTERSECTION => T_TYPE_INTERSECTION, + T_NULL => T_NULL, + T_TRUE => T_TRUE, + T_FALSE => T_FALSE, + T_SELF => T_SELF, + T_PARENT => T_PARENT, + ]; + + /** + * Called to process class member vars. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * + * @return void + */ + public function processMemberVar(File $phpcsFile, $stackPtr) { + $tokens = $phpcsFile->getTokens(); + $ignore = self::$ignoredTokens; + + for ($commentEnd = ($stackPtr - 1); $commentEnd >= 0; $commentEnd--) { + if (isset($ignore[$tokens[$commentEnd]['code']]) === true) { + continue; + } + + if ( + $tokens[$commentEnd]['code'] === T_ATTRIBUTE_END + && isset($tokens[$commentEnd]['attribute_opener']) === true + ) { + $commentEnd = $tokens[$commentEnd]['attribute_opener']; + continue; + } + + break; + } + + if ( + $commentEnd === false + || ($tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG + && $tokens[$commentEnd]['code'] !== T_COMMENT) + ) { + $phpcsFile->addError('Missing member variable doc comment', $stackPtr, 'Missing'); + return; + } + + if ($tokens[$commentEnd]['code'] === T_COMMENT) { + $phpcsFile->addError('You must use "/**" style comments for a member variable comment', $stackPtr, 'WrongStyle'); + return; + } + + $commentStart = $tokens[$commentEnd]['comment_opener']; + + $foundVar = null; + foreach ($tokens[$commentStart]['comment_tags'] as $tag) { + if ($tokens[$tag]['content'] === '@var') { + if ($foundVar !== null) { + $error = 'Only one @var tag is allowed in a member variable comment'; + $phpcsFile->addError($error, $tag, 'DuplicateVar'); + } else { + $foundVar = $tag; + } + } elseif ($tokens[$tag]['content'] === '@see') { + // Make sure the tag isn't empty. + $string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $tag, $commentEnd); + if ($string === false || $tokens[$string]['line'] !== $tokens[$tag]['line']) { + $error = 'Content missing for @see tag in member variable comment'; + $phpcsFile->addError($error, $tag, 'EmptySees'); + } + } else { + $error = '%s tag is not allowed in member variable comment'; + $data = [$tokens[$tag]['content']]; + $phpcsFile->addWarning($error, $tag, 'TagNotAllowed', $data); + } + } + + // The @var tag is the only one we require. + if ($foundVar === null) { + $error = 'Missing @var tag in member variable comment'; + $phpcsFile->addError($error, $commentEnd, 'MissingVar'); + return; + } + + $firstTag = $tokens[$commentStart]['comment_tags'][0]; + if ($foundVar !== null && $tokens[$firstTag]['content'] !== '@var') { + $error = 'The @var tag must be the first tag in a member variable comment'; + $phpcsFile->addError($error, $foundVar, 'VarOrder'); + } + + // Make sure the tag isn't empty and has the correct padding. + $string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $foundVar, $commentEnd); + if ($string === false || $tokens[$string]['line'] !== $tokens[$foundVar]['line']) { + $error = 'Content missing for @var tag in member variable comment'; + $phpcsFile->addError($error, $foundVar, 'EmptyVar'); + return; + } + + // Support both a var type and a description. + preg_match('`^((?:\|?(?:array\([^\)]*\)|[\\\\a-z0-9\[\]]+))*)( .*)?`i', $tokens[($foundVar + 2)]['content'], $varParts); + $varType = $varParts[1]; + + // Check var type (can be multiple, separated by '|'). + $typeNames = explode('|', $varType); + $suggestedNames = []; + foreach ($typeNames as $i => $typeName) { + $suggestedName = self::suggestType($typeName); + if (in_array($suggestedName, $suggestedNames, true) === false) { + $suggestedNames[] = $suggestedName; + } + } + + $suggestedType = implode('|', $suggestedNames); + if ($varType !== $suggestedType) { + $error = 'Expected "%s" but found "%s" for @var tag in member variable comment'; + $data = [ + $suggestedType, + $varType, + ]; + + $fix = $phpcsFile->addFixableError($error, $foundVar, 'IncorrectVarType', $data); + if ($fix === true) { + $replacement = $suggestedType; + if (empty($varParts[2]) === false) { + $replacement .= $varParts[2]; + } + + $phpcsFile->fixer->replaceToken(($foundVar + 2), $replacement); + unset($replacement); + } + } + } + + /** + * Processes normal variables within a method. + * + * @param File $file The file where this token was found. + * @param int $stackptr The position where the token was found. + * + * @return void + */ + protected function processVariable(File $phpcsFile, $stackPtr) { + // Find the method that this variable is declared in. + $methodPtr = $phpcsFile->findPrevious(T_FUNCTION, $stackPtr); + if ($methodPtr === false) { + // Not in a method. + return; // @codeCoverageIgnore + } + + $methodName = ObjectDeclarations::getName($phpcsFile, $methodPtr); + if ($methodName !== '__construct') { + // Not in a constructor. + return; + } + + $method = $phpcsFile->getTokens()[$methodPtr]; + if ($method['parenthesis_opener'] < $stackPtr && $method['parenthesis_closer'] > $stackPtr) { + $this->processMemberVar($phpcsFile, $stackPtr); + return; + } + } + + /** + * Returns a valid variable type for param/var tags. + * + * If type is not one of the standard types, it must be a custom type. + * Returns the correct type name suggestion if type name is invalid. + * + * @param string $varType The variable type to process. + * + * @return string + */ + protected static function suggestType(string $varType): string { + if (in_array($varType, self::$allowedTypes, true) === true) { + return $varType; + } elseif (substr($varType, -2) === '[]') { + return sprintf( + '%s[]', + self::suggestType(substr($varType, 0, -2)) + ); + } else { + $lowerVarType = strtolower($varType); + switch ($lowerVarType) { + case 'bool': + case 'boolean': + return 'bool'; + case 'double': + case 'real': + case 'float': + return 'float'; + case 'int': + case 'integer': + return 'int'; + case 'array()': + case 'array': + return 'array'; + } + + if (strpos($lowerVarType, 'array(') !== false) { + // Valid array declaration: + // array, array(type), array(type1 => type2). + $matches = []; + $pattern = '/^array\(\s*([^\s^=^>]*)(\s*=>\s*(.*))?\s*\)/i'; + if (preg_match($pattern, $varType, $matches) !== 0) { + $type1 = ''; + if (isset($matches[1]) === true) { + $type1 = $matches[1]; + } + + $type2 = ''; + if (isset($matches[3]) === true) { + $type2 = $matches[3]; + } + + $type1 = self::suggestType($type1); + $type2 = self::suggestType($type2); + if ($type2 !== '') { + $type2 = ' => ' . $type2; + } + + return "array($type1$type2)"; + } else { + return 'array'; + } + } elseif (in_array($lowerVarType, self::$allowedTypes, true) === true) { + // A valid type, but not lower cased. + return $lowerVarType; + } else { + // Must be a custom type name. + return $varType; + } + } + } + + /** + * @codeCoverageIgnore + */ + protected function processVariableInString(File $phpcsFile, $stackPtr) { + } +} diff --git a/moodle/Tests/Sniffs/Commenting/VariableCommentSniffTest.php b/moodle/Tests/Sniffs/Commenting/VariableCommentSniffTest.php new file mode 100644 index 0000000..480ae04 --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/VariableCommentSniffTest.php @@ -0,0 +1,125 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\Commenting; + +use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase; + +/** + * Test the VariableCommentSniff sniff. + * + * @copyright 2024 onwards Andrew Lyons + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\Commenting\VariableCommentSniff + */ +class VariableCommentSniffTest extends MoodleCSBaseTestCase +{ + /** + * @dataProvider fixtureProvider + */ + public function testSniffWithFixtures( + string $fixture, + array $errors, + array $warnings + ): void { + $this->setStandard('moodle'); + $this->setSniff('moodle.Commenting.VariableComment'); + $this->setFixture(sprintf("%s/fixtures/VariableComment/%s.php", __DIR__, $fixture)); + $this->setErrors($errors); + $this->setWarnings($warnings); + + $this->verifyCsResults(); + } + + public static function fixtureProvider(): array { + $cases = [ + 'Multiline docblocks' => [ + 'fixture' => 'multiline', + 'errors' => [ + 23 => 'Expected "int" but found "integer" for @var tag in member variable comment', + 31 => 'Only one @var tag is allowed in a member variable comment', + 51 => 'You must use "/**" style comments for a member variable comment', + 53 => 'Missing member variable doc comment', + 67 => 'Content missing for @see tag in member variable comment', + 75 => 'Missing @var tag in member variable comment', + 90 => 'The @var tag must be the first tag in a member variable comment', + ], + 'warnings' => [ + 82 => '@deprecated tag is not allowed in member variable comment', + ], + ], + 'Single line variable declarations' => [ + 'fixture' => 'singleline', + 'errors' => [ + 12 => 'Expected "int" but found "integer" for @var tag in member variable comment', + 22 => 'Missing @var tag in member variable comment', + 25 => 'Missing member variable doc comment', + 36 => 'Expected "int" but found "INT" for @var tag in member variable comment', + 39 => 'Content missing for @var tag in member variable comment', + ], + 'warnings' => [], + ], + 'Type corrections' => [ + 'fixture' => 'typechecks', + 'errors' => [ + 6 => 'Expected "int" but found "INT" for @var tag in member variable comment', + 9 => 'Expected "string" but found "String" for @var tag in member variable comment', + 12 => 'Expected "float" but found "double" for @var tag in member variable comment', + 15 => 'Expected "float" but found "real" for @var tag in member variable comment', + 21 => 'Expected "array" but found "ARRAY()" for @var tag in member variable comment', + 27 => 'Expected "bool" but found "boolean" for @var tag in member variable comment', + 30 => 'Expected "bool[]" but found "boolean[]" for @var tag in member variable comment', + 36 => 'Expected "array" but found "array(int > bool)" for @var tag in member variable comment', + ], + 'warnings' => [], + ], + ]; + + if (version_compare(PHP_VERSION, '8.0.0') >= 0) { + // Since PHP 8.0. + $cases['Constructor Property Promotions'] = [ + 'fixture' => 'constructor_property_promotion', + 'errors' => [ + 7 => 'Missing member variable doc comment', + 13 => 'Expected "bool" but found "BOOLEAN" for @var tag in member variable comment', + 20 => 'The @var tag must be the first tag in a member variable commen', + ], + 'warnings' => [ + 19 => '@deprecated tag is not allowed in member variable comment', + ], + ]; + } + + if (version_compare(PHP_VERSION, '8.1.0') >= 0) { + // Since PHP 8.1. + $cases['Constructor Property Promotions with Readonly params'] = [ + 'fixture' => 'constructor_property_promotion_readonly', + 'errors' => [ + 7 => 'Missing member variable doc comment', + 13 => 'Expected "bool" but found "BOOLEAN" for @var tag in member variable comment', + 20 => 'The @var tag must be the first tag in a member variable commen', + ], + 'warnings' => [ + 19 => '@deprecated tag is not allowed in member variable comment', + ], + ]; + } + + return $cases; + } +} diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion.php b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion.php new file mode 100644 index 0000000..a101d8e --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion.php @@ -0,0 +1,36 @@ +attribute = 'example'; + } + + public function someStandardMethod( + string $example, + bool $otherExample, + \moodle_page $page, + ): void { + } +} diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion.php.fixed b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion.php.fixed new file mode 100644 index 0000000..621048e --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion.php.fixed @@ -0,0 +1,36 @@ +attribute = 'example'; + } + + public function someStandardMethod( + string $example, + bool $otherExample, + \moodle_page $page, + ): void { + } +} diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion_readonly.php b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion_readonly.php new file mode 100644 index 0000000..38888b4 --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_property_promotion_readonly.php @@ -0,0 +1,28 @@ + bool) Mapping of ints to bools */ + protected array $arrayofthings; + + /** @var array(int > bool) List of ints */ + protected array $arrayofints; + +} diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/typechecks.php.fixed b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/typechecks.php.fixed new file mode 100644 index 0000000..33e0f80 --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/typechecks.php.fixed @@ -0,0 +1,39 @@ + bool) Mapping of ints to bools */ + protected array $arrayofthings; + + /** @var array List of ints */ + protected array $arrayofints; + +}