Skip to content

Commit f7b6a02

Browse files
phpstan-botclaude
phpstan-bot
and
claude
committed
Unify specifyOnly and rootExpr into single rootExpr mechanism with duplicate detection
Replace the `specifyOnly` flag with `rootExpr` + a sureType for the call expression in both StrContainingTypeSpecifyingExtension and ArrayKeyExistsFunctionTypeSpecifyingExtension. This unifies their behavior with equality assertions (`@phpstan-assert-if-true =Type`), which already use rootExpr. Add duplicate call detection: when a function/method call with rootExpr has already been evaluated in scope (via hasExpressionType), report the nested identical call as always-true. This works for str_contains, str_ends_with, array_key_exists, and equality assertion methods. For equality assertions, add the call itself as a sureType with ConstantBooleanType so it gets stored in scope's expressionTypes, enabling the same duplicate detection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d6e4e48 commit f7b6a02

8 files changed

Lines changed: 84 additions & 51 deletions

‎src/Analyser/SpecifiedTypes.php‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ final class SpecifiedTypes
1313

1414
private bool $overwrite = false;
1515

16-
private bool $specifyOnly = false;
17-
1816
/** @var array<string, ConditionalExpressionHolder[]> */
1917
private array $newConditionalExpressionHolders = [];
2018

@@ -53,28 +51,6 @@ public function setAlwaysOverwriteTypes(): self
5351
{
5452
$self = new self($this->sureTypes, $this->sureNotTypes);
5553
$self->overwrite = true;
56-
$self->specifyOnly = $this->specifyOnly;
57-
$self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders;
58-
$self->rootExpr = $this->rootExpr;
59-
60-
return $self;
61-
}
62-
63-
/**
64-
* When set, the sureTypes are only used for narrowing — ImpossibleCheckTypeHelper
65-
* will not use them to determine whether the check is always-true/always-false.
66-
*
67-
* Use this when the sureTypes are a side effect of the check
68-
* (e.g. str_contains narrowing haystack to non-empty-string)
69-
* rather than the determining condition.
70-
*
71-
* @api
72-
*/
73-
public function setSpecifyOnly(): self
74-
{
75-
$self = new self($this->sureTypes, $this->sureNotTypes);
76-
$self->overwrite = $this->overwrite;
77-
$self->specifyOnly = true;
7854
$self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders;
7955
$self->rootExpr = $this->rootExpr;
8056

@@ -88,7 +64,6 @@ public function setRootExpr(?Expr $rootExpr): self
8864
{
8965
$self = new self($this->sureTypes, $this->sureNotTypes);
9066
$self->overwrite = $this->overwrite;
91-
$self->specifyOnly = $this->specifyOnly;
9267
$self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders;
9368
$self->rootExpr = $rootExpr;
9469

@@ -102,7 +77,6 @@ public function setNewConditionalExpressionHolders(array $newConditionalExpressi
10277
{
10378
$self = new self($this->sureTypes, $this->sureNotTypes);
10479
$self->overwrite = $this->overwrite;
105-
$self->specifyOnly = $this->specifyOnly;
10680
$self->newConditionalExpressionHolders = $newConditionalExpressionHolders;
10781
$self->rootExpr = $this->rootExpr;
10882

@@ -132,11 +106,6 @@ public function shouldOverwrite(): bool
132106
return $this->overwrite;
133107
}
134108

135-
public function isSpecifyOnly(): bool
136-
{
137-
return $this->specifyOnly;
138-
}
139-
140109
/**
141110
* @return array<string, ConditionalExpressionHolder[]>
142111
*/
@@ -159,7 +128,6 @@ public function removeExpr(string $exprString): self
159128

160129
$self = new self($sureTypes, $sureNotTypes);
161130
$self->overwrite = $this->overwrite;
162-
$self->specifyOnly = $this->specifyOnly;
163131
$self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders;
164132
$self->rootExpr = $this->rootExpr;
165133

@@ -199,9 +167,6 @@ public function intersectWith(SpecifiedTypes $other): self
199167
if ($this->overwrite && $other->overwrite) {
200168
$result = $result->setAlwaysOverwriteTypes();
201169
}
202-
if ($this->specifyOnly || $other->specifyOnly) {
203-
$result = $result->setSpecifyOnly();
204-
}
205170

206171
return $result->setRootExpr($rootExpr);
207172
}
@@ -239,9 +204,6 @@ public function unionWith(SpecifiedTypes $other): self
239204
if ($this->overwrite || $other->overwrite) {
240205
$result = $result->setAlwaysOverwriteTypes();
241206
}
242-
if ($this->specifyOnly || $other->specifyOnly) {
243-
$result = $result->setSpecifyOnly();
244-
}
245207

246208
$conditionalExpressionHolders = $this->newConditionalExpressionHolders;
247209
foreach ($other->newConditionalExpressionHolders as $exprString => $holders) {
@@ -273,9 +235,6 @@ public function normalize(Scope $scope): self
273235
if ($this->overwrite) {
274236
$result = $result->setAlwaysOverwriteTypes();
275237
}
276-
if ($this->specifyOnly) {
277-
$result = $result->setSpecifyOnly();
278-
}
279238

280239
return $result->setRootExpr($this->rootExpr);
281240
}

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1856,7 +1856,19 @@ static function (Type $type, callable $traverse) use ($templateTypeMap, &$contai
18561856
$assertedType,
18571857
$assert->isNegated() ? TypeSpecifierContext::createFalse() : TypeSpecifierContext::createTrue(),
18581858
$scope,
1859-
)->setRootExpr($containsUnresolvedTemplate || $assert->isEquality() ? $call : null);
1859+
);
1860+
if ($assert->isEquality()) {
1861+
$assertContext = $assert->isNegated() ? TypeSpecifierContext::createFalse() : TypeSpecifierContext::createTrue();
1862+
$newTypes = $newTypes->unionWith(
1863+
$this->create(
1864+
$call,
1865+
new ConstantBooleanType($assertContext->true()),
1866+
TypeSpecifierContext::createTrue(),
1867+
$scope,
1868+
),
1869+
);
1870+
}
1871+
$newTypes = $newTypes->setRootExpr($containsUnresolvedTemplate || $assert->isEquality() ? $call : null);
18601872
$types = $types !== null ? $types->unionWith($newTypes) : $newTypes;
18611873

18621874
if (!$context->null() || !$assertedType instanceof ConstantBooleanType) {

‎src/Rules/Comparison/ImpossibleCheckTypeHelper.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,16 @@ public function findSpecifiedType(
278278

279279
$rootExpr = $specifiedTypes->getRootExpr();
280280
if ($rootExpr !== null) {
281+
if ($scope->hasExpressionType($rootExpr)->yes()) {
282+
$rootExprType = $this->treatPhpDocTypesAsCertain ? $scope->getType($rootExpr) : $scope->getNativeType($rootExpr);
283+
if ($rootExprType->isTrue()->yes()) {
284+
return true;
285+
}
286+
if ($rootExprType->isFalse()->yes()) {
287+
return false;
288+
}
289+
}
290+
281291
if (self::isSpecified($typeSpecifierScope, $node, $rootExpr)) {
282292
return null;
283293
}
@@ -290,10 +300,6 @@ public function findSpecifiedType(
290300
return null;
291301
}
292302

293-
if ($specifiedTypes->isSpecifyOnly()) {
294-
return null;
295-
}
296-
297303
$results = [];
298304

299305
$assignedInCallVars = [];

‎src/Type/Php/ArrayKeyExistsFunctionTypeSpecifyingExtension.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use PHPStan\Type\Accessory\HasOffsetType;
1616
use PHPStan\Type\Accessory\NonEmptyArrayType;
1717
use PHPStan\Type\ArrayType;
18+
use PHPStan\Type\Constant\ConstantBooleanType;
1819
use PHPStan\Type\Constant\ConstantIntegerType;
1920
use PHPStan\Type\Constant\ConstantStringType;
2021
use PHPStan\Type\FunctionTypeSpecifyingExtension;
@@ -112,7 +113,14 @@ public function specifyTypes(
112113
$arrayType->getIterableValueType(),
113114
$context,
114115
$scope,
115-
))->setSpecifyOnly();
116+
))->unionWith(
117+
$this->typeSpecifier->create(
118+
$node,
119+
new ConstantBooleanType(true),
120+
$context,
121+
$scope,
122+
),
123+
)->setRootExpr($node);
116124
}
117125

118126
return new SpecifiedTypes();

‎src/Type/Php/StrContainingTypeSpecifyingExtension.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\ShouldNotHappenException;
1414
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
1515
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
16+
use PHPStan\Type\Constant\ConstantBooleanType;
1617
use PHPStan\Type\FunctionTypeSpecifyingExtension;
1718
use PHPStan\Type\IntersectionType;
1819
use PHPStan\Type\StringType;
@@ -84,7 +85,14 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n
8485
new IntersectionType($accessories),
8586
$context,
8687
$scope,
87-
)->setSpecifyOnly();
88+
)->unionWith(
89+
$this->typeSpecifier->create(
90+
$node,
91+
new ConstantBooleanType(true),
92+
$context,
93+
$scope,
94+
),
95+
)->setRootExpr($node);
8896
}
8997
}
9098

‎tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,18 @@ public function testNonEmptySpecifiedString(): void
510510
public function testBug14705(): void
511511
{
512512
$this->treatPhpDocTypesAsCertain = true;
513-
$this->analyse([__DIR__ . '/data/bug-14705.php'], []);
513+
$this->analyse([__DIR__ . '/data/bug-14705.php'], [
514+
[
515+
'Call to function str_ends_with() with non-empty-string and non-empty-string will always evaluate to true.',
516+
75,
517+
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
518+
],
519+
[
520+
'Call to function str_contains() with non-empty-string and non-empty-string will always evaluate to true.',
521+
87,
522+
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
523+
],
524+
]);
514525
}
515526

516527
public function testBug2755(): void

‎tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,18 @@ public function testBug10337(): void
308308
$this->analyse([__DIR__ . '/data/bug-10337.php'], []);
309309
}
310310

311+
public function testBug14705(): void
312+
{
313+
$this->treatPhpDocTypesAsCertain = true;
314+
$this->analyse([__DIR__ . '/data/bug-14705.php'], [
315+
[
316+
'Call to method Bug14705\Foo::isValid() with non-empty-string will always evaluate to true.',
317+
104,
318+
'If Bug14705\Foo::isValid() is impure, add <fg=cyan>@phpstan-impure</> PHPDoc tag above its declaration. Learn more: <fg=cyan>https://phpstan.org/blog/remembering-and-forgetting-returned-values</>',
319+
],
320+
]);
321+
}
322+
311323
public function testInTrait(): void
312324
{
313325
$this->treatPhpDocTypesAsCertain = true;

‎tests/PHPStan/Rules/Comparison/data/bug-14705.php‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public function arrayKeyExistsNonEmpty(array $array, string $key): void
7272
public function strEndsWithDuplicate(string $haystack, string $needle): void
7373
{
7474
if (str_ends_with($haystack, $needle)) {
75-
if (str_ends_with($haystack, $needle)) {
75+
if (str_ends_with($haystack, $needle)) { // reported as always-true
7676

7777
}
7878
}
@@ -84,7 +84,24 @@ public function strEndsWithDuplicate(string $haystack, string $needle): void
8484
public function strContainsDuplicate(string $haystack, string $needle): void
8585
{
8686
if (str_contains($haystack, $needle)) {
87-
if (str_contains($haystack, $needle)) {
87+
if (str_contains($haystack, $needle)) { // reported as always-true
88+
89+
}
90+
}
91+
}
92+
93+
/**
94+
* @phpstan-assert-if-true =non-empty-string $foo
95+
*/
96+
public function isValid(string $foo): bool
97+
{
98+
return $foo !== '';
99+
}
100+
101+
public function equalityAssertDuplicate(string $task): void
102+
{
103+
if ($this->isValid($task)) {
104+
if ($this->isValid($task)) { // reported as always-true
88105

89106
}
90107
}

0 commit comments

Comments
 (0)