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.

Sign up for GitHub

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

Jump to bottom

Ensure all template parameters are specified in all extended|used classlikes #7492

Merged
merged 6 commits into from Jan 31, 2022
Merged

Ensure all template parameters are specified in all extended|used classlikes #7492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8d0c1c6
Ensure all template parameters are specified in classes, interfaces, …
danog Jan 26, 2022
ada778e
Remove unused field
danog Jan 26, 2022
52872c0
Fixes
danog Jan 26, 2022
54b3f24
Update
danog Jan 26, 2022
d0c9272
Update baseline
danog Jan 26, 2022
6f51b4a
Fix tests
danog Jan 31, 2022
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 UPGRADING.md
View file
Open in desktop
Expand Up @@ -122,6 +122,7 @@
- [BC] Method `Psalm\Type\Union::hasFormerStaticObject()` was renamed to `hasStaticObject()`
- [BC] Function assertions (from `@psalm-assert Foo $bar`) have been converted from strings to specific `Assertion` objects.
- [BC] Property `Psalm\Storage\ClassLikeStorage::$invalid_dependencies` changed from `array<string>` to `array<string, true>`.
- [BC] Property `Psalm\Storage\ClassLikeStorage::$template_extended_count` was renamed to `$template_type_extends_count`, its type was changed from `int|null` to `array<string, int>|null`.
- [BC] Event classes became final and their constructors were marked `@internal`:
- `Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent`
- `Psalm\Plugin\EventHandler\Event\AfterAnalysisEvent`
Expand Down
3 changes: 3 additions & 0 deletions docs/running_psalm/issues/InvalidTraversableImplementation.md
View file
Open in desktop
Expand Up @@ -6,5 +6,8 @@ implemented by implementing either `IteratorAggregate` or `Iterator`
```php
<?php

/**
* @implements Traversable<mixed, mixed>
*/
final class C implements Traversable {} // will cause fatal error
```
27 changes: 1 addition & 26 deletions psalm-baseline.xml
View file
Open in desktop
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="dev-master@093edcbe1d5c90ad574efcddd62fe741819e7cf2">
<files psalm-version="dev-master@485e516088a22efd18e902926a2efdad1b040d72">
<file src="examples/TemplateChecker.php">
<PossiblyUndefinedIntArrayOffset occurrences="2">
<code>$comment_block-&gt;tags['variablesfrom'][0]</code>
Expand All @@ -18,13 +18,6 @@
<code>$symbol_parts[1]</code>
</PossiblyUndefinedIntArrayOffset>
</file>
<file src="src/Psalm/Config.php">
<DeprecatedMethod occurrences="3">
<code>getAdditionalFileExtensions</code>
<code>getAdditionalFileTypeAnalyzers</code>
<code>getAdditionalFileTypeScanners</code>
</DeprecatedMethod>
</file>
<file src="src/Psalm/Config/FileFilter.php">
<PossiblyUndefinedIntArrayOffset occurrences="1">
<code>explode('::', $method_id)[1]</code>
Expand All @@ -38,10 +31,6 @@
</PossiblyUndefinedIntArrayOffset>
</file>
<file src="src/Psalm/Internal/Analyzer/ClassAnalyzer.php">
<DeprecatedProperty occurrences="2">
<code>$storage-&gt;template_extended_count</code>
<code>$storage-&gt;template_extended_count</code>
</DeprecatedProperty>
<PossiblyUndefinedIntArrayOffset occurrences="3">
<code>$comments[0]</code>
<code>$stmt-&gt;props[0]</code>
Expand All @@ -61,11 +50,6 @@
<code>$iterator_atomic_type-&gt;type_params[1]</code>
</PossiblyUndefinedIntArrayOffset>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php">
<ReferenceConstraintViolation occurrences="1">
<code>$newly_reconciled_var_ids</code>
</ReferenceConstraintViolation>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php">
<PossiblyUndefinedIntArrayOffset occurrences="1">
<code>$pre_conditions[0]</code>
Expand Down Expand Up @@ -274,9 +258,6 @@
</PossiblyUndefinedIntArrayOffset>
</file>
<file src="src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php">
<DeprecatedProperty occurrences="1">
<code>$storage-&gt;template_extended_count</code>
</DeprecatedProperty>
<PossiblyUndefinedIntArrayOffset occurrences="3">
<code>$l[4]</code>
<code>$r[4]</code>
Expand Down Expand Up @@ -361,12 +342,6 @@
<code>VirtualConst</code>
</PropertyNotSetInConstructor>
</file>
<file src="src/Psalm/PluginRegistrationSocket.php">
<DeprecatedMethod occurrences="2">
<code>addFileExtension</code>
<code>addFileExtension</code>
</DeprecatedMethod>
</file>
<file src="src/Psalm/Type/Atomic.php">
<PossiblyUndefinedIntArrayOffset occurrences="1">
<code>array_keys($template_type_map[$value])[0]</code>
Expand Down
242 changes: 30 additions & 212 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
View file
Open in desktop
Expand Up @@ -39,14 +39,12 @@
use Psalm\Issue\InternalClass;
use Psalm\Issue\InvalidEnumCaseValue;
use Psalm\Issue\InvalidExtendClass;
use Psalm\Issue\InvalidTemplateParam;
use Psalm\Issue\InvalidTraversableImplementation;
use Psalm\Issue\MethodSignatureMismatch;
use Psalm\Issue\MismatchingDocblockPropertyType;
use Psalm\Issue\MissingConstructor;
use Psalm\Issue\MissingImmutableAnnotation;
use Psalm\Issue\MissingPropertyType;
use Psalm\Issue\MissingTemplateParam;
use Psalm\Issue\MutableDependency;
use Psalm\Issue\NoEnumProperties;
use Psalm\Issue\NonInvariantDocblockPropertyType;
Expand All @@ -55,7 +53,6 @@
use Psalm\Issue\ParseError;
use Psalm\Issue\PropertyNotSetInConstructor;
use Psalm\Issue\ReservedWord;
use Psalm\Issue\TooManyTemplateParams;
use Psalm\Issue\UndefinedClass;
use Psalm\Issue\UndefinedInterface;
use Psalm\Issue\UndefinedTrait;
Expand Down Expand Up @@ -87,7 +84,6 @@
use function array_map;
use function array_merge;
use function array_pop;
use function array_search;
use function array_values;
use function assert;
use function count;
Expand Down Expand Up @@ -586,18 +582,16 @@ public function analyze(

$fq_trait_name_lc = strtolower($fq_trait_name);

if (isset($storage->template_type_uses_count[$fq_trait_name_lc])) {
$this->checkTemplateParams(
$codebase,
$storage,
$trait_storage,
new CodeLocation(
$this,
$trait
),
$storage->template_type_uses_count[$fq_trait_name_lc]
);
}
$this->checkTemplateParams(
$codebase,
$storage,
$trait_storage,
new CodeLocation(
$this,
$trait
),
$storage->template_type_uses_count[$fq_trait_name_lc] ?? 0
);

foreach ($trait_node->stmts as $trait_stmt) {
if ($trait_stmt instanceof PhpParser\Node\Stmt\Property) {
Expand Down Expand Up @@ -1974,178 +1968,6 @@ public static function analyzeClassMethodReturnType(
);
}

private function checkTemplateParams(
Codebase $codebase,
ClassLikeStorage $storage,
ClassLikeStorage $parent_storage,
CodeLocation $code_location,
int $given_param_count
): void {
$expected_param_count = $parent_storage->template_types === null
? 0
: count($parent_storage->template_types);

if ($expected_param_count > $given_param_count) {
IssueBuffer::maybeAdd(
new MissingTemplateParam(
$storage->name . ' has missing template params when extending ' . $parent_storage->name
. ' , expecting ' . $expected_param_count,
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
} elseif ($expected_param_count < $given_param_count) {
IssueBuffer::maybeAdd(
new TooManyTemplateParams(
$storage->name . ' has too many template params when extending ' . $parent_storage->name
. ' , expecting ' . $expected_param_count,
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
}

$storage_param_count = ($storage->template_types ? count($storage->template_types) : 0);

if ($parent_storage->enforce_template_inheritance
&& $expected_param_count !== $storage_param_count
) {
if ($expected_param_count > $storage_param_count) {
IssueBuffer::maybeAdd(
new MissingTemplateParam(
$storage->name . ' requires the same number of template params as ' . $parent_storage->name
. ' but saw ' . $storage_param_count,
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
} else {
IssueBuffer::maybeAdd(
new TooManyTemplateParams(
$storage->name . ' requires the same number of template params as ' . $parent_storage->name
. ' but saw ' . $storage_param_count,
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
}
}

if ($parent_storage->template_types && $storage->template_extended_params) {
$i = 0;

$previous_extended = [];

foreach ($parent_storage->template_types as $template_name => $type_map) {
// declares the variables
foreach ($type_map as $declaring_class => $template_type) {
}

if (isset($storage->template_extended_params[$parent_storage->name][$template_name])) {
$extended_type = $storage->template_extended_params[$parent_storage->name][$template_name];

if (isset($parent_storage->template_covariants[$i])
&& !$parent_storage->template_covariants[$i]
) {
foreach ($extended_type->getAtomicTypes() as $t) {
if ($t instanceof TTemplateParam
&& $storage->template_types
&& $storage->template_covariants
&& ($local_offset
= array_search($t->param_name, array_keys($storage->template_types)))
!== false
&& !empty($storage->template_covariants[$local_offset])
) {
IssueBuffer::maybeAdd(
new InvalidTemplateParam(
'Cannot extend an invariant template param ' . $template_name
. ' into a covariant context',
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
}
}
}

if ($parent_storage->enforce_template_inheritance) {
foreach ($extended_type->getAtomicTypes() as $t) {
if (!$t instanceof TTemplateParam
|| !isset($storage->template_types[$t->param_name])
) {
IssueBuffer::maybeAdd(
new InvalidTemplateParam(
'Cannot extend a strictly-enforced parent template param '
. $template_name
. ' with a non-template type',
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
} elseif ($storage->template_types[$t->param_name][$storage->name]->getId()
!== $template_type->getId()
) {
IssueBuffer::maybeAdd(
new InvalidTemplateParam(
'Cannot extend a strictly-enforced parent template param '
. $template_name
. ' with constraint ' . $template_type->getId()
. ' with a child template param ' . $t->param_name
. ' with different constraint '
. $storage->template_types[$t->param_name][$storage->name]->getId(),
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
}
}
}

if (!$template_type->isMixed()) {
$template_type_copy = clone $template_type;

$template_result = new TemplateResult(
$previous_extended ?: [],
[]
);

$template_type_copy = TemplateStandinTypeReplacer::replace(
$template_type_copy,
$template_result,
$codebase,
null,
$extended_type,
null,
null
);

if (!UnionTypeComparator::isContainedBy($codebase, $extended_type, $template_type_copy)) {
IssueBuffer::maybeAdd(
new InvalidTemplateParam(
'Extended template param ' . $template_name
. ' expects type ' . $template_type_copy->getId()
. ', type ' . $extended_type->getId() . ' given',
$code_location
),
$storage->suppressed_issues + $this->getSuppressedIssues()
);
} else {
$previous_extended[$template_name] = [
$declaring_class => $extended_type
];
}
} else {
$previous_extended[$template_name] = [
$declaring_class => $extended_type
];
}
}

$i++;
}
}
}

/**
* @param PhpParser\Node\Stmt\Class_|PhpParser\Node\Stmt\Enum_ $class
*/
Expand Down Expand Up @@ -2234,15 +2056,13 @@ private function checkImplementedInterfaces(
);
}

if (isset($storage->template_type_implements_count[$fq_interface_name_lc])) {
$this->checkTemplateParams(
$codebase,
$storage,
$interface_storage,
$code_location,
$storage->template_type_implements_count[$fq_interface_name_lc]
);
}
$this->checkTemplateParams(
$codebase,
$storage,
$interface_storage,
$code_location,
$storage->template_type_implements_count[$fq_interface_name_lc] ?? 0
);
}

foreach ($storage->class_implements as $fq_interface_name_lc => $fq_interface_name) {
Expand Down Expand Up @@ -2555,22 +2375,20 @@ private function checkParentClass(
);
}

if ($storage->template_extended_count !== null || $parent_class_storage->enforce_template_inheritance) {
$code_location = new CodeLocation(
$this,
$class->name ?: $class,
$class_context->include_location ?? null,
true
);
$code_location = new CodeLocation(
$this,
$class->name ?: $class,
$class_context->include_location ?? null,
true
);

$this->checkTemplateParams(
$codebase,
$storage,
$parent_class_storage,
$code_location,
$storage->template_extended_count ?? 0
);
}
$this->checkTemplateParams(
$codebase,
$storage,
$parent_class_storage,
$code_location,
$storage->template_type_extends_count[$parent_fq_class_name] ?? 0
);
} catch (InvalidArgumentException $e) {
// do nothing
}
Expand Down