There is a fatal error caused by a missing use ArrayIterator; statement in core/lib/Drupal/Core/Template/Attribute.php

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

psynaptic’s picture

Status: Active » Needs review
FileSize
732 bytes

Patch attached.

andypost’s picture

Priority: Critical » Major
Status: Needs review » Needs work
Issue tags: +Needs tests

So we need a test for this! Not sure it's critical

psynaptic’s picture

Assigned: psynaptic » Unassigned
Fabianx’s picture

Assigned: Unassigned » psynaptic
Issue tags: +Twig

Found as part of twig engine conversion.

I am not sure this really needs a test and how to test this. Any ideas?

I think it is bad practice to use a class inside of a namespace and not add a use statement.

So I'd rather say this is a regression of the Attribute patch as calling new Attribute() from a different namespace can error out with fatal error.

In this case a new Attribute() added to template_preprocess_table lead to fatal error of:

[07-Oct-2012 17:46:33] PHP Fatal error:  Class 'Drupal\Core\Template\ArrayIterator' not found in /Users/fabian/Sites/twig/drupal_8_theme_and_twig_sprint/core/lib/Drupal/Core/Template/Attribute.php on line 132
Fabianx’s picture

Assigned: psynaptic » Unassigned

Unassign again due to x-post.

Eric_A’s picture

Status: Needs work » Needs review
FileSize
726 bytes

Found another one. Setting status to needs review for the bot.

Eric_A’s picture

Status: Needs review » Needs work

So not one test in core actually uses these iterators?

Berdir’s picture

According to the new guideline, global namespace objects should be referenced using \ArrayIterator, without explicit use statements.

rbayliss’s picture

Here are two patches that provide a test and bring these two files up to standards in light of #8. The first one (test only) should fail, the second one should pass.

rbayliss’s picture

Status: Needs work » Needs review

Whoops. Set status.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Looks good to me.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome, thanks for the fix and for the tests.

Committed and pushed to 8.x.

Automatically closed -- issue fixed for 2 weeks with no activity.