6.x: Add stricter naming convention sniffs

.github/workflows/ci.yml

@@ -5,6 +5,7 @@ on:
     branches:
       - master
       - '5.x'
+      - '6.x'
   pull_request:
     branches:
       - '*'
@@ -15,10 +16,10 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        php-version: ['8.1', '8.2', '8.3', '8.4']
+        php-version: ['8.4', '8.5']
         dependencies: ['highest']
         include:
-          - php-version: '8.1'
+          - php-version: '8.4'
             dependencies: 'lowest'
 
     steps:
@@ -48,7 +49,7 @@ jobs:
     - name: Setup PHP
       uses: shivammathur/setup-php@v2
       with:
-        php-version: '8.1'
+        php-version: '8.4'
         tools: phive, cs2pr
         coverage: none
 

CakePHP/Sniffs/Classes/ReturnTypeHintSniff.php

@@ -41,40 +41,67 @@ public function process(File $phpcsFile, $stackPtr)
         $closeParenthesisIndex = $tokens[$openParenthesisIndex]['parenthesis_closer'];
 
         $colonIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $closeParenthesisIndex + 1, null, true);
-        if (!$colonIndex) {
+
+        if (!$this->isChainingMethod($phpcsFile, $stackPtr)) {
+            if ($colonIndex && $tokens[$colonIndex]['code'] === T_COLON) {
+                $this->assertNotThisOrStatic($phpcsFile, $stackPtr);
+            }
+
             return;
         }
 
-        $startIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $colonIndex + 1, $colonIndex + 3, true);
-        if (!$startIndex) {
+        // We skip for interface methods
+        if (empty($tokens[$stackPtr]['scope_opener']) || empty($tokens[$stackPtr]['scope_closer'])) {
             return;
         }
 
-        if (!$this->isChainingMethod($phpcsFile, $stackPtr)) {
-            $this->assertNotThisOrStatic($phpcsFile, $stackPtr);
+        // No colon means no return type hint - add static
+        if (!$colonIndex || $tokens[$colonIndex]['code'] !== T_COLON) {
+            $fix = $phpcsFile->addFixableError(
+                'Chaining methods (@return $this) should have "static" return type.',
+                $closeParenthesisIndex,
+                'MissingStatic',
+            );
+            if (!$fix) {
+                return;
+            }
+
+            $phpcsFile->fixer->beginChangeset();
+            $phpcsFile->fixer->addContent($closeParenthesisIndex, ': static');
+            $phpcsFile->fixer->endChangeset();
 
             return;
         }
 
-        // We skip for interface methods
-        if (empty($tokens[$stackPtr]['scope_opener']) || empty($tokens[$stackPtr]['scope_closer'])) {
+        $startIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $colonIndex + 1, $colonIndex + 3, true);
+        if (!$startIndex) {
             return;
         }
 
         $returnTokenCode = $tokens[$startIndex]['code'];
+        if ($returnTokenCode === T_STATIC) {
+            return;
+        }
+
         if ($returnTokenCode !== T_SELF) {
-            // Then we can only warn, but not auto-fix
-            $phpcsFile->addError(
-                'Chaining methods (@return $this) should not have any return-type-hint.',
+            $fix = $phpcsFile->addFixableError(
+                'Chaining methods (@return $this) should have "static" return type.',
                 $startIndex,
                 'InvalidSelf',
             );
+            if (!$fix) {
+                return;
+            }
+
+            $phpcsFile->fixer->beginChangeset();
+            $phpcsFile->fixer->replaceToken($startIndex, 'static');
+            $phpcsFile->fixer->endChangeset();
 
             return;
         }
 
         $fix = $phpcsFile->addFixableError(
-            'Chaining methods (@return $this) should not have any return-type-hint (Remove "self").',
+            'Chaining methods (@return $this) should have "static" return type instead of "self".',
             $startIndex,
             'InvalidSelf',
         );
@@ -83,9 +110,7 @@ public function process(File $phpcsFile, $stackPtr)
         }
 
         $phpcsFile->fixer->beginChangeset();
-        for ($i = $colonIndex; $i <= $startIndex; $i++) {
-            $phpcsFile->fixer->replaceToken($i, '');
-        }
+        $phpcsFile->fixer->replaceToken($startIndex, 'static');
         $phpcsFile->fixer->endChangeset();
     }
 
@@ -173,7 +198,7 @@ protected function assertNotThisOrStatic(File $phpCsFile, int $stackPointer): vo
             }
 
             $phpCsFile->addError(
-                'Class name repeated, expected `self` or `$this`.',
+                'Class name repeated, expected `static` or `$this`.',
                 $classNameIndex,
                 'InvalidClass',
             );

CakePHP/Sniffs/NamingConventions/ValidEnumNameSniff.php

@@ -0,0 +1,76 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://github.com/cakephp/cakephp-codesniffer
+ * @since         CakePHP CodeSniffer 0.1.10
+ * @license       https://www.opensource.org/licenses/mit-license.php MIT License
+ */
+
+/**
+ * Ensures enum names use the Enum suffix.
+ */
+namespace CakePHP\Sniffs\NamingConventions;
+
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Sniffs\Sniff;
+
+class ValidEnumNameSniff implements Sniff
+{
+    /**
+     * @inheritDoc
+     */
+    public function register()
+    {
+        return [T_ENUM];
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function process(File $phpcsFile, $stackPtr)
+    {
+        $enumName = $phpcsFile->getDeclarationName($stackPtr);
+
+        if (!str_ends_with($enumName, 'Enum') && !$this->isInEnumNamespace($phpcsFile)) {
+            $error = 'Enums must have an "Enum" suffix (or be in an Enum namespace).';
+            $phpcsFile->addError($error, $stackPtr, 'InvalidEnumName');
+        }
+    }
+
+    /**
+     * Check if the file's namespace contains "Enum" as a segment.
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
+     * @return bool
+     */
+    protected function isInEnumNamespace(File $phpcsFile): bool
+    {
+        $tokens = $phpcsFile->getTokens();
+        $namespacePtr = $phpcsFile->findNext(T_NAMESPACE, 0);
+
+        if ($namespacePtr === false) {
+            return false;
+        }
+
+        $namespaceEnd = $phpcsFile->findNext([T_SEMICOLON, T_OPEN_CURLY_BRACKET], $namespacePtr);
+        $namespace = '';
+
+        for ($i = $namespacePtr + 1; $i < $namespaceEnd; $i++) {
+            if ($tokens[$i]['code'] === T_STRING || $tokens[$i]['code'] === T_NAME_QUALIFIED) {
+                $namespace .= $tokens[$i]['content'];
+            }
+        }
+
+        // Check if namespace ends with \Enum or contains \Enum\
+        return (bool)preg_match('/\\\\Enum(\\\\|$)/', $namespace);
+    }
+}

CakePHP/Sniffs/NamingConventions/ValidFunctionNameSniff.php

@@ -27,9 +27,9 @@ class ValidFunctionNameSniff extends AbstractScopeSniff
     /**
      * A list of all PHP magic methods.
      *
-     * @var array
+     * @var array<string>
      */
-    protected array $_magicMethods = [
+    protected array $magicMethods = [
         'construct',
         'destruct',
         'call',
@@ -54,7 +54,7 @@ class ValidFunctionNameSniff extends AbstractScopeSniff
      */
     public function __construct()
     {
-        parent::__construct([T_CLASS, T_INTERFACE, T_TRAIT], [T_FUNCTION], true);
+        parent::__construct([T_CLASS, T_INTERFACE, T_TRAIT, T_ENUM], [T_FUNCTION], true);
     }
 
     /**
@@ -71,7 +71,7 @@ protected function processTokenWithinScope(File $phpcsFile, $stackPtr, $currScop
         $errorData = [$className . '::' . $methodName];
 
         // Ignore magic methods
-        if (preg_match('/^__(' . implode('|', $this->_magicMethods) . ')$/', $methodName)) {
+        if (preg_match('/^__(' . implode('|', $this->magicMethods) . ')$/', $methodName)) {
             return;
         }
 
@@ -89,6 +89,17 @@ protected function processTokenWithinScope(File $phpcsFile, $stackPtr, $currScop
 
             return;
         }
+
+        // Check non-public methods for underscore prefix
+        if ($isPublic === false && $methodName[0] === '_') {
+            // Allow CakePHP Entity accessor/mutator pattern: _getField(), _setField()
+            if (preg_match('/^_(get|set)[A-Z]/', $methodName)) {
+                return;
+            }
+
+            $error = 'Non-public method name "%s" should not be prefixed with underscore';
+            $phpcsFile->addError($error, $stackPtr, 'ProtectedWithUnderscore', $errorData);
+        }
     }
 
     /**

CakePHP/Sniffs/NamingConventions/ValidTraitNameSniff.php

@@ -39,7 +39,7 @@ public function process(File $phpcsFile, $stackPtr)
         $tokens = $phpcsFile->getTokens();
         $traitName = $tokens[$stackPtr + 2]['content'];
 
-        if (substr($traitName, -5) !== 'Trait') {
+        if (!str_ends_with($traitName, 'Trait')) {
             $error = 'Traits must have a "Trait" suffix.';
             $phpcsFile->addError($error, $stackPtr, 'InvalidTraitName');
         }

CakePHP/Tests/Classes/ReturnTypeHintUnitTest.inc

@@ -6,14 +6,21 @@ class Foo
     /**
      * @return $this
      */
-    public function correct()
+    public function correct(): static
     {
     }
 
     /**
      * @return $this
      */
-    public function incorrect(): Foo
+    public function incorrect()
+    {
+    }
+
+    /**
+     * @return $this
+     */
+    public function incorrectClass(): Foo
     {
     }
 

CakePHP/Tests/Classes/ReturnTypeHintUnitTest.inc.fixed

@@ -6,21 +6,28 @@ class Foo
     /**
      * @return $this
      */
-    public function correct()
+    public function correct(): static
     {
     }
 
     /**
      * @return $this
      */
-    public function incorrect(): Foo
+    public function incorrect(): static
     {
     }
 
     /**
      * @return $this
      */
-    public function incorrectSelf()
+    public function incorrectClass(): static
+    {
+    }
+
+    /**
+     * @return $this
+     */
+    public function incorrectSelf(): static
     {
     }
 }

CakePHP/Tests/Classes/ReturnTypeHintUnitTest.php

@@ -14,6 +14,7 @@ public function getErrorList()
         return [
             16 => 1,
             23 => 1,
+            30 => 1,
         ];
     }
 

CakePHP/Tests/Commenting/TypeHintUnitTest.1.inc

@@ -45,3 +45,12 @@ function test()
 function intersection($param)
 {
 }
+
+/**
+ * Void must be last, after null.
+ *
+ * @return int|void|null
+ */
+function voidAfterNull()
+{
+}

CakePHP/Tests/Commenting/TypeHintUnitTest.1.inc.fixed

@@ -45,3 +45,12 @@ function test()
 function intersection($param)
 {
 }
+
+/**
+ * Void must be last, after null.
+ *
+ * @return int|null|void
+ */
+function voidAfterNull()
+{
+}

CakePHP/Tests/Commenting/TypeHintUnitTest.php

@@ -32,6 +32,7 @@ public function getWarningList($testFile = '')
                     27 => 1,
                     37 => 1,
                     42 => 1,
+                    52 => 1,
                 ];
 
             default:

CakePHP/Tests/NamingConventions/ValidEnumNameUnitTest.1.inc

@@ -0,0 +1,8 @@
+<?php
+enum Status: string {
+    case Draft = 'draft';
+}
+
+enum StatusEnum: string {
+    case Published = 'published';
+}

CakePHP/Tests/NamingConventions/ValidEnumNameUnitTest.2.inc

@@ -0,0 +1,12 @@
+<?php
+namespace App\Model\Enum;
+
+// Valid: no suffix needed when in Enum namespace
+enum Status: string {
+    case Draft = 'draft';
+}
+
+// Also valid: suffix is still allowed
+enum StatusEnum: string {
+    case Published = 'published';
+}