Page MenuHomePhabricator

Add a linter rule for use of `is_a`
ClosedPublic

Authored by joshuaspence on Nov 25 2015, 7:47 PM.
Tags
None
Referenced Files
F14497065: D14573.diff
Fri, Jan 3, 4:02 PM
F14494207: D14573.id35571.diff
Fri, Jan 3, 4:49 AM
Unknown Object (File)
Thu, Dec 26, 2:30 AM
Unknown Object (File)
Mon, Dec 16, 12:20 AM
Unknown Object (File)
Mon, Dec 9, 1:30 PM
Unknown Object (File)
Thu, Dec 5, 9:52 AM
Unknown Object (File)
Nov 27 2024, 12:24 PM
Unknown Object (File)
Nov 19 2024, 2:08 PM
Subscribers

Details

Summary

instanceof should be used instead of is_a. I need to do a bit more research here to see if there are any edge cases.

Test Plan

Added unit tests.

Diff Detail

Repository
rARC Arcanist
Branch
master
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 9736
Build 11686: Run Core Tests
Build 11685: arc lint + arc unit

Event Timeline

joshuaspence retitled this revision from to Add a linter rule for use of `is_a`.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

I think this is an edge case:

$subclass = 'Subclass';
is_a($subclass, 'Base', $allow_string = true); // true
$subclass instanceof Base; // false, string is not an instance of anything

I'm also not sure offhand what the distinction between is_a() and is_subclass_of() is.

epriestley edited edge metadata.

(Probably an edge case earlier?)

This revision now requires changes to proceed.Dec 1 2015, 12:21 PM
joshuaspence edited edge metadata.

Seems to work now

epriestley edited edge metadata.
epriestley added inline comments.
src/lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php
30–32

We could probably make evalStatic() smarter about this and have the code do something like:

if (!$allow_string->isSomethingWeCanCallEvalStaticOn()) {
  continue;
}

if ($allow_string->evalStatic()) {
  continue;
}

...to catch 0, etc., some day far in the future.

57

Missing text at end of sentence ("which has... what?")

60

Prefer ($replacement === null) for consistency.

This revision is now accepted and ready to land.Dec 22 2015, 1:43 PM
joshuaspence marked 2 inline comments as done.
joshuaspence edited edge metadata.

Fix a few inlines

This revision was automatically updated to reflect the committed changes.