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

Mixed issues not being reported #9619

Open
ADmad opened this issue Apr 7, 2023 · 11 comments
Open

Mixed issues not being reported #9619

ADmad opened this issue Apr 7, 2023 · 11 comments
Labels
bug easy problems Issues that can be fixed without background knowledge of Psalm good first issue Help wanted

Comments

@ADmad
Copy link
Contributor

ADmad commented Apr 7, 2023

For the following config

<?xml version="1.0"?>
<psalm
    errorLevel="5"
    resolveFromConfigFile="true"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
    findUnusedBaselineEntry="false"
    findUnusedCode="false"
    usePhpDocPropertiesWithoutMagicCall="true"
    reportMixedIssues="true"
>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
    <issueHandlers>
        <MixedArgument errorLevel="error" />
        <MixedAssignment errorLevel="error" />
        <MixedPropertyAssignment errorLevel="error" />
    </issueHandlers>
</psalm>

running psalm for this code

<?php

/**
 * Safely access the values in $this->params.
 *
 * @param string $name The name or dotted path to parameter.
 * @param mixed $default The default value if `$name` is not set. Default `null`.
 * @return mixed
 */
function getParam(string $name, $default = null)
{
    if ($name && $default) {} // suppress unused name warnings
    return null;
}

$type = getParam('type');
$type = strtolower($type);

echo $type;

I would expect the same errors to be reported as seen on https://psalm.dev/r/34a696dcc4

yet I get No errors found!.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/34a696dcc4
<?php

/**
 * Safely access the values in $this->params.
 *
 * @param string $name The name or dotted path to parameter.
 * @param mixed $default The default value if `$name` is not set. Default `null`.
 * @return mixed
 */
function getParam(string $name, $default = null)
{
    if ($name && $default) {} // suppress unused name warnings
    return null;
}

$type = getParam('type');
$type = strtolower($type);

echo $type;
Psalm output (using commit 82b3a7c):

INFO: MixedAssignment - 16:1 - Unable to determine the type that $type is being assigned to

INFO: MixedArgument - 17:20 - Argument 1 of strtolower cannot be mixed, expecting string

@orklah
Copy link
Collaborator

orklah commented Apr 7, 2023

Can you try to see if you get them at errorLevel="1"?

@ADmad
Copy link
Contributor Author

ADmad commented Apr 7, 2023

Yes I do the same results as the playground with errorLevel="1" but that's not want I want.

I expect using reportMixedIssues="true" (and explicitly enabling the specific handlers) to show the mixed errors as documented here even at error level 5 (or any other level).

@orklah
Copy link
Collaborator

orklah commented Apr 7, 2023

Yeah, seems like the doc is wrongly worded. It seems currently that it was only made for silencing mixed issues. For legacy reasons, it's somewhat of a mess.

Note: I don't know what your motives are for doing that, it seems like a bad idea to me on first glance. A lot of issues above level 5 will cause mixed to appear out of nowhere. If you don't display those, you'll see the mixed part without seeing the cause of it.

However, if you want to fix things, it should not be too difficult. The main thing to understand is:

'reportMixedIssues' => 'show_mixed_issues',

This is what turns a config in xml into a property of Config. Once you get that, the property is used twice. It should be easy to track that and find out why it doesn't behave properly

@orklah orklah added bug easy problems Issues that can be fixed without background knowledge of Psalm Help wanted good first issue labels Apr 7, 2023
@ADmad
Copy link
Contributor Author

ADmad commented Apr 7, 2023

Note: I don't know what your motives are for doing that, it seems like a bad idea to me on first glance. A lot of issues above level 5 will cause mixed to appear out of nowhere. If you don't display those, you'll see the mixed part without seeing the cause of it.

I am working with an existing codebase and even level 4 generates hundreds of errors and going through the list is too tedious. Passing mixed type values to arguments which expects string (like the various string functions) is a common type of error that's occurring after turning on strict typing, so I was hoping to turn on just the related issue handlers and potentially get a more manageable list of errors.

However, if you want to fix things, it should not be too difficult.

I'll see if I can find the time to dig into it 🙂.

@orklah
Copy link
Collaborator

orklah commented Apr 7, 2023

I am working with an existing codebase and even level 4 generates hundreds of errors

Seems like a job for the baseline :). Just ignore everything and fix incoming errors as they arrive

In my experience, on existing codebase, the list of Mixed issue is rarely the shortest!

@ADmad
Copy link
Contributor Author

ADmad commented Apr 7, 2023

Just ignore everything...

The problem is we have (naively) added declare(strict_types=1) too already too all files, which is causing multiple TypeErrors which need to be addressed and can't be brushed under the rug 😄.

@orklah
Copy link
Collaborator

orklah commented Apr 7, 2023

You might want to take a look at https://github.com/orklah/psalm-strict-types

This is very much not perfect, but it should tell you if you have flagrant issues with strict_types and also tell you if entire files are safe to add strict_types.

@ADmad
Copy link
Contributor Author

ADmad commented Apr 7, 2023

You might want to take a look at https://github.com/orklah/psalm-strict-types

Thank you!! I'll give it a shot.

@ADmad
Copy link
Contributor Author

ADmad commented Apr 7, 2023

The plugin has "vimeo/psalm": "^4.4.0".

So can't try it with psalm 5.9 😞

@orklah
Copy link
Collaborator

orklah commented Apr 8, 2023

I think it would be ready to go for Psalm 5, all is missing is someone to test it so if you're up for it, just try it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug easy problems Issues that can be fixed without background knowledge of Psalm good first issue Help wanted
Projects
None yet
Development

No branches or pull requests

2 participants