From e7506b1c65e3bb46c5ccb94628ee0b29a7b78187 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 31 Mar 2024 21:58:03 +0800 Subject: [PATCH] Properties should not be tested for regular constructors (Fixes #142) Only properties which define a visibility and therefore are promoted constructor properties are required to define a variable docblock. --- .../Commenting/VariableCommentSniff.php | 19 +++++++++++++++ .../Commenting/VariableCommentSniffTest.php | 8 +++++++ ...structor_with_mixed_property_promotion.php | 24 +++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_with_mixed_property_promotion.php diff --git a/moodle/Sniffs/Commenting/VariableCommentSniff.php b/moodle/Sniffs/Commenting/VariableCommentSniff.php index 4f91743..fc05cf6 100644 --- a/moodle/Sniffs/Commenting/VariableCommentSniff.php +++ b/moodle/Sniffs/Commenting/VariableCommentSniff.php @@ -179,6 +179,25 @@ protected function processVariable(File $phpcsFile, $stackPtr) { $method = $phpcsFile->getTokens()[$methodPtr]; if ($method['parenthesis_opener'] < $stackPtr && $method['parenthesis_closer'] > $stackPtr) { + // Only apply to properties declared in the constructor. + // Constructor Promoted Properties canbe detected by a visbility keyword. + // These can be found, amongst others like READONLY in Collections::propertyModifierKeywords(). + // When searching, only look back to the previous arg (comma), or the opening parenthesis. + $lookBackTo = max( + $method['parenthesis_opener'], + $phpcsFile->findPrevious(T_COMMA, $stackPtr) + ); + $modifierPtr = $phpcsFile->findPrevious( + Collections::propertyModifierKeywords(), + $stackPtr, + $lookBackTo + ); + if ($modifierPtr === false) { + // No modifier found, so not a promoted property. + return; + } + + // This is a promoted property. Handle it in the same way as other properties. $this->processMemberVar($phpcsFile, $stackPtr); return; } diff --git a/moodle/Tests/Sniffs/Commenting/VariableCommentSniffTest.php b/moodle/Tests/Sniffs/Commenting/VariableCommentSniffTest.php index ad87824..8c479fd 100644 --- a/moodle/Tests/Sniffs/Commenting/VariableCommentSniffTest.php +++ b/moodle/Tests/Sniffs/Commenting/VariableCommentSniffTest.php @@ -90,6 +90,14 @@ public static function fixtureProvider(): array { ], 'warnings' => [], ], + 'Constructor with mixed CPP' => [ + 'fixture' => 'constructor_with_mixed_property_promotion', + 'errors' => [ + 21 => 'Missing member variable doc comment', + ], + 'warnings' => [ + ], + ], ]; if (version_compare(PHP_VERSION, '8.0.0') >= 0) { diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_with_mixed_property_promotion.php b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_with_mixed_property_promotion.php new file mode 100644 index 0000000..9495e94 --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/fixtures/VariableComment/constructor_with_mixed_property_promotion.php @@ -0,0 +1,24 @@ +