Page MenuHomePhabricator

Allow `arc liberate` to handle traits and namespaces
Needs ReviewPublic

Authored by Firehed on Oct 20 2014, 8:00 PM.

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

Previously, the arc liberate workflow would choke on codebases using 5.3+ features such as traits and namespaces despite XHPAST being updated to parse this code. This change adds the necessary changes into the symbol map script to resolve most namespaces and handle trait use and declaration.

There are two main limitations:

  • Files which use multiple namespaces remain unsupported. It's unlikely to be a problem in practice (unless trying to liberate test fixtures) as there's effectively no overlap between people/codebases that use PHP namespaces and those that would not put them in their own file
  • There's no support for resolving needed namespaced functions. Due to how the PHP engine works this can only be done at runtime, so all "need"s are assumed to be in the global namespace (exception: the function was declared in the same file as the code that requires it)

See T4725

Test Plan

Ran arc liberate on a codebase using both traits and namespaces. Examined the generated map files, which seemed reasonable. Looked at the JSON output of the phutil_symbols.php script of a pretty crazy source file that threw in as many edge cases as seemed reasonable, which also looked correct. I will attach both.

- Source File
- Output of scripts/phutil_symbols.php

Diff Detail

Repository
rPHU libphutil
Branch
liberate_namespaces
Lint
Lint OK
SeverityLocationCodeMessage
Advicescripts/phutil_symbols.php:125XHP16TODO Comment
Advicescripts/phutil_symbols.php:126XHP16TODO Comment
Advicescripts/phutil_symbols.php:127XHP16TODO Comment
Unit
Unit Tests OK
Build Status
Buildable 2866
Build 2870: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

Firehed updated this revision to Diff 25754.Oct 20 2014, 8:00 PM
Firehed retitled this revision from to Allow `arc liberate` to handle traits and namespaces.
Firehed updated this object.
Firehed edited the test plan for this revision. (Show Details)
Firehed added a reviewer: epriestley.
Firehed updated this object.Oct 20 2014, 8:02 PM
Firehed edited the test plan for this revision. (Show Details)
Firehed edited edge metadata.
Firehed added inline comments.Oct 20 2014, 8:11 PM
scripts/phutil_symbols.php
125

Note: these are currently unsupported by XHPAST, as it hasn't pulled in the 5.6 syntax additions yet. Once that is done, adding these should be reasonably straightforward (loop over what will probably be n_USE_FUNCTION and n_USE_CONST and stuff them into $need)

joshuaspence added a subscriber: joshuaspence.
wjiang added a subscriber: wjiang.Dec 11 2016, 10:54 PM

Is this diff still in progress ?