diff --git a/.github/workflows/code_analysis.yaml b/.github/workflows/code_analysis.yaml index 3600e580d46..095bc05b37f 100644 --- a/.github/workflows/code_analysis.yaml +++ b/.github/workflows/code_analysis.yaml @@ -53,7 +53,7 @@ jobs: - name: 'Active Classes' run: | - vendor/bin/class-leak check bin config src rules utils --skip-suffix "Rector" --skip-type="Rector\\Utils\\Compiler\\Unprefixer" --skip-type="Rector\\NodeCollector\\BinaryOpConditionsCollector" --skip-type="Rector\\Set\\Contract\\SetListInterface" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\DuplicateRegistrationFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\SingleRegistrationFactory" --skip-type="Rector\\ChangesReporting\\Contract\\Output\\OutputFormatterInterface" --skip-type="Rector\\NodeTypeResolver\\Contract\\NodeTypeResolverInterface" --skip-type="Rector\\CodingStyle\\Contract\\ClassNameImport\\ClassNameImportSkipVoterInterface" --skip-type="Rector\\Php80\\Contract\\ConverterAttributeDecoratorInterface" --skip-type="Rector\\NodeNameResolver\\Contract\\NodeNameResolverInterface" --skip-type="Rector\\StaticTypeMapper\\Contract\\PhpDocParser\\PhpDocTypeMapperInterface" --skip-type="Rector\\PhpAttribute\\Contract\\AnnotationToAttributeMapperInterface" + vendor/bin/class-leak check bin config src rules utils --skip-suffix "Rector" --skip-type="Rector\\Utils\\Compiler\\Unprefixer" --skip-type="Rector\\NodeCollector\\BinaryOpConditionsCollector" --skip-type="Rector\\Set\\Contract\\SetListInterface" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\DuplicateRegistrationFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\SingleRegistrationFactory" --skip-type="Rector\\ChangesReporting\\Contract\\Output\\OutputFormatterInterface" --skip-type="Rector\\NodeTypeResolver\\Contract\\NodeTypeResolverInterface" --skip-type="Rector\\CodingStyle\\Contract\\ClassNameImport\\ClassNameImportSkipVoterInterface" --skip-type="Rector\\Php80\\Contract\\ConverterAttributeDecoratorInterface" --skip-type="Rector\\NodeNameResolver\\Contract\\NodeNameResolverInterface" --skip-type="Rector\\StaticTypeMapper\\Contract\\PhpDocParser\\PhpDocTypeMapperInterface" --skip-type="Rector\\PhpAttribute\\Contract\\AnnotationToAttributeMapperInterface" --skip-type="Rector\\PHPStanStaticTypeMapper\\Contract\\TypeMapperInterface" - name: 'Compatible PHPStan versions' diff --git a/src/DependencyInjection/LazyContainerFactory.php b/src/DependencyInjection/LazyContainerFactory.php index eee3925cd2a..e759c2eaf1a 100644 --- a/src/DependencyInjection/LazyContainerFactory.php +++ b/src/DependencyInjection/LazyContainerFactory.php @@ -73,37 +73,11 @@ use Rector\PhpParser\NodeVisitor\PhpVersionConditionNodeVisitor; use Rector\PhpParser\NodeVisitor\PropertyOrClassConstDefaultNodeVisitor; use Rector\PhpParser\NodeVisitor\StaticVariableNodeVisitor; -use Rector\PHPStanStaticTypeMapper\Contract\TypeMapperInterface; use Rector\PHPStanStaticTypeMapper\PHPStanStaticTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\AccessoryArrayTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\AccessoryStringTypeMapper; use Rector\PHPStanStaticTypeMapper\TypeMapper\ArrayTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\BooleanTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\CallableTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\ClassStringTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\ClosureTypeMapper; use Rector\PHPStanStaticTypeMapper\TypeMapper\ConditionalTypeForParameterMapper; use Rector\PHPStanStaticTypeMapper\TypeMapper\ConditionalTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\ConstantArrayTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\FloatTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\HasMethodTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\HasPropertyTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\IntegerTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\IntersectionTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\IterableTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\MixedTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\NeverTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\NullTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\ObjectTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\ObjectWithoutClassTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\ParentStaticTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\ResourceTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\StaticTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\StrictMixedTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\StringTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\TypeWithClassNameTypeMapper; use Rector\PHPStanStaticTypeMapper\TypeMapper\UnionTypeMapper; -use Rector\PHPStanStaticTypeMapper\TypeMapper\VoidTypeMapper; use Rector\PostRector\Application\PostFileProcessor; use Rector\Rector\AbstractRector; use Rector\Skipper\Skipper\Skipper; @@ -154,41 +128,6 @@ final class LazyContainerFactory ArgNotAcceptingClosureNodeVisitor::class, ]; - /** - * @var array> - */ - private const array TYPE_MAPPER_CLASSES = [ - AccessoryArrayTypeMapper::class, - AccessoryStringTypeMapper::class, - ConstantArrayTypeMapper::class, - ArrayTypeMapper::class, - BooleanTypeMapper::class, - CallableTypeMapper::class, - ClassStringTypeMapper::class, - ClosureTypeMapper::class, - ConditionalTypeForParameterMapper::class, - ConditionalTypeMapper::class, - FloatTypeMapper::class, - HasMethodTypeMapper::class, - HasPropertyTypeMapper::class, - IntegerTypeMapper::class, - IntersectionTypeMapper::class, - IterableTypeMapper::class, - MixedTypeMapper::class, - NeverTypeMapper::class, - NullTypeMapper::class, - ObjectTypeMapper::class, - ObjectWithoutClassTypeMapper::class, - ParentStaticTypeMapper::class, - ResourceTypeMapper::class, - StaticTypeMapper::class, - StrictMixedTypeMapper::class, - StringTypeMapper::class, - TypeWithClassNameTypeMapper::class, - UnionTypeMapper::class, - VoidTypeMapper::class, - ]; - /** * @var array> */ @@ -394,7 +333,7 @@ private function registerTaggedServices(RectorConfig $rectorConfig): void // PHP 8.0 attributes $rectorConfig->autodiscover(__DIR__ . '/../PhpAttribute/AnnotationToAttributeMapper'); - $this->registerTagged($rectorConfig, self::TYPE_MAPPER_CLASSES, TypeMapperInterface::class); + $rectorConfig->autodiscover(__DIR__ . '/../PHPStanStaticTypeMapper/TypeMapper'); $rectorConfig->autodiscover(__DIR__ . '/../StaticTypeMapper/PhpDocParser'); $rectorConfig->autodiscover(__DIR__ . '/../NodeNameResolver/NodeNameResolver'); $rectorConfig->autodiscover(__DIR__ . '/../NodeTypeResolver/NodeTypeResolver'); diff --git a/src/PHPStanStaticTypeMapper/PHPStanStaticTypeMapper.php b/src/PHPStanStaticTypeMapper/PHPStanStaticTypeMapper.php index a2a75b7a1d9..5dbdbb96081 100644 --- a/src/PHPStanStaticTypeMapper/PHPStanStaticTypeMapper.php +++ b/src/PHPStanStaticTypeMapper/PHPStanStaticTypeMapper.php @@ -27,15 +27,12 @@ public function __construct( public function mapToPHPStanPhpDocTypeNode(Type $type): TypeNode { - foreach ($this->typeMappers as $typeMapper) { - if (! $this->doesTypeMatch($type, $typeMapper)) { - continue; - } - - return $typeMapper->mapToPHPStanPhpDocTypeNode($type); + $typeMapper = $this->matchTypeMapper($type); + if (! $typeMapper instanceof TypeMapperInterface) { + throw new NotImplementedYetException(__METHOD__ . ' for ' . $type::class); } - throw new NotImplementedYetException(__METHOD__ . ' for ' . $type::class); + return $typeMapper->mapToPHPStanPhpDocTypeNode($type); } /** @@ -43,22 +40,38 @@ public function mapToPHPStanPhpDocTypeNode(Type $type): TypeNode */ public function mapToPhpParserNode(Type $type, string $typeKind): Name|ComplexType|Identifier|null { - foreach ($this->typeMappers as $typeMapper) { - if (! $this->doesTypeMatch($type, $typeMapper)) { - continue; - } - - return $typeMapper->mapToPhpParserNode($type, $typeKind); + $typeMapper = $this->matchTypeMapper($type); + if (! $typeMapper instanceof TypeMapperInterface) { + throw new NotImplementedYetException(__METHOD__ . ' for ' . $type::class); } - throw new NotImplementedYetException(__METHOD__ . ' for ' . $type::class); + return $typeMapper->mapToPhpParserNode($type, $typeKind); } /** - * @param TypeMapperInterface $typeMapper + * Match the most specific mapper: when a type is handled by both a mapper for a parent + * class and one for its subclass, the subclass mapper wins, regardless of registration order. + * + * @return TypeMapperInterface|null */ - private function doesTypeMatch(Type $type, TypeMapperInterface $typeMapper): bool + private function matchTypeMapper(Type $type): ?TypeMapperInterface { - return array_any($typeMapper->getNodeClasses(), fn (string $nodeClass): bool => $type instanceof $nodeClass); + $matchedTypeMapper = null; + $matchedNodeClass = null; + + foreach ($this->typeMappers as $typeMapper) { + foreach ($typeMapper->getNodeClasses() as $nodeClass) { + if (! $type instanceof $nodeClass) { + continue; + } + + if ($matchedNodeClass === null || is_a($nodeClass, $matchedNodeClass, true)) { + $matchedNodeClass = $nodeClass; + $matchedTypeMapper = $typeMapper; + } + } + } + + return $matchedTypeMapper; } } diff --git a/tests/PHPStanStaticTypeMapper/TypeMapperOrderTest.php b/tests/PHPStanStaticTypeMapper/TypeMapperOrderTest.php index 2010b2ae5cc..3a357eec35a 100644 --- a/tests/PHPStanStaticTypeMapper/TypeMapperOrderTest.php +++ b/tests/PHPStanStaticTypeMapper/TypeMapperOrderTest.php @@ -4,48 +4,35 @@ namespace Rector\Tests\PHPStanStaticTypeMapper; -use Rector\PHPStanStaticTypeMapper\Contract\TypeMapperInterface; +use PHPStan\Type\ClassStringType; +use PHPStan\Type\StringType; +use Rector\PHPStanStaticTypeMapper\PHPStanStaticTypeMapper; use Rector\Testing\PHPUnit\AbstractLazyTestCase; final class TypeMapperOrderTest extends AbstractLazyTestCase { - /** - * The mappers are matched with is_a() in registration order, so a mapper for a child type - * must come before the mapper for its parent type. Otherwise the parent one always wins - * and the child one is never reached. - */ - public function testChildTypeMapperIsRegisteredBeforeItsParent(): void + private PHPStanStaticTypeMapper $phpStanStaticTypeMapper; + + protected function setUp(): void { - $typeMappers = $this->resolveTypeMappers(); - - foreach ($typeMappers as $position => $typeMapper) { - foreach ($typeMapper->getNodeClasses() as $nodeClass) { - for ($earlierPosition = 0; $earlierPosition < $position; ++$earlierPosition) { - $earlierTypeMapper = $typeMappers[$earlierPosition]; - - foreach ($earlierTypeMapper->getNodeClasses() as $earlierNodeClass) { - $this->assertFalse(is_a($nodeClass, $earlierNodeClass, true), sprintf( - 'The "%s" is registered after "%s", but "%s" is a "%s". It can never be reached, register it earlier.', - $typeMapper::class, - $earlierTypeMapper::class, - $nodeClass, - $earlierNodeClass - )); - } - } - } - } + parent::setUp(); + + $this->phpStanStaticTypeMapper = $this->make(PHPStanStaticTypeMapper::class); } /** - * @return TypeMapperInterface[] + * The most specific mapper wins regardless of registration order: ClassStringType extends + * StringType, so the ClassStringType mapper must match it over the StringType one. */ - private function resolveTypeMappers(): array + public function testMostSpecificMapperWinsOverParent(): void { - $typeMappers = self::getContainer()->findByContract(TypeMapperInterface::class); - - $this->assertNotEmpty($typeMappers); + $typeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode(new ClassStringType()); + $this->assertSame('class-string', (string) $typeNode); + } - return $typeMappers; + public function testParentMapperStillMatchesParentType(): void + { + $typeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode(new StringType()); + $this->assertSame('string', (string) $typeNode); } }