Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid most magic methods on enums #8890

Merged
merged 2 commits into from Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions config.xsd
Expand Up @@ -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" />
Expand Down
1 change: 1 addition & 0 deletions docs/running_psalm/error_levels.md
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions docs/running_psalm/issues.md
Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions 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";
}
}
```
4 changes: 4 additions & 0 deletions src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
Expand Up @@ -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);
}
Expand Down
45 changes: 42 additions & 3 deletions src/Psalm/Internal/Analyzer/MethodAnalyzer.php
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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
)
);
Expand All @@ -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
));
}
}
}
9 changes: 9 additions & 0 deletions 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;
}
3 changes: 2 additions & 1 deletion tests/DocumentationTest.php
Expand Up @@ -310,6 +310,7 @@ public function providerInvalidCodeParse(): array
case 'DuplicateEnumCaseValue':
case 'InvalidEnumBackingType':
case 'InvalidEnumCaseValue':
case 'InvalidEnumMethod':
case 'NoEnumProperties':
case 'OverriddenFinalConstant':
$php_version = '8.1';
Expand Down Expand Up @@ -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");
}
Expand Down
11 changes: 11 additions & 0 deletions tests/EnumTest.php
Expand Up @@ -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',
],
];
}
}