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

[Feature] Optional chaining operator #3260

Open
hason opened this issue Feb 7, 2020 · 33 comments
Open

[Feature] Optional chaining operator #3260

hason opened this issue Feb 7, 2020 · 33 comments

Comments

@hason
Copy link
Contributor

hason commented Feb 7, 2020

Add support for the optional chaining operator ?. that is available in:

{# Before #}
{{ foo.bar.baz is defined ? foo.bar.baz }}

{# After #}
{{ foo?.bar?.baz }}
@VincentLanglet
Copy link
Contributor

it should even be the default behaviour.

@stof
Copy link
Member

stof commented Feb 20, 2020

-1 for being the default behavior. That would make it a lot harder to identify mistakes (typo in a property name) when you don't expect it to be optional.

@VincentLanglet
Copy link
Contributor

VincentLanglet commented Feb 20, 2020

-1 for being the default behavior. That would make it a lot harder to identify mistakes (typo in a property name) when you don't expect it to be optional.

In php, there is tools like phpstan returning error for

$foo->getBar()->getBaz()

if getBar can be null. But there is nothing like this for twig. So mistake is easier to make.

I find sad the fact that

{% if foo.bar.baz %}
...
{% endif %}

throw an exception and display an error 500 in production when foo.bar is null.

This should be user-first and not dev-first.

What about a strict mode, enable in dev ?

@stof
Copy link
Member

stof commented Feb 20, 2020

Well, first, it does not throw an exception by default in production, as strict_variables is opt-in and will make . return null for non-existent attributes.

If what you want is to have everything optional in prod and strict in dev, Twig already has that feature since the 1.0 release (and probably even before in some 0.x releases).

To me, the benefit of a ?. operator would be that it could be used even in strict mode

@VincentLanglet
Copy link
Contributor

Well, first, it does not throw an exception by default in production, as strict_variables is opt-in and will make . return null for non-existent attributes.

If what you want is to have everything optional in prod and strict in dev, Twig already has that feature since the 1.0 release (and probably even before in some 0.x releases).

To be, the benefit of a ?. operator would be that it could be used even in strict mode

Oh sorry, didn't know that. That's great. I agree with you then.

@Herz3h
Copy link

Herz3h commented Sep 15, 2020

Would be cool to have this feature 👍

@hason
Copy link
Contributor Author

hason commented Dec 30, 2020

Null-safe operator was introduced in PHP 8 https://wiki.php.net/rfc/nullsafe_operator

@Bilge
Copy link

Bilge commented Aug 20, 2021

We should have this.

@acalvino4
Copy link

Have any maintainers taken a look at this yet? this would be an awesome feature that would aid in writing shorter, more expressive templating. I know maintainers often have a lot on their plate, so I understand if there are other priorities, but I would at least like to know if this is being considered / on a roadmap.

I am guessing that the use of this construct would just get translated to the underlying php operator, in which case it would probably be pretty easy to implement, but would only be compatible with php8, so maybe that is holding the feature back?

@spackmat
Copy link

As mentioned in #3609 this would also be a nice little improvement for filters like in myNullableString?|myStringFilter so that those don't get called at all and don't throw a deprecation in PHP 8.1 when the value to filter is null.

@acalvino4
Copy link

acalvino4 commented Jan 19, 2022

@nicolas-grekas @hason As top contributors, do you know (or know who would know) if an implementation of this would be accepted or is maybe even on the roadmap?

PS - sorry for the direct tag, but as no maintainer has replied to my request for comment here or on the slack channel, I didn't know of a better way to get a response.

@Bilge
Copy link

Bilge commented Jan 19, 2022

You should ask @fabpot. It's a lot more likely to be accepted if there's a PR, but I can understand the reluctance to work on something that might be rejected.

@fabpot
Copy link
Contributor

fabpot commented Jan 20, 2022

I would review a PR implementing this new behavior. But if we want to be "simple" to implement, it might mean bumping the min PHP version to 8.0 (7.2 right now for the 3.x branch).

@Bilge
Copy link

Bilge commented Jan 20, 2022

Not that I am in any way an expert, but I can't see any reason why the implementation has to be constrained by platform feature support of the same. Twig has a history of implementing features before PHP (e.g. named parameters) and that's a good thing.

@fabpot
Copy link
Contributor

fabpot commented Jan 20, 2022

Implementation will tell us.

@stof
Copy link
Member

stof commented Jan 20, 2022

@Bilge Twig named parameters are a compile-time feature (which is why Twig macros don't support named parameters btw).

@fabpot I'm not sure we can use the native optional chaining to implement that in Twig, due to . compiling to a twig_get_attribute function call for instance. A Twig chain will not compile to a PHP chain.

@fabpot
Copy link
Contributor

fabpot commented Jan 20, 2022

Indeed stof. But again, let’s talk about implementation in a PR

@xepozz
Copy link

xepozz commented Jan 6, 2023

Any news?

@xabbuh
Copy link
Contributor

xabbuh commented Jan 6, 2023

@xepozz Until now nobody proposed a pull request implementing this feature.

@realjjaveweb
Copy link

realjjaveweb commented Feb 27, 2023

I don't really have time for PR and refinements, but here's some inspiration, it shouldn't be that hard:

class NullsafeExtension extends \Twig\Extension\AbstractExtension
{
    public function getTokenParsers()
    {
        return array(new NullsafeTokenParser());
    }
}

class NullsafeTokenParser extends \Twig\TokenParser\AbstractTokenParser
{
    public function parse(\Twig\Token $token)
    {
        $stream = $this->parser->getStream();
        $node = $this->parser->getExpressionParser()->parseExpression();
        while ($stream->test(\Twig\Token::OPERATOR_TYPE, '?.')) {
            $stream->next();
            $node = new \Twig\Node\Expr\Nullsafe(
                $node, $this->parser->getExpressionParser()->parsePrimaryExpression()
            );
        }
        return $node;
    }
    public function getTag()
    {
        return 'nullsafe';
    }
}

Then somehow load it into Twig environment

$twig = new Twig_Environment($loader);
$twig->addExtension(new NullsafeExtension());

and finally it coul be used as

{{ some.?somethingMayNotBeThere.?somethingElseMayNotBeThere() }}

@realjjaveweb
Copy link

I would like to also note, that I have a feeling that some.somethingMayNotBeThere.somethingElseMayNotBeThere()|default('some fallback value') can work pretty much the same, not sure if that relies on some strict/nonstrict mode though

@acalvino4
Copy link

acalvino4 commented May 9, 2023

I started playing around with an implementation of this, and as soon as I got a basic something working, I realized that I can't actually think of a situation where it is needed/helpful. Some of these points have been made earlier in the conversation here, but I think it may be helpful to summarize.

Say we have:

{% set foo = { bar: 'baz' } %}

First of all (as @stof pointed out), without strict mode (the default), . already just returns null at any point in a chain where a variable is undefined:

{# with `strict_variables` set to false #}
{{ foo.bar }} {# output: 'baz' #}
{{ foo.bad }} {# output: '' #}
{{ bad.bar }} {# output: '' #}

I think it is pretty common practice for prod environments to be set to non-strict variables (you don't want visitors seeing error messages), and dev environments to be set to strict variables; hence that case does need to be handled too, and I think it is.

If the goal is to perform a test, the is defined avoids an error in all cases and returns what you'd expect:

{# with `strict_variables` set to true or false #}
{% if foo.bar is defined %}defined{% else %}not defined{% endif %} {# output: 'defined' #}
{% if foo.bad is defined %}defined{% else %}not defined{% endif %} {# output: 'not defined' #}
{% if bad.bar is defined %}defined{% else %}not defined{% endif %} {# output: 'not defined' #}

If the goal is to assign to a variable, the default filter will do the job (as @realjjaveweb pointed out):

{# with `strict_variables` set to true or false #}
{% set var = foo.bar|default(null) %}{{ var }} {# output: 'baz' #}
{% set var = foo.bad|default(null) %}{{ var }} {# output: '' #}
{% set var = bad.bar|default(null) %}{{ var }} {# output: '' #}

So the only place where the ?. construct might be helpful is when you have strict_variables enabled and want to avoid using the default filter. But that would only deal with the last line of the last code block; the middle line (with var = foo.bad) would still require |default to avoid a twig error because the nullsafe operator only checks the expression before itself for null, not the expression after (at least in typical implementations of the construct).

I'm curious if given this summary, anyone still knows of a use for this in twig. My current take, given Twig's current behaviors and abilities is

  • If in strict variables mode, . already behaves like ?. would, so you don't need this.
  • If not, use is defined and |default, and you still won't need this.

@stof
Copy link
Member

stof commented May 10, 2023

@acalvino4 if you look at the behavior of the |default filter, you will see that it does not really do the job. Any defined value considered as empty will trigger the default.

@acalvino4
Copy link

I see, so for the case {% set var = foo.bad|default(null) %} where foo.bad is defined but equal to '', then var === null rather than var === ''.

So I acknowledge that there is slightly different behavior. If all you are doing is printing var somewhere ({{ var }}) that difference is moot.

Hence I am curious how many real-world use-cases can't easily be handled with default or is defined. Not saying there aren't any - just saying that all the one's I've thought of and come across I can handle with existing twig functionality..

@realjjaveweb
Copy link

@acalvino4 I totally agree. In 99% cases it's about textual output. Outputing null is unlikely and if that's happening very often in the app one should create custom function/filter since that's quite an edge case.
Only more "real" less edgy case I can think of is outputing boolean - however - most likely you will want to provide some custom common output - so either if it's often - define custom filter, otherwise I will remind that twig does support ternary operator so you can do

{{ foo|default(false) ? 'yes' : 'no' }}
{% set foo = false %}
{{ foo|default(false) ? 'yes' : 'no' }}
{% set foo = true %}
{{ foo|default(false) ? 'yes' : 'no' }
### OUTPUT: ###
no
no
yes

You can test it on https://twigfiddle.com/

@realjjaveweb
Copy link

One more important note note - @stof is a bit misleading here default filter is NOT the same thing as PHP's empty()! The twig's empty docs say:

empty checks if a variable is an empty string, an empty array, an empty hash, exactly false, or exactly null

so this...

{% set foo = 0 %}
{{ foo|default('not there') }}

=> outputs 0. rest was already said by @acalvino4

What wasn't said here is the documentation says that if you want to tackle undefined booleans - you should use ?? operator:

Using the default filter on a boolean variable might trigger unexpected behavior, as false is treated as an empty value. Consider using ?? instead:
{% set foo = false %}
{{ foo|default(true) }} {# true #}
{{ foo ?? true }} {# false #}

@stof
Copy link
Member

stof commented May 12, 2023

@realjjaveweb I never said it was the same that PHP empty. It is the same than Twig* empty. but false is empty for instance.

@spackmat
Copy link

The discussion started to drift away from the initially described problem: The optional chaining operator only really gets useful, when there is a chain of objects. Sure, when the chain only has two elements, there are pretty simple other ways to handle a nullable object (the default-filter or tenary were named). But when there is a real chain, like nullableObject.nullableRelatedObject.nullableOtherRelatedObject.property, a ?. operator like the ?-> in PHP can be very handy to avoid nasty checks or even more nasty rare error conditions.

@realjjaveweb
Copy link

@spackmat yes, but common chains in twig can be handled by |default filter. The issue is basically only for null vs bool.
@stof sorry, my mistake, it sounded to me a bit like it, my bad

@element-code
Copy link

element-code commented May 16, 2023

#3260 (comment)

If the goal is to perform a test, the is defined avoids an error in all cases and returns what you'd expect: [...]
If the goal is to assign to a variable, the default filter will do the job (as @realjjaveweb pointed out): [...]

There is a third goal: output of content.

Imagine the Output {{ order.orderCustomer.customer.customFields.acmeExtensionCustomersSAPCustomerNumber }}, having to check with defined beforehand kinda sucks, defining variables for each output sucks too.
However, this case can be handled with the default filter (as long as you don't care about 0 or false being handled as null)

@realjjaveweb
Copy link

@element-code quick correction: default only falls to default for not defined and "empty string, an empty array, an empty hash, exactly false, or exactly null", NOT for 0/'0'. BTW: defined DOESN'T do the null check! (it's not like PHP's isset).

@ryanleichty
Copy link

Just started working with twig files, coming from JavaScript. Would love to have this 🔥

@rudiedirkx
Copy link

rudiedirkx commented Apr 27, 2024

@acalvino4 @spackmat My usecase (in strict_variables mode) is

{{ site.getLatestResult('host').getHtmlPill() }}

Exception: Impossible to invoke a method ("getHtmlPill") on a null variable.

getLatestResult('host') is expensive, so the only way to do that now is

{% set hostResult = site.getLatestResult('host') %}
{{ hostResult ? hostResult.getHtmlPill() }}

Not horrible, but not as readable as

{{ site.getLatestResult('host')?.getHtmlPill() }}

Using default() doesn't throw the strict error, BUT it calls getLatestResult() twice (that's what default() compiles into). (So does ?? unfortunately, but that aside.) I assume ?. (or .?) wouldn't.


With |default('XXX'):

yield Twig\Extension\EscaperExtension::escape(
	$this->env,
	(
		// Once to see if it's empty
		TwigBridge\Node\GetAttrNode::attribute(
			$this->env,
			$this->source,
			TwigBridge\Node\GetAttrNode::attribute(
				$this->env,
				$this->source,
				$context["site"],
				"getLatestResult",
				["host"],
				"method",
				false,
				true,
				false,
				58
			),
			"getHtmlPill",
			[],
			"method",
			true,
			true,
			false,
			58
		) ? (
			Twig\Extension\CoreExtension::defaultFilter(
				// Once again if it's not
				TwigBridge\Node\GetAttrNode::attribute(
					$this->env,
					$this->source,
					TwigBridge\Node\GetAttrNode::attribute(
						$this->env,
						$this->source,
						$context["site"],
						"getLatestResult",
						["host"],
						"method",
						false,
						true,
						false,
						58
					),
					"getHtmlPill",
					[],
					"method",
					false,
					false,
					false,
					58
				),
				"XXX"
			)
		) : ("XXX")
	),
	"html",
	null,
	true
);

Actually that looks like a bug... Why is it called twice? Once before defaultFilter() ...? With the default value "XXX" twice... Twig 3.9.3

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

No branches or pull requests