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

Nullability false positive on a complex condition #987

Closed
janedbal opened this issue May 4, 2018 · 10 comments
Closed

Nullability false positive on a complex condition #987

janedbal opened this issue May 4, 2018 · 10 comments
Labels

Comments

@janedbal
Copy link
Contributor

janedbal commented May 4, 2018

Code snippet that reproduces the problem

class HelloWorld
{
	public function sayHello(?int $a, ?int $b): void
	{
		
		if ($a === null && $b === null) {
			throw new \LogicException();
		}
		
		$this->foo($a !== null ? $a : $b); // error: Parameter #1 $c of method HelloWorld::foo() expects int, int|null given.  
	}
	
	public function foo(int $c){}
}

Expected output

  • calling foo should be ok
@ondrejmirtes ondrejmirtes added this to the 0.10 milestone May 4, 2018
@ondrejmirtes
Copy link
Member

Probably the same problem as #966 but I'll leave this open for referring to this specific case.

@muglug
Copy link
Contributor

muglug commented May 6, 2018

There are a whole bunch of similar logic tests in TypeAlgebraTest. Most, if not all, fail in PHPStan but I imagine it's not that hard to alter your engine for them to pass.

Psalm doesn't deal with #966 well either, as its algebraic assertions don't cover comparisons between variables.

@ondrejmirtes
Copy link
Member

I misread the example - this is more complex than #966 (#966 does not work because I broke something on dev-master, it works on 0.9.2). This will require something I now call "Dependent types". Because of their lack, PHPStan can't solve these examples either:

Therefore I'm postponing solving this to after the next release because it's a bigger topic.

@ondrejmirtes ondrejmirtes modified the milestones: 0.10, Dependent types May 6, 2018
@muglug
Copy link
Contributor

muglug commented May 6, 2018

Yeah, supporting this is on an ongoing basis is non-trivial. It requires keeping track of the assertions made in previous expressions as well as current types. A year-old description of that process is here: https://medium.com/vimeo-engineering-blog/automated-type-inference-for-dynamically-typed-programs-6e79197e5420

@ondrejmirtes
Copy link
Member

@janedbal
Copy link
Contributor Author

Seems like it works on latest version https://phpstan.org/r/622c3f59-0fd7-4442-b89e-356e25287fd6

@ondrejmirtes
Copy link
Member

@janedbal It doesn't, it just moved to level 8: https://phpstan.org/r/9cc9771c-0c03-412c-b186-7f99bac5120f

@ondrejmirtes ondrejmirtes reopened this Mar 15, 2021
@phpstan-bot
Copy link
Contributor

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

@@ @@
-29: Function hello() should return array<string> but returns array<string>|null.
+No errors

@phpstan-bot
Copy link
Contributor

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

@@ @@
-12: Parameter #1 $c of method HelloWorld::foo() expects int, int|null given.
+No errors

@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 Apr 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants