From 7d6bbecd0a8a03b6bb50874ff66845ba138421a3 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Mon, 7 Jul 2025 02:22:05 +0900 Subject: [PATCH 1/8] Simplify property memoization for Flyweight pattern by replacing it with `??=` --- src/Analyser/Ignore/IgnoreLexer.php | 4 +- ...micReturnTypeExtensionRegistryProvider.php | 16 ++-- ...nTypeResolverExtensionRegistryProvider.php | 10 +-- ...ypeSpecifyingExtensionRegistryProvider.php | 10 +-- ...eNodeResolverExtensionRegistryProvider.php | 12 +-- src/PhpDoc/ResolvedPhpDocBlock.php | 82 ++++++------------- .../AnnotationMethodReflection.php | 25 +++--- .../PhpFunctionFromParserNodeReflection.php | 26 +++--- src/Reflection/Php/PhpFunctionReflection.php | 26 +++--- src/Reflection/Php/PhpMethodReflection.php | 26 +++--- src/Reflection/Php/PhpParameterReflection.php | 14 ++-- src/Reflection/Php/PhpPropertyReflection.php | 26 ++---- ...aysUsedClassConstantsExtensionProvider.php | 6 +- ...ingFunctionsDynamicReturnTypeExtension.php | 6 +- src/Type/Traits/LateResolvableTypeTrait.php | 6 +- src/Type/TypeAlias.php | 12 +-- 16 files changed, 109 insertions(+), 198 deletions(-) diff --git a/src/Analyser/Ignore/IgnoreLexer.php b/src/Analyser/Ignore/IgnoreLexer.php index dbfa2ebf42..4f95757861 100644 --- a/src/Analyser/Ignore/IgnoreLexer.php +++ b/src/Analyser/Ignore/IgnoreLexer.php @@ -41,9 +41,7 @@ final class IgnoreLexer */ public function tokenize(string $input): array { - if ($this->regexp === null) { - $this->regexp = $this->generateRegexp(); - } + $this->regexp ??= $this->generateRegexp(); $matches = Strings::matchAll($input, $this->regexp, PREG_SET_ORDER); diff --git a/src/DependencyInjection/Type/LazyDynamicReturnTypeExtensionRegistryProvider.php b/src/DependencyInjection/Type/LazyDynamicReturnTypeExtensionRegistryProvider.php index d69b06cb2a..f84db5f927 100644 --- a/src/DependencyInjection/Type/LazyDynamicReturnTypeExtensionRegistryProvider.php +++ b/src/DependencyInjection/Type/LazyDynamicReturnTypeExtensionRegistryProvider.php @@ -20,16 +20,12 @@ public function __construct(private Container $container) public function getRegistry(): DynamicReturnTypeExtensionRegistry { - if ($this->registry === null) { - $this->registry = new DynamicReturnTypeExtensionRegistry( - $this->container->getByType(ReflectionProvider::class), - $this->container->getServicesByTag(BrokerFactory::DYNAMIC_METHOD_RETURN_TYPE_EXTENSION_TAG), - $this->container->getServicesByTag(BrokerFactory::DYNAMIC_STATIC_METHOD_RETURN_TYPE_EXTENSION_TAG), - $this->container->getServicesByTag(BrokerFactory::DYNAMIC_FUNCTION_RETURN_TYPE_EXTENSION_TAG), - ); - } - - return $this->registry; + return $this->registry ??= new DynamicReturnTypeExtensionRegistry( + $this->container->getByType(ReflectionProvider::class), + $this->container->getServicesByTag(BrokerFactory::DYNAMIC_METHOD_RETURN_TYPE_EXTENSION_TAG), + $this->container->getServicesByTag(BrokerFactory::DYNAMIC_STATIC_METHOD_RETURN_TYPE_EXTENSION_TAG), + $this->container->getServicesByTag(BrokerFactory::DYNAMIC_FUNCTION_RETURN_TYPE_EXTENSION_TAG), + ); } } diff --git a/src/DependencyInjection/Type/LazyExpressionTypeResolverExtensionRegistryProvider.php b/src/DependencyInjection/Type/LazyExpressionTypeResolverExtensionRegistryProvider.php index de695de8bb..6efc5fcb80 100644 --- a/src/DependencyInjection/Type/LazyExpressionTypeResolverExtensionRegistryProvider.php +++ b/src/DependencyInjection/Type/LazyExpressionTypeResolverExtensionRegistryProvider.php @@ -19,13 +19,9 @@ public function __construct(private Container $container) public function getRegistry(): ExpressionTypeResolverExtensionRegistry { - if ($this->registry === null) { - $this->registry = new ExpressionTypeResolverExtensionRegistry( - $this->container->getServicesByTag(BrokerFactory::EXPRESSION_TYPE_RESOLVER_EXTENSION_TAG), - ); - } - - return $this->registry; + return $this->registry ??= new ExpressionTypeResolverExtensionRegistry( + $this->container->getServicesByTag(BrokerFactory::EXPRESSION_TYPE_RESOLVER_EXTENSION_TAG), + ); } } diff --git a/src/DependencyInjection/Type/LazyOperatorTypeSpecifyingExtensionRegistryProvider.php b/src/DependencyInjection/Type/LazyOperatorTypeSpecifyingExtensionRegistryProvider.php index 2913c0fa01..ead76923a3 100644 --- a/src/DependencyInjection/Type/LazyOperatorTypeSpecifyingExtensionRegistryProvider.php +++ b/src/DependencyInjection/Type/LazyOperatorTypeSpecifyingExtensionRegistryProvider.php @@ -19,13 +19,9 @@ public function __construct(private Container $container) public function getRegistry(): OperatorTypeSpecifyingExtensionRegistry { - if ($this->registry === null) { - $this->registry = new OperatorTypeSpecifyingExtensionRegistry( - $this->container->getServicesByTag(BrokerFactory::OPERATOR_TYPE_SPECIFYING_EXTENSION_TAG), - ); - } - - return $this->registry; + return $this->registry ??= new OperatorTypeSpecifyingExtensionRegistry( + $this->container->getServicesByTag(BrokerFactory::OPERATOR_TYPE_SPECIFYING_EXTENSION_TAG), + ); } } diff --git a/src/PhpDoc/LazyTypeNodeResolverExtensionRegistryProvider.php b/src/PhpDoc/LazyTypeNodeResolverExtensionRegistryProvider.php index b8d27c130c..eb0a48b337 100644 --- a/src/PhpDoc/LazyTypeNodeResolverExtensionRegistryProvider.php +++ b/src/PhpDoc/LazyTypeNodeResolverExtensionRegistryProvider.php @@ -17,14 +17,10 @@ public function __construct(private Container $container) public function getRegistry(): TypeNodeResolverExtensionRegistry { - if ($this->registry === null) { - $this->registry = new TypeNodeResolverExtensionAwareRegistry( - $this->container->getByType(TypeNodeResolver::class), - $this->container->getServicesByTag(TypeNodeResolverExtension::EXTENSION_TAG), - ); - } - - return $this->registry; + return $this->registry ??= new TypeNodeResolverExtensionAwareRegistry( + $this->container->getByType(TypeNodeResolver::class), + $this->container->getServicesByTag(TypeNodeResolverExtension::EXTENSION_TAG), + ); } } diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index be3b794d72..94274f4c46 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -733,12 +733,9 @@ public function getDeprecatedTag(): ?DeprecatedTag public function isDeprecated(): bool { - if ($this->isDeprecated === null) { - $this->isDeprecated = $this->phpDocNodeResolver->resolveIsDeprecated( - $this->phpDocNode, - ); - } - return $this->isDeprecated; + return $this->isDeprecated ??= $this->phpDocNodeResolver->resolveIsDeprecated( + $this->phpDocNode, + ); } /** @@ -746,52 +743,37 @@ public function isDeprecated(): bool */ public function isNotDeprecated(): bool { - if ($this->isNotDeprecated === null) { - $this->isNotDeprecated = $this->phpDocNodeResolver->resolveIsNotDeprecated( - $this->phpDocNode, - ); - } - return $this->isNotDeprecated; + return $this->isNotDeprecated ??= $this->phpDocNodeResolver->resolveIsNotDeprecated( + $this->phpDocNode, + ); } public function isInternal(): bool { - if ($this->isInternal === null) { - $this->isInternal = $this->phpDocNodeResolver->resolveIsInternal( - $this->phpDocNode, - ); - } - return $this->isInternal; + return $this->isInternal ??= $this->phpDocNodeResolver->resolveIsInternal( + $this->phpDocNode, + ); } public function isFinal(): bool { - if ($this->isFinal === null) { - $this->isFinal = $this->phpDocNodeResolver->resolveIsFinal( - $this->phpDocNode, - ); - } - return $this->isFinal; + return $this->isFinal ??= $this->phpDocNodeResolver->resolveIsFinal( + $this->phpDocNode, + ); } public function hasConsistentConstructor(): bool { - if ($this->hasConsistentConstructor === null) { - $this->hasConsistentConstructor = $this->phpDocNodeResolver->resolveHasConsistentConstructor( - $this->phpDocNode, - ); - } - return $this->hasConsistentConstructor; + return $this->hasConsistentConstructor ??= $this->phpDocNodeResolver->resolveHasConsistentConstructor( + $this->phpDocNode, + ); } public function acceptsNamedArguments(): bool { - if ($this->acceptsNamedArguments === null) { - $this->acceptsNamedArguments = $this->phpDocNodeResolver->resolveAcceptsNamedArguments( - $this->phpDocNode, - ); - } - return $this->acceptsNamedArguments; + return $this->acceptsNamedArguments ??= $this->phpDocNodeResolver->resolveAcceptsNamedArguments( + $this->phpDocNode, + ); } public function getTemplateTypeMap(): TemplateTypeMap @@ -826,33 +808,23 @@ public function isPure(): ?bool public function isReadOnly(): bool { - if ($this->isReadOnly === null) { - $this->isReadOnly = $this->phpDocNodeResolver->resolveIsReadOnly( - $this->phpDocNode, - ); - } - return $this->isReadOnly; + return $this->isReadOnly ??= $this->phpDocNodeResolver->resolveIsReadOnly( + $this->phpDocNode, + ); } public function isImmutable(): bool { - if ($this->isImmutable === null) { - $this->isImmutable = $this->phpDocNodeResolver->resolveIsImmutable( - $this->phpDocNode, - ); - } - return $this->isImmutable; + return $this->isImmutable ??= $this->phpDocNodeResolver->resolveIsImmutable( + $this->phpDocNode, + ); } public function isAllowedPrivateMutation(): bool { - if ($this->isAllowedPrivateMutation === null) { - $this->isAllowedPrivateMutation = $this->phpDocNodeResolver->resolveAllowPrivateMutation( - $this->phpDocNode, - ); - } - - return $this->isAllowedPrivateMutation; + return $this->isAllowedPrivateMutation ??= $this->phpDocNodeResolver->resolveAllowPrivateMutation( + $this->phpDocNode, + ); } /** diff --git a/src/Reflection/Annotations/AnnotationMethodReflection.php b/src/Reflection/Annotations/AnnotationMethodReflection.php index 696e0e5b08..9b1886b544 100644 --- a/src/Reflection/Annotations/AnnotationMethodReflection.php +++ b/src/Reflection/Annotations/AnnotationMethodReflection.php @@ -68,20 +68,17 @@ public function getName(): string public function getVariants(): array { - if ($this->variants === null) { - $this->variants = [ - new ExtendedFunctionVariant( - $this->templateTypeMap, - null, - $this->parameters, - $this->isVariadic, - $this->returnType, - $this->returnType, - new MixedType(), - ), - ]; - } - return $this->variants; + return $this->variants ??= [ + new ExtendedFunctionVariant( + $this->templateTypeMap, + null, + $this->parameters, + $this->isVariadic, + $this->returnType, + $this->returnType, + new MixedType(), + ), + ]; } public function getOnlyVariant(): ExtendedParametersAcceptor diff --git a/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php b/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php index 02a5b642af..d49cb57f4a 100644 --- a/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php +++ b/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php @@ -105,21 +105,17 @@ public function getName(): string public function getVariants(): array { - if ($this->variants === null) { - $this->variants = [ - new ExtendedFunctionVariant( - $this->getTemplateTypeMap(), - $this->getResolvedTemplateTypeMap(), - $this->getParameters(), - $this->isVariadic(), - $this->getReturnType(), - $this->getPhpDocReturnType(), - $this->getNativeReturnType(), - ), - ]; - } - - return $this->variants; + return $this->variants ??= [ + new ExtendedFunctionVariant( + $this->getTemplateTypeMap(), + $this->getResolvedTemplateTypeMap(), + $this->getParameters(), + $this->isVariadic(), + $this->getReturnType(), + $this->getPhpDocReturnType(), + $this->getNativeReturnType(), + ), + ]; } public function getOnlyVariant(): ExtendedParametersAcceptor diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index b6670aa21f..38604e8761 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -92,21 +92,17 @@ public function getFileName(): ?string public function getVariants(): array { - if ($this->variants === null) { - $this->variants = [ - new ExtendedFunctionVariant( - $this->templateTypeMap, - null, - $this->getParameters(), - $this->isVariadic(), - $this->getReturnType(), - $this->getPhpDocReturnType(), - $this->getNativeReturnType(), - ), - ]; - } - - return $this->variants; + return $this->variants ??= [ + new ExtendedFunctionVariant( + $this->templateTypeMap, + null, + $this->getParameters(), + $this->isVariadic(), + $this->getReturnType(), + $this->getPhpDocReturnType(), + $this->getNativeReturnType(), + ), + ]; } public function getOnlyVariant(): ExtendedParametersAcceptor diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index c0bbad283d..c9b2b132c7 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -198,21 +198,17 @@ private function getMethodNameWithCorrectCase(string $lowercaseMethodName, strin */ public function getVariants(): array { - if ($this->variants === null) { - $this->variants = [ - new ExtendedFunctionVariant( - $this->templateTypeMap, - null, - $this->getParameters(), - $this->isVariadic(), - $this->getReturnType(), - $this->getPhpDocReturnType(), - $this->getNativeReturnType(), - ), - ]; - } - - return $this->variants; + return $this->variants ??= [ + new ExtendedFunctionVariant( + $this->templateTypeMap, + null, + $this->getParameters(), + $this->isVariadic(), + $this->getReturnType(), + $this->getPhpDocReturnType(), + $this->getNativeReturnType(), + ), + ]; } public function getOnlyVariant(): ExtendedParametersAcceptor diff --git a/src/Reflection/Php/PhpParameterReflection.php b/src/Reflection/Php/PhpParameterReflection.php index 73293225ee..8469f7bef4 100644 --- a/src/Reflection/Php/PhpParameterReflection.php +++ b/src/Reflection/Php/PhpParameterReflection.php @@ -104,15 +104,11 @@ public function hasNativeType(): bool public function getNativeType(): Type { - if ($this->nativeType === null) { - $this->nativeType = TypehintHelper::decideTypeFromReflection( - $this->reflection->getType(), - selfClass: $this->declaringClass, - isVariadic: $this->isVariadic(), - ); - } - - return $this->nativeType; + return $this->nativeType ??= TypehintHelper::decideTypeFromReflection( + $this->reflection->getType(), + selfClass: $this->declaringClass, + isVariadic: $this->isVariadic(), + ); } public function getDefaultValue(): ?Type diff --git a/src/Reflection/Php/PhpPropertyReflection.php b/src/Reflection/Php/PhpPropertyReflection.php index 23d8f50e63..2e7a4eb41e 100644 --- a/src/Reflection/Php/PhpPropertyReflection.php +++ b/src/Reflection/Php/PhpPropertyReflection.php @@ -101,15 +101,11 @@ public function isReadOnlyByPhpDoc(): bool public function getReadableType(): Type { - if ($this->type === null) { - $this->type = TypehintHelper::decideTypeFromReflection( - $this->nativeType, - $this->phpDocType, - $this->declaringClass, - ); - } - - return $this->type; + return $this->type ??= TypehintHelper::decideTypeFromReflection( + $this->nativeType, + $this->phpDocType, + $this->declaringClass, + ); } public function getWritableType(): Type @@ -172,14 +168,10 @@ public function hasNativeType(): bool public function getNativeType(): Type { - if ($this->finalNativeType === null) { - $this->finalNativeType = TypehintHelper::decideTypeFromReflection( - $this->nativeType, - selfClass: $this->declaringClass, - ); - } - - return $this->finalNativeType; + return $this->finalNativeType ??= TypehintHelper::decideTypeFromReflection( + $this->nativeType, + selfClass: $this->declaringClass, + ); } public function isReadable(): bool diff --git a/src/Rules/Constants/LazyAlwaysUsedClassConstantsExtensionProvider.php b/src/Rules/Constants/LazyAlwaysUsedClassConstantsExtensionProvider.php index 9425eb8849..cc325cdc33 100644 --- a/src/Rules/Constants/LazyAlwaysUsedClassConstantsExtensionProvider.php +++ b/src/Rules/Constants/LazyAlwaysUsedClassConstantsExtensionProvider.php @@ -18,11 +18,7 @@ public function __construct(private Container $container) public function getExtensions(): array { - if ($this->extensions === null) { - $this->extensions = $this->container->getServicesByTag(AlwaysUsedClassConstantsExtensionProvider::EXTENSION_TAG); - } - - return $this->extensions; + return $this->extensions ??= $this->container->getServicesByTag(AlwaysUsedClassConstantsExtensionProvider::EXTENSION_TAG); } } diff --git a/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php b/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php index a407414ddd..8ed3c6b605 100644 --- a/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php +++ b/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php @@ -76,11 +76,7 @@ public function getTypeFromFunctionCall( private function getHelper(): ImpossibleCheckTypeHelper { - if ($this->helper === null) { - $this->helper = new ImpossibleCheckTypeHelper($this->reflectionProvider, $this->typeSpecifier, $this->universalObjectCratesClasses, $this->treatPhpDocTypesAsCertain); - } - - return $this->helper; + return $this->helper ??= new ImpossibleCheckTypeHelper($this->reflectionProvider, $this->typeSpecifier, $this->universalObjectCratesClasses, $this->treatPhpDocTypesAsCertain); } } diff --git a/src/Type/Traits/LateResolvableTypeTrait.php b/src/Type/Traits/LateResolvableTypeTrait.php index 5eb703077f..e0015381c5 100644 --- a/src/Type/Traits/LateResolvableTypeTrait.php +++ b/src/Type/Traits/LateResolvableTypeTrait.php @@ -584,11 +584,7 @@ public function getFiniteTypes(): array public function resolve(): Type { - if ($this->result === null) { - return $this->result = $this->getResult(); - } - - return $this->result; + return $this->result ??= $this->getResult(); } abstract protected function getResult(): Type; diff --git a/src/Type/TypeAlias.php b/src/Type/TypeAlias.php index 7dc7803af0..17bd6373e5 100644 --- a/src/Type/TypeAlias.php +++ b/src/Type/TypeAlias.php @@ -28,14 +28,10 @@ public static function invalid(): self public function resolve(TypeNodeResolver $typeNodeResolver): Type { - if ($this->resolvedType === null) { - $this->resolvedType = $typeNodeResolver->resolve( - $this->typeNode, - $this->nameScope, - ); - } - - return $this->resolvedType; + return $this->resolvedType ??= $typeNodeResolver->resolve( + $this->typeNode, + $this->nameScope, + ); } } From ef19d69bdd9937b77c030043d473e33aeddb12f2 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Mon, 7 Jul 2025 05:12:27 +0900 Subject: [PATCH 2/8] Add MemoizationPropertyRule --- .../PHPStan/Build/MemoizationPropertyRule.php | 113 ++++++++++++++++++ build/phpstan.neon | 1 + .../Build/MemoizationPropertyRuleTest.php | 41 +++++++ .../Build/data/memoization-property.php | 94 +++++++++++++++ .../Build/data/memoization-property.php.fixed | 86 +++++++++++++ 5 files changed, 335 insertions(+) create mode 100644 build/PHPStan/Build/MemoizationPropertyRule.php create mode 100644 tests/PHPStan/Build/MemoizationPropertyRuleTest.php create mode 100644 tests/PHPStan/Build/data/memoization-property.php create mode 100644 tests/PHPStan/Build/data/memoization-property.php.fixed diff --git a/build/PHPStan/Build/MemoizationPropertyRule.php b/build/PHPStan/Build/MemoizationPropertyRule.php new file mode 100644 index 0000000000..ecb0aa4218 --- /dev/null +++ b/build/PHPStan/Build/MemoizationPropertyRule.php @@ -0,0 +1,113 @@ + + */ +final class MemoizationPropertyRule implements Rule +{ + + public function __construct(private FileHelper $fileHelper, private bool $skipTests = true) + { + } + + public function getNodeType(): string + { + return InClassMethodNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $methodNode = $node->getOriginalNode(); + if (count($methodNode->params) !== 0) { + return []; + } + + $stmts = $methodNode->getStmts(); + if ($stmts === null || count($stmts) !== 2) { + return []; + } + + [$ifNode, $returnNode] = $stmts; + + if (!$returnNode instanceof Return_ || + !$returnNode->expr instanceof PropertyFetch + ) { + return []; + } + + if (!$ifNode instanceof If_ + || count($ifNode->stmts) !== 1 + || !$ifNode->stmts[0] instanceof Expression + || count($ifNode->elseifs) !== 0 + || $ifNode->else !== null + || !$ifNode->cond instanceof Identical + || !$ifNode->cond->left instanceof PropertyFetch + || !$ifNode->cond->right instanceof ConstFetch + || strcasecmp($ifNode->cond->right->name->name, 'null') !== 0 + ) { + return []; + } + + $ifThenNode = $ifNode->stmts[0]->expr; + + if (!$ifThenNode instanceof Assign || !$ifThenNode->var instanceof PropertyFetch) { + return []; + } + + if (get_class($returnNode->expr) !== get_class($ifNode->cond->left) + || get_class($returnNode->expr->var) !== get_class($ifNode->cond->left->var) + || !$returnNode->expr->name instanceof Identifier + || !$ifNode->cond->left->name instanceof Identifier + || $returnNode->expr->name->name !== $ifNode->cond->left->name->name + ) { + return []; + } + + if ($this->skipTests && str_starts_with($this->fileHelper->normalizePath($scope->getFile()), $this->fileHelper->normalizePath(dirname(__DIR__, 3) . '/tests'))) { + return []; + } + + $classReflection = $node->getClassReflection(); + $methodName = $methodNode->name->name; + $errorBuilder = RuleErrorBuilder::message( + sprintf('Method %s::%s() for memoization can be simplified.', $classReflection->getDisplayName(), $methodName), + )->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) use ($ifThenNode) { + $method->stmts = [ + new Return_( + new Coalesce($ifThenNode->var, $ifThenNode->expr), + ), + ]; + + return $method; + })->identifier('phpstan.memoizationProperty'); + + return [ + $errorBuilder->build(), + ]; + } + +} diff --git a/build/phpstan.neon b/build/phpstan.neon index 9fbb0126bd..1ff96a3d92 100644 --- a/build/phpstan.neon +++ b/build/phpstan.neon @@ -123,6 +123,7 @@ rules: - PHPStan\Build\NamedArgumentsRule - PHPStan\Build\OverrideAttributeThirdPartyMethodRule - PHPStan\Build\SkipTestsWithRequiresPhpAttributeRule + - PHPStan\Build\MemoizationPropertyRule services: - diff --git a/tests/PHPStan/Build/MemoizationPropertyRuleTest.php b/tests/PHPStan/Build/MemoizationPropertyRuleTest.php new file mode 100644 index 0000000000..4a4c0e1d32 --- /dev/null +++ b/tests/PHPStan/Build/MemoizationPropertyRuleTest.php @@ -0,0 +1,41 @@ + + */ +final class MemoizationPropertyRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new MemoizationPropertyRule(self::getContainer()->getByType(FileHelper::class), false); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/memoization-property.php'], [ + [ + 'Method MemoizationProperty\A::getFoo() for memoization can be simplified.', + 11, + ], + [ + 'Method MemoizationProperty\A::getBar() for memoization can be simplified.', + 20, + ], + ]); + } + + #[RequiresPhp('>= 8.0')] + public function testFix(): void + { + $this->fix(__DIR__ . '/data/memoization-property.php', __DIR__ . '/data/memoization-property.php.fixed'); + } + +} diff --git a/tests/PHPStan/Build/data/memoization-property.php b/tests/PHPStan/Build/data/memoization-property.php new file mode 100644 index 0000000000..f34c196f0b --- /dev/null +++ b/tests/PHPStan/Build/data/memoization-property.php @@ -0,0 +1,94 @@ += 8.0 + +namespace MemoizationProperty; + +final class A +{ + private ?string $foo = null; + private ?string $bar = null; + private string|false $buz = false; + + public function getFoo() + { + if ($this->foo === null) { + $this->foo = random_bytes(1); + } + + return $this->foo; + } + + public function getBar() + { + if ($this->bar === null) { + $this->bar = random_bytes(1); + } + + return $this->bar; + } + + /** Not applicable because it has an else clause in the if. */ + public function getBarElse() + { + if ($this->bar === null) { + $this->bar = random_bytes(1); + } else { + // no-op + } + + return $this->bar; + } + + /** Not applicable because it has an elseif clause in the if. */ + public function getBarElseIf() + { + if ($this->bar === null) { + $this->bar = random_bytes(1); + } elseif (false) { + // no-op + } + + return $this->bar; + } + + /** Not applicable because it receives parameters. */ + public function getBarReceiveParam(int $length) + { + if ($this->bar === null) { + $this->bar = random_bytes($length); + } + + return $this->bar; + } + + /** Not applicable because the body of if is not just an assignment. */ + public function getBarComplex() + { + if ($this->bar === null) { + $rand = random_bytes(1); + $this->bar = $rand; + } + + return $this->bar; + } + + /** Not applicable because it is comparing a property with a non-null value. */ + public function getBuz() + { + if ($this->buz === false) { + $this->buz = random_bytes(1); + } + + return $this->buz; + } + + /** Not applicable because it doesn't return a memoized property. */ + public function printFoo(): void + { + if ($this->foo === null) { + $this->foo = random_bytes(1); + } + + echo $this->foo; + } + +} diff --git a/tests/PHPStan/Build/data/memoization-property.php.fixed b/tests/PHPStan/Build/data/memoization-property.php.fixed new file mode 100644 index 0000000000..7fe7d6063b --- /dev/null +++ b/tests/PHPStan/Build/data/memoization-property.php.fixed @@ -0,0 +1,86 @@ += 8.0 + +namespace MemoizationProperty; + +final class A +{ + private ?string $foo = null; + private ?string $bar = null; + private string|false $buz = false; + + public function getFoo() + { + return $this->foo ??= random_bytes(1); + } + + public function getBar() + { + return $this->bar ??= random_bytes(1); + } + + /** Not applicable because it has an else clause in the if. */ + public function getBarElse() + { + if ($this->bar === null) { + $this->bar = random_bytes(1); + } else { + // no-op + } + + return $this->bar; + } + + /** Not applicable because it has an elseif clause in the if. */ + public function getBarElseIf() + { + if ($this->bar === null) { + $this->bar = random_bytes(1); + } elseif (false) { + // no-op + } + + return $this->bar; + } + + /** Not applicable because it receives parameters. */ + public function getBarReceiveParam(int $length) + { + if ($this->bar === null) { + $this->bar = random_bytes($length); + } + + return $this->bar; + } + + /** Not applicable because the body of if is not just an assignment. */ + public function getBarComplex() + { + if ($this->bar === null) { + $rand = random_bytes(1); + $this->bar = $rand; + } + + return $this->bar; + } + + /** Not applicable because it is comparing a property with a non-null value. */ + public function getBuz() + { + if ($this->buz === false) { + $this->buz = random_bytes(1); + } + + return $this->buz; + } + + /** Not applicable because it doesn't return a memoized property. */ + public function printFoo(): void + { + if ($this->foo === null) { + $this->foo = random_bytes(1); + } + + echo $this->foo; + } + +} From 2154e3302213aec2007696881fb94ee9e0cf8535 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Mon, 7 Jul 2025 05:17:35 +0900 Subject: [PATCH 3/8] Simplify property memoization for Flyweight pattern by replacing it with `??=` --- src/Reflection/Php/PhpMethodReflection.php | 36 ++++++++----------- ...zyReadWritePropertiesExtensionProvider.php | 6 +--- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index c9b2b132c7..22982ca1f3 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -226,20 +226,16 @@ public function getNamedArgumentsVariants(): ?array */ private function getParameters(): array { - if ($this->parameters === null) { - $this->parameters = array_map(fn (ReflectionParameter $reflection): PhpParameterReflection => new PhpParameterReflection( - $this->initializerExprTypeResolver, - $reflection, - $this->phpDocParameterTypes[$reflection->getName()] ?? null, - $this->getDeclaringClass(), - $this->phpDocParameterOutTypes[$reflection->getName()] ?? null, - $this->immediatelyInvokedCallableParameters[$reflection->getName()] ?? TrinaryLogic::createMaybe(), - $this->phpDocClosureThisTypeParameters[$reflection->getName()] ?? null, - $this->attributeReflectionFactory->fromNativeReflection($reflection->getAttributes(), InitializerExprContext::fromReflectionParameter($reflection)), - ), $this->reflection->getParameters()); - } - - return $this->parameters; + return $this->parameters ??= array_map(fn (ReflectionParameter $reflection): PhpParameterReflection => new PhpParameterReflection( + $this->initializerExprTypeResolver, + $reflection, + $this->phpDocParameterTypes[$reflection->getName()] ?? null, + $this->getDeclaringClass(), + $this->phpDocParameterOutTypes[$reflection->getName()] ?? null, + $this->immediatelyInvokedCallableParameters[$reflection->getName()] ?? TrinaryLogic::createMaybe(), + $this->phpDocClosureThisTypeParameters[$reflection->getName()] ?? null, + $this->attributeReflectionFactory->fromNativeReflection($reflection->getAttributes(), InitializerExprContext::fromReflectionParameter($reflection)), + ), $this->reflection->getParameters()); } private function isVariadic(): bool @@ -342,14 +338,10 @@ private function getPhpDocReturnType(): Type private function getNativeReturnType(): Type { - if ($this->nativeReturnType === null) { - $this->nativeReturnType = TypehintHelper::decideTypeFromReflection( - $this->reflection->getReturnType(), - selfClass: $this->declaringClass, - ); - } - - return $this->nativeReturnType; + return $this->nativeReturnType ??= TypehintHelper::decideTypeFromReflection( + $this->reflection->getReturnType(), + selfClass: $this->declaringClass, + ); } public function getDeprecatedDescription(): ?string diff --git a/src/Rules/Properties/LazyReadWritePropertiesExtensionProvider.php b/src/Rules/Properties/LazyReadWritePropertiesExtensionProvider.php index 07dfafea6b..603c685105 100644 --- a/src/Rules/Properties/LazyReadWritePropertiesExtensionProvider.php +++ b/src/Rules/Properties/LazyReadWritePropertiesExtensionProvider.php @@ -18,11 +18,7 @@ public function __construct(private Container $container) public function getExtensions(): array { - if ($this->extensions === null) { - $this->extensions = $this->container->getServicesByTag(ReadWritePropertiesExtensionProvider::EXTENSION_TAG); - } - - return $this->extensions; + return $this->extensions ??= $this->container->getServicesByTag(ReadWritePropertiesExtensionProvider::EXTENSION_TAG); } } From 3c4bcf2f2d31ec045442e936d4def01efe4fb2cf Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Mon, 7 Jul 2025 06:17:28 +0900 Subject: [PATCH 4/8] fixup! Add MemoizationPropertyRule --- build/PHPStan/Build/MemoizationPropertyRule.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/build/PHPStan/Build/MemoizationPropertyRule.php b/build/PHPStan/Build/MemoizationPropertyRule.php index ecb0aa4218..102246328b 100644 --- a/build/PHPStan/Build/MemoizationPropertyRule.php +++ b/build/PHPStan/Build/MemoizationPropertyRule.php @@ -8,6 +8,7 @@ use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\If_; @@ -19,7 +20,6 @@ use PHPStan\Rules\RuleErrorBuilder; use function count; use function dirname; -use function get_class; use function sprintf; use function str_starts_with; use function strcasecmp; @@ -52,9 +52,10 @@ public function processNode(Node $node, Scope $scope): array } [$ifNode, $returnNode] = $stmts; - - if (!$returnNode instanceof Return_ || - !$returnNode->expr instanceof PropertyFetch + if (!$returnNode instanceof Return_ + || !$returnNode->expr instanceof PropertyFetch + || !$returnNode->expr->var instanceof Variable + || !$returnNode->expr->name instanceof Identifier ) { return []; } @@ -66,6 +67,8 @@ public function processNode(Node $node, Scope $scope): array || $ifNode->else !== null || !$ifNode->cond instanceof Identical || !$ifNode->cond->left instanceof PropertyFetch + || !$ifNode->cond->left->var instanceof Variable + || !$ifNode->cond->left->name instanceof Identifier || !$ifNode->cond->right instanceof ConstFetch || strcasecmp($ifNode->cond->right->name->name, 'null') !== 0 ) { @@ -73,15 +76,11 @@ public function processNode(Node $node, Scope $scope): array } $ifThenNode = $ifNode->stmts[0]->expr; - if (!$ifThenNode instanceof Assign || !$ifThenNode->var instanceof PropertyFetch) { return []; } - if (get_class($returnNode->expr) !== get_class($ifNode->cond->left) - || get_class($returnNode->expr->var) !== get_class($ifNode->cond->left->var) - || !$returnNode->expr->name instanceof Identifier - || !$ifNode->cond->left->name instanceof Identifier + if ($returnNode->expr->var->name !== $ifNode->cond->left->var->name || $returnNode->expr->name->name !== $ifNode->cond->left->name->name ) { return []; From 6d8d1e3dabe1a7d9b46766bd477e60eab8c392d9 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Tue, 8 Jul 2025 00:23:28 +0900 Subject: [PATCH 5/8] Simplify property memoization for Flyweight pattern by replacing it with `??=` --- src/Type/Regex/RegexGroupParser.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Type/Regex/RegexGroupParser.php b/src/Type/Regex/RegexGroupParser.php index 832bb70140..80151501be 100644 --- a/src/Type/Regex/RegexGroupParser.php +++ b/src/Type/Regex/RegexGroupParser.php @@ -53,10 +53,8 @@ public function __construct( public function parseGroups(string $regex): ?RegexAstWalkResult { - if (self::$parser === null) { - /** @throws void */ - self::$parser = Llk::load(new Read(__DIR__ . '/../../../resources/RegexGrammar.pp')); - } + /** @throws void */ + self::$parser ??= Llk::load(new Read(__DIR__ . '/../../../resources/RegexGrammar.pp')); try { Strings::match('', $regex); From 9d72e31632017110c47192507cf791800c6d883b Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Tue, 8 Jul 2025 01:02:00 +0900 Subject: [PATCH 6/8] MemoizationPropertyRule supports static properties --- .../PHPStan/Build/MemoizationPropertyRule.php | 96 ++++++++++++++++--- .../Build/MemoizationPropertyRuleTest.php | 4 + .../Build/data/memoization-property.php | 21 ++++ .../Build/data/memoization-property.php.fixed | 17 ++++ 4 files changed, 125 insertions(+), 13 deletions(-) diff --git a/build/PHPStan/Build/MemoizationPropertyRule.php b/build/PHPStan/Build/MemoizationPropertyRule.php index 102246328b..9a29e94cae 100644 --- a/build/PHPStan/Build/MemoizationPropertyRule.php +++ b/build/PHPStan/Build/MemoizationPropertyRule.php @@ -3,16 +3,20 @@ namespace PHPStan\Build; use PhpParser\Node; +use PhpParser\Node\Expr; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\AssignOp\Coalesce; use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\StaticPropertyFetch; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; +use PhpParser\Node\Name; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Return_; +use PhpParser\Node\VarLikeIdentifier; use PHPStan\Analyser\Scope; use PHPStan\File\FileHelper; use PHPStan\Node\InClassMethodNode; @@ -20,6 +24,7 @@ use PHPStan\Rules\RuleErrorBuilder; use function count; use function dirname; +use function is_string; use function sprintf; use function str_starts_with; use function strcasecmp; @@ -52,11 +57,7 @@ public function processNode(Node $node, Scope $scope): array } [$ifNode, $returnNode] = $stmts; - if (!$returnNode instanceof Return_ - || !$returnNode->expr instanceof PropertyFetch - || !$returnNode->expr->var instanceof Variable - || !$returnNode->expr->name instanceof Identifier - ) { + if (!$returnNode instanceof Return_ || !$this->isSupportedFetchNode($returnNode->expr)) { return []; } @@ -66,9 +67,7 @@ public function processNode(Node $node, Scope $scope): array || count($ifNode->elseifs) !== 0 || $ifNode->else !== null || !$ifNode->cond instanceof Identical - || !$ifNode->cond->left instanceof PropertyFetch - || !$ifNode->cond->left->var instanceof Variable - || !$ifNode->cond->left->name instanceof Identifier + || !$this->isSupportedFetchNode($ifNode->cond->left) || !$ifNode->cond->right instanceof ConstFetch || strcasecmp($ifNode->cond->right->name->name, 'null') !== 0 ) { @@ -76,13 +75,11 @@ public function processNode(Node $node, Scope $scope): array } $ifThenNode = $ifNode->stmts[0]->expr; - if (!$ifThenNode instanceof Assign || !$ifThenNode->var instanceof PropertyFetch) { + if (!$ifThenNode instanceof Assign || !$this->isSupportedFetchNode($ifThenNode->var)) { return []; } - if ($returnNode->expr->var->name !== $ifNode->cond->left->var->name - || $returnNode->expr->name->name !== $ifNode->cond->left->name->name - ) { + if ($this->areNodesNotEqual($returnNode->expr, [$ifNode->cond->left, $ifThenNode->var])) { return []; } @@ -91,9 +88,15 @@ public function processNode(Node $node, Scope $scope): array } $classReflection = $node->getClassReflection(); + $methodReflection = $node->getMethodReflection(); $methodName = $methodNode->name->name; $errorBuilder = RuleErrorBuilder::message( - sprintf('Method %s::%s() for memoization can be simplified.', $classReflection->getDisplayName(), $methodName), + sprintf( + '%s %s::%s() for memoization can be simplified.', + $methodReflection->isStatic() ? 'Static method' : 'Method', + $classReflection->getDisplayName(), + $methodName, + ), )->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) use ($ifThenNode) { $method->stmts = [ new Return_( @@ -109,4 +112,71 @@ public function processNode(Node $node, Scope $scope): array ]; } + /** + * @phpstan-assert-if-true PropertyFetch|StaticPropertyFetch $node + */ + private function isSupportedFetchNode(?Expr $node): bool + { + return $node instanceof PropertyFetch || $node instanceof StaticPropertyFetch; + } + + /** + * @param list $otherNodes + */ + private function areNodesNotEqual(PropertyFetch|StaticPropertyFetch $node, array $otherNodes): bool + { + if ($node instanceof PropertyFetch) { + if (!$node->var instanceof Variable + || !is_string($node->var->name) + || !$node->name instanceof Identifier + ) { + return true; + } + + foreach ($otherNodes as $otherNode) { + if (!$otherNode instanceof PropertyFetch) { + return true; + } + if (!$otherNode->var instanceof Variable + || !is_string($otherNode->var->name) + || !$otherNode->name instanceof Identifier + ) { + return true; + } + + if ($node->var->name !== $otherNode->var->name + || $node->name->name !== $otherNode->name->name + ) { + return true; + } + } + + return false; + } + + if (!$node->class instanceof Name || !$node->name instanceof VarLikeIdentifier) { + return true; + } + + foreach ($otherNodes as $otherNode) { + if (!$otherNode instanceof StaticPropertyFetch) { + return true; + } + + if (!$otherNode->class instanceof Name + || !$otherNode->name instanceof VarLikeIdentifier + ) { + return true; + } + + if ($node->class->toLowerString() !== $otherNode->class->toLowerString() + || $node->name->toString() !== $otherNode->name->toString() + ) { + return true; + } + } + + return false; + } + } diff --git a/tests/PHPStan/Build/MemoizationPropertyRuleTest.php b/tests/PHPStan/Build/MemoizationPropertyRuleTest.php index 4a4c0e1d32..c2cf998ea6 100644 --- a/tests/PHPStan/Build/MemoizationPropertyRuleTest.php +++ b/tests/PHPStan/Build/MemoizationPropertyRuleTest.php @@ -29,6 +29,10 @@ public function testRule(): void 'Method MemoizationProperty\A::getBar() for memoization can be simplified.', 20, ], + [ + 'Static method MemoizationProperty\A::singleton() for memoization can be simplified.', + 96, + ], ]); } diff --git a/tests/PHPStan/Build/data/memoization-property.php b/tests/PHPStan/Build/data/memoization-property.php index f34c196f0b..24fc03fd60 100644 --- a/tests/PHPStan/Build/data/memoization-property.php +++ b/tests/PHPStan/Build/data/memoization-property.php @@ -91,4 +91,25 @@ public function printFoo(): void echo $this->foo; } + private static ?self $singleton = null; + + public static function singleton(): self + { + if (self::$singleton === null) { + self::$singleton = new self(); + } + + return self::$singleton; + } + + /** Not applicable because property names are not matched. */ + public static function singletonBadProperty(): self + { + if (self::$singleton === null) { + self::$singletom = new self(); + } + + return self::$singleton; + } + } diff --git a/tests/PHPStan/Build/data/memoization-property.php.fixed b/tests/PHPStan/Build/data/memoization-property.php.fixed index 7fe7d6063b..00333db395 100644 --- a/tests/PHPStan/Build/data/memoization-property.php.fixed +++ b/tests/PHPStan/Build/data/memoization-property.php.fixed @@ -83,4 +83,21 @@ final class A echo $this->foo; } + private static ?self $singleton = null; + + public static function singleton(): self + { + return self::$singleton ??= new self(); + } + + /** Not applicable because property names are not matched. */ + public static function singletonBadProperty(): self + { + if (self::$singleton === null) { + self::$singletom = new self(); + } + + return self::$singleton; + } + } From 50d4c643e4bbd25d9179c889ae34645d371576ad Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Tue, 8 Jul 2025 10:20:58 +0900 Subject: [PATCH 7/8] fixup! Add MemoizationPropertyRule --- .../PHPStan/Build/MemoizationPropertyRule.php | 48 ++++--------------- .../Build/MemoizationPropertyRuleTest.php | 18 +++++-- .../Build/data/memoization-property.php | 2 - .../Build/data/memoization-property.php.fixed | 22 ++++----- 4 files changed, 32 insertions(+), 58 deletions(-) diff --git a/build/PHPStan/Build/MemoizationPropertyRule.php b/build/PHPStan/Build/MemoizationPropertyRule.php index 9a29e94cae..e245294c2f 100644 --- a/build/PHPStan/Build/MemoizationPropertyRule.php +++ b/build/PHPStan/Build/MemoizationPropertyRule.php @@ -15,7 +15,6 @@ use PhpParser\Node\Name; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\If_; -use PhpParser\Node\Stmt\Return_; use PhpParser\Node\VarLikeIdentifier; use PHPStan\Analyser\Scope; use PHPStan\File\FileHelper; @@ -25,12 +24,11 @@ use function count; use function dirname; use function is_string; -use function sprintf; use function str_starts_with; use function strcasecmp; /** - * @implements Rule + * @implements Rule */ final class MemoizationPropertyRule implements Rule { @@ -41,28 +39,14 @@ public function __construct(private FileHelper $fileHelper, private bool $skipTe public function getNodeType(): string { - return InClassMethodNode::class; + return If_::class; } public function processNode(Node $node, Scope $scope): array { - $methodNode = $node->getOriginalNode(); - if (count($methodNode->params) !== 0) { - return []; - } - - $stmts = $methodNode->getStmts(); - if ($stmts === null || count($stmts) !== 2) { - return []; - } - - [$ifNode, $returnNode] = $stmts; - if (!$returnNode instanceof Return_ || !$this->isSupportedFetchNode($returnNode->expr)) { - return []; - } + $ifNode = $node; - if (!$ifNode instanceof If_ - || count($ifNode->stmts) !== 1 + if (count($ifNode->stmts) !== 1 || !$ifNode->stmts[0] instanceof Expression || count($ifNode->elseifs) !== 0 || $ifNode->else !== null @@ -79,7 +63,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($this->areNodesNotEqual($returnNode->expr, [$ifNode->cond->left, $ifThenNode->var])) { + if ($this->areNodesNotEqual($ifNode->cond->left, [$ifThenNode->var])) { return []; } @@ -87,25 +71,9 @@ public function processNode(Node $node, Scope $scope): array return []; } - $classReflection = $node->getClassReflection(); - $methodReflection = $node->getMethodReflection(); - $methodName = $methodNode->name->name; - $errorBuilder = RuleErrorBuilder::message( - sprintf( - '%s %s::%s() for memoization can be simplified.', - $methodReflection->isStatic() ? 'Static method' : 'Method', - $classReflection->getDisplayName(), - $methodName, - ), - )->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) use ($ifThenNode) { - $method->stmts = [ - new Return_( - new Coalesce($ifThenNode->var, $ifThenNode->expr), - ), - ]; - - return $method; - })->identifier('phpstan.memoizationProperty'); + $errorBuilder = RuleErrorBuilder::message('Memoization property can be simplified.') + ->fixNode($node, static fn (If_ $node) => new Expression(new Coalesce($ifThenNode->var, $ifThenNode->expr))) + ->identifier('phpstan.memoizationProperty'); return [ $errorBuilder->build(), diff --git a/tests/PHPStan/Build/MemoizationPropertyRuleTest.php b/tests/PHPStan/Build/MemoizationPropertyRuleTest.php index c2cf998ea6..34e0e6ecc4 100644 --- a/tests/PHPStan/Build/MemoizationPropertyRuleTest.php +++ b/tests/PHPStan/Build/MemoizationPropertyRuleTest.php @@ -22,15 +22,23 @@ public function testRule(): void { $this->analyse([__DIR__ . '/data/memoization-property.php'], [ [ - 'Method MemoizationProperty\A::getFoo() for memoization can be simplified.', - 11, + 'Memoization property can be simplified.', + 13, ], [ - 'Method MemoizationProperty\A::getBar() for memoization can be simplified.', - 20, + 'Memoization property can be simplified.', + 22, ], [ - 'Static method MemoizationProperty\A::singleton() for memoization can be simplified.', + 'Memoization property can be simplified.', + 55, + ], + [ + 'Memoization property can be simplified.', + 85, + ], + [ + 'Memoization property can be simplified.', 96, ], ]); diff --git a/tests/PHPStan/Build/data/memoization-property.php b/tests/PHPStan/Build/data/memoization-property.php index 24fc03fd60..cccf56642a 100644 --- a/tests/PHPStan/Build/data/memoization-property.php +++ b/tests/PHPStan/Build/data/memoization-property.php @@ -50,7 +50,6 @@ public function getBarElseIf() return $this->bar; } - /** Not applicable because it receives parameters. */ public function getBarReceiveParam(int $length) { if ($this->bar === null) { @@ -81,7 +80,6 @@ public function getBuz() return $this->buz; } - /** Not applicable because it doesn't return a memoized property. */ public function printFoo(): void { if ($this->foo === null) { diff --git a/tests/PHPStan/Build/data/memoization-property.php.fixed b/tests/PHPStan/Build/data/memoization-property.php.fixed index 00333db395..73a40f8ac7 100644 --- a/tests/PHPStan/Build/data/memoization-property.php.fixed +++ b/tests/PHPStan/Build/data/memoization-property.php.fixed @@ -10,12 +10,16 @@ final class A public function getFoo() { - return $this->foo ??= random_bytes(1); + $this->foo ??= random_bytes(1); + + return $this->foo; } public function getBar() { - return $this->bar ??= random_bytes(1); + $this->bar ??= random_bytes(1); + + return $this->bar; } /** Not applicable because it has an else clause in the if. */ @@ -42,12 +46,9 @@ final class A return $this->bar; } - /** Not applicable because it receives parameters. */ public function getBarReceiveParam(int $length) { - if ($this->bar === null) { - $this->bar = random_bytes($length); - } + $this->bar ??= random_bytes($length); return $this->bar; } @@ -73,12 +74,9 @@ final class A return $this->buz; } - /** Not applicable because it doesn't return a memoized property. */ public function printFoo(): void { - if ($this->foo === null) { - $this->foo = random_bytes(1); - } + $this->foo ??= random_bytes(1); echo $this->foo; } @@ -87,7 +85,9 @@ final class A public static function singleton(): self { - return self::$singleton ??= new self(); + self::$singleton ??= new self(); + + return self::$singleton; } /** Not applicable because property names are not matched. */ From a6e89c9c74072213c1089f556ae0b575593f5c9c Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Tue, 8 Jul 2025 10:24:34 +0900 Subject: [PATCH 8/8] Simplify property memoization for Flyweight pattern by replacing it with `??=` --- src/Parser/VariadicFunctionsVisitor.php | 4 +--- src/Parser/VariadicMethodsVisitor.php | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Parser/VariadicFunctionsVisitor.php b/src/Parser/VariadicFunctionsVisitor.php index d083767a53..70d19bacec 100644 --- a/src/Parser/VariadicFunctionsVisitor.php +++ b/src/Parser/VariadicFunctionsVisitor.php @@ -44,9 +44,7 @@ public function beforeTraverse(array $nodes): ?array #[Override] public function enterNode(Node $node): ?Node { - if ($this->topNode === null) { - $this->topNode = $node; - } + $this->topNode ??= $node; if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { $this->inNamespace = $node->name->toString(); diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 6fdfa0bc67..4e5bf66d75 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -53,9 +53,7 @@ public function beforeTraverse(array $nodes): ?array #[Override] public function enterNode(Node $node): ?Node { - if ($this->topNode === null) { - $this->topNode = $node; - } + $this->topNode ??= $node; if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { $this->inNamespace = $node->name->toString();