smarty-php / smarty-php/smarty

Uneccessary call to hasVaraible and inconsistent compile results for isset and empty

Open
#1,158 0 comments 1 reaction 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.