smarty-php / smarty-php/smarty
Uneccessary call to hasVaraible and inconsistent compile results for isset and empty
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 2.3k
- Forks
- 709
- PR merge metrics
- No merged PRs in 30d
Description
TL;DR
I think a hasVariable call just before a getValue call is redundant and can be removed from here
https://github.com/smarty-php/smarty/blob/aa6edc3c0bf8fc77cda9a06320021328c924cd68/src/Parser/TemplateParser.y#L885
Context
I noticed a strange thing in mt compiled templates: {$foo|isset} and {isset($foo)} do not result in the same compiled code. And interestingly, {isset($bar.a)} does not call hasVaraible and is almost identical to the result of {$bar.a|isset} except for two additional true &&
// {$foo|isset}
((null !== ($_smarty_tpl->getValue('foo') ?? null)))
// {isset($foo)}
((true && ($_smarty_tpl->hasVariable('foo') && null !== ($_smarty_tpl->getValue('foo') ?? null))))
// {$bar.a|isset}
((null !== ($_smarty_tpl->getValue('bar')['a'] ?? null)))
// {isset($bar.a)}
((true && (true && null !== ($_smarty_tpl->getValue('bar')['a'] ?? null))))
Ideally, both functions result in the same compiled code. Historically, I used {isset($bar.a)} because the other syntax was throwing warnings in some circumstances. This seems to be no longer the case.
I also found at least one instance in my code where an {empty($var)} resulted in a hasVariable call. Not sure if this can happen for other functions.
As a side note. The unnecessary true && seems to be only there to avoid handling the first iteration in a loop in the parser.
https://github.com/smarty-php/smarty/blob/aa6edc3c0bf8fc77cda9a06320021328c924cd68/src/Parser/TemplateParser.y#L1117
Also the ?? null part is unnecessary as long as there is no subarray. However, this behavior seems to be more complex to change since the part that adds ?? null does not see what the part in front looks like.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/Parser/TemplateParser.y at the referenced lines 885 and 1117, then compare compiled output for {$foo|isset}, {isset($foo)}, {isset($bar.a)}, and {empty($var)}. The work is done when equivalent isset and empty forms compile consistently without redundant hasVariable or true && expressions, while preserving the reported behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100