From e23971ca29c176116eb4fef682ec632cfc4189d4 Mon Sep 17 00:00:00 2001 From: robchett Date: Thu, 31 Aug 2023 18:21:47 +0100 Subject: [PATCH 1/2] InheritorViolation was only being triggered on grand-childen classes Fixes #10167 --- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 17 +++++++++++++++++ .../Internal/Analyzer/ClassLikeAnalyzer.php | 19 ------------------- tests/ClassTest.php | 3 --- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php index bdb3573260c..bcd63764a0f 100644 --- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php @@ -36,6 +36,7 @@ use Psalm\Issue\ExtensionRequirementViolation; use Psalm\Issue\ImplementationRequirementViolation; use Psalm\Issue\InaccessibleMethod; +use Psalm\Issue\InheritorViolation; use Psalm\Issue\InternalClass; use Psalm\Issue\InvalidEnumCaseValue; use Psalm\Issue\InvalidExtendClass; @@ -271,6 +272,22 @@ public function analyze( ); } + $class_union = new Union([new TNamedObject($fq_class_name)]); + foreach ($storage->parent_classes + $storage->direct_class_interfaces as $parent_class) { + $parent_storage = $codebase->classlikes->getStorageFor($parent_class); + if ($parent_storage && $parent_storage->inheritors) { + if (!UnionTypeComparator::isContainedBy($codebase, $class_union, $parent_storage->inheritors)) { + IssueBuffer::maybeAdd( + new InheritorViolation( + 'Class ' . $fq_class_name . ' is not an allowed inheritor of parent class ' . $parent_class, + new CodeLocation($this, $this->class), + ), + $this->getSuppressedIssues(), + ); + } + } + } + if ($storage->template_types) { foreach ($storage->template_types as $param_name => $_) { diff --git a/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php index a6c909191b8..2c32967f805 100644 --- a/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php @@ -14,7 +14,6 @@ use Psalm\Internal\Type\TemplateResult; use Psalm\Internal\Type\TemplateStandinTypeReplacer; use Psalm\Issue\InaccessibleProperty; -use Psalm\Issue\InheritorViolation; use Psalm\Issue\InvalidClass; use Psalm\Issue\InvalidTemplateParam; use Psalm\Issue\MissingDependency; @@ -29,7 +28,6 @@ use Psalm\StatementsSource; use Psalm\Storage\ClassLikeStorage; use Psalm\Type; -use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Atomic\TTemplateParam; use Psalm\Type\Union; use UnexpectedValueException; @@ -332,23 +330,6 @@ public static function checkFullyQualifiedClassLikeName( return null; } - - $classUnion = new Union([new TNamedObject($fq_class_name)]); - foreach ($class_storage->parent_classes + $class_storage->direct_class_interfaces as $parent_class) { - $parent_storage = $codebase->classlikes->getStorageFor($parent_class); - if ($parent_storage && $parent_storage->inheritors) { - if (!UnionTypeComparator::isContainedBy($codebase, $classUnion, $parent_storage->inheritors)) { - IssueBuffer::maybeAdd( - new InheritorViolation( - 'Class ' . $fq_class_name . ' is not an allowed inheritor of parent class ' . $parent_class, - $code_location, - ), - $suppressed_issues, - ); - } - } - } - foreach ($class_storage->invalid_dependencies as $dependency_class_name => $_) { // if the implemented/extended class is stubbed, it may not yet have // been hydrated diff --git a/tests/ClassTest.php b/tests/ClassTest.php index 89778446a46..e1e85b10692 100644 --- a/tests/ClassTest.php +++ b/tests/ClassTest.php @@ -1393,7 +1393,6 @@ class Bar extends Foo {} */ class BaseClass {} class BazClass extends BaseClass {} // this is an error - $a = new BazClass(); PHP, 'error_message' => 'InheritorViolation', 'ignored_issues' => [], @@ -1406,7 +1405,6 @@ class BazClass extends BaseClass {} // this is an error */ interface BaseInterface {} class BazClass implements BaseInterface {} - $a = new BazClass(); PHP, 'error_message' => 'InheritorViolation', 'ignored_issues' => [], @@ -1423,7 +1421,6 @@ interface InterfaceA {} */ interface InterfaceB {} class BazClass implements InterFaceA, InterFaceB {} - $a = new BazClass(); PHP, 'error_message' => 'InheritorViolation', 'ignored_issues' => [], From d0c4d170b0f0dc7ed92402ba5b652a1560e944ef Mon Sep 17 00:00:00 2001 From: robchett Date: Thu, 31 Aug 2023 19:12:58 +0100 Subject: [PATCH 2/2] Apply psalm-inheritors to interfaces too --- .../Internal/Analyzer/InterfaceAnalyzer.php | 21 +++++++++++ tests/ClassTest.php | 37 ++++++++++++------- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php b/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php index dd2a107498b..c1f8ed6b4e5 100644 --- a/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php @@ -11,9 +11,13 @@ use Psalm\Internal\Analyzer\Statements\Expression\ClassConstAnalyzer; use Psalm\Internal\FileManipulation\FileManipulationBuffer; use Psalm\Internal\Provider\NodeDataProvider; +use Psalm\Internal\Type\Comparator\UnionTypeComparator; +use Psalm\Issue\InheritorViolation; use Psalm\Issue\ParseError; use Psalm\Issue\UndefinedInterface; use Psalm\IssueBuffer; +use Psalm\Type\Atomic\TNamedObject; +use Psalm\Type\Union; use UnexpectedValueException; use function strtolower; @@ -109,6 +113,23 @@ public function analyze(): void } } + $class_union = new Union([new TNamedObject($fq_interface_name)]); + foreach ($class_storage->direct_interface_parents as $parent_interface) { + $parent_storage = $codebase->classlikes->getStorageFor($parent_interface); + if ($parent_storage && $parent_storage->inheritors) { + if (!UnionTypeComparator::isContainedBy($codebase, $class_union, $parent_storage->inheritors)) { + IssueBuffer::maybeAdd( + new InheritorViolation( + 'Interface ' . $fq_interface_name . ' + is not an allowed inheritor of parent interface ' . $parent_interface, + new CodeLocation($this, $this->class), + ), + $this->getSuppressedIssues(), + ); + } + } + } + $fq_interface_name = $this->getFQCLN(); if (!$fq_interface_name) { diff --git a/tests/ClassTest.php b/tests/ClassTest.php index e1e85b10692..110b4da95d3 100644 --- a/tests/ClassTest.php +++ b/tests/ClassTest.php @@ -857,7 +857,6 @@ private final function __construct() {} */ class BaseClass {} class FooClass extends BaseClass {} - $a = new FooClass(); PHP, ], 'unionInheritorIsAllowed' => [ @@ -868,9 +867,7 @@ class FooClass extends BaseClass {} */ class BaseClass {} class FooClass extends BaseClass {} - $a = new FooClass(); class BarClass extends FooClass {} - $b = new BarClass(); PHP, ], 'multiInheritorIsAllowed' => [ @@ -881,9 +878,7 @@ class BarClass extends FooClass {} */ class BaseClass {} class FooClass extends BaseClass {} - $a = new FooClass(); class BarClass extends FooClass {} - $b = new BarClass(); PHP, ], 'skippedInheritorIsAllowed' => [ @@ -894,9 +889,7 @@ class BarClass extends FooClass {} */ class BaseClass {} class FooClass extends BaseClass {} - $a = new FooClass(); class BarClass extends FooClass {} - $b = new BarClass(); PHP, ], 'CompositeInheritorIsAllowed' => [ @@ -908,7 +901,6 @@ class BarClass extends FooClass {} class BaseClass {} interface FooInterface {} class BarClass extends BaseClass implements FooInterface {} - $b = new BarClass(); PHP, ], 'InterfaceInheritorIsAllowed' => [ @@ -919,12 +911,10 @@ class BarClass extends BaseClass implements FooInterface {} */ interface BaseInterface {} class FooClass implements BaseInterface {} - $a = new FooClass(); class BarClass implements BaseInterface {} - $b = new BarClass(); PHP, - ], - 'MultiInterfaceInheritorIsAllowed' => [ + ], + 'MultiInterfaceInheritorIsAllowed' => [ 'code' => <<<'PHP' [ + 'code' => <<<'PHP' + 'InheritorViolation', 'ignored_issues' => [], ], + 'interfaceCannotImplementIfNotInInheritors' => [ + 'code' => <<<'PHP' + 'InheritorViolation', + 'ignored_issues' => [], + ], 'UnfulfilledInterfaceInheritors' => [ 'code' => <<<'PHP'