Page MenuHomePhabricator

Improve the "self member reference" linter rule
ClosedPublic

Authored by joshuaspence on Aug 19 2015, 2:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 6, 5:03 AM
Unknown Object (File)
Sun, Mar 3, 7:36 PM
Unknown Object (File)
Sun, Mar 3, 7:36 PM
Unknown Object (File)
Sun, Mar 3, 7:36 PM
Unknown Object (File)
Sun, Mar 3, 7:36 PM
Unknown Object (File)
Sun, Mar 3, 7:36 PM
Unknown Object (File)
Sun, Mar 3, 7:36 PM
Unknown Object (File)
Sun, Mar 3, 7:36 PM
Subscribers

Details

Summary

Extends ArcanistSelfMemberReferenceXHPASTLinterRule to warn about the use of a hardcoded class name instead of self when instantiating a class. Arguably, we should maybe rename the linter rule from ArcanistSelfMemberReferenceXHPASTLinterRule to ArcanistSelfUsageXHPASTLinterRule, or even maybe split this rule into three separate rules:

  • ArcanistSelfMemberReferenceXHPASTLinterRule
  • ArcanistSelfSpacingXHPASTLinterRule
  • ArcanistSelfClassReferenceXHPASTLinterRule.
Test Plan

Added to existing test cases.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

joshuaspence retitled this revision from to Improve the "self member reference" linter rule.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
epriestley edited edge metadata.

This construction looks a little bizarre to me, but it seems reasonable to adopt given that we've adopted the member rule, I suppose.

This revision is now accepted and ready to land.Aug 20 2015, 12:42 PM

This construction looks a little bizarre to me, but it seems reasonable to adopt given that we've adopted the member rule, I suppose.

The more I thought about this the more I have come to favor splitting this rule into three smaller rules. Without doing this, it is impossible to customize the severities individually. Are you supporting of this idea?

Yeah, I think that's reasonable. This one definitely looks a little more bizarre than the others.

joshuaspence edited edge metadata.

Split into three separate rules

joshuaspence edited edge metadata.

Wanted to get a pair of eyes on the change I made to ArcanistKeywordCasingXHPASTLinterRule.

  • Minor changes
  • Add some additional tests
epriestley edited edge metadata.
This revision is now accepted and ready to land.Dec 22 2015, 4:15 PM
This revision was automatically updated to reflect the committed changes.