Skip to content

Commit

Permalink
Improve class constant static analysis.
Browse files Browse the repository at this point in the history
Add class const covariance support (fixes #5589).
Add check for overriding const from interface in PHP < 8.1 (fixes #7108).
Add check for ambiguous const inheritance.
  • Loading branch information
AndrolGenhald committed Dec 13, 2021
1 parent 3e03dc6 commit 71f5b09
Show file tree
Hide file tree
Showing 18 changed files with 456 additions and 24 deletions.
13 changes: 8 additions & 5 deletions config.xsd
Expand Up @@ -218,9 +218,9 @@

<xs:complexType name="IssueHandlersType">
<xs:choice minOccurs="0" maxOccurs="unbounded">
<xs:element name="PluginIssue" type="PluginIssueHandlerType" minOccurs="0" />
<xs:element name="AbstractInstantiation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="AbstractMethodCall" type="IssueHandlerType" minOccurs="0" />
<xs:element name="AmbiguousConstantInheritance" type="ClassConstantIssueHandlerType" minOccurs="0" />
<xs:element name="ArgumentTypeCoercion" type="ArgumentIssueHandlerType" minOccurs="0" />
<xs:element name="AssignmentToVoid" type="IssueHandlerType" minOccurs="0" />
<xs:element name="CircularReference" type="IssueHandlerType" minOccurs="0" />
Expand Down Expand Up @@ -313,6 +313,7 @@
<xs:element name="InvalidToString" type="IssueHandlerType" minOccurs="0" />
<xs:element name="InvalidTraversableImplementation" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="InvalidTypeImport" type="IssueHandlerType" minOccurs="0" />
<xs:element name="LessSpecificClassConstantType" type="ClassConstantIssueHandlerType" minOccurs="0" />
<xs:element name="LessSpecificImplementedReturnType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="LessSpecificReturnStatement" type="IssueHandlerType" minOccurs="0" />
<xs:element name="LessSpecificReturnType" type="IssueHandlerType" minOccurs="0" />
Expand Down Expand Up @@ -358,10 +359,10 @@
<xs:element name="NamedArgumentNotAllowed" type="ArgumentIssueHandlerType" minOccurs="0" />
<xs:element name="NoEnumProperties" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="NoInterfaceProperties" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="NonStaticSelfCall" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NoValue" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NonInvariantDocblockPropertyType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NonInvariantPropertyType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NonStaticSelfCall" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NoValue" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NullableReturnStatement" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NullArgument" type="ArgumentIssueHandlerType" minOccurs="0" />
<xs:element name="NullArrayAccess" type="IssueHandlerType" minOccurs="0" />
Expand All @@ -372,11 +373,13 @@
<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="OverriddenInterfaceConstant" type="ClassConstantIssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenMethodAccess" type="IssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenPropertyAccess" type="PropertyIssueHandlerType" minOccurs="0" />
<xs:element name="ParadoxicalCondition" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ParamNameMismatch" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ParentNotFound" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PluginIssue" type="PluginIssueHandlerType" minOccurs="0" />
<xs:element name="PossibleRawObjectIteration" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyFalseArgument" type="ArgumentIssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyFalseIterator" type="IssueHandlerType" minOccurs="0" />
Expand Down Expand Up @@ -425,8 +428,8 @@
<xs:element name="RedundantCastGivenDocblockType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantCondition" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantConditionGivenDocblockType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantPropertyInitializationCheck" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantIdentityWithTrue" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantPropertyInitializationCheck" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ReferenceConstraintViolation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ReservedWord" type="IssueHandlerType" minOccurs="0" />
<xs:element name="StringIncrement" type="IssueHandlerType" minOccurs="0" />
Expand Down Expand Up @@ -484,8 +487,8 @@
<xs:element name="UnrecognizedStatement" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnresolvableConstant" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnresolvableInclude" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnsafeInstantiation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnsafeGenericInstantiation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnsafeInstantiation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClass" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedConstructor" type="MethodIssueHandlerType" minOccurs="0" />
Expand Down
42 changes: 42 additions & 0 deletions docs/running_psalm/issues/AmbiguousConstantInheritance.md
@@ -0,0 +1,42 @@
# AmbiguousConstantInheritance

Emitted when a constant is inherited from multiple sources.

```php
<?php

interface Foo
{
/** @var non-empty-string */
public const CONSTANT='foo';
}

interface Bar
{
/**
* @psalm-suppress OverriddenInterfaceConstant
* @var non-empty-string
*/
public const CONSTANT='bar';
}

interface Baz extends Foo, Bar {}
```

```php
<?php

interface Foo
{
/** @var non-empty-string */
public const CONSTANT='foo';
}

class Bar
{
/** @var non-empty-string */
public const CONSTANT='bar';
}

class Baz extends Bar implements Foo {}
```
28 changes: 28 additions & 0 deletions docs/running_psalm/issues/LessSpecificClassConstantType.md
@@ -0,0 +1,28 @@
# LessSpecificClassConstantType

Emitted when a constant type in a child is less specific than 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 */
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.
17 changes: 17 additions & 0 deletions docs/running_psalm/issues/OverriddenInterfaceConstant.md
@@ -0,0 +1,17 @@
# OverriddenInterfaceConstant

Emitted when a constant declared on an interface is overridden by a child (illegal in PHP < 8.1).

```php
<?php

interface Foo
{
public const BAR='baz';
}

interface Bar extends Foo
{
public const BAR='foobar';
}
```
3 changes: 3 additions & 0 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
Expand Up @@ -15,6 +15,7 @@
use Psalm\Exception\DocblockParseException;
use Psalm\FileManipulation;
use Psalm\Internal\Analyzer\Statements\Expression\Call\ClassTemplateParamCollector;
use Psalm\Internal\Analyzer\Statements\Expression\ClassConstAnalyzer;
use Psalm\Internal\Analyzer\Statements\Expression\Fetch\AtomicPropertyFetchAnalyzer;
use Psalm\Internal\FileManipulation\FileManipulationBuffer;
use Psalm\Internal\FileManipulation\PropertyDocblockManipulator;
Expand Down Expand Up @@ -526,6 +527,8 @@ public function analyze(
$statements_analyzer = new StatementsAnalyzer($this, new NodeDataProvider());
$statements_analyzer->analyze($member_stmts, $class_context, $global_context, true);

ClassConstAnalyzer::analyze($storage, $this->getCodebase());

$config = Config::getInstance();

if ($class instanceof PhpParser\Node\Stmt\Class_) {
Expand Down
36 changes: 36 additions & 0 deletions src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php
Expand Up @@ -6,12 +6,17 @@
use PhpParser;
use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\FileManipulation;
use Psalm\Internal\Analyzer\Statements\Expression\ClassConstAnalyzer;
use Psalm\Internal\FileManipulation\FileManipulationBuffer;
use Psalm\Internal\Provider\NodeDataProvider;
use Psalm\Issue\ParseError;
use Psalm\Issue\UndefinedInterface;
use Psalm\IssueBuffer;
use UnexpectedValueException;

use function strtolower;

/**
* @internal
*/
Expand All @@ -31,6 +36,8 @@ public function analyze(): void
throw new LogicException('Something went badly wrong');
}

$interface_context = new Context($this->fq_class_name);

$project_analyzer = $this->file_analyzer->project_analyzer;
$codebase = $project_analyzer->getCodebase();
$config = $project_analyzer->getConfig();
Expand Down Expand Up @@ -105,6 +112,7 @@ public function analyze(): void
);
}

$member_stmts = [];
foreach ($this->class->stmts as $stmt) {
if ($stmt instanceof PhpParser\Node\Stmt\ClassMethod) {
$method_analyzer = new MethodAnalyzer($stmt, $this);
Expand Down Expand Up @@ -140,7 +148,35 @@ public function analyze(): void
);

return;
} elseif ($stmt instanceof PhpParser\Node\Stmt\ClassConst) {
$member_stmts[] = $stmt;

foreach ($stmt->consts as $const) {
$const_id = strtolower($this->fq_class_name) . '::' . $const->name;

foreach ($codebase->class_constants_to_rename as $original_const_id => $new_const_name) {
if ($const_id === $original_const_id) {
$file_manipulations = [
new FileManipulation(
(int) $const->name->getAttribute('startFilePos'),
(int) $const->name->getAttribute('endFilePos') + 1,
$new_const_name
)
];

FileManipulationBuffer::add(
$this->getFilePath(),
$file_manipulations
);
}
}
}
}
}

$statements_analyzer = new StatementsAnalyzer($this, new NodeDataProvider());
$statements_analyzer->analyze($member_stmts, $interface_context, null, true);

ClassConstAnalyzer::analyze($this->storage, $this->getCodebase());
}
}

0 comments on commit 71f5b09

Please sign in to comment.