Page MenuHomePhabricator

Allow linters to share resources by shoving them on the Engine
ClosedPublic

Authored by epriestley on May 11 2014, 11:58 PM.

Details

Summary

Currently, the Phutil XHPAST Linter and vanilla XHPAST Linter reuse the same parse tree, but do this by having explicit knowledge of one another.

Instead, let them synchronize by writing to a glorified array of globals on the Engine. They no longer require knowledge of one another, so this can work under .arclint.

(This could probably be a little cleaner by putting more logic in the shared base class, but Facebook has some kind of goofy subclass of this thing and this patch won't disrupt it, while a cleaner one might.)

This should unblock D9057.

Test Plan

Ran unit tests and normal lint, got accurate looking results without duplicate invocations showing up in --trace.

Diff Detail

Repository
rARC Arcanist
Branch
engineshare
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 376
Build 376: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Allow linters to share resources by shoving them on the Engine.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added reviewers: joshuaspence, btrahan.
  • Explicitly force the Phutil linter to run after the main linter so the parse trees are always reused.
joshuaspence requested changes to this revision.May 12 2014, 12:23 AM
joshuaspence edited edge metadata.

Some minor inlines.

src/lint/linter/ArcanistPhutilXHPASTLinter.php
12–13

It would almost be nice if this method was static, in which case we could do 2 * ArcanistXHPASTLinter::getLinterPriority().

Maybe (just in case) we should add a unit test which checks that ArcanistPhutilXHPASTLinter has a larger (numerically) priority than ArcanistXHPASTLinter.

62

Maybe check that $linter is an instance of ArcanistXHPASTLinter? Just in case...

This revision now requires changes to proceed.May 12 2014, 12:23 AM

I'll see if I can restructure this so the linters are order-independent without affecting hypothetical Facebook subclasses.

src/lint/linter/ArcanistPhutilXHPASTLinter.php
12–13

We can't make this static until late static binding, introduced in PHP 5.3.0.

epriestley updated this revision to Diff 21514.May 12 2014, 2:04 AM
epriestley edited edge metadata.
  • Allow subclasses to coordinate regardless of order.
joshuaspence accepted this revision.May 12 2014, 2:13 AM
joshuaspence edited edge metadata.

This looks much better.

This revision is now accepted and ready to land.May 12 2014, 2:13 AM
epriestley closed this revision.May 12 2014, 2:28 AM
epriestley updated this revision to Diff 21517.

Closed by commit rARC86cbcb9f564b (authored by @epriestley).