Single-pass expression analysis groundwork - answer type questions from ExpressionResults

build/PHPStan/Build/NoScopeTypeReadInEngineRule.php

@@ -0,0 +1,111 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Build;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use function array_key_exists;
+use function in_array;
+use function sprintf;
+use function str_starts_with;
+
+/**
+ * The single-pass engine must not price expressions through Scope::getType()
+ * and friends - every walked node's type lives in its ExpressionResult
+ * (threaded via ArgsResult, storage, or gathered statement scopes). A scope
+ * read in engine code either re-walks a node on demand or silently diverges
+ * from the stored result. The allowlist below names the documented seams
+ * (rule-facing bridge asks and ask-ahead-of-walk reads).
+ *
+ * @implements Rule<MethodCall>
+ */
+final class NoScopeTypeReadInEngineRule implements Rule
+{
+
+	private const ENGINE_NAMESPACES = [
+		'PHPStan\\Analyser\\NodeScopeResolver',
+		'PHPStan\\Analyser\\StmtHandler\\',
+		'PHPStan\\Analyser\\ExprHandler\\',
+	];
+
+	private const BANNED_METHODS = ['getType', 'getNativeType', 'getKeepVoidType'];
+
+	private const ALLOWED_SEAMS = [
+		// rule-facing bridge asks: the scope carries no storage
+		'PHPStan\\Analyser\\ExprHandler\\Helper\\ClosureTypeResolver' => ['readExprType', 'resolveArrowFunctionReturnType'],
+		'PHPStan\\Analyser\\ExprHandler\\Helper\\CountNarrowingHelper' => ['isNormalCountCall'],
+		// immediately invoked closures: the invocation args are walked after the closure
+		'PHPStan\\Analyser\\ExprHandler\\Helper\\ClosureTypeResolver#buildParametersAndAcceptors' => [],
+	];
+
+	public function getNodeType(): string
+	{
+		return MethodCall::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if (!$node->name instanceof Node\Identifier) {
+			return [];
+		}
+		$methodName = $node->name->toString();
+		if (!in_array($methodName, self::BANNED_METHODS, true)) {
+			return [];
+		}
+
+		$currentClassReflection = $scope->getClassReflection();
+		if ($currentClassReflection === null) {
+			return [];
+		}
+		$namespace = $currentClassReflection->getName();
+		$inEngine = false;
+		foreach (self::ENGINE_NAMESPACES as $prefix) {
+			if ($namespace === $prefix || str_starts_with($namespace, $prefix)) {
+				$inEngine = true;
+				break;
+			}
+		}
+		if (!$inEngine) {
+			return [];
+		}
+
+		$calledOnType = $scope->getType($node->var);
+		if ($calledOnType->getObjectClassNames() === []) {
+			return [];
+		}
+		$isScope = false;
+		foreach ($calledOnType->getObjectClassReflections() as $classReflection) {
+			if ($classReflection->is(Scope::class)) {
+				$isScope = true;
+				break;
+			}
+		}
+		if (!$isScope) {
+			return [];
+		}
+
+		$function = $scope->getFunction();
+		$functionName = $function !== null ? $function->getName() : null;
+		if (
+			array_key_exists($namespace, self::ALLOWED_SEAMS)
+			&& $functionName !== null
+			&& in_array($functionName, self::ALLOWED_SEAMS[$namespace], true)
+		) {
+			return [];
+		}
+		if ($functionName !== null && array_key_exists($namespace . '#' . $functionName, self::ALLOWED_SEAMS)) {
+			return [];
+		}
+
+		return [
+			RuleErrorBuilder::message(sprintf(
+				'Engine code must not price expressions via Scope::%s() - read the expression\'s stored ExpressionResult (ArgsResult, storage, gathered scopes) instead.',
+				$methodName,
+			))->identifier('phpstanBuild.scopeTypeReadInEngine')->build(),
+		];
+	}
+
+}

build/phpstan.neon

@@ -183,6 +183,7 @@ parameters:
 
 rules:
 	- PHPStan\Build\FinalClassRule
+	- PHPStan\Build\NoScopeTypeReadInEngineRule
 	- PHPStan\Build\AttributeNamedArgumentsRule
 	- PHPStan\Build\NamedArgumentsRule
 	- PHPStan\Build\OverrideAttributeThirdPartyMethodRule

phpstan-baseline.neon

@@ -37,39 +37,27 @@ parameters:
 			path: src/Analyser/ExprHandler/BinaryOpHandler.php
 
 		-
-			rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantBooleanType is error-prone and deprecated. Use Type::isTrue() or Type::isFalse() instead.'
-			identifier: phpstanApi.instanceofType
-			count: 1
-			path: src/Analyser/ExprHandler/BooleanNotHandler.php
-
-		-
-			rawMessage: 'Doing instanceof PHPStan\Type\ConstantScalarType is error-prone and deprecated. Use Type::isConstantScalarValue() or Type::getConstantScalarTypes() or Type::getConstantScalarValues() instead.'
-			identifier: phpstanApi.instanceofType
-			count: 2
-			path: src/Analyser/ExprHandler/Helper/EqualityTypeSpecifyingHelper.php
-
-		-
-			rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantBooleanType is error-prone and deprecated. Use Type::isTrue() or Type::isFalse() instead.'
-			identifier: phpstanApi.instanceofType
+			rawMessage: Casting to string something that's already string.
+			identifier: cast.useless
 			count: 3
-			path: src/Analyser/ExprHandler/Helper/EqualityTypeSpecifyingHelper.php
+			path: src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php
 
 		-
 			rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantStringType is error-prone and deprecated. Use Type::getConstantStrings() instead.'
 			identifier: phpstanApi.instanceofType
 			count: 2
-			path: src/Analyser/ExprHandler/IssetHandler.php
+			path: src/Analyser/ExprHandler/Helper/DefaultNarrowingHelper.php
 
 		-
 			rawMessage: 'Only numeric types are allowed in pre-increment, float|int|string|null given.'
 			identifier: preInc.nonNumeric
 			count: 1
-			path: src/Analyser/ExprHandler/PreIncHandler.php
+			path: src/Analyser/ExprHandler/Helper/IncDecTypeHelper.php
 
 		-
 			rawMessage: Casting to string something that's already string.
 			identifier: cast.useless
-			count: 1
+			count: 4
 			path: src/Analyser/MutatingScope.php
 
 		-
@@ -108,24 +96,12 @@ parameters:
 			count: 1
 			path: src/Analyser/RuleErrorTransformer.php
 
-		-
-			rawMessage: Casting to string something that's already string.
-			identifier: cast.useless
-			count: 2
-			path: src/Analyser/ScopeOps.php
-
 		-
 			rawMessage: 'Parameter #2 $node of method PHPStan\BetterReflection\SourceLocator\Ast\Strategy\NodeToReflection::__invoke() expects PhpParser\Node\Expr\ArrowFunction|PhpParser\Node\Expr\Closure|PhpParser\Node\Expr\FuncCall|PhpParser\Node\Stmt\Class_|PhpParser\Node\Stmt\Const_|PhpParser\Node\Stmt\Enum_|PhpParser\Node\Stmt\Function_|PhpParser\Node\Stmt\Interface_|PhpParser\Node\Stmt\Trait_, PhpParser\Node\Stmt\ClassLike given.'
 			identifier: argument.type
 			count: 1
 			path: src/Analyser/StmtHandler/ClassLikeHandler.php
 
-		-
-			rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantBooleanType is error-prone and deprecated. Use Type::isTrue() or Type::isFalse() instead.'
-			identifier: phpstanApi.instanceofType
-			count: 2
-			path: src/Analyser/TypeSpecifier.php
-
 		-
 			rawMessage: 'Template type TNodeType is declared as covariant, but occurs in contravariant position in parameter node of method PHPStan\Collectors\Collector::processNode().'
 			identifier: generics.variance

src/Analyser/ArgsResult.php

@@ -2,26 +2,76 @@
 
 namespace PHPStan\Analyser;
 
+use PhpParser\Node\Expr;
 use PHPStan\Reflection\ParametersAcceptor;
+use PHPStan\ShouldNotHappenException;
+use function get_class;
+use function spl_object_id;
+use function sprintf;
 
 /**
  * Result of NodeScopeResolver::processArgs(): the scope/throw/impure state after
  * processing all arguments (wrapped ExpressionResult) plus the ParametersAcceptor
  * resolved from the arg types gathered on the arg-to-arg evolving scope. The
  * resolved acceptor is type-driven (selectFromTypes) so its generics are resolved
- * against the actual argument types - callers wire it into the call's return
- * type. Null when the call had no variants (dynamic callee).
+ * against the actual argument types - callers wire it into the call expression's
+ * stored return type. Null when the call had no variants (dynamic callee).
  */
 final class ArgsResult
 {
 
+	/**
+	 * @param array<int, ExpressionResult> $argResults keyed by spl_object_id of each argument's value expression
+	 */
 	public function __construct(
 		private ExpressionResult $expressionResult,
 		private ?ParametersAcceptor $resolvedParametersAcceptor,
+		private array $argResults,
 	)
 	{
 	}
 
+	/**
+	 * Membership query: the ExpressionResult of the given expression if it is one of
+	 * the call's processed arguments. Null means the expression is not such an
+	 * argument - either it is a different expression altogether (the callee name, a
+	 * synthetic node, a narrowed subexpression), or it is a default-value argument
+	 * ArgumentsNormalizer synthesized for an omitted optional parameter, which has no
+	 * source code behind it and so was never processed. Call sites holding an actual
+	 * argument of the call use requireArgResult() instead.
+	 */
+	public function findArgResult(Expr $argValue): ?ExpressionResult
+	{
+		return $this->argResults[spl_object_id($argValue)] ?? null;
+	}
+
+	/**
+	 * @return array<int, ExpressionResult> keyed by spl_object_id of each argument's value expression
+	 */
+	public function getArgResults(): array
+	{
+		return $this->argResults;
+	}
+
+	/**
+	 * The stored ExpressionResult of a call argument's value expression; the
+	 * argument must have been processed by processArgs() - engine code reads
+	 * argument types through this instead of re-asking the scope.
+	 */
+	public function requireArgResult(Expr $argValue): ExpressionResult
+	{
+		$result = $this->argResults[spl_object_id($argValue)] ?? null;
+		if ($result === null) {
+			throw new ShouldNotHappenException(sprintf(
+				'No stored ExpressionResult for a %s argument on line %d.',
+				get_class($argValue),
+				$argValue->getStartLine(),
+			));
+		}
+
+		return $result;
+	}
+
 	public function getScope(): MutatingScope
 	{
 		return $this->expressionResult->getScope();
@@ -53,6 +103,19 @@ public function getImpurePoints(): array
 		return $this->expressionResult->getImpurePoints();
 	}
 
+	/**
+	 * The same processed arguments under another acceptor: an immediately
+	 * invoked closure's arguments are walked on its declared signature, the
+	 * acceptor the call resolves from is the walked closure's.
+	 */
+	public function withResolvedParametersAcceptor(?ParametersAcceptor $resolvedParametersAcceptor): self
+	{
+		$clone = clone $this;
+		$clone->resolvedParametersAcceptor = $resolvedParametersAcceptor;
+
+		return $clone;
+	}
+
 	public function getResolvedParametersAcceptor(): ?ParametersAcceptor
 	{
 		return $this->resolvedParametersAcceptor;

src/Analyser/CalledMethodProcessor.php

@@ -87,9 +87,10 @@ public function processCalledMethod(NodeScopeResolver $nodeScopeResolver, Method
 		if ($returnStatement !== null) {
 			foreach ($returnStatement->getExecutionEnds() as $executionEnd) {
 				$statementResult = $executionEnd->getStatementResult();
-				$endNode = $executionEnd->getNode();
-				if ($endNode instanceof Node\Stmt\Expression) {
-					$exprType = $statementResult->getScope()->getType($endNode->expr);
+				$endExprResult = $executionEnd->getExprResult();
+				if ($endExprResult !== null) {
+					$walkScope = $statementResult->getScope()->toWalkScope();
+					$exprType = $endExprResult->getTypeOnScope($walkScope, $walkScope->nativeTypesPromoted);
 					if ($exprType instanceof NeverType && $exprType->isExplicit()) {
 						continue;
 					}

src/Analyser/ConditionalExpressionHolderRecipe.php

@@ -22,7 +22,7 @@ final class ConditionalExpressionHolderRecipe
 {
 
 	/**
-	 * @param list<array{string, Expr, bool, Type}> $conditionEntries [exprString, expr, fromSureTypes, type]
+	 * @param list<array{string, Expr, bool, Type, ?ExpressionResult}> $conditionEntries [exprString, expr, fromSureTypes, type, the operand walk's result for the expr]
 	 * @param list<array{string, Expr, Type, ?Type}> $holderEntries [exprString, expr, type, target type pinned at compose time (null = read the applying scope)]
 	 */
 	public function __construct(
@@ -43,8 +43,15 @@ public function evaluate(MutatingScope $scope): array
 		// the unnarrowed type of each condition expression, for the
 		// dropped-self-condition complement below
 		$conditionOriginalTypes = [];
-		foreach ($this->conditionEntries as [$exprString, $expr, $fromSureTypes, $type]) {
-			$scopeType = $scope->getType($expr);
+		foreach ($this->conditionEntries as [$exprString, $expr, $fromSureTypes, $type, $conditionResult]) {
+			// through the expression's own result, which answers from the applying
+			// scope's state where that scope owns the expression - a narrowing
+			// subject no walk produced (an extension is free to specify a type for
+			// an expression the source never evaluated on its own) has no result
+			// and is read from the scope state directly
+			$scopeType = $conditionResult !== null
+				? $conditionResult->getTypeOnScope($scope, $scope->nativeTypesPromoted)
+				: $scope->getStateType($expr);
 			$conditionType = $fromSureTypes
 				? TypeCombinator::remove($scopeType, $type)
 				: TypeCombinator::intersect($scopeType, $type);
@@ -81,7 +88,7 @@ public function evaluate(MutatingScope $scope): array
 				continue;
 			}
 
-			$targetType = $pinnedTargetType ?? $scope->getType($expr);
+			$targetType = $pinnedTargetType ?? $scope->getStateType($expr);
 			$holderType = $this->holdersFromSureTypes
 				? TypeCombinator::intersect($targetType, $type)
 				: TypeCombinator::remove($targetType, $type);