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

non-empty-literal-string|literal-string is inferred as string #10925

Closed
VincentLanglet opened this issue Apr 28, 2024 · 4 comments · Fixed by #10930
Closed

non-empty-literal-string|literal-string is inferred as string #10925

VincentLanglet opened this issue Apr 28, 2024 · 4 comments · Fixed by #10930

Comments

@VincentLanglet
Copy link
Contributor

https://psalm.dev/r/21adcbe806

Copy link

I found these snippets:

https://psalm.dev/r/21adcbe806
<?php

/**
 * @param non-empty-literal-string $a
 * @param literal-string $b
 */
function foo(string $a, string $b) {
    /** @psalm-trace $c */
    $c = random_int(0, 12) ? $a : $b;
}
Psalm output (using commit 08afc45):

INFO: Trace - 9:5 - $c: string

INFO: UnusedVariable - 9:5 - $c is never referenced or the value is not used

INFO: UnusedParam - 7:32 - Param b is never referenced in this method

INFO: UnusedParam - 7:21 - Param a is never referenced in this method

INFO: MissingReturnType - 7:10 - Method foo does not have a return type, expecting void

@VincentLanglet
Copy link
Contributor Author

Hi @orklah @weirdan,

I assume this could be an easy fix but I have no idea where such type resolution is done in the psalm codebase.
Any idea which method does such job ?

@orklah
Copy link
Collaborator

orklah commented Apr 29, 2024

Yeah, it should be around

private static function scrapeStringProperties(

The TypeCombiner is the class that makes true|false become bool or non-empty-string|non-falsy-string becomes non-empty-string for example.

specifically, scrapeStringProperties handles strings of different shapes, you can see things like $has_only_numeric_strings or $has_only_non_empty_strings that are flags to check that every string match the property you want to conserve at the end

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Apr 29, 2024

Thanks, the PR is done #10930

(I also will need your help for #10927 (comment))

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

Successfully merging a pull request may close this issue.

2 participants