Page MenuHomePhabricator

Fold `ArcanistPhutilXHPASTLinter` into `ArcanistXHPASTLinter`
ClosedPublic

Authored by joshuaspence on Aug 11 2015, 1:27 PM.
Tags
None
Referenced Files
F14076072: D13867.id34993.diff
Thu, Nov 21, 2:36 PM
F14075755: D13867.id35004.diff
Thu, Nov 21, 1:12 PM
F14075732: D13867.id34993.diff
Thu, Nov 21, 1:06 PM
F14075682: D13867.id33493.diff
Thu, Nov 21, 12:49 PM
Unknown Object (File)
Wed, Nov 20, 1:38 PM
Unknown Object (File)
Wed, Nov 20, 1:38 PM
Unknown Object (File)
Wed, Nov 20, 1:27 PM
Unknown Object (File)
Wed, Nov 20, 1:27 PM
Subscribers

Details

Summary

I've been thinking about this for a while... why not just fold ArcanistPhutilXHPASTLinter into ArcanistXHPASTLinter?

Test Plan

arc unit

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

joshuaspence retitled this revision from to Fold `ArcanistPhutilXHPASTLinter` into `ArcanistXHPASTLinter`.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

I don't like that third parties who are developing things like Phabricator applications and arc extensions need to maintain a list of un-disables on these rules. There's no way for them to keep this list in sync with the "correct" list in the upstream.

One possibility that I was considering was to introduce an xhpast.standard which can be phutil or psr-2 (see D13512 for another use case).

Alternatively, we could do something like this:

public function getLintSeverity() {
  if ($this->isPhutilLibrary()) {
      return ArcanistLintSeverity::SEVERITY_ADVICE;
  }

  return ArcanistLintSeverity::SEVERITY_DISABLED;
}

I think I like the "standard" approach. The other one feels maybe a little too magical to me.

epriestley edited edge metadata.

I think this is good to go after D13942.

This revision is now accepted and ready to land.Aug 27 2015, 11:50 AM
This revision was automatically updated to reflect the committed changes.