Paths

Table of Contentst

Improve parsing of namespaces
ClosedPublic
Actions

Authored by joshuaspence on Nov 17 2015, 7:29 AM.

Details

Reviewers

epriestley
• jim_reed

Group Reviewers

Blessed Reviewers

Commits

rPHU7124fef0f0d0: Improve parsing of namespaces

Summary

Currently PHP namespaces are parsed in two different ways, depending on the syntax used. For example, consider the following example:

<?php

namespace A;

class B {}

namespace A\B\C {
 class D {}
}

namespace {
  class A {}
}

This is currently parsed by XHPAST as follows (see https://secure.phabricator.com/xhpast/view/915/):

This generally makes it difficult to determine the namespace in which a class is declared (unless the namespace is defined using the namespace { ... } syntax, which is somewhat rare). Instead, always parse namespaces as if they were of the namespace { ... } form.

Test Plan

Added unit tests. Also inspected the AST after this change:

Diff Detail

Repository

rPHU libphutil

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

joshuaspence updated this revision to Diff 35071.Nov 17 2015, 7:29 AM

joshuaspence retitled this revision from to Improve parsing of namespaces.

joshuaspence updated this object.

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptNov 17 2015, 7:29 AM

Herald added a subscriber: Korvin. · View Herald Transcript

joshuaspence mentioned this in D14496: Add previous and next sibling pointers to `AASTNode`.Nov 17 2015, 7:29 AM

邀请您做代码查看**谢谢 thx

joshuaspence updated this object.Nov 17 2015, 10:26 AM

joshuaspence edited edge metadata.

joshuaspence added a child revision: D14499: Fix brace formatting linter rule after XHPAST changes.Nov 17 2015, 10:38 AM

Unrelated, but I wonder if we can find a better way to represent the test cases -- dumping the JSON turns out to be totally unreadable.

Maybe we could just sort of mark up the file in some way so that it was approximately human-readable.

This revision is now accepted and ready to land.Nov 17 2015, 3:18 PM

epriestley mentioned this in D14500: Improve test data format for XHPAST unit tests.Nov 17 2015, 3:21 PM

One thing that would slightly help readability would be to use token names rather than token IDs.

Closed by commit rPHU7124fef0f0d0: Improve parsing of namespaces (authored by joshuaspence, committed by joshuaspence). · Explain WhyNov 17 2015, 8:19 PM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in rARCa4dba24c468c: Prevent double rendering of unit test results.Nov 17 2015, 8:23 PM

The mixed syntax is actually invalid. (Maybe known, just saw this one fly by)

That should be fine, we're just parsing the two different forms in a similar way, not requiring then to appear in the same file.

That said, maybe top_statement should be top_statement_list? Do we parse this file correctly (with no statements)?

<?php namespace x;

In D14498#163309, @epriestley wrote:
That should be fine, we're just parsing the two different forms in a similar way, not requiring then to appear in the same file.

That said, maybe top_statement should be top_statement_list? Do we parse this file correctly (with no statements)?
<?php namespace x;

No we don't handle that correctly. Let me see if I can fix that...

That file is pretty silly, of course, but it'd be nice to handle it.

In D14498#163309, @epriestley wrote:

That said, maybe top_statement should be top_statement_list? Do we parse this file correctly (with no statements)?

It's not quite that simple...

bison --verbose -Wall --defines=parser.yacc.hpp --output=parser.yacc.cpp parser.y
parser.y: error: shift/reduce conflicts: 86 found, 5 expected
Makefile:57: recipe for target 'parser.yacc.hpp' failed
make: *** [parser.yacc.hpp] Error 1

joshuaspence mentioned this in D14506: Improve parsing of namespaces.Nov 18 2015, 1:34 AM

joshuaspence mentioned this in D14512: Add an `XHPAST::getNamespace()` method.Nov 19 2015, 7:41 PM

joshuaspence mentioned this in rPHU44113cfd0db1: Add an `XHPAST::getNamespace()` method.Nov 19 2015, 7:47 PM

avivey mentioned this in Q594: Support PHP namespaces.Mar 17 2017, 4:44 PM