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

Add $not constructor parameter to AST classes #10267

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Dec 4, 2022

While reviewing #10219, I noticed the inconsistent typing of negation flags ($not) on various AST classes. In most cases, we populate $someAstExpression->not right after construction the object. In some cases, we don't assign any value to it, but assume it to be false afterwards.

With all those public properties, I think we should aim at making all those classes immutable in 3.0. Adding $not to constructors is a first step in that direction. I've also split the InExpression class into two classes because it actually covers two distinct cases that we need to distinguish whenever we deal with an instance of that class.

@derrabus derrabus force-pushed the improvement/ast-not-constructor branch from d9b850a to 59d612b Compare December 5, 2022 00:40
@derrabus derrabus merged commit 5a8541b into doctrine:2.14.x Dec 5, 2022
@derrabus derrabus deleted the improvement/ast-not-constructor branch December 5, 2022 10:45
derrabus added a commit to derrabus/orm that referenced this pull request Dec 5, 2022
* 2.14.x:
  Add $not constructor parameter to AST classes (doctrine#10267)
derrabus added a commit to derrabus/orm that referenced this pull request Dec 5, 2022
* 2.14.x:
  Add $not constructor parameter to AST classes (doctrine#10267)
@derrabus derrabus mentioned this pull request Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants