Page MenuHomePhabricator

Improve parsing of namespaces
ClosedPublic

Authored by joshuaspence on Nov 17 2015, 7:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 26, 4:31 PM
Unknown Object (File)
Sat, Mar 23, 6:41 AM
Unknown Object (File)
Wed, Mar 20, 9:48 PM
Unknown Object (File)
Sun, Mar 17, 8:40 PM
Unknown Object (File)
Sun, Mar 17, 7:45 PM
Unknown Object (File)
Sat, Mar 16, 6:44 PM
Unknown Object (File)
Sat, Mar 16, 3:39 PM
Unknown Object (File)
Thu, Mar 14, 6:33 PM

Details

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/):

xhpast-before.png (799×297 px, 28 KB)

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:

xhpast-after.png (708×243 px, 25 KB)

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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.
JolanYacht added a subscriber: JolanYacht.
  • 邀请您做代码查看**谢谢 thx
epriestley edited edge metadata.

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

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

This revision was automatically updated to reflect the committed changes.

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;

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.

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