Skip to content

Commit

Permalink
Merge pull request #7363 from zoonru/81_returntypewillchange
Browse files Browse the repository at this point in the history
PHP 8.1: Report missing typehints in overridden native methods
  • Loading branch information
orklah committed Jan 19, 2022
2 parents d274d52 + 1914be4 commit bbfdd57
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 1 deletion.
1 change: 1 addition & 0 deletions config.xsd
Expand Up @@ -325,6 +325,7 @@
<xs:element name="LoopInvalidation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="MethodSignatureMismatch" type="IssueHandlerType" minOccurs="0" />
<xs:element name="MethodSignatureMustOmitReturnType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="MethodSignatureMustProvideReturnType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="MismatchingDocblockParamType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="MismatchingDocblockPropertyType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="MismatchingDocblockReturnType" type="IssueHandlerType" minOccurs="0" />
Expand Down
1 change: 1 addition & 0 deletions docs/running_psalm/error_levels.md
Expand Up @@ -56,6 +56,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
- [InvalidThrow](issues/InvalidThrow.md)
- [LoopInvalidation](issues/LoopInvalidation.md)
- [MethodSignatureMustOmitReturnType](issues/MethodSignatureMustOmitReturnType.md)
- [MethodSignatureMustProvideReturnType](issues/MethodSignatureMustProvideReturnType.md)
- [MissingDependency](issues/MissingDependency.md)
- [MissingFile](issues/MissingFile.md)
- [MissingImmutableAnnotation](issues/MissingImmutableAnnotation.md)
Expand Down
1 change: 1 addition & 0 deletions docs/running_psalm/issues.md
Expand Up @@ -99,6 +99,7 @@
- [LoopInvalidation](issues/LoopInvalidation.md)
- [MethodSignatureMismatch](issues/MethodSignatureMismatch.md)
- [MethodSignatureMustOmitReturnType](issues/MethodSignatureMustOmitReturnType.md)
- [MethodSignatureMustProvideReturnType](issues/MethodSignatureMustProvideReturnType.md)
- [MismatchingDocblockParamType](issues/MismatchingDocblockParamType.md)
- [MismatchingDocblockPropertyType](issues/MismatchingDocblockPropertyType.md)
- [MismatchingDocblockReturnType](issues/MismatchingDocblockReturnType.md)
Expand Down
17 changes: 17 additions & 0 deletions docs/running_psalm/issues/MethodSignatureMustProvideReturnType.md
@@ -0,0 +1,17 @@
# MethodSignatureMustProvideReturnType

In PHP 8.1+, [most non-final internal methods now require overriding methods to declare a compatible return type, otherwise a deprecated notice is emitted during inheritance validation](https://www.php.net/manual/en/migration81.incompatible.php#migration81.incompatible.core.type-compatibility-internal).

This issue is emitted when a method overriding a native method is defined without a return type.

**Only if** the return type cannot be declared to keep support for PHP 7, a `#[ReturnTypeWillChange]` attribute can be added to silence the PHP deprecation notice and Psalm issue.

```php
<?php

class A implements JsonSerializable {
public function jsonSerialize() {
return ['type' => 'A'];
}
}
```
35 changes: 34 additions & 1 deletion src/Psalm/Internal/Analyzer/MethodComparator.php
Expand Up @@ -21,12 +21,14 @@
use Psalm\Issue\ImplementedReturnTypeMismatch;
use Psalm\Issue\LessSpecificImplementedReturnType;
use Psalm\Issue\MethodSignatureMismatch;
use Psalm\Issue\MethodSignatureMustProvideReturnType;
use Psalm\Issue\MissingImmutableAnnotation;
use Psalm\Issue\MoreSpecificImplementedParamType;
use Psalm\Issue\OverriddenMethodAccess;
use Psalm\Issue\ParamNameMismatch;
use Psalm\Issue\TraitMethodSignatureMismatch;
use Psalm\IssueBuffer;
use Psalm\Storage\AttributeStorage;
use Psalm\Storage\ClassLikeStorage;
use Psalm\Storage\FunctionLikeParameter;
use Psalm\Storage\MethodStorage;
Expand All @@ -35,6 +37,7 @@
use Psalm\Type\Atomic\TTemplateParam;
use Psalm\Type\Union;

use function array_filter;
use function in_array;
use function strpos;
use function strtolower;
Expand Down Expand Up @@ -113,6 +116,29 @@ public static function compare(
);
}

if (!$guide_classlike_storage->user_defined
&& $implementer_classlike_storage->user_defined
&& $codebase->analysis_php_version_id >= 80100
&& ($guide_method_storage->return_type
|| $guide_method_storage->signature_return_type
)
&& !$implementer_method_storage->signature_return_type
&& !array_filter(
$implementer_method_storage->attributes,
function (AttributeStorage $s) {
return $s->fq_class_name === 'ReturnTypeWillChange';
}
)
) {
IssueBuffer::maybeAdd(
new MethodSignatureMustProvideReturnType(
'Method ' . $cased_implementer_method_id . ' must have a return type signature!',
$implementer_method_storage->location ?: $code_location
),
$suppressed_issues + $implementer_classlike_storage->suppressed_issues
);
}

if ($guide_method_storage->return_type
&& $implementer_method_storage->return_type
&& !$implementer_method_storage->inherited_return_type
Expand Down Expand Up @@ -862,7 +888,14 @@ private static function compareMethodSignatureReturnTypes(
$implementer_signature_return_type,
$guide_signature_return_type
)
: UnionTypeComparator::isContainedByInPhp($implementer_signature_return_type, $guide_signature_return_type);
: (!$implementer_signature_return_type
&& $guide_signature_return_type->isMixed()
? false
: UnionTypeComparator::isContainedByInPhp(
$implementer_signature_return_type,
$guide_signature_return_type
)
);

if (!$is_contained_by) {
if ($codebase->php_major_version >= 8
Expand Down
9 changes: 9 additions & 0 deletions src/Psalm/Issue/MethodSignatureMustProvideReturnType.php
@@ -0,0 +1,9 @@
<?php

namespace Psalm\Issue;

class MethodSignatureMustProvideReturnType extends CodeIssue
{
public const ERROR_LEVEL = -1;
public const SHORTCODE = 282;
}
4 changes: 4 additions & 0 deletions tests/DocumentationTest.php
Expand Up @@ -218,6 +218,10 @@ public function testInvalidCode($code, $error_message, $error_levels = [], $chec
$this->markTestSkipped();
}

if (strpos($error_message, 'MethodSignatureMustProvideReturnType') !== false) {
$php_version = '8.1';
}

$this->project_analyzer->setPhpVersion($php_version, 'tests');

if ($check_references) {
Expand Down
31 changes: 31 additions & 0 deletions tests/MethodSignatureTest.php
Expand Up @@ -1569,6 +1569,37 @@ public function bar(string ...$_args): void {}
',
'error_message' => 'MethodSignatureMismatch',
],
'noMixedTypehintInDescendant' => [
'<?php
class a {
public function test(): mixed {
return 0;
}
}
class b extends a {
public function test() {
return 0;
}
}
',
'error_message' => 'MethodSignatureMismatch',
[],
false,
'8.0'
],
'noTypehintInNativeDescendant' => [
'<?php
class a implements JsonSerializable {
public function jsonSerialize() {
return 0;
}
}
',
'error_message' => 'MethodSignatureMustProvideReturnType',
[],
false,
'8.1'
],
];
}
}

0 comments on commit bbfdd57

Please sign in to comment.