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

More class const improvements. #7666

Merged
merged 1 commit into from Feb 16, 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
2 changes: 2 additions & 0 deletions config.xsd
Expand Up @@ -249,6 +249,7 @@
<xs:element name="InvalidCast" type="IssueHandlerType" minOccurs="0" />
<xs:element name="InvalidCatch" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="InvalidClass" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="InvalidClassConstantType" type="ClassConstantIssueHandlerType" minOccurs="0" />
<xs:element name="InvalidClone" type="IssueHandlerType" minOccurs="0" />
<xs:element name="InvalidConstantAssignmentValue" type="ClassConstantIssueHandlerType" minOccurs="0" />
<xs:element name="InvalidDocblock" type="IssueHandlerType" minOccurs="0" />
Expand Down Expand Up @@ -343,6 +344,7 @@
<xs:element name="NullPropertyAssignment" type="PropertyIssueHandlerType" minOccurs="0" />
<xs:element name="NullPropertyFetch" type="PropertyIssueHandlerType" minOccurs="0" />
<xs:element name="NullReference" type="IssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenFinalConstant" type="ClassConstantIssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenInterfaceConstant" type="ClassConstantIssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenMethodAccess" type="IssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenPropertyAccess" type="PropertyIssueHandlerType" minOccurs="0" />
Expand Down
2 changes: 2 additions & 0 deletions docs/running_psalm/issues.md
Expand Up @@ -61,6 +61,7 @@
- [InvalidCast](issues/InvalidCast.md)
- [InvalidCatch](issues/InvalidCatch.md)
- [InvalidClass](issues/InvalidClass.md)
- [InvalidClassConstantType](issues/InvalidClassConstantType.md)
- [InvalidClone](issues/InvalidClone.md)
- [InvalidConstantAssignmentValue](issues/InvalidConstantAssignmentValue.md)
- [InvalidDocblock](issues/InvalidDocblock.md)
Expand Down Expand Up @@ -155,6 +156,7 @@
- [NullPropertyAssignment](issues/NullPropertyAssignment.md)
- [NullPropertyFetch](issues/NullPropertyFetch.md)
- [NullReference](issues/NullReference.md)
- [OverriddenFinalConstant](issues/OverriddenFinalConstant.md)
- [OverriddenInterfaceConstant](issues/OverriddenInterfaceConstant.md)
- [OverriddenMethodAccess](issues/OverriddenMethodAccess.md)
- [OverriddenPropertyAccess](issues/OverriddenPropertyAccess.md)
Expand Down
28 changes: 28 additions & 0 deletions docs/running_psalm/issues/InvalidClassConstantType.md
@@ -0,0 +1,28 @@
# InvalidClassConstantType

Emitted when a constant type in a child does not satisfy the type in the parent.

```php
<?php

class Foo
{
/** @var int<1,max> */
public const CONSTANT = 3;

public static function bar(): array
{
return str_split("foobar", static::CONSTANT);
}
}

class Bar extends Foo
{
/** @var int<min,-1> */
public const CONSTANT = -1;
}

Bar::bar(); // Error: str_split argument 2 must be greater than 0
```

This issue will always show up when overriding a constant that doesn't have a docblock type. Psalm will infer the most specific type for the constant that it can, you have to add a type annotation to tell it what type constraint you wish to be applied. Otherwise Psalm has no way of telling if you mean for the constant to be a literal `1`, `int<1, max>`, `int`, `numeric`, etc.
19 changes: 19 additions & 0 deletions docs/running_psalm/issues/OverriddenFinalConstant.md
@@ -0,0 +1,19 @@
# OverriddenFinalConstant

Emitted when a constant declared as final is overridden in a child class or interface.

```php
<?php

class Foo
{
/** @var string */
final public const BAR='baz';
}

class Bar extends Foo
{
/** @var string */
public const BAR='foobar';
}
```
Expand Up @@ -23,11 +23,14 @@
use Psalm\Issue\DeprecatedConstant;
use Psalm\Issue\InaccessibleClassConstant;
use Psalm\Issue\InternalClass;
use Psalm\Issue\InvalidClassConstantType;
use Psalm\Issue\InvalidConstantAssignmentValue;
use Psalm\Issue\LessSpecificClassConstantType;
use Psalm\Issue\NonStaticSelfCall;
use Psalm\Issue\OverriddenFinalConstant;
use Psalm\Issue\OverriddenInterfaceConstant;
use Psalm\Issue\ParentNotFound;
use Psalm\Issue\ParseError;
use Psalm\Issue\UndefinedConstant;
use Psalm\IssueBuffer;
use Psalm\Storage\ClassConstantStorage;
Expand Down Expand Up @@ -721,38 +724,89 @@ public static function analyze(
Codebase $codebase
): void {
foreach ($class_storage->constants as $const_name => $const_storage) {
// Check covariance
[$parent_classlike_storage, $parent_const_storage] = self::getOverriddenConstant(
$class_storage,
$const_storage,
$const_name,
$codebase
);

$type_location = $const_storage->location ?? $const_storage->stmt_location;
if ($type_location === null) {
continue;
}

if ($parent_const_storage !== null) {
assert($parent_classlike_storage !== null);
$location = $const_storage->type_location ?? $const_storage->stmt_location;
if ($location !== null
&& $const_storage->type !== null

// Check covariance
if ($const_storage->type !== null
&& $parent_const_storage->type !== null
&& !UnionTypeComparator::isContainedBy(
$codebase,
$const_storage->type,
$parent_const_storage->type
)
) {
if (UnionTypeComparator::isContainedBy(
$codebase,
$parent_const_storage->type,
$const_storage->type,
)) {
// Contravariant
IssueBuffer::maybeAdd(
new LessSpecificClassConstantType(
"The type \"{$const_storage->type->getId()}\" for {$class_storage->name}::"
. "{$const_name} is more general than the type "
. "\"{$parent_const_storage->type->getId()}\" inherited from "
. "{$parent_classlike_storage->name}::{$const_name}",
$type_location,
"{$class_storage->name}::{$const_name}"
),
$const_storage->suppressed_issues
);
} else {
// Completely different
IssueBuffer::maybeAdd(
new InvalidClassConstantType(
"The type \"{$const_storage->type->getId()}\" for {$class_storage->name}::"
. "{$const_name} does not satisfy the type "
. "\"{$parent_const_storage->type->getId()}\" inherited from "
. "{$parent_classlike_storage->name}::{$const_name}",
$type_location,
"{$class_storage->name}::{$const_name}"
),
$const_storage->suppressed_issues
);
}
}

// Check overridden final
if ($parent_const_storage->final) {
IssueBuffer::maybeAdd(
new LessSpecificClassConstantType(
"The type '{$const_storage->type->getId()}' for {$class_storage->name}::"
. "{$const_name} is more general than the type "
. "'{$parent_const_storage->type->getId()}' inherited from "
. "{$parent_classlike_storage->name}::{$const_name}",
$location,
new OverriddenFinalConstant(
"{$const_name} cannot be overridden because it is marked as final in "
. $parent_classlike_storage->name,
$type_location,
"{$class_storage->name}::{$const_name}"
),
$const_storage->suppressed_issues
);
}
}

if ($const_storage->stmt_location !== null) {
// Check final in PHP < 8.1
if ($codebase->analysis_php_version_id < 8_01_00 && $const_storage->final) {
IssueBuffer::maybeAdd(
new ParseError(
"Class constants cannot be marked final before PHP 8.1",
$const_storage->stmt_location,
),
$const_storage->suppressed_issues
);
weirdan marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

Expand Down
Expand Up @@ -989,7 +989,7 @@ private function extendTemplatedType(
$extended_type_parameters = [];

$storage->template_type_extends_count[$atomic_type->value] = count($atomic_type->type_params);

foreach ($atomic_type->type_params as $type_param) {
$extended_type_parameters[] = $type_param;
}
Expand Down Expand Up @@ -1340,6 +1340,7 @@ private function visitClassConstDeclaration(
}

$constant_storage->description = $description;
$constant_storage->final = $stmt->isFinal();

foreach ($stmt->attrGroups as $attr_group) {
foreach ($attr_group->attrs as $attr) {
Expand Down
9 changes: 9 additions & 0 deletions src/Psalm/Issue/InvalidClassConstantType.php
@@ -0,0 +1,9 @@
<?php

namespace Psalm\Issue;

final class InvalidClassConstantType extends ClassConstantIssue
{
public const ERROR_LEVEL = 1;
public const SHORTCODE = 309;
}
9 changes: 9 additions & 0 deletions src/Psalm/Issue/OverriddenFinalConstant.php
@@ -0,0 +1,9 @@
<?php

namespace Psalm\Issue;

final class OverriddenFinalConstant extends ClassConstantIssue
{
public const ERROR_LEVEL = 6;
public const SHORTCODE = 310;
}
5 changes: 5 additions & 0 deletions src/Psalm/Storage/ClassConstantStorage.php
Expand Up @@ -58,6 +58,11 @@ final class ClassConstantStorage
*/
public $deprecated = false;

/**
* @var bool
*/
public $final = false;

/**
* @var list<AttributeStorage>
*/
Expand Down
50 changes: 49 additions & 1 deletion tests/ConstantTest.php
Expand Up @@ -1879,10 +1879,58 @@ class Baz implements Foo, Bar
public const BAR="";
}
',
'error_message' => "LessSpecificClassConstantType",
'error_message' => "InvalidClassConstantType",
'ignored_issues' => [],
'php_version' => '8.1',
],
'overrideFinalClassConstFromExtendedClass' => [
'code' => '<?php
class Foo
{
/** @var string */
final public const BAR="baz";
}

class Baz extends Foo
{
/** @var string */
public const BAR="foobar";
}
',
'error_message' => "OverriddenFinalConstant",
'ignored_issues' => [],
'php_version' => '8.1',
],
'overrideFinalClassConstFromImplementedInterface' => [
'code' => '<?php
interface Foo
{
/** @var string */
final public const BAR="baz";
}

class Baz implements Foo
{
/** @var string */
public const BAR="foobar";
}
',
'error_message' => "OverriddenFinalConstant",
'ignored_issues' => [],
'php_version' => '8.1',
],
'finalConstantIsIllegalBefore8.1' => [
'code' => '<?php
class Foo
{
/** @var string */
final public const BAR="baz";
}
',
'error_message' => 'ParseError - src' . DIRECTORY_SEPARATOR . 'somefile.php:5:44',
'ignored_issues' => [],
'php_version' => '8.0',
],
];
}
}
21 changes: 6 additions & 15 deletions tests/DocumentationTest.php
Expand Up @@ -268,22 +268,12 @@ public function providerInvalidCodeParse(): array
$php_version = '8.0';
$ignored_issues = [];
switch ($issue_name) {
case 'MissingThrowsDocblock':
continue 2;

case 'UncaughtThrowInGlobalScope':
continue 2;

case 'InvalidStringClass':
continue 2;

case 'MissingThrowsDocblock':
case 'PluginClass':
continue 2;

case 'RedundantIdentityWithTrue':
continue 2;

case 'TraitMethodSignatureMismatch':
case 'UncaughtThrowInGlobalScope':
continue 2;

/** @todo reinstate this test when the issue is restored */
Expand Down Expand Up @@ -320,12 +310,13 @@ public function providerInvalidCodeParse(): array
break;

case 'AmbiguousConstantInheritance':
case 'InvalidEnumBackingType':
case 'InvalidEnumCaseValue':
case 'DeprecatedConstant':
case 'DuplicateEnumCase':
case 'DuplicateEnumCaseValue':
case 'InvalidEnumBackingType':
case 'InvalidEnumCaseValue':
case 'NoEnumProperties':
case 'DeprecatedConstant':
case 'OverriddenFinalConstant':
$php_version = '8.1';
break;
}
Expand Down