Problem/Motivation
Discovered via PHPStan in #3178534: Start running PHPStan on Drupal core (level 0), there is invalid PHP syntax in \Drupal\Tests\node\Functional\AssertButtonsTrait.
$ php -l ./core/modules/node/tests/src/Functional/AssertButtonsTrait.php
PHP Parse error: syntax error, unexpected '+', expecting :: (T_PAAMAYIM_NEKUDOTAYIM) in ./core/modules/node/tests/src/Functional/AssertButtonsTrait.php on line 30
The line in question is
$this->assertSession()->elementTextEquals('xpath', "(//div[@class='dropbutton-wrapper']//input[@type='submit'])[{$i + 1}]", $buttons[$i]);
Only variables, array elements or object properties can be accessed using the {} syntax inside strings; other expressions are not allowed.
This was last changed in #3203476: Convert assertions involving use of xpath on divs to WebAssert, and although we don't use this trait in core (two uses in contrib), I don't know how this slipped through as I thought we had a PHP lint step in DrupalCI?
Steps to reproduce
Proposed resolution
Fix the syntax.
Add a basic test that uses this trait.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3213734-12-9.2.x.patch | 959 bytes | longwave |
Comments
Comment #2
longwaveThis trait doesn't seem worth maintaining in core.
It was only ever used in NodeFormButtonsTest. It was added in #2511584: Move NodeFormButtonsTest::assertButtons() to a trait and then the original test was removed from core in #2068063: Change "Save and keep un-/published" buttons to a "Published" checkbox and an included "Save" button
I propose fixing the syntax but deprecating it with no replacement.
Comment #3
mondrakeNeeds a CR and its number reflected in the deprecation message. Other than that, this looks good.
Comment #4
longwaveAdded draft CR: https://www.drupal.org/node/3215411
Comment #5
longwaveComment #6
mondrakeSlightly edited the CR. Thanks!
Comment #7
grimreaper+1 for this, big thanks!
Comment #9
catchCommitted ee6a1a7 and pushed to 9.3.x. Thanks!
Comment #10
mondrakeI would suggest to backport the fix to the PHP syntax (not the deprecation) to 9.2.x to avoid static analysis complaining
Comment #11
mondrakeComment #12
longwaveComment #13
mondrakeComment #15
catchCommitted 9da6062 and pushed to 9.2.x. Thanks!
Comment #17
quietone commentedPublished change record