phpstan-webmozart-assert icon indicating copy to clipboard operation
phpstan-webmozart-assert copied to clipboard

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

Open simPod opened this issue 4 years ago • 17 comments

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

simPod avatar Jul 05 '21 10:07 simPod

Yeah, this is unfortunate, it means that this code shouldn't exist https://github.com/phpstan/phpstan-src/blob/3c3ea2f21898e8e411b0a7263910722738839288/src/Rules/Comparison/ImpossibleCheckTypeHelper.php#L85-L134 and all of that should be handled directly in https://github.com/phpstan/phpstan-src/blob/master/src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php.

ondrejmirtes avatar Jul 05 '21 12:07 ondrejmirtes

Hi there! :wave:

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 🤖

mergeable[bot] avatar Jul 21 '21 17:07 mergeable[bot]

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)

staabm avatar Aug 08 '21 06:08 staabm

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: https://github.com/phpstan/phpstan-webmozart-assert/blob/978e524796e5e81851d09d5967b173cda173d422/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php#L404-L413
  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().

ondrejmirtes avatar Aug 12 '21 13:08 ondrejmirtes

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.

p4veI avatar Sep 10 '21 13:09 p4veI

@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.

ondrejmirtes avatar Sep 10 '21 14:09 ondrejmirtes

imo fixed in latest release ;)

p4veI avatar Jun 20 '22 10:06 p4veI

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

staabm avatar Jun 20 '22 10:06 staabm

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

herndlm avatar Jul 11 '22 15:07 herndlm

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 https://github.com/phpstan/phpstan-webmozart-assert/issues/142#issuecomment-1160296906

ondrejmirtes avatar Jul 11 '22 15:07 ondrejmirtes

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 :)

herndlm avatar Jul 11 '22 15:07 herndlm

Enjoy :D https://github.com/phpstan/phpstan/issues/6705

ondrejmirtes avatar Jul 11 '22 15:07 ondrejmirtes

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.

VincentLanglet avatar Aug 30 '22 11:08 VincentLanglet