added RequireOverrideAttributeSniff that checks existence of #[Override] attribute for overriden methods (#3858)

Author: TomasLudvik

ecs.php

@@ -104,6 +104,7 @@
 use Shopsys\CodingStandards\Sniffs\ForbiddenExitSniff;
 use Shopsys\CodingStandards\Sniffs\ForbiddenSuperGlobalSniff;
 use Shopsys\CodingStandards\Sniffs\ObjectIsCreatedByFactorySniff;
+use Shopsys\CodingStandards\Sniffs\RequireOverrideAttributeSniff;
 use Shopsys\CodingStandards\Sniffs\ValidVariableNameSniff;
 use SlevomatCodingStandard\Sniffs\Arrays\TrailingArrayCommaSniff;
 use SlevomatCodingStandard\Sniffs\Classes\ClassLengthSniff;
@@ -146,11 +147,6 @@
         SetList::DOCBLOCK,
         SetList::NAMESPACES,
     ])
-    ->withRules([
-        InlineDocCommentDeclarationSniff::class,
-        NullableTypeForNullDefaultValueSniff::class,
-        ReturnTypeHintSpacingSniff::class,
-    ])
     ->withConfiguredRule(
         ForbiddenClassesSniff::class, [
         'forbiddenClasses' => [
@@ -159,6 +155,9 @@
         ],
     ])
     ->withRules([
+        InlineDocCommentDeclarationSniff::class,
+        NullableTypeForNullDefaultValueSniff::class,
+        ReturnTypeHintSpacingSniff::class,
         InheritDocFormatFixer::class,
         ForbiddenDumpFixer::class,
         MissingButtonTypeFixer::class,
@@ -259,6 +258,7 @@
         DisallowEmptySniff::class,
         ParentCallSpacingSniff::class,
         UselessIfConditionWithReturnSniff::class,
+        RequireOverrideAttributeSniff::class,
     ])
     ->withConfiguredRule(CyclomaticComplexitySniff::class, [
         'absoluteComplexity' => CyclomaticComplexitySniffSetting::DEFAULT_ABSOLUTE_COMPLEXITY,

src/Sniffs/RequireOverrideAttributeSniff.php

@@ -0,0 +1,202 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shopsys\CodingStandards\Sniffs;
+
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Sniffs\Sniff;
+use ReflectionClass;
+use ReflectionException;
+
+class RequireOverrideAttributeSniff implements Sniff
+{
+    /**
+     * @return array
+     */
+    public function register(): array
+    {
+        return [T_FUNCTION];
+    }
+
+    /**
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile
+     * @param mixed $stackPtr
+     */
+    public function process(File $phpcsFile, $stackPtr): void
+    {
+        $tokens = $phpcsFile->getTokens();
+
+        $classPtr = $phpcsFile->findPrevious([T_CLASS, T_TRAIT], $stackPtr);
+
+        if ($classPtr === false) {
+            return;
+        }
+
+        $classNamePtr = $phpcsFile->findNext(T_STRING, $classPtr);
+
+        if ($classNamePtr === false) {
+            return;
+        }
+        $className = $tokens[$classNamePtr]['content'];
+
+        $methodNamePtr = $phpcsFile->findNext(T_STRING, $stackPtr);
+
+        if ($methodNamePtr === false) {
+            return;
+        }
+        $methodName = $tokens[$methodNamePtr]['content'];
+
+        if ($methodName === '__construct') {
+            return;
+        }
+
+        $parentClassPtr = $phpcsFile->findNext(T_EXTENDS, $classNamePtr);
+
+        if ($parentClassPtr === false) {
+            return;
+        }
+
+        $parentClassNamePtr = $phpcsFile->findNext(T_STRING, $parentClassPtr);
+
+        if ($parentClassNamePtr === false) {
+            return;
+        }
+
+        $parentClassName = $tokens[$parentClassNamePtr]['content'];
+        $parentClassFqn = $this->getFqnOfClass($phpcsFile, $tokens, $parentClassName);
+
+        if ($parentClassFqn === '') {
+            return;
+        }
+
+
+        try {
+            $parentClassReflection = new ReflectionClass($parentClassFqn);
+
+            $hasMethod = $this->hasParentClassMethod($parentClassReflection, $methodName);
+        } catch (ReflectionException $e) {
+            return;
+        }
+
+        if ($hasMethod === false) {
+            return;
+        }
+
+        $hasOverride = $this->checkMethodHasOverrideAttribute($phpcsFile, $methodNamePtr, $tokens, $hasOverride);
+
+        if ($hasOverride === true) {
+            return;
+        }
+
+        $error = "Method {$className}::{$methodName} overrides {$parentClassName}::{$methodName} but is missing #[Override] attribute.";
+        $fix = $phpcsFile->addFixableError($error, $stackPtr, 'MissingOverride');
+
+        if (!$fix) {
+            return;
+        }
+
+        $phpcsFile->fixer->beginChangeset();
+
+        $previousLine = $phpcsFile->findPrevious(T_WHITESPACE, $methodNamePtr, value: PHP_EOL);
+        $indent = $tokens[$previousLine + 1]['content'];
+        $phpcsFile->fixer->addContentBefore($previousLine + 1, "{$indent}#[Override]" . PHP_EOL);
+
+        $overrideFqn = $this->getFqnOfClass($phpcsFile, $tokens, 'Override');
+
+        if ($overrideFqn === '') {
+            $usePtr = $phpcsFile->findNext(T_USE, 0);
+            $previousLine = $phpcsFile->findPrevious(T_WHITESPACE, $usePtr, value: PHP_EOL);
+            $phpcsFile->fixer->addContentBefore($previousLine + 1, 'use Override;' . PHP_EOL);
+        }
+
+        $phpcsFile->fixer->endChangeset();
+    }
+
+    /**
+     * @param \ReflectionClass $parentClass
+     * @param string $methodName
+     * @return bool
+     */
+    protected function hasParentClassMethod(ReflectionClass $parentClass, string $methodName): bool
+    {
+        if ($parentClass->hasMethod($methodName)) {
+            return true;
+        }
+
+        $parentClass = $parentClass->getParentClass();
+
+        if ($parentClass === false) {
+            return false;
+        }
+
+        return $parentClass->hasMethod($methodName);
+    }
+
+    /**
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile
+     * @param array $tokens
+     * @param string $parentClassName
+     * @return string
+     */
+    private function getFqnOfClass(File $phpcsFile, array $tokens, mixed $parentClassName): string
+    {
+        $usePtr = $phpcsFile->findNext(T_USE, 0);
+
+        $parentClassFqn = '';
+
+        while ($usePtr !== false) {
+            $semicolonPtr = $phpcsFile->findNext(T_SEMICOLON, $usePtr);
+            $fqnClassNamePtr = $phpcsFile->findPrevious(T_STRING, $semicolonPtr);
+
+            $fqnClassName = $tokens[$fqnClassNamePtr]['content'];
+
+            if ($fqnClassName !== $parentClassName) {
+                $usePtr = $phpcsFile->findNext(T_USE, $usePtr + 1);
+
+                continue;
+            }
+
+            $fqnCurrentPtr = $usePtr + 2;
+
+            while ($tokens[$fqnCurrentPtr]['type'] !== 'T_WHITESPACE' && $tokens[$fqnCurrentPtr]['type'] !== 'T_SEMICOLON') {
+                $parentClassFqn .= $tokens[$fqnCurrentPtr]['content'];
+                $fqnCurrentPtr++;
+            }
+
+            break;
+        }
+
+        return $parentClassFqn;
+    }
+
+    /**
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile
+     * @param int $methodNamePtr
+     * @param array $tokens
+     * @return bool
+     */
+    private function checkMethodHasOverrideAttribute(File $phpcsFile, int $methodNamePtr, array $tokens): bool
+    {
+        $hasOverride = false;
+        $previousLinePtr = $phpcsFile->findPrevious(T_WHITESPACE, $methodNamePtr, value: PHP_EOL);
+
+        for ($i = $previousLinePtr - 1; $i >= 0; $i--) {
+            if ($tokens[$i]['code'] === T_ATTRIBUTE) {
+                $attributeNamePtr = $phpcsFile->findNext(T_STRING, $i);
+
+                if ($attributeNamePtr !== false && $tokens[$attributeNamePtr]['content'] === 'Override') {
+                    $hasOverride = true;
+
+                    break;
+                }
+            }
+
+            if ($tokens[$i]['code'] === T_DOC_COMMENT_CLOSE_TAG || $tokens[$i]['code'] === T_FUNCTION) {
+                break;
+            }
+        }
+
+        return $hasOverride;
+    }
+}

src/Sniffs/ValidVariableNameSniff.php

@@ -4,6 +4,7 @@
 
 namespace Shopsys\CodingStandards\Sniffs;
 
+use Override;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\AbstractVariableSniff;
 use PHP_CodeSniffer\Util\Common;
@@ -23,6 +24,7 @@ final class ValidVariableNameSniff extends AbstractVariableSniff
      * @param \PHP_CodeSniffer\Files\File $file
      * @param int $position
      */
+    #[Override]
     protected function processVariable(File $file, $position): void
     {
         $errorMessageFormat = 'Variable "$%s" should be camel case';
@@ -33,6 +35,7 @@ protected function processVariable(File $file, $position): void
      * @param \PHP_CodeSniffer\Files\File $file
      * @param int $position
      */
+    #[Override]
     protected function processVariableInString(File $file, $position): void
     {
     }
@@ -41,6 +44,7 @@ protected function processVariableInString(File $file, $position): void
      * @param \PHP_CodeSniffer\Files\File $file
      * @param int $position
      */
+    #[Override]
     protected function processMemberVar(File $file, $position): void
     {
         $errorMessageFormat = 'Class member variable "$%s" should be camel case';

tests/Unit/CsFixer/Constraint/IsIdenticalString.php

@@ -4,6 +4,7 @@
 
 namespace Tests\CodingStandards\Unit\CsFixer\Constraint;
 
+use Override;
 use PHPUnit\Framework\Constraint\Constraint;
 use PHPUnit\Framework\Constraint\IsIdentical;
 use PHPUnit\Framework\ExpectationFailedException;
@@ -30,6 +31,7 @@ public function __construct(private readonly mixed $value)
      * @param bool $returnResult
      * @return bool|null
      */
+    #[Override]
     public function evaluate($other, string $description = '', bool $returnResult = false): ?bool
     {
         try {
@@ -54,6 +56,7 @@ public function evaluate($other, string $description = '', bool $returnResult =
     /**
      * @return string
      */
+    #[Override]
     public function toString(): string
     {
         return $this->isIdentical->toString();
@@ -63,6 +66,7 @@ public function toString(): string
      * @param mixed $other
      * @return string
      */
+    #[Override]
     protected function additionalFailureDescription($other): string
     {
         $pattern = '/(\r\n|\n\r|\r)/';

tests/Unit/CsFixer/ForbiddenDumpFixer/ForbiddenDumpFixerTest.php

@@ -4,6 +4,7 @@
 
 namespace Tests\CodingStandards\Unit\CsFixer\ForbiddenDumpFixer;
 
+use Override;
 use Shopsys\CodingStandards\CsFixer\ForbiddenDumpFixer;
 use Tests\CodingStandards\Unit\CsFixer\AbstractFixerTestCase;
 
@@ -12,6 +13,7 @@ final class ForbiddenDumpFixerTest extends AbstractFixerTestCase
     /**
      * @return \Shopsys\CodingStandards\CsFixer\ForbiddenDumpFixer
      */
+    #[Override]
     protected function createFixerService(): ForbiddenDumpFixer
     {
         return new ForbiddenDumpFixer();
@@ -20,6 +22,7 @@ protected function createFixerService(): ForbiddenDumpFixer
     /**
      * {@inheritdoc}
      */
+    #[Override]
     public static function getTestingFiles(): iterable
     {
         yield [__DIR__ . '/fixed/fixed.html.twig', __DIR__ . '/wrong/wrong.html.twig'];

tests/Unit/CsFixer/ForbiddenPrivateVisibilityFixer/ForbiddenPrivateVisibilityFixerTest.php

@@ -4,6 +4,7 @@
 
 namespace Tests\CodingStandards\Unit\CsFixer\ForbiddenPrivateVisibilityFixer;
 
+use Override;
 use Shopsys\CodingStandards\CsFixer\ForbiddenPrivateVisibilityFixer;
 use Tests\CodingStandards\Unit\CsFixer\AbstractFixerTestCase;
 
@@ -12,6 +13,7 @@ final class ForbiddenPrivateVisibilityFixerTest extends AbstractFixerTestCase
     /**
      * @return \Shopsys\CodingStandards\CsFixer\ForbiddenPrivateVisibilityFixer
      */
+    #[Override]
     protected function createFixerService(): ForbiddenPrivateVisibilityFixer
     {
         $fixer = new ForbiddenPrivateVisibilityFixer();
@@ -25,6 +27,7 @@ protected function createFixerService(): ForbiddenPrivateVisibilityFixer
     /**
      * {@inheritdoc}
      */
+    #[Override]
     public static function getTestingFiles(): iterable
     {
         yield [__DIR__ . '/fixed/constructor_property_promotion.php', __DIR__ . '/wrong/constructor_property_promotion.php'];

tests/Unit/CsFixer/MissingButtonTypeFixer/MissingButtonTypeFixerTest.php

@@ -4,6 +4,7 @@
 
 namespace Tests\CodingStandards\Unit\CsFixer\MissingButtonTypeFixer;
 
+use Override;
 use Shopsys\CodingStandards\CsFixer\MissingButtonTypeFixer;
 use Tests\CodingStandards\Unit\CsFixer\AbstractFixerTestCase;
 
@@ -12,6 +13,7 @@ final class MissingButtonTypeFixerTest extends AbstractFixerTestCase
     /**
      * @return \Shopsys\CodingStandards\CsFixer\MissingButtonTypeFixer
      */
+    #[Override]
     protected function createFixerService(): MissingButtonTypeFixer
     {
         return new MissingButtonTypeFixer();
@@ -20,6 +22,7 @@ protected function createFixerService(): MissingButtonTypeFixer
     /**
      * {@inheritdoc}
      */
+    #[Override]
     public static function getTestingFiles(): iterable
     {
         yield [__DIR__ . '/fixed/fixed.html.twig', __DIR__ . '/wrong/wrong.html.twig'];