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

Type inferrence with negated condition with OR #4466

Closed
orklah opened this issue Nov 1, 2020 · 10 comments
Closed

Type inferrence with negated condition with OR #4466

orklah opened this issue Nov 1, 2020 · 10 comments

Comments

@orklah
Copy link
Collaborator

orklah commented Nov 1, 2020

Hi,

In the example below, I'd have expected the type to be trait-string|class-string
https://psalm.dev/r/b09d68b1ed

Thanks

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/b09d68b1ed
<?php

function specifyString(string $className): void{
    if (! (class_exists($className, false) || interface_exists($className, false) || trait_exists($className, false))) {
        return;
    }
 	/** @psalm-trace $className */;
    
    new ReflectionClass($className);
}
Psalm output (using commit 8f05cc9):

INFO: Trace - 7:33 - $className: string

ERROR: ArgumentTypeCoercion - 9:25 - Argument 1 of ReflectionClass::__construct expects class-string|object|trait-string, parent type string provided

@weirdan
Copy link
Collaborator

weirdan commented Nov 1, 2020

This seems to be partially caused by wrong handling for autoloader flag for these functions: https://psalm.dev/r/3313a99852

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/3313a99852
<?php

function specifyString(string $className): void{
    if (!class_exists($className, false)) {
        // class does not exist or is not loaded
        return;
    }
 	/** @psalm-trace $className */;
    // class exists and is loaded, but Psalm still complain
    new ReflectionClass($className);
}
Psalm output (using commit f0a30b9):

INFO: Trace - 8:33 - $className: string

ERROR: ArgumentTypeCoercion - 10:25 - Argument 1 of ReflectionClass::__construct expects class-string|object|trait-string, parent type string provided

@weirdan weirdan added the bug label Nov 1, 2020
@weirdan
Copy link
Collaborator

weirdan commented Nov 1, 2020

Short-circuiting behaviour of || also doesn't seem to be taken into account: https://psalm.dev/r/750ff3309a

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/750ff3309a
<?php

function specifyString(string $className): void{
    if (! (class_exists($className, true) || interface_exists($className, true) || trait_exists($className, true))) {
        return;
    }
    // expected: class-string|trait-string
 	/** @psalm-trace $className */;
    
    new ReflectionClass($className);
}
Psalm output (using commit f0a30b9):

INFO: Trace - 8:33 - $className: trait-string

@muglug muglug added the wontfix label Nov 1, 2020
@muglug
Copy link
Collaborator

muglug commented Nov 1, 2020

I'm gonna say, with the trait_exists check, that this is a no-fix. Just too much of an edge-case.

@muglug muglug closed this as completed Nov 1, 2020
@weirdan
Copy link
Collaborator

weirdan commented Nov 1, 2020

@muglug https://psalm.dev/r/3313a99852 is clearly wrong though, isn't it? And it doesn't involve trait_exists()

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/3313a99852
<?php

function specifyString(string $className): void{
    if (!class_exists($className, false)) {
        // class does not exist or is not loaded
        return;
    }
 	/** @psalm-trace $className */;
    // class exists and is loaded, but Psalm still complain
    new ReflectionClass($className);
}
Psalm output (using commit 966b139):

INFO: Trace - 8:33 - $className: string

ERROR: ArgumentTypeCoercion - 10:25 - Argument 1 of ReflectionClass::__construct expects class-string|object|trait-string, parent type string provided

@muglug muglug reopened this Nov 1, 2020
@muglug
Copy link
Collaborator

muglug commented Nov 1, 2020

Yeah, that should be negatable

@muglug muglug closed this as completed in 6922caf Nov 1, 2020
@orklah
Copy link
Collaborator Author

orklah commented Nov 1, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants