[ExpressionLanguage] Add support for null-safe operator #45795

mytuny · 2022-03-20T14:51:18Z

Q	A
Branch?	6.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #45411, #21691
License	MIT
Doc PR	symfony/symfony-docs#16630

This is a long-time-lasting feature for the ExpressionLanguage component. I've been waiting for the support of Nullsafe operator in expressions dealing with mutable objects, until I finally decided to work on it once for all 👍

The lack of nullsafety feature has been repeatedly reported as a BUG several time (e.g #45411 & #21691) when it is actually a missing feature.

Currently, expressions like foo.bar assumes that the property bar "always" exists on the object foo and if doesn't the parser throws a RuntimeException. Although, sometimes, that's exactly the behavior we need, some other times we may work with mutable objects with uncontrolled structure, thus, such assumption is error-prone and will force adding extra checks making the expression uglier and less readable.

The proposed work, introduces the support for the ?. syntax alongside with the usual . syntax to help working with objects with dynamic structure. The two notations works identically in all normal cases. The difference occurs when trying to access non-existant properties and/or methods where the . notation will throw a RuntimeException as usual and the ?. notation will return null instead and no errors nor exceptions will be thrown. Hence the name "Null-Safe".

PS: This work account ONLY for accessing object's properties and methods. It does not account for non-existant array items which is a seperate problem that can be addressed by introducing the null coalescing operator. Another feature that I'm currently working on as well 💯

carsonbot · 2022-03-20T14:51:23Z

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

Always add tests
Keep backward compatibility (see https://symfony.com/bc).
Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
Features and deprecations must be submitted against the 6.1 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

mytuny · 2022-03-20T14:57:00Z

I start working on this PR before the 6.1 release, therefore, this work is based on branch 5.4 instead.

nicolas-grekas

That's great thanks! Here are some minor comments

src/Symfony/Component/ExpressionLanguage/CHANGELOG.md

src/Symfony/Component/ExpressionLanguage/Node/GetAttrNode.php

src/Symfony/Component/ExpressionLanguage/Parser.php

nicolas-grekas · 2022-03-20T15:01:39Z

Right, please rebase to target ~~5.4~~6.1 as that's the branch where new features are merged.

fabpot · 2022-03-20T16:13:51Z

@nicolas-grekas wanted to say 6.1, not 5.4, for the branch :)

mytuny · 2022-03-20T21:56:27Z

I just rebased to 6.1, however, this seem to pollute the PR commits history :/ + had to resolve few conflits.
Don't you think that is better to re-open a clean PR based on 6.1 branch ?

stof · 2022-03-21T11:47:41Z

@mytuny this polluted it, because you were still targetting the 5.4 branch in the PR. I edited the PR to change the target branch to 6.1, which fixed it.

stof · 2022-03-21T11:49:43Z

also, it looks like you merged your rebased branch with the upstream branch instead of forcing the push, thus duplicating the commits in the history. See https://symfony.com/doc/current/contributing/code/pull_requests.html#rebase-your-pull-request

mytuny · 2022-03-21T11:49:51Z

@mytuny this polluted it, because you were still targetting the 5.4 branch in the PR. I edited the PR to change the target branch to 6.1, which fixed it.

Brilliant! Many thanks 👏

mytuny · 2022-03-21T14:40:40Z

also, it looks like you merged your rebased branch with the upstream branch instead of forcing the push, thus duplicating the commits in the history. See https://symfony.com/doc/current/contributing/code/pull_requests.html#rebase-your-pull-request

Although I followed exactly the steps mentioned there, a conflit-resolving commits that I've added during the rebase of 6.1 has introduced that duplication. The conflits occured with files that have been changed both in 6.1 and in my PR as well comparing to 5.4, namely the CHANGELOG file and the ConstantNode.php file. That's why I asked if it is better to re-submit a PR based on 6.1 right from the start 🤔

stof · 2022-03-21T14:46:04Z

@mytuny the process I linked to is using a force push, precisely to avoid doing a merge of the remote branch after the rebase.
You should not have the commit Merge remote-tracking branch 'origin/feature_expressionlanguage_nulls at all in your branch.

mytuny · 2022-03-21T14:47:51Z

Yess, exactly :( any suggestions ?

stof · 2022-03-21T14:55:10Z

@mytuny use git reset --hard bffdcd22cabac5e105697b44e8e34c080dbe5f39 to reset your branch to the commit before the bad merge, and then actually follow the end of the process as documented

mytuny · 2022-03-21T15:30:03Z

@stof All right! the hard reset does the trick.
This time I forced the push and we seem to have a cleaner commits history by now...
Thank you :)

src/Symfony/Component/ExpressionLanguage/CHANGELOG.md

src/Symfony/Component/ExpressionLanguage/Node/ConstantNode.php

src/Symfony/Component/ExpressionLanguage/Parser.php

src/Symfony/Component/ExpressionLanguage/CHANGELOG.md

src/Symfony/Component/ExpressionLanguage/Node/ConstantNode.php

src/Symfony/Component/ExpressionLanguage/Node/GetAttrNode.php

src/Symfony/Component/ExpressionLanguage/Tests/ExpressionLanguageTest.php

nicolas-grekas

almost good to me :)

src/Symfony/Component/ExpressionLanguage/Node/GetAttrNode.php

src/Symfony/Component/ExpressionLanguage/Tests/ExpressionLanguageTest.php

nicolas-grekas

I just fixed my own comments and rebased. LGTM 👍

nicolas-grekas · 2022-04-04T13:18:26Z

Note that the nullsafe operator only guards against null. It does not guard against undefined properties/keys.

nicolas-grekas · 2022-04-04T13:40:51Z

Thank you @mytuny.

nicolas-grekas · 2022-04-04T13:43:44Z

Note that the nullsafe operator only guards against null. It does not guard against undefined properties/keys.

About this, we might want to add support for the null-coalescing operator ??, which is able to ignore undefined vars/properties/keys.

@mytuny up to give this a try as a follow up? Please have a look at the changes I made they might help (esp. the part where the lexer is modified);

mytuny · 2022-04-04T13:57:29Z

Thank you @nicolas-grekas !!
I'm so glad to help improve this incredible project :)

mytuny · 2022-04-04T14:00:53Z

Note that the nullsafe operator only guards against null. It does not guard against undefined properties/keys.

About this, we might want to add support for the null-coalescing operator ??, which is able to ignore undefined vars/properties/keys.

@mytuny up to give this a try as a follow up? Please have a look at the changes I made they might help (esp. the part where the lexer is modified);

Yup! sure, I'm working on that null-coalescing operator right now. Your comments did and will help me even more to have the feature ready for the next release too.

Thank you once more @nicolas-grekas

…tax for accessing object's properties and methods (Sofien NAAS) This PR was submitted for the 5.4 branch but it was merged into the 6.1 branch instead. Discussion ---------- [ExpressionLanguage] Add support for Nullsafe syntax for accessing object's properties and methods [ExpressionLanguage] [NEW FEATURE] This PR introduces the support for `nullsafe operator` in expressions that works with accessing object's properties and methods. The proposed change here, includes a paragraph under the sub-page `/expression_language/syntax` to describe the new feature usage. The sub-paragraph has a title of `Nullsafe operator` under the paragraph `Working with Objects`. The actual work related to this Doc PR available as Symfony PR [#45795](symfony/symfony#45795). Commits ------- 23bf15d [ExpressionLanguage] Add support for Nullsafe syntax for accessing object's properties and methods.

…yntax (mytuny) This PR was merged into the 6.2 branch. Discussion ---------- [ExpressionLanguage] Add support for null coalescing syntax | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #45411, #21691 | License | MIT | Doc PR | symfony/symfony-docs#16743 This is another waited feature for the syntax of the expression-language component. The [null-coalescing](https://wiki.php.net/rfc/isset_ternary) operator ``??`` becomes a need for variant programming needs these days. Following my previous PR introducing the null-safe operator (#45795). I'm hereby introducing yet another essential operator to make the syntax even more complete. The null-coalescing operator is a syntactic sugar for a common use of ternary in conjunction with ``isset()`` (in PHP) or equivalent in other languages. This is such a common use-case to the point that almost all majors programming syntax nowadays support a sort of a short-hand for that operation namely coalescing operator. Now it's time for the syntax of Expression-Language to do so! Expressions like: * ``foo.bar ?? 'default'`` * ``foo[3] ?? 'default'`` * ``foo.bar ?? foo['bar'] ?? 'default'`` will default to the expression in the right-hand-side of the ``??`` operator whenever the expression in the left-hand-side of it does not exist or it's ``null``. Note that this coalescing behavior can be chained and the validation logic takes decreasing priority from left to right. Commits ------- 8e3c505 [ExpressionLanguage] Add support for null coalescing syntax

…ytuny) This PR was merged into the 6.1 branch. Discussion ---------- [ExpressionLanguage] Feature Null-coalescing operator  [ExpressionLanguage] [NEW FEATURE] This PR introduces the support for null-coalescing operator ``??``. The proposed change, includes a paragraph under the sub-page /expression_language/syntax to describe the new feature usage. The sub-paragraph has a title of **Null-coalescing operator** under the paragraph **Supported Operators**. The actual work related to this Doc PR available as Symfony PR <!--symfony/symfony#45795>. Commits ------- 81204c3 [ExpressionLanguage] Feature Null-coalescing operator

carsonbot added Status: Needs Review ExpressionLanguage Feature labels Mar 20, 2022

carsonbot added this to the 5.4 milestone Mar 20, 2022

carsonbot changed the title ~~[ExpressionLanguage] [Feature] Add Support For Nullsafe Operator~~ [ExpressionLanguage] Add Support For Nullsafe Operator Mar 20, 2022

mytuny mentioned this pull request Mar 20, 2022

[ExpressionLanguage] Add support for Nullsafe syntax for accessing object's properties and methods symfony/symfony-docs#16630

Merged

nicolas-grekas reviewed Mar 20, 2022

View reviewed changes

nicolas-grekas modified the milestones: 5.4, 6.1 Mar 20, 2022

mytuny requested review from lyrixx, yceruto, wouterj, chalasr, dunglas, OskarStark, jderusse and xabbuh as code owners March 20, 2022 21:49

stof changed the base branch from 5.4 to 6.1 March 21, 2022 11:46

mytuny force-pushed the feature_expressionlanguage_nullsafe branch from 97a7c84 to bffdcd2 Compare March 21, 2022 15:18

nicolas-grekas reviewed Mar 21, 2022

View reviewed changes

src/Symfony/Component/ExpressionLanguage/CHANGELOG.md Outdated Show resolved Hide resolved

nicolas-grekas reviewed Mar 22, 2022

View reviewed changes

src/Symfony/Component/ExpressionLanguage/Node/ConstantNode.php Outdated Show resolved Hide resolved

src/Symfony/Component/ExpressionLanguage/Node/GetAttrNode.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Mar 22, 2022

View reviewed changes

src/Symfony/Component/ExpressionLanguage/Tests/ExpressionLanguageTest.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Mar 31, 2022

View reviewed changes

wouterj removed request for wouterj and OskarStark March 31, 2022 16:47

nicolas-grekas changed the title ~~[ExpressionLanguage] Add Support For Nullsafe Operator~~ [ExpressionLanguage] Add support for nullsafe operator Apr 4, 2022

nicolas-grekas force-pushed the feature_expressionlanguage_nullsafe branch from 2363842 to 998d802 Compare April 4, 2022 13:11

nicolas-grekas approved these changes Apr 4, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Apr 4, 2022

nicolas-grekas force-pushed the feature_expressionlanguage_nullsafe branch from 998d802 to b161f1e Compare April 4, 2022 13:37

[ExpressionLanguage] Add support for null-safe operator

946c59f

nicolas-grekas changed the title ~~[ExpressionLanguage] Add support for nullsafe operator~~ [ExpressionLanguage] Add support for null-safe operator Apr 4, 2022

nicolas-grekas force-pushed the feature_expressionlanguage_nullsafe branch from b161f1e to 946c59f Compare April 4, 2022 13:40

nicolas-grekas merged commit 1b75cc0 into symfony:6.1 Apr 4, 2022

fabpot mentioned this pull request Apr 15, 2022

Release v6.1.0-BETA1 #46049

Merged

mytuny mentioned this pull request Apr 22, 2022

[ExpressionLanguage] Add support for null coalescing syntax #46142

Merged

fmata mentioned this pull request Jul 4, 2022

[ExpressionLanguage] Revert readonly flag on ConstantNode::$isNullSafe #46840

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ExpressionLanguage] Add support for null-safe operator #45795

[ExpressionLanguage] Add support for null-safe operator #45795

mytuny commented Mar 20, 2022 •

edited

Loading

carsonbot commented Mar 20, 2022

mytuny commented Mar 20, 2022

nicolas-grekas left a comment

nicolas-grekas commented Mar 20, 2022 •

edited

Loading

fabpot commented Mar 20, 2022

mytuny commented Mar 20, 2022

stof commented Mar 21, 2022

stof commented Mar 21, 2022

mytuny commented Mar 21, 2022

mytuny commented Mar 21, 2022

stof commented Mar 21, 2022

mytuny commented Mar 21, 2022

stof commented Mar 21, 2022

mytuny commented Mar 21, 2022

nicolas-grekas left a comment

nicolas-grekas left a comment

nicolas-grekas commented Apr 4, 2022

nicolas-grekas commented Apr 4, 2022

nicolas-grekas commented Apr 4, 2022

mytuny commented Apr 4, 2022

mytuny commented Apr 4, 2022

[ExpressionLanguage] Add support for null-safe operator #45795

[ExpressionLanguage] Add support for null-safe operator #45795

Conversation

mytuny commented Mar 20, 2022 • edited Loading

carsonbot commented Mar 20, 2022

mytuny commented Mar 20, 2022

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Mar 20, 2022 • edited Loading

fabpot commented Mar 20, 2022

mytuny commented Mar 20, 2022

stof commented Mar 21, 2022

stof commented Mar 21, 2022

mytuny commented Mar 21, 2022

mytuny commented Mar 21, 2022

stof commented Mar 21, 2022

mytuny commented Mar 21, 2022

stof commented Mar 21, 2022

mytuny commented Mar 21, 2022

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Apr 4, 2022

nicolas-grekas commented Apr 4, 2022

nicolas-grekas commented Apr 4, 2022

mytuny commented Apr 4, 2022

mytuny commented Apr 4, 2022

mytuny commented Mar 20, 2022 •

edited

Loading

nicolas-grekas commented Mar 20, 2022 •

edited

Loading