Skip to content

Commit

Permalink
Merge pull request #7154 from AndrolGenhald/feature/class-const-impro…
Browse files Browse the repository at this point in the history
…vements

Improve class constant static analysis
  • Loading branch information
orklah committed Jan 26, 2022
2 parents efe9c2b + b68c611 commit 66343de
Show file tree
Hide file tree
Showing 19 changed files with 457 additions and 23 deletions.
13 changes: 8 additions & 5 deletions config.xsd
Expand Up @@ -198,9 +198,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 @@ -292,6 +292,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 @@ -338,10 +339,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 @@ -352,11 +353,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 @@ -407,8 +410,8 @@
<xs:element name="RedundantConditionGivenDocblockType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantFunctionCall" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantFunctionCallGivenDocblockType" 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 @@ -466,8 +469,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
3 changes: 3 additions & 0 deletions docs/running_psalm/issues.md
Expand Up @@ -2,6 +2,7 @@

- [AbstractInstantiation](issues/AbstractInstantiation.md)
- [AbstractMethodCall](issues/AbstractMethodCall.md)
- [AmbiguousConstantInheritance](issues/AmbiguousConstantInheritance.md)
- [ArgumentTypeCoercion](issues/ArgumentTypeCoercion.md)
- [AssignmentToVoid](issues/AssignmentToVoid.md)
- [CircularReference](issues/CircularReference.md)
Expand Down Expand Up @@ -93,6 +94,7 @@
- [InvalidToString](issues/InvalidToString.md)
- [InvalidTraversableImplementation](issues/InvalidTraversableImplementation.md)
- [InvalidTypeImport](issues/InvalidTypeImport.md)
- [LessSpecificClassConstantType](issues/LessSpecificClassConstantType.md)
- [LessSpecificImplementedReturnType](issues/LessSpecificImplementedReturnType.md)
- [LessSpecificReturnStatement](issues/LessSpecificReturnStatement.md)
- [LessSpecificReturnType](issues/LessSpecificReturnType.md)
Expand Down Expand Up @@ -153,6 +155,7 @@
- [NullPropertyAssignment](issues/NullPropertyAssignment.md)
- [NullPropertyFetch](issues/NullPropertyFetch.md)
- [NullReference](issues/NullReference.md)
- [OverriddenInterfaceConstant](issues/OverriddenInterfaceConstant.md)
- [OverriddenMethodAccess](issues/OverriddenMethodAccess.md)
- [OverriddenPropertyAccess](issues/OverriddenPropertyAccess.md)
- [ParadoxicalCondition](issues/ParadoxicalCondition.md)
Expand Down
41 changes: 41 additions & 0 deletions docs/running_psalm/issues/AmbiguousConstantInheritance.md
@@ -0,0 +1,41 @@
# AmbiguousConstantInheritance

Emitted when a constant is inherited from multiple sources.

```php
<?php

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

interface Bar
{
/**
* @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 @@ -17,6 +17,7 @@
use Psalm\FileManipulation;
use Psalm\Internal\Analyzer\FunctionLike\ReturnTypeAnalyzer;
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 @@ -533,6 +534,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 @@ -7,12 +7,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 @@ -32,6 +37,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 @@ -106,6 +113,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 @@ -141,7 +149,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 66343de

Please sign in to comment.