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

Unnecessary isset / array_key_exists checks not detected #9759

Open
M393 opened this issue May 8, 2023 · 3 comments
Open

Unnecessary isset / array_key_exists checks not detected #9759

M393 opened this issue May 8, 2023 · 3 comments

Comments

@M393
Copy link

M393 commented May 8, 2023

In the examples below the property x always exists, thus the check is redundant and psalm should give a warning.

https://psalm.dev/r/b32581926c
https://psalm.dev/r/a6dfa404ce

@psalm-github-bot
Copy link

I found these snippets:

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

/** @param array{x: int} $a */
function f(array $a): bool {
    if (!array_key_exists('x', $a)) {
        return false;
    }
    return true;
}
Psalm output (using commit d4c5f85):

No issues!
https://psalm.dev/r/a6dfa404ce
<?php

/** @param array{x: int} $a */
function f(array $a): bool {
    if (!isset($a['x'])) {
        return false;
    }
    return true;
}
Psalm output (using commit d4c5f85):

No issues!

@kkmuffme
Copy link
Contributor

kkmuffme commented Aug 4, 2023

Same with "empty"
Related to #8507

Also in very basic cases:
https://psalm.dev/r/a22cb83a32

EDIT: some of the issues are caused by the Mixed type latest changed #7663 (was there before though) where the not part is mixed instead of never

@psalm-github-bot
Copy link

I found these snippets:

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


function foo( $bar ) : void {
 	if ( !isset( $bar ) ) {
        if ( !isset( $bar ) ) {
			
        }    
    }
}
Psalm output (using commit 73ebe22):

INFO: MissingParamType - 4:15 - Parameter $bar has no provided type

kkmuffme added a commit to kkmuffme/psalm that referenced this issue Nov 18, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs.
It's still possible to run the validation of docblock docs though: now with 1 @psalm-suppress (instead of 2) and a @var declaration that contains both possible types, to ensure later code won't escape any checks

This is also a required preparation to fix some isset issues of vimeo#9759
kkmuffme added a commit to kkmuffme/psalm that referenced this issue Nov 18, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs.
It's still possible to run the validation of docblock docs though: a @var declaration that contains both possible types, to ensure later code won't escape any checks (and no @psalm-suppress needed at all)

This is also a required preparation to fix some isset issues of vimeo#9759
kkmuffme added a commit to kkmuffme/psalm that referenced this issue Nov 18, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs.
It's still possible to run the validation of docblock docs though: a @var declaration that contains both possible types, to ensure later code won't escape any checks (and no @psalm-suppress needed at all)

This is also a required preparation to fix some isset issues of vimeo#9759
kkmuffme added a commit to kkmuffme/psalm that referenced this issue Nov 21, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs.
It's still possible to run the validation of docblock docs though: a @var declaration that contains both possible types, to ensure later code won't escape any checks (and no @psalm-suppress needed at all)

This is also a required preparation to fix some isset issues of vimeo#9759
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