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

Acessing globally imported macro in another macro fails with PHP fatal error #1052

Closed
dmzkrsk opened this issue Apr 10, 2013 · 3 comments · Fixed by #3012
Closed

Acessing globally imported macro in another macro fails with PHP fatal error #1052

dmzkrsk opened this issue Apr 10, 2013 · 3 comments · Fixed by #3012
Labels

Comments

@dmzkrsk
Copy link

dmzkrsk commented Apr 10, 2013

Twig 1.12.3 (2013-04-08)

Assume we have this template:

{% import "macros/clipart.twig" as clm %}

{% macro show(something) %}
    {{ unknown_variable.moody }}
    {{ clm.moody }}
{% endmacro %}

The docs say, that macros have no access to global scope. That's fine. But they still track imported macroses!

Line {{ unknown_variable.moody }} runs without errors producing an empty strings (with strict variables disabled)

The other line {{ clm.moody }} fails with fatal error Call to a member function getmoody() on a non-object

As far as I could dig into the sources, I found that in ExperssionParser.php:368 uses $this->parser->getImportedSymbol to look for clm variable, that uses Parser::importedSymbols which is global. But clm isn't included in the $context of the generated template method. And strict_variables don't help here.

And in generated template method we have echo $_clm_->moody(); (and $_clm_ is null since it doesn't exists in $context) with no sanity checks.

Solutions:

  • make imported modules visible within macros context (against the docs)
  • don't recognize them as modules within macros (according to the docs)
@hason hason mentioned this issue Jul 16, 2013
fabpot added a commit that referenced this issue Aug 3, 2013
This PR was merged into the master branch.

Discussion
----------

Improved macros

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #861, #867, #929, #1052
| License       | MIT
| Doc        | yes

* Fixed fatal error for unknown macro
* Added support for named arguments for macros
* Added support for directly call macros defined in the same template

Commits
-------

3004ba1 Added support for directly call macros defined in the same template
63615a6 Added support for named arguments for macros
09fedb8 Fixed fatal error for unknown macro
9e96837 Refactored parsing of macros
@fabpot fabpot closed this as completed Aug 3, 2013
@hason
Copy link
Contributor

hason commented Oct 3, 2013

Please reopen this issue due to 74586e9

@fabpot fabpot reopened this Oct 3, 2013
hason added a commit to hason/Twig that referenced this issue Jan 2, 2014
hason added a commit to hason/Twig that referenced this issue Jul 8, 2014
hason added a commit to hason/Twig that referenced this issue Jul 14, 2014
@fabpot fabpot added the Macros label Nov 13, 2016
@stof
Copy link
Member

stof commented Mar 3, 2017

@fabpot the parser should be taught that the body of a macro tag is always isolated in scope, and so should have its own set of imported symbols, due to the way macros are implemented currently

@fabpot
Copy link
Contributor

fabpot commented May 16, 2019

#3009 fixed the error message to get "Variable "clm" does not exist..." like any other variable.
#3012 makes the code valid.

@fabpot fabpot closed this as completed May 16, 2019
fabpot added a commit that referenced this issue May 17, 2019
…ble in macros without re-importing them (fabpot)

This PR was merged into the 2.x branch.

Discussion
----------

Macros imported "globally" in a template are now available in macros without re-importing them

closes #1790, closes #1052, closes #2414, closes #2442

Commits
-------

e138164 macros imported "globally" in a template are now available in macros without re-importing them
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 participants