Page MenuHomePhabricator

PHP 7 compatibility: Embedded variable syntax
Closed, ResolvedPublic

Description

The new PHP7 parser and AST interpret a few edge cases differently from the PHP5 parser. The PHAN tool (https://github.com/etsy/phan) is able to detect these. We should scan for these and make sure we're not using them.

https://www.youtube.com/watch?v=eOCyPxqFSTI&t=16m

Event Timeline

Krinkle created this task.Dec 7 2015, 6:43 PM
Krinkle updated the task description. (Show Details)
Krinkle raised the priority of this task from to Needs Triage.
Krinkle added a subscriber: Krinkle.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptDec 7 2015, 6:43 PM
Alexia added a subscriber: Alexia.Dec 14 2015, 6:02 AM

I ran php7mar against this and did not find any current instances PHP 7 breaking variable interpolation. I could not get phan to work as it just produces segmentation faults.

Would prefer a second glance with another tool to verify my results.

Perhaps this is something we can catch with phpcs as well to cover our projects on a wider scale beyond just mediawiki/core? (@Legoktm)

Danny_B moved this task from Unsorted to PHP 7 on the [DO NOT USE] NewPHP board.May 9 2016, 12:14 AM
Krinkle removed a subscriber: Krinkle.May 10 2016, 9:01 PM
Ricordisamoa updated the task description. (Show Details)Feb 9 2017, 12:39 AM

I ran phan against latest core code. It did not find any instances of issues with embedded variable syntax.

I temporarily added a few instances of embedded variable syntax locally just to confirm that I had phan configured properly, and it did successfully find them. It did.

Given that the concern is a change in syntax interpretation between PHP5 and PHP7, and we are now on PHP7, there should be no future problems of this sort. The only reason I can think of that we'd encounter an inconsistency in the future is if someone pasted PHP5 code from somewhere like Stack Exchange, and the pasted code behaved differently under PHP7. This seems an unlikely edge case that should be caught in testing and code review.

In my opinion, we are already be good on this issue, with no further action required.

BPirkle closed this task as Resolved.Oct 12 2018, 4:10 AM