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

False positive: Call to inArray() with different object instances will always evaluate to true #142

Open
simPod opened this issue Jul 5, 2021 · 13 comments
Labels
bug Something isn't working

Comments

@simPod
Copy link
Contributor

simPod commented Jul 5, 2021

class A {
	public function __construct(private string $val) {
	}	
}

function foo(A $a) {
    Assert::inArray($a, [new A('a')]);
}

gives:

Call to static method Webmozart\Assert\Assert::inArray() with arguments A and array(A) will always evaluate to true

@ondrejmirtes
Copy link
Member

@ondrejmirtes ondrejmirtes transferred this issue from phpstan/phpstan-webmozart-assert Jul 21, 2021
@mergeable mergeable bot closed this as completed Jul 21, 2021
@mergeable
Copy link

mergeable bot commented Jul 21, 2021

Hi there! 👋

Looks like you opened an issue without following one of the issue templates:

Bug report 🐛 (open an issue)

If something isn't working as expected 🤔.

Feature request 🚀 (open an issue)

I have a suggestion (and may want to implement it 🙂)!

Support question ❓ (open a discussion)

I need some help with my code because PHPStan doesn't like it.


The current issue will be closed. This is a precaution to save maintainer's time, I hope you'll understand.

Sincerely, the bot 🤖

@staabm
Copy link
Contributor

staabm commented Aug 8, 2021

tried working on it, but I don't get how to formulate the unit test to reproduce the problem (I don't get how the LegacyNodeScopeResolverTest works for this case)

@ondrejmirtes
Copy link
Member

You'd need to:

  1. Create a new test case that looks like https://github.com/phpstan/phpstan-src/blob/master/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php and tests the same rule
  2. Create a new type-specifying extension that mimics what in_array does. You can copy and reduce this file to a minimal working example:
    'inArray' => function (Scope $scope, Arg $needle, Arg $array): \PhpParser\Node\Expr {
    return new \PhpParser\Node\Expr\FuncCall(
    new \PhpParser\Node\Name('in_array'),
    [
    $needle,
    $array,
    new Arg(new \PhpParser\Node\Expr\ConstFetch(new \PhpParser\Node\Name('true'))),
    ]
    );
    },
  3. Create foo.neon that registers this extension
  4. Return the path to foo.neon in the testcase's (from 1)) method public static function getAdditionalConfigFiles().

@p4veI
Copy link

p4veI commented Sep 10, 2021

Hello, I just came across a similar issue as per the following example:

/**
 * @param mixed $value
 * @param array<mixed> $array 
 */
public function test($value, array $array) : void
{
  Assert::inArray($value, $array);
}

phpstan reports:
Call to static method Webmozart\Assert\Assert::inArray() with mixed and array will always evaluate to true.

However this is not true, the webmozart assert function is using the exact same param annotations.

I'm not completely sure if this is related to what @simPod has reported.

@ondrejmirtes
Copy link
Member

@p4veI In fact this is exactly the same issue :) In fact this issue was moved from phpstan-webmozart-repo here because it needs to be fixed in PHPStan's core.

@p4veI
Copy link

p4veI commented Jun 20, 2022

imo fixed in latest release ;)

@staabm
Copy link
Contributor

staabm commented Jun 20, 2022

Please send a regression test, or it didn't happen :-)

@ondrejmirtes ondrejmirtes transferred this issue from phpstan/phpstan Jun 20, 2022
@herndlm
Copy link
Contributor

herndlm commented Jul 11, 2022

Interesting, I just tried to add a regression test for this. The current state is

@ondrejmirtes
Copy link
Member

Not surprising - there's still a massive codeblock about in_array in ImpossibleCheckTypeHelper: https://github.com/phpstan/phpstan-src/blob/a5f8d2dd0095a80f69fc23364f9cddae67ed7c44/src/Rules/Comparison/ImpossibleCheckTypeHelper.php#L80-L138

It's there to prevent false-positives about in_array calls, but it's not going to work if an extension delegates its method call to in_array extension via TypeSpecifier.

My comment still stands #142 (comment)

@herndlm
Copy link
Contributor

herndlm commented Jul 11, 2022

ah right, I was just super confused because I could not reproduce it in a phpstan snippet. actually it never reported even when it definitely should. that explains it all :) thx
Maybe I try to look at the in_array stuff in phpstan soon. that feels more like my domain :)

@ondrejmirtes
Copy link
Member

Enjoy :D phpstan/phpstan#6705

@VincentLanglet
Copy link

Just encounter the issue with

Assert::inArray('from__r', $fields);
Assert::inArray('to__r', $fields);
Assert::inArray('fromEnd__r', $fields);
Assert::inArray('toEnd__r', $fields);

Only the second, third and fourth check are reported as always true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants