Merge pull request #8890 from weirdan/enum-forbidden-methods

Fixes #8889 Fixes #8888
vimeo · Dec 12, 2022 · fb685a1 · fb685a1
2 parents ca0b2a1 + 9db0eb3
commit fb685a1
Show file tree

Hide file tree

Showing 9 changed files with 86 additions and 4 deletions.
diff --git a/config.xsd b/config.xsd
@@ -264,6 +264,7 @@
             <xs:element name="InvalidDocblockParamName" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidEnumBackingType" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidEnumCaseValue" type="ClassIssueHandlerType" minOccurs="0" />
+            <xs:element name="InvalidEnumMethod" type="MethodIssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidExtendClass" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidFalsableReturnType" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidFunctionCall" type="IssueHandlerType" minOccurs="0" />

diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md
@@ -45,6 +45,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
  - [InaccessibleProperty](issues/InaccessibleProperty.md)
  - [InterfaceInstantiation](issues/InterfaceInstantiation.md)
  - [InvalidAttribute](issues/InvalidAttribute.md)
+ - [InvalidEnumMethod](issues/InvalidEnumMethod.md)
  - [InvalidExtendClass](issues/InvalidExtendClass.md)
  - [InvalidGlobal](issues/InvalidGlobal.md)
  - [InvalidParamDefault](issues/InvalidParamDefault.md)

diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
@@ -69,6 +69,7 @@
  - [InvalidDocblockParamName](issues/InvalidDocblockParamName.md)
  - [InvalidEnumBackingType](issues/InvalidEnumBackingType.md)
  - [InvalidEnumCaseValue](issues/InvalidEnumCaseValue.md)
+ - [InvalidEnumMethod](issues/InvalidEnumMethod.md)
  - [InvalidExtendClass](issues/InvalidExtendClass.md)
  - [InvalidFalsableReturnType](issues/InvalidFalsableReturnType.md)
  - [InvalidFunctionCall](issues/InvalidFunctionCall.md)

diff --git a/docs/running_psalm/issues/InvalidEnumMethod.md b/docs/running_psalm/issues/InvalidEnumMethod.md
@@ -0,0 +1,15 @@
+# InvalidEnumMethod
+
+Enums may not define most of the magic methods like `__get`, `__toString`, etc.
+
+```php
+<?php
+enum Status: string {
+    case Open = 'open';
+    case Closed = 'closed';
+
+    public function __toString(): string {
+        return "SomeStatus";
+    }
+}
+```
diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -2020,6 +2020,10 @@ private function getFunctionInformation(
 
             MethodAnalyzer::checkMethodSignatureMustOmitReturnType($storage, $codeLocation);
 
+            if ($appearing_class_storage->is_enum) {
+                MethodAnalyzer::checkForbiddenEnumMethod($storage);
+            }
+
             if (!$context->calling_method_id || !$context->collect_initializations) {
                 $context->calling_method_id = strtolower((string)$method_id);
             }

diff --git a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php
@@ -9,6 +9,7 @@
 use Psalm\Context;
 use Psalm\Internal\Codebase\InternalCallMapHandler;
 use Psalm\Internal\MethodIdentifier;
+use Psalm\Issue\InvalidEnumMethod;
 use Psalm\Issue\InvalidStaticInvocation;
 use Psalm\Issue\MethodSignatureMustOmitReturnType;
 use Psalm\Issue\NonStaticSelfCall;
@@ -27,6 +28,24 @@
  */
 class MethodAnalyzer extends FunctionLikeAnalyzer
 {
+    // https://github.com/php/php-src/blob/a83923044c48982c80804ae1b45e761c271966d3/Zend/zend_enum.c#L77-L95
+    private const FORBIDDEN_ENUM_METHODS = [
+        '__construct',
+        '__destruct',
+        '__clone',
+        '__get',
+        '__set',
+        '__unset',
+        '__isset',
+        '__tostring',
+        '__debuginfo',
+        '__serialize',
+        '__unserialize',
+        '__sleep',
+        '__wakeup',
+        '__set_state',
+    ];
+
     /** @psalm-external-mutation-free */
     public function __construct(
         PhpParser\Node\Stmt\ClassMethod $function,
@@ -266,13 +285,17 @@ public static function checkMethodSignatureMustOmitReturnType(
             return;
         }
 
-        $cased_method_name = $method_storage->cased_name;
+        if ($method_storage->cased_name === null) {
+            return;
+        }
+
+        $method_name_lc = strtolower($method_storage->cased_name);
         $methodsOfInterest = ['__clone', '__construct', '__destruct'];
 
-        if (in_array($cased_method_name, $methodsOfInterest)) {
+        if (in_array($method_name_lc, $methodsOfInterest, true)) {
             IssueBuffer::maybeAdd(
                 new MethodSignatureMustOmitReturnType(
-                    'Method ' . $cased_method_name . ' must not declare a return type',
+                    'Method ' . $method_storage->cased_name . ' must not declare a return type',
                     $code_location
                 )
             );
@@ -288,4 +311,20 @@ public function getMethodId(?string $context_self = null): MethodIdentifier
             strtolower($function_name)
         );
     }
+
+    public static function checkForbiddenEnumMethod(MethodStorage $method_storage): void
+    {
+        if ($method_storage->cased_name === null || $method_storage->location === null) {
+            return;
+        }
+
+        $method_name_lc = strtolower($method_storage->cased_name);
+        if (in_array($method_name_lc, self::FORBIDDEN_ENUM_METHODS, true)) {
+            IssueBuffer::maybeAdd(new InvalidEnumMethod(
+                'Enums cannot define ' . $method_storage->cased_name,
+                $method_storage->location,
+                $method_storage->defining_fqcln . '::' . $method_storage->cased_name
+            ));
+        }
+    }
 }
diff --git a/src/Psalm/Issue/InvalidEnumMethod.php b/src/Psalm/Issue/InvalidEnumMethod.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Psalm\Issue;
+
+final class InvalidEnumMethod extends MethodIssue
+{
+    public const ERROR_LEVEL = -1;
+    public const SHORTCODE = 314;
+}
diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php
@@ -310,6 +310,7 @@ public function providerInvalidCodeParse(): array
                 case 'DuplicateEnumCaseValue':
                 case 'InvalidEnumBackingType':
                 case 'InvalidEnumCaseValue':
+                case 'InvalidEnumMethod':
                 case 'NoEnumProperties':
                 case 'OverriddenFinalConstant':
                     $php_version = '8.1';
@@ -436,7 +437,7 @@ public function testIssuesIndex(): void
             throw new UnexpectedValueException("Issues index not found");
         }
 
-        $issues_index_contents = file($issues_index, FILE_IGNORE_NEW_LINES|FILE_SKIP_EMPTY_LINES);
+        $issues_index_contents = file($issues_index, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
         if ($issues_index_contents === false) {
             throw new UnexpectedValueException("Issues index returned false");
         }

diff --git a/tests/EnumTest.php b/tests/EnumTest.php
@@ -686,6 +686,17 @@ enum Foo {
                 'ignored_issues' => [],
                 'php_version' => '8.1',
             ],
+            'forbiddenMethod' => [
+                'code' => '<?php
+                    enum Foo {
+                        case A;
+                        public function __get() {}
+                    }
+                ',
+                'error_message' => 'InvalidEnumMethod',
+                'ignored_issues' => [],
+                'php_version' => '8.1',
+            ],
         ];
     }
 }