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

False positive with array_merge #2567

Closed
MarioHoberg opened this issue Nov 9, 2019 · 16 comments · Fixed by phpstan/phpstan-src#1155
Closed

False positive with array_merge #2567

MarioHoberg opened this issue Nov 9, 2019 · 16 comments · Fixed by phpstan/phpstan-src#1155
Labels
Milestone

Comments

@MarioHoberg
Copy link

I am getting a "Property ... (array<string, bool|int|string>) does not accept array<string, int>" error when working with array_merge().

https://phpstan.org/r/82e6d926-5634-4f25-ab61-c08cf403582b

I would expect to not get such an error when each of the passed associative arrays should work.

@ondrejmirtes
Copy link
Member

ArrayMergeFunctionDynamicReturnTypeExtension does not preserve ConstantArrayType. Feel free to send a fix.

@greg0ire
Copy link

greg0ire commented Aug 5, 2020

Note for anyone willing to work on this, (might be me in the future, who knows):

ConstantArrayType is a subtype of what ArrayMergeFunctionDynamicReturnTypeExtension::getTypeFromFunctionCall
returns
.
I think it represents arrays that have a fixed number of keys (so array shapes, written like so: array{…}).
While writing the patch, one should not forget to account for numeric arrays (keys are lost and renumbering happens, starting at 0).
There seems to be existing tests in NodeScopeResolverTest, because that's where the test suite crashes when you throw from inside ArrayMergeFunctionDynamicReturnTypeExtension::getTypeFromFunctionCall.

I think new test data should be added to tests/PHPStan/Analyser/data/constantTypes.php

@greg0ire
Copy link

greg0ire commented Aug 6, 2020

@dereuromark
Copy link
Contributor

Is this a dup of #1516 ?

@phpstan-bot
Copy link
Contributor

@MarioLipinski PHPStan now reports different result with your code snippet:

@@ @@
+PHP 7.3 – 8.0 (1 error)
+==========
+
+10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
+
+PHP 7.2 (1 error)
+==========
+
+ 6: Function foo() has parameter $a with no value type specified in iterable type array.
+
+PHP 7.1 (1 error)
+==========
+
 10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
Full report

PHP 7.3 – 8.0 (1 error)

Line Error
10 Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.

PHP 7.2 (1 error)

Line Error
6 Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.

PHP 7.1 (1 error)

Line Error
10 Function foo() has parameter $a with no value type specified in iterable type array.

@phpstan-bot
Copy link
Contributor

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

@@ @@
-10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
+10: Parameter #1 $a of function foo expects array('foo' => string), array<non-empty-string, ''> given.
Full report
Line Error
10 Parameter #1 $a of function foo expects array('foo' => string), array<non-empty-string, ''> given.

@phpstan-bot
Copy link
Contributor

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

@@ @@
-10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
+10: Parameter #1 $a of function foo expects array('foo' => string), array<non-empty-string, ''>&nonEmpty given.
Full report
Line Error
10 Parameter #1 $a of function foo expects array('foo' => string), array<non-empty-string, ''>&nonEmpty given.

@phpstan-bot
Copy link
Contributor

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

@@ @@
-10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
+No errors

@ondrejmirtes
Copy link
Member

Fixed by: phpstan/phpstan-src#622

@ondrejmirtes
Copy link
Member

Unfortunately I had to revert this extension: phpstan/phpstan-src@53817aa

It's because of performance impact on real-world projects. The extension itself may not be the fault but it proves that having more ConstantArrayType instances during analysis leads to a slowdown.

The only way I see is to build a benchmarking mode into PHPStan itself that would compare the analysis times of project files and point out which file slowed down after this commit. Then isolate what exactly in that file causes it and try to optimize that. Unfortunately it's not my priority right now.

@ondrejmirtes ondrejmirtes reopened this Aug 30, 2021
@phpstan-bot
Copy link
Contributor

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

@@ @@
-10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
+10: Parameter #1 $a of function foo expects array('foo' => string), array<literal-string&non-empty-string, ''>&nonEmpty given.
Full report
Line Error
10 Parameter #1 $a of function foo expects array('foo' => string), array<literal-string&non-empty-string, ''>&nonEmpty given.

@phpstan-bot
Copy link
Contributor

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

@@ @@
-10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
+10: Parameter #1 $a of function foo expects array('foo' => string), non-empty-array<literal-string&non-empty-string, ''> given.
Full report
Line Error
10 Parameter #1 $a of function foo expects array('foo' => string), non-empty-array<literal-string&non-empty-string, ''> given.

@phpstan-bot
Copy link
Contributor

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

@@ @@
-10: Parameter #1 $a of function foo expects array('foo' => string), array<string, ''> given.
+10: Parameter #1 $a of function foo expects array{foo: string}, non-empty-array<literal-string&non-empty-string, ''> given.
Full report
Line Error
10 Parameter #1 $a of function foo expects array{foo: string}, non-empty-array<literal-string&non-empty-string, ''> given.

@staabm
Copy link
Contributor

staabm commented Mar 30, 2022

fixed by phpstan/phpstan-src#1135

@ondrejmirtes
Copy link
Member

Needs a regression test :) /cc @VincentLanglet

@github-actions
Copy link

github-actions bot commented May 3, 2022

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 May 3, 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.

6 participants