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

[treatPhpDocTypesAsCertain: false] Call to static method Assert::notNull() with DateTime|null will always evaluate to false. #4657

Closed
ruudk opened this issue Mar 4, 2021 · 8 comments
Labels
Milestone

Comments

@ruudk
Copy link
Contributor

ruudk commented Mar 4, 2021

Bug report

phpstan/phpstan                         0.12.80
phpstan/phpstan-phpunit                 0.12.17
phpstan/phpstan-strict-rules            0.12.9
phpstan/phpstan-webmozart-assert        0.12.12
$value = null;
$callback = function () use (&$value) : void {
    $value = new DateTime();
};
$callback();

// phpstan: Call to static method Webmozart\Assert\Assert::notNull() with DateTime|null will always evaluate to false.
\Webmozart\Assert\Assert::notNull($value);

Same happens when using PHPUnit's $this->assertNotNull($command); together with phpstan/phpstan-phpunit. So I believe this is not a webmozart / phpunit extension issue.

// phpstan: Call to method PHPUnit\Framework\Assert::assertNotNull() with DateTime|null will always evaluate to false.
$this->assertNotNull($value);

It's interesting because the error correctly says $value is DateTime|null.

Code snippet that reproduces the problem

Not possible as phpstan/phpstan-webmozart-assert is not installed. It would be great if the official extensions could be enabled on the playground.

Expected output

No error.

@mergeable
Copy link

mergeable bot commented Mar 4, 2021

This bug report is missing a link to reproduction on phpstan.org.

It will most likely be closed after manual review.

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Mar 4, 2021
@ruudk
Copy link
Contributor Author

ruudk commented Mar 4, 2021

I see this is marked as easy fix. Do you have any recommendations on how to fix this? I could try to create a PR and fix it myself :)

@ondrejmirtes
Copy link
Member

Hi, I thought this would be easily reproducible. This code is equivalent but it doesn't show any error: https://phpstan.org/r/60856b1b-041c-4f7f-bca7-1eb05e228888 Once we have it reproduced with vanilla PHPStan, I'll tell you about the fix. Essentially we'd need MutatingScope::$nativeExpressionTypes to contain the right type guarded by native typehints after the closure... And it's tested with assertNativeType() in NodeScopeResolver::testFileAsserts() (check the list of dataProviders).

@ondrejmirtes
Copy link
Member

When you're at it, this one should be really easy to fix: #4650

We just need these lines https://github.com/phpstan/phpstan-src/blob/6f7cadf96c7b55a0df21b51730e4b53ad13a7545/src/Analyser/MutatingScope.php#L757-L758 to call ->getNativeType() if !$this->treatPhpDocTypesAsCertain.

And the test should prove that assertNativeType('bool', $idx === []), not false.

@ruudk
Copy link
Contributor Author

ruudk commented Mar 16, 2021

@ondrejmirtes I'm still not sure how to fix this issue. I fixed the other issue in phpstan/phpstan-src#476 but I cannot reproduce my problem without the phpstan-webmozart-assert on the playground. Any advice on how to proceed?

@ondrejmirtes
Copy link
Member

Reproduced: https://phpstan.org/r/f371c7db-7deb-440a-b1f6-398fd8a62ffe

I guess the problem is that somewhere around ImpossibleCheckTypeHelper we should call $scope->getNativeType() instead of $scope->getType().

@ondrejmirtes
Copy link
Member

Fixed by: phpstan/phpstan-src#477

@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

2 participants