-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix PHPStan for Twig 4.x #4466
base: 4.x
Are you sure you want to change the base?
Fix PHPStan for Twig 4.x #4466
Conversation
@VincentLanglet I've just merged 3.x into 4.x to keep this PR only about phpstan. |
cb89b75
to
e8fe4a1
Compare
private array $unaryOperators; | ||
/** @var array<string, array{precedence: int, precedence_change?: OperatorPrecedenceChange, class: class-string<AbstractBinary>, associativity: self::OPERATOR_*}> */ | ||
/** @var array<string, array{precedence: int, precedence_change?: OperatorPrecedenceChange, class: class-string<AbstractExpression>, associativity: self::OPERATOR_*}> */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 2 changes look wrong to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those values are set in construct
$this->unaryOperators = $env->getUnaryOperators();
$this->binaryOperators = $env->getBinaryOperators();
They call the method on $this->extensionSet
.
The ExtensionSet gets them from multiple ExtensionInterface,
and getOperators is
/**
* Returns a list of operators to add to the existing list.
*
* @return array<array> First array of unary operators, second array of binary operators
*
* @psalm-return array{
* array<string, array{precedence: int, precedence_change?: OperatorPrecedenceChange, class: class-string<AbstractExpression>}>,
* array<string, array{precedence: int, precedence_change?: OperatorPrecedenceChange, class?: class-string<AbstractExpression>, associativity: ExpressionParser::OPERATOR_*}>
* }
*/
public function getOperators(): array;
This is because
'??' => ['precedence' => 5, 'class' => NullCoalesceExpression::class, 'associativity' => ExpressionParser::OPERATOR_RIGHT],
is not an AbstractBinary but just an AbtractExpression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. I suppose all binary operators should extend AbstractBinary. I'm going to have a look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. I suppose all binary operators should extend AbstractBinary. I'm going to have a look at it.
The issue already exists on 3.x if you want to take a look and fix it on the lower branch.
You can ping me after the fix, I'll update the PHPDoc if needed :)
402e96f
to
97ef15c
Compare
For the iterable/Iterator issue, I think it's an issue with PHPStan instead. |
Let's revert the change then. |
I reverted the extra if I added. We still need |
Hi @fabpot, to help introducing PHPStan on 4.x, I made the merge 3.x into 4.x
I have
A commit with the merge 6a82e16
A commit with the update needed to get PHPStan build green in 4.x cb89b75