From 37f0c392a5331de09d0ede5c2646af1e066f9403 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Sat, 16 May 2026 19:41:54 +0000 Subject: [PATCH] Add regression tests for `null !== $x = expr()` assignment-in-condition pattern with `&&` - Add NSRT test verifying type narrowing works correctly for the parenthesized pattern: (null !== $x = expr()) && use($x) - Add DefinedVariableRule test documenting that the unparenthesized version correctly reports "Undefined variable" (due to PHP operator precedence, && binds tighter than =) - Cover variations: logical 'and'/'or' (lower precedence), chained assignments, false !== pattern, while loops, || with === null The reported code `null !== $x = expr() && use($x)` parses as `null !== ($x = (expr() && use($x)))` due to && having higher precedence than =. The variable $x is genuinely undefined inside the BooleanAnd's right side. PHPStan correctly reports this error. The correctly parenthesized version `(null !== $x = expr()) && use($x)` already works correctly in PHPStan - variable is defined and narrowed to non-null in the right side of &&. Closes https://github.com/phpstan/phpstan/issues/4662 --- tests/PHPStan/Analyser/nsrt/bug-4662.php | 117 ++++++++++++++++++ .../Variables/DefinedVariableRuleTest.php | 14 +++ .../PHPStan/Rules/Variables/data/bug-4662.php | 57 +++++++++ 3 files changed, 188 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-4662.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-4662.php diff --git a/tests/PHPStan/Analyser/nsrt/bug-4662.php b/tests/PHPStan/Analyser/nsrt/bug-4662.php new file mode 100644 index 00000000000..071c3204e53 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-4662.php @@ -0,0 +1,117 @@ +programStartDate; + } + +} + +function testParenthesized(Foo $foo): void +{ + $now = new \DateTimeImmutable('now'); + + // Correct: with parentheses, the assignment happens in the left side of && + // and the narrowed variable is available in the right side + if ((null !== $programStartDate = $foo->getProgramStartDate()) && $now > $programStartDate) { + assertType('DateTimeImmutable', $programStartDate); + } +} + +function testParenthesizedReversed(Foo $foo): void +{ + $now = new \DateTimeImmutable('now'); + + // Correct: reversed comparison style + if (($programStartDate = $foo->getProgramStartDate()) !== null && $now > $programStartDate) { + assertType('DateTimeImmutable', $programStartDate); + } +} + +function testWithLogicalAnd(Foo $foo): void +{ + $now = new \DateTimeImmutable('now'); + + // 'and' has lower precedence than '=', so this parses as: + // (null !== ($programStartDate = $foo->getProgramStartDate())) and ($now > $programStartDate) + if (null !== $programStartDate = $foo->getProgramStartDate() and $now > $programStartDate) { + assertType('DateTimeImmutable', $programStartDate); + } +} + +function testChained(Foo $foo): void +{ + // Multiple assignments chained with && + if ((null !== $a = $foo->getProgramStartDate()) && (null !== $b = $foo->getProgramStartDate()) && $a > $b) { + assertType('DateTimeImmutable', $a); + assertType('DateTimeImmutable', $b); + } +} + +function testFalseCheck(string $haystack, string $needle): void +{ + // false !== with assignment and && + if ((false !== $pos = strpos($haystack, $needle)) && $pos > 5) { + assertType('int<6, max>', $pos); + } +} + +function testWithOr(Foo $foo): void +{ + // Using || with === null (the inverse pattern) + if (($programStartDate = $foo->getProgramStartDate()) === null || $programStartDate->getTimestamp() < 0) { + assertType('DateTimeImmutable|null', $programStartDate); + } else { + assertType('DateTimeImmutable', $programStartDate); + } +} + +function testLogicalOr(Foo $foo): void +{ + // 'or' has lower precedence than '=' + if (null === $programStartDate = $foo->getProgramStartDate() or $programStartDate->getTimestamp() < 0) { + assertType('DateTimeImmutable|null', $programStartDate); + } else { + assertType('DateTimeImmutable', $programStartDate); + } +} + +function testNested(Foo $foo): void +{ + // Assignment result used in chained && + if ((null !== $a = $foo->getProgramStartDate()) && ($b = $a->format('Y-m-d')) !== '') { + assertType('DateTimeImmutable', $a); + assertType('non-falsy-string', $b); + } +} + +function testAssignInWhile(Foo $foo): void +{ + // Assignment in while condition with && + $i = 0; + while ((null !== $val = $foo->getProgramStartDate()) && $i < 10) { + assertType('DateTimeImmutable', $val); + $i++; + } +} + +function testSimpleAssignInCondition(Foo $foo): void +{ + // Simple assignment in condition without && + if (null !== $programStartDate = $foo->getProgramStartDate()) { + assertType('DateTimeImmutable', $programStartDate); + } else { + assertType('null', $programStartDate); + } + assertType('DateTimeImmutable|null', $programStartDate); +} diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 67f77fa6bfe..63a6802713d 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1626,4 +1626,18 @@ public function testBug6833(): void ]); } + public function testBug4662(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-4662.php'], [ + [ + 'Undefined variable: $programStartDate', + 27, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-4662.php b/tests/PHPStan/Rules/Variables/data/bug-4662.php new file mode 100644 index 00000000000..71385159cd6 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-4662.php @@ -0,0 +1,57 @@ +programStartDate; + } + +} + +function testUnparenthesized(Foo $foo): void +{ + $now = new \DateTimeImmutable('now'); + + // Without parentheses, && has higher precedence than = + // so this parses as: null !== ($programStartDate = (getProgramStartDate() && ($now > $programStartDate))) + // $programStartDate is used inside the BooleanAnd RHS before it's assigned + if ( + null !== $programStartDate = $foo->getProgramStartDate() + && $now > $programStartDate + ) { + echo 'ok'; + } +} + +function testParenthesized(Foo $foo): void +{ + $now = new \DateTimeImmutable('now'); + + // With parentheses, this works correctly - no undefined variable + if ( + (null !== $programStartDate = $foo->getProgramStartDate()) + && $now > $programStartDate + ) { + echo 'ok'; + } +} + +function testLogicalAnd(Foo $foo): void +{ + $now = new \DateTimeImmutable('now'); + + // 'and' has lower precedence than '=' so this works without extra parentheses + if ( + null !== $programStartDate = $foo->getProgramStartDate() + and $now > $programStartDate + ) { + echo 'ok'; + } +}