phpstan / phpstan-webmozart-assert

PHPStan extension for webmozart/assert
160 stars 27 forks source link

allIsInstanceOf / allNotNull does not tell phpstan that its evaluated as expected #171

Closed nopenopenope closed 5 months ago

nopenopenope commented 5 months ago

Hello,

today, I am observing a strange behavior that I cannot really understand, maybe it is expected, but it feels like a bug.

I have this code:

private function getTimePeriod(VacationRequestInterface $request): DatePeriod
    {
        $startDate = $request->getStartDate();
        $endDate = $request->getEndDate();
        Assert::allIsInstanceOf([$startDate, $endDate], \DateTimeImmutable::class);
        Assert::allNotNull([$startDate, $endDate]);

        return new DatePeriod(
            $startDate,
            new DateInterval('P1D'),
            $endDate,
            DatePeriod::INCLUDE_END_DATE,
        );
    }

So, I'd expect that both $startDate and $endDate would be evaluated to be DateTimeImmutable (the allNotNull check is only there for a sanity check and not really required technically). However, once I run phpstan, I receive this error:

 85     Parameter #1 $start of class DatePeriod constructor expects DateTimeInterface, DateTimeInterface|null given.                                                                                      
 87     Parameter #3 $end of class DatePeriod constructor expects DateTimeInterface, DateTimeInterface|null given.

Now, if I do singular checks with isInstanceOf instead of allIsInstanceOf, the results are as expected.

Is it happening because I create a new array inside the allIsInstanceOf, which is later not used to get the individual - and evaluated - elements? Maybe people think it make sense to allow evaluation like this, and we could adapt the module.

ondrejmirtes commented 5 months ago

Is it happening because I create a new array inside the

Yes :) In case of two variables like that you can assert them separately using Assert::notNull right after they are assigned.

nopenopenope commented 5 months ago

Okay, thanks, than my assumption were correct; it makes sense to me to some extend, but it would of course have a charme if I can check ample elements in one call rather than having multiple calls for the same thing; but I get it. Thanks for your constant work on this amazing piece of software, Ondrej. :heart:

github-actions[bot] commented 4 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.