From edbe641ce2fcd23c6c9ff8cb0cf42fab4ae219c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pudil?= Date: Sun, 12 Feb 2023 11:25:11 +0100 Subject: [PATCH 1/5] wip: hide class-level template types from static members Amp-Thread-ID: https://ampcode.com/threads/T-01a19219-6ecc-7668-9fe2-6e4d943f7b8e --- src/Type/FileTypeMapper.php | 10 ++-- .../nsrt/static-methods-template-types.php | 49 +++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/static-methods-template-types.php diff --git a/src/Type/FileTypeMapper.php b/src/Type/FileTypeMapper.php index a61ad12860e..920bb804640 100644 --- a/src/Type/FileTypeMapper.php +++ b/src/Type/FileTypeMapper.php @@ -553,13 +553,18 @@ function (Node $node) use ($fileName, $lookForTrait, &$traitFound, $traitMethodA } if ($node instanceof Node\Stmt\ClassLike || $node instanceof Node\Stmt\ClassMethod || $node instanceof Node\Stmt\Function_ || $node instanceof Node\PropertyHook) { + $parentNameScope = array_last($typeMapStack); + + // static methods exist in their own scope + if ($node instanceof Node\Stmt\ClassMethod && $node->isStatic()) { + $parentNameScope = null; + } + if ($phpDocNode !== null) { if ($node instanceof Node\Stmt\ClassLike) { $typeAliasStack[] = $this->getTypeAliasesMap($phpDocNode); } - $parentNameScope = array_last($typeMapStack); - $typeMapStack[] = new IntermediaryNameScope( $namespaceUsesTracker->getNamespaceUses(), $className, @@ -572,7 +577,6 @@ function (Node $node) use ($fileName, $lookForTrait, &$traitFound, $traitMethodA } elseif ($node instanceof Node\Stmt\ClassLike) { $typeAliasStack[] = []; } else { - $parentNameScope = array_last($typeMapStack); $typeMapStack[] = new IntermediaryNameScope( $namespaceUsesTracker->getNamespaceUses(), $className, diff --git a/tests/PHPStan/Analyser/nsrt/static-methods-template-types.php b/tests/PHPStan/Analyser/nsrt/static-methods-template-types.php new file mode 100644 index 00000000000..065bd86dfd5 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/static-methods-template-types.php @@ -0,0 +1,49 @@ + Date: Sun, 12 Feb 2023 20:10:15 +0100 Subject: [PATCH 2/5] Test unknown type --- .../ExistingClassesInTypehintsRuleTest.php | 4 ++++ .../PHPStan/Rules/Methods/data/typehints.php | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/tests/PHPStan/Rules/Methods/ExistingClassesInTypehintsRuleTest.php b/tests/PHPStan/Rules/Methods/ExistingClassesInTypehintsRuleTest.php index 1a5628f6584..3b1546da5a6 100644 --- a/tests/PHPStan/Rules/Methods/ExistingClassesInTypehintsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ExistingClassesInTypehintsRuleTest.php @@ -153,6 +153,10 @@ public function testExistingClassInTypehint(): void 'Template type U of method TestMethodTypehints\TemplateTypeMissingInParameter::doFoo() is not referenced in a parameter.', 130, ], + [ + 'Method TestMethodTypehints\TemplateInStaticMethod::doBar() has invalid return type TestMethodTypehints\T.', + 163, + ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/typehints.php b/tests/PHPStan/Rules/Methods/data/typehints.php index f967e44906d..a06bded239e 100644 --- a/tests/PHPStan/Rules/Methods/data/typehints.php +++ b/tests/PHPStan/Rules/Methods/data/typehints.php @@ -142,3 +142,27 @@ public function doBar(string $class): void } } + +/** + * @template T + */ +class TemplateInStaticMethod +{ + + /** + * @return T + */ + public function doFoo() + { + + } + + /** + * @return T + */ + public static function doBar() + { + + } + +} From 8c76be616b09f35aef47a24843b73b5f9c73f1ea Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 12 Feb 2023 20:13:31 +0100 Subject: [PATCH 3/5] Forget class-level templates in static memebers only in bleeding edge Amp-Thread-ID: https://ampcode.com/threads/T-01a19219-6ecc-7668-9fe2-6e4d943f7b8e --- conf/bleedingEdge.neon | 1 + conf/config.neon | 1 + conf/parametersSchema.neon | 1 + conf/services.neon | 1 + src/Type/FileTypeMapper.php | 15 +++++++++++++-- tests/PHPStan/Type/FileTypeMapperTest.php | 1 + 6 files changed, 18 insertions(+), 2 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 7098877fb44..d1027950124 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -34,3 +34,4 @@ parameters: unusedVariable: true unusedParameters: true reuseCollectedDataRuleErrors: true + disallowClassLevelTemplatesInStatic: true diff --git a/conf/config.neon b/conf/config.neon index 0eb6c0c764b..5e907acf641 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -66,6 +66,7 @@ parameters: unusedVariable: false unusedParameters: false reuseCollectedDataRuleErrors: false + disallowClassLevelTemplatesInStatic: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 62dab9d9c3b..71f5292fb13 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -63,6 +63,7 @@ parametersSchema: unusedVariable: bool() unusedParameters: bool() reuseCollectedDataRuleErrors: bool() + disallowClassLevelTemplatesInStatic: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/conf/services.neon b/conf/services.neon index 1ec53e8e20c..1c73a51c84a 100644 --- a/conf/services.neon +++ b/conf/services.neon @@ -138,6 +138,7 @@ services: phpParser: @stubParser resolvedPhpDocBlockCacheCountMax: %cache.resolvedPhpDocBlockCacheCountMax% nameScopeMapMemoryCacheCountMax: %cache.nameScopeMapMemoryCacheCountMax% + disallowClassLevelTemplatesInStatic: %featureToggles.disallowClassLevelTemplatesInStatic% autowired: false fileExcluderAnalyse: diff --git a/src/Type/FileTypeMapper.php b/src/Type/FileTypeMapper.php index 920bb804640..41628c2af00 100644 --- a/src/Type/FileTypeMapper.php +++ b/src/Type/FileTypeMapper.php @@ -91,6 +91,8 @@ public function __construct( private int $resolvedPhpDocBlockCacheCountMax, #[AutowiredParameter(ref: '%cache.nameScopeMapMemoryCacheCountMax%')] int $nameScopeMapMemoryCacheCountMax, + #[AutowiredParameter(ref: '%featureToggles.disallowClassLevelTemplatesInStatic%')] + private bool $disallowClassLevelTemplatesInStatic, ) { // 0 kept one entry here rather than meaning "no limit" as it does for the other bounded @@ -387,7 +389,12 @@ private function getNameScopeMap(string $fileName): array // a map built from the simplified AST of a file outside the analysed paths lacks // the name scopes of classes declared inside function bodies, so it must not be // reused once the file gets analysed, nor the other way around - $cacheKey = sprintf('ftm-%s%s', $this->returnsSimplifiedAst($fileName) ? 'simplified-' : '', $fileName); + $cacheKey = sprintf( + 'ftm-%s%s%s', + $this->returnsSimplifiedAst($fileName) ? 'simplified-' : '', + $this->disallowClassLevelTemplatesInStatic ? 'static-own-scope-' : '', + $fileName, + ); // v6: v5 entries may be poisoned by the turbo comment-loss bug // (https://github.com/phpstan/phpstan/issues/15037) - a parse that // silently dropped every comment cached PHPDoc-less name-scope maps, @@ -556,7 +563,11 @@ function (Node $node) use ($fileName, $lookForTrait, &$traitFound, $traitMethodA $parentNameScope = array_last($typeMapStack); // static methods exist in their own scope - if ($node instanceof Node\Stmt\ClassMethod && $node->isStatic()) { + if ( + $this->disallowClassLevelTemplatesInStatic + && $node instanceof Node\Stmt\ClassMethod + && $node->isStatic() + ) { $parentNameScope = null; } diff --git a/tests/PHPStan/Type/FileTypeMapperTest.php b/tests/PHPStan/Type/FileTypeMapperTest.php index 49658c33707..68168316538 100644 --- a/tests/PHPStan/Type/FileTypeMapperTest.php +++ b/tests/PHPStan/Type/FileTypeMapperTest.php @@ -303,6 +303,7 @@ private function createFileTypeMapper(CacheStorage $storage): FileTypeMapper $container->getByType(FileContentHasher::class), 2048, 128, + false, ); } From b16a24b4b4420892ae993db1794863dc546e4190 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 12 Feb 2023 20:30:04 +0100 Subject: [PATCH 4/5] Fix tests --- tests/PHPStan/Analyser/nsrt/generics.php | 2 +- .../MethodSignatureVarianceRuleTest.php | 28 ++----------------- .../Methods/CallStaticMethodsRuleTest.php | 7 ++++- .../Rules/Methods/ReturnTypeRuleTest.php | 24 +++++++++++++++- tests/PHPStan/Rules/Methods/data/bug-6249.php | 13 +++++++++ 5 files changed, 45 insertions(+), 29 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/generics.php b/tests/PHPStan/Analyser/nsrt/generics.php index 6e150f33349..4a792501687 100644 --- a/tests/PHPStan/Analyser/nsrt/generics.php +++ b/tests/PHPStan/Analyser/nsrt/generics.php @@ -1410,7 +1410,7 @@ class CarFactoryProcessor { */ public function process($class): void { $car = $class::create(); - assertType(Car::class, $car); + assertType(T::class, $car); // @template T is unknown in static method } } diff --git a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php index 7f19aa0edeb..966f6b20079 100644 --- a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php +++ b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php @@ -186,32 +186,8 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/method-signature-variance-constructor.php'], []); - $this->analyse([__DIR__ . '/data/method-signature-variance-static.php'], [ - [ - 'Template type X is declared as covariant, but occurs in contravariant position in parameter a of method MethodSignatureVariance\StaticMethod\B::a().', - 43, - ], - [ - 'Template type X is declared as covariant, but occurs in contravariant position in parameter c of method MethodSignatureVariance\StaticMethod\B::a().', - 43, - ], - [ - 'Template type X is declared as covariant, but occurs in contravariant position in return type of method MethodSignatureVariance\StaticMethod\B::c().', - 49, - ], - [ - 'Template type X is declared as contravariant, but occurs in covariant position in parameter b of method MethodSignatureVariance\StaticMethod\C::a().', - 62, - ], - [ - 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\StaticMethod\C::b().', - 65, - ], - [ - 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\StaticMethod\C::d().', - 71, - ], - ]); + // class-level @template X is unknown in static methods + $this->analyse([__DIR__ . '/data/method-signature-variance-static.php'], []); } public function testBug8880(): void diff --git a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php index 33f7859302b..de5ef96e6f7 100644 --- a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php @@ -541,7 +541,12 @@ public function testBug6249(): void // discussion https://github.com/phpstan/phpstan/discussions/6249 $this->checkThisOnly = false; $this->checkExplicitMixed = true; - $this->analyse([__DIR__ . '/data/bug-6249.php'], []); + $this->analyse([__DIR__ . '/data/bug-6249.php'], [ + [ + 'Parameter #1 $iterable of static method Bug6249N3\Cw<(int|string),mixed>::fromIterable() expects iterable, Bug6249N2\Eii given.', + 145, + ], + ]); } public function testBug5749(): void diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index 260fde1475f..86174f2feb0 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1248,7 +1248,29 @@ public function testBug13043(): void public function testBug12739(): void { - $this->analyse([__DIR__ . '/data/bug-12739.php'], []); + // @template TScalar is unknown in static methods + $this->analyse([__DIR__ . '/data/bug-12739.php'], [ + [ + "Method MyInt::getClassIdentify() should return class-string> but returns 'MyInt'.", + 20, + ], + [ + "Method MyString::getClassIdentify() should return class-string> but returns 'MyString'.", + 29, + ], + [ + "Method MyLowerString::getClassIdentify() should return class-string> but returns 'MyLowerString'.", + 38, + ], + [ + "Method MyUpperString::getClassIdentify() should return class-string> but returns 'MyUpperString'.", + 47, + ], + [ + "Method MyClassString::getClassIdentify() should return class-string> but returns 'MyClassString'.", + 56, + ], + ]); } public function testBug12928(): void diff --git a/tests/PHPStan/Rules/Methods/data/bug-6249.php b/tests/PHPStan/Rules/Methods/data/bug-6249.php index 7d2db1ed3e3..dd0535e3a62 100644 --- a/tests/PHPStan/Rules/Methods/data/bug-6249.php +++ b/tests/PHPStan/Rules/Methods/data/bug-6249.php @@ -106,6 +106,18 @@ public static function fromIterable(iterable $iterable): self return new self($iterable); } + /** + * @template UKey of array-key + * @template U + * @param iterable $iterable + * + * @return self + */ + public static function fromIterableCorrect(iterable $iterable): self + { + return new self($iterable); + } + public function count(): int { if (is_array($this->iterable) || $this->iterable instanceof Countable) { @@ -131,6 +143,7 @@ class Foo public function doFoo() { \Bug6249N3\Cw::fromIterable(new \Bug6249N2\Eii([])); + \Bug6249N3\Cw::fromIterableCorrect(new \Bug6249N2\Eii([])); } } From 6aa8715097027de0f9ae3e163047e72cf4e2d571 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 12 Feb 2023 20:40:06 +0100 Subject: [PATCH 5/5] Rename feature toggle Amp-Thread-ID: https://ampcode.com/threads/T-01a19219-6ecc-7668-9fe2-6e4d943f7b8e --- conf/bleedingEdge.neon | 2 +- conf/config.neon | 2 +- conf/parametersSchema.neon | 2 +- conf/services.neon | 2 +- src/Type/FileTypeMapper.php | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index d1027950124..040b3b281be 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -34,4 +34,4 @@ parameters: unusedVariable: true unusedParameters: true reuseCollectedDataRuleErrors: true - disallowClassLevelTemplatesInStatic: true + disallowClassLevelTemplatesInStaticMethods: true diff --git a/conf/config.neon b/conf/config.neon index 5e907acf641..95d09f63cad 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -66,7 +66,7 @@ parameters: unusedVariable: false unusedParameters: false reuseCollectedDataRuleErrors: false - disallowClassLevelTemplatesInStatic: false + disallowClassLevelTemplatesInStaticMethods: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 71f5292fb13..0fea5c221ef 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -63,7 +63,7 @@ parametersSchema: unusedVariable: bool() unusedParameters: bool() reuseCollectedDataRuleErrors: bool() - disallowClassLevelTemplatesInStatic: bool() + disallowClassLevelTemplatesInStaticMethods: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/conf/services.neon b/conf/services.neon index 1c73a51c84a..cc5be33af60 100644 --- a/conf/services.neon +++ b/conf/services.neon @@ -138,7 +138,7 @@ services: phpParser: @stubParser resolvedPhpDocBlockCacheCountMax: %cache.resolvedPhpDocBlockCacheCountMax% nameScopeMapMemoryCacheCountMax: %cache.nameScopeMapMemoryCacheCountMax% - disallowClassLevelTemplatesInStatic: %featureToggles.disallowClassLevelTemplatesInStatic% + disallowClassLevelTemplatesInStaticMethods: %featureToggles.disallowClassLevelTemplatesInStaticMethods% autowired: false fileExcluderAnalyse: diff --git a/src/Type/FileTypeMapper.php b/src/Type/FileTypeMapper.php index 41628c2af00..fa86a52db2d 100644 --- a/src/Type/FileTypeMapper.php +++ b/src/Type/FileTypeMapper.php @@ -91,8 +91,8 @@ public function __construct( private int $resolvedPhpDocBlockCacheCountMax, #[AutowiredParameter(ref: '%cache.nameScopeMapMemoryCacheCountMax%')] int $nameScopeMapMemoryCacheCountMax, - #[AutowiredParameter(ref: '%featureToggles.disallowClassLevelTemplatesInStatic%')] - private bool $disallowClassLevelTemplatesInStatic, + #[AutowiredParameter(ref: '%featureToggles.disallowClassLevelTemplatesInStaticMethods%')] + private bool $disallowClassLevelTemplatesInStaticMethods, ) { // 0 kept one entry here rather than meaning "no limit" as it does for the other bounded @@ -392,7 +392,7 @@ private function getNameScopeMap(string $fileName): array $cacheKey = sprintf( 'ftm-%s%s%s', $this->returnsSimplifiedAst($fileName) ? 'simplified-' : '', - $this->disallowClassLevelTemplatesInStatic ? 'static-own-scope-' : '', + $this->disallowClassLevelTemplatesInStaticMethods ? 'static-own-scope-' : '', $fileName, ); // v6: v5 entries may be poisoned by the turbo comment-loss bug @@ -564,7 +564,7 @@ function (Node $node) use ($fileName, $lookForTrait, &$traitFound, $traitMethodA // static methods exist in their own scope if ( - $this->disallowClassLevelTemplatesInStatic + $this->disallowClassLevelTemplatesInStaticMethods && $node instanceof Node\Stmt\ClassMethod && $node->isStatic() ) {