Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

array_pop/array_shift false positive #2851

Closed
dereuromark opened this issue Jan 15, 2020 · 12 comments · Fixed by phpstan/phpstan-src#1651
Closed

array_pop/array_shift false positive #2851

dereuromark opened this issue Jan 15, 2020 · 12 comments · Fixed by phpstan/phpstan-src#1651
Labels
Milestone

Comments

@dereuromark
Copy link
Contributor

Bug report

array_* false positive

while (count($arguments) > 0) {
	$words .= array_pop($arguments);
	if (count($arguments) > 0) {
		$words .= ' ';
	}
}

Code snippet that reproduces the problem

$arguments = ['x', 'y'];

$words = '';
while (count($arguments) > 0) {
	$words .= array_pop($arguments);
	if (count($arguments) > 0) {
		$words .= ' ';
	}
}

echo $words;

https://phpstan.org/r/172e2161-c350-4466-8867-de074489c34f

Expected output

No error

@ondrejmirtes
Copy link
Member

Not sure why is this still happening - /cc @cs278

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Jan 16, 2020
@cs278
Copy link
Contributor

cs278 commented Jan 16, 2020

Reverting phpstan/phpstan-src@b1fd47b results in an error at the if statement:

Comparison operation ">" between 1|2 and 0 is always true.

Which makes sense as the array_pop() now invalidates the known value of count($arguments). At a guess I think PHPStan is only ever seeing the constant array that was initially defined, I suppose the code needs to either pop the element off the constant array or change the type to not be a constant array.

@ondrejmirtes
Copy link
Member

There's most likely a bug in handling the while loop.

@cs278
Copy link
Contributor

cs278 commented Dec 17, 2021

Hah think I've just found this again.

https://phpstan.org/r/eff591cd-41f7-4e5e-a236-f000f32236d5

Presumably this
https://github.com/phpstan/phpstan-src/blob/e4b71d41de38a83ae4a9dcd2a9bdf342e10f3c3a/src/Analyser/NodeScopeResolver.php#L1849-L1878 should union on an empty array as that is a possible outcome of a array_{shift,pop}() call.

@phpstan-bot
Copy link
Contributor

@cs278 After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
  5: Method HelloWorld::sayHello() has parameter $input with no value type specified in iterable type iterable.
 18: Dumped type: array{0: 1|(literal-string&non-empty-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'}
 20: Call to function assert() with false will always evaluate to false.
-20: Strict comparison using === between array{0: 1|(literal-string&non-empty-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'} and array{} will always evaluate to false.
-21: Call to function assert() with false will always evaluate to false.
-21: Strict comparison using === between int<1, max> and 0 will always evaluate to false.
+20: Strict comparison using === between array{0: 1|(literal-string&non-empty-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'} and array{} will always evaluate to false.
Full report
Line Error
5 Method HelloWorld::sayHello() has parameter $input with no value type specified in iterable type iterable.
18 `Dumped type: array{0: 1
20 Call to function assert() with false will always evaluate to false.
20 `Strict comparison using === between array{0: 1

@phpstan-bot
Copy link
Contributor

@dereuromark After the latest commit in 1.6.x, PHPStan now reports different result with your code snippet:

@@ @@
- 6: Comparison operation ">" between 1|2 and 0 is always true.
+ 6: Comparison operation ">" between int<1, 2> and 0 is always true.
 13: Unreachable statement - code above always terminates.
Full report
Line Error
6 Comparison operation ">" between int<1, 2> and 0 is always true.
13 Unreachable statement - code above always terminates.

@Seldaek
Copy link
Contributor

Seldaek commented May 5, 2022

Another one I discovered today due to the changes to DateTime::modify

https://phpstan.org/r/213421a3-bdf9-4ee1-93b9-6e3cc368021a

It seems after an array_shift the whole static array is lost but IMO it should be able to retain the values as a union of static strings?

@phpstan-bot
Copy link
Contributor

@cs278 After the latest push in 1.8.x, PHPStan now reports different result with your code snippet:

@@ @@
  5: Method HelloWorld::sayHello() has parameter $input with no value type specified in iterable type iterable.
-18: Dumped type: array{0: 1|(literal-string&non-empty-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'}
+18: Dumped type: array{0: 1|(literal-string&non-falsy-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'}
 20: Call to function assert() with false will always evaluate to false.
-20: Strict comparison using === between array{0: 1|(literal-string&non-empty-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'} and array{} will always evaluate to false.
-21: Call to function assert() with false will always evaluate to false.
-21: Strict comparison using === between int<1, max> and 0 will always evaluate to false.
+20: Strict comparison using === between array{0: 1|(literal-string&non-falsy-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'} and array{} will always evaluate to false.
Full report
Line Error
5 Method HelloWorld::sayHello() has parameter $input with no value type specified in iterable type iterable.
18 `Dumped type: array{0: 1
20 Call to function assert() with false will always evaluate to false.
20 `Strict comparison using === between array{0: 1

@phpstan-bot
Copy link
Contributor

@Seldaek After the latest push in 1.8.x, PHPStan now reports different result with your code snippet:

@@ @@
  7: Dumped type: array{'+1week', '+1months', '+6months', '+17months'}
- 9: Expected type array<0|1|2|3, '+1week'|'+1months'|'+6months'|'+17months'>, actual: array<0|1|2|3, literal-string&non-empty-string>
+ 9: Expected type array<0|1|2|3, '+1week'|'+1months'|'+6months'|'+17months'>, actual: array<0|1|2|3, literal-string&non-falsy-string>
 10: Cannot call method modify() on DateTimeImmutable|false.
Full report
Line Error
7 Dumped type: array{'+1week', '+1months', '+6months', '+17months'}
9 `Expected type array<0
10 `Cannot call method modify() on DateTimeImmutable

@phpstan-bot
Copy link
Contributor

@dereuromark After the latest push in 1.8.x, PHPStan now reports different result with your code snippet:

@@ @@
- 6: Comparison operation ">" between 1|2 and 0 is always true.
-13: Unreachable statement - code above always terminates.
+No errors

@phpstan-bot
Copy link
Contributor

@cs278 After the latest push in 1.8.x, PHPStan now reports different result with your code snippet:

@@ @@
  5: Method HelloWorld::sayHello() has parameter $input with no value type specified in iterable type iterable.
-18: Dumped type: array{0: 1|(literal-string&non-empty-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'}
-20: Call to function assert() with false will always evaluate to false.
-20: Strict comparison using === between array{0: 1|(literal-string&non-empty-string)|false, 1: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'} and array{} will always evaluate to false.
-21: Call to function assert() with false will always evaluate to false.
-21: Strict comparison using === between int<1, max> and 0 will always evaluate to false.
+18: Dumped type: array{0?: 1|(literal-string&non-falsy-string)|false, 1?: 1|'x'|'y', 2?: 'x'|'y', 3?: 'y'}
Full report
Line Error
5 Method HelloWorld::sayHello() has parameter $input with no value type specified in iterable type iterable.
18 `Dumped type: array{0?: 1

staabm added a commit to staabm/phpstan-src that referenced this issue Aug 25, 2022
staabm added a commit to staabm/phpstan-src that referenced this issue Aug 25, 2022
ondrejmirtes pushed a commit to phpstan/phpstan-src that referenced this issue Aug 25, 2022
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants