Skip to content

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Oct 20, 2024

Comment on lines +1113 to +989
"• 'publishDate' is not lowercase.
• 'approvedAt' is not lowercase.
• 'allowedValues' is not lowercase.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the comparison is between 'publishDate'|'approvedAt'|'allowedValues' and lowercase string, and because of how UnionType::isSuperTypeOfWithReason works

$result = IsSuperTypeOfResult::createNo()->or(...array_map(static fn (Type $innerType) => $innerType->isSuperTypeOfWithReason($otherType), $this->types));
		if ($result->yes()) {
			return $result;
		}

we have a list with three reasons.

Is it ok for you @ondrejmirtes ?

@VincentLanglet VincentLanglet force-pushed the impossibleReason branch 2 times, most recently from 0543a24 to 0087cd2 Compare November 19, 2024 19:55
@VincentLanglet VincentLanglet marked this pull request as ready for review November 19, 2024 21:53
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@VincentLanglet
Copy link
Contributor Author

Not sure it's a satisfying solution

@ondrejmirtes
Copy link
Member

There are conflicts now.

@ondrejmirtes
Copy link
Member

Also - maybe base this on 2.1.x now.

@VincentLanglet VincentLanglet changed the base branch from 1.12.x to 2.1.x January 15, 2025 09:32
@VincentLanglet VincentLanglet force-pushed the impossibleReason branch 4 times, most recently from c9618d9 to 944d18b Compare January 15, 2025 09:50
@VincentLanglet VincentLanglet marked this pull request as draft January 15, 2025 11:29
@VincentLanglet
Copy link
Contributor Author

Also - maybe base this on 2.1.x now.

It stopped working on 2.1.x when I rebased for the case

in_array($sortBy, ['publishDate', 'approvedAt', 'allowedValues'], true)

because:

  • In 1.12.x, $specifiedTypes->getRootExpr() returns NULL
  • In 2.1.x, it now returns PhpParser\Node\Expr\BinaryOp\Identical. So the code used is
$rootExprType = ($this->treatPhpDocTypesAsCertain ? $scope->getType($rootExpr) : $scope->getNativeType($rootExpr));
if ($rootExprType instanceof ConstantBooleanType) {
     return [$rootExprType->getValue(), []];
}

It seems like in_array($sortBy, ['publishDate', 'approvedAt', 'allowedValues'] is now understood as $sortBy === 'publishDate' || $sortBy === 'approvedAt' || $sortBy === 'allowedValues'.

The issue is that when using $rootExprType we don't have the reason.

I dunno if you see a fix/solution and/or if you're still interested in a way by this PR since it might have some inconsistency between when a reason is given and when it's not given @ondrejmirtes ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In_array always true error may be more precise

3 participants