Page MenuHomePhabricator
Files F15380766

D14573.diff
No OneTemporary
Actions

D14573.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -196,6 +196,8 @@
'ArcanistInvalidModifiersXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidModifiersXHPASTLinterRuleTestCase.php',
'ArcanistInvalidOctalNumericScalarXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInvalidOctalNumericScalarXHPASTLinterRule.php',
'ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase.php',
+ 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php',
+ 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php',
'ArcanistJSHintLinter' => 'lint/linter/ArcanistJSHintLinter.php',
'ArcanistJSHintLinterTestCase' => 'lint/linter/__tests__/ArcanistJSHintLinterTestCase.php',
'ArcanistJSONLintLinter' => 'lint/linter/ArcanistJSONLintLinter.php',
@@ -608,6 +610,8 @@
'ArcanistInvalidModifiersXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistInvalidOctalNumericScalarXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
+ 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+ 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistJSHintLinter' => 'ArcanistExternalLinter',
'ArcanistJSHintLinterTestCase' => 'ArcanistExternalLinterTestCase',
'ArcanistJSONLintLinter' => 'ArcanistExternalLinter',
diff --git a/src/lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php
@@ -0,0 +1,69 @@
+<?php
+
+final class ArcanistIsAShouldBeInstanceOfXHPASTLinterRule
+ extends ArcanistXHPASTLinterRule {
+
+ const ID = 111;
+
+ public function getLintName() {
+ return pht('`%s` Should Be `%s`', 'is_a', 'instanceof');
+ }
+
+ public function getLintSeverity() {
+ return ArcanistLintSeverity::SEVERITY_ADVICE;
+ }
+
+ public function process(XHPASTNode $root) {
+ $calls = $this->getFunctionCalls($root, array('is_a'));
+
+ foreach ($calls as $call) {
+ $parameters = $call->getChildOfType(1, 'n_CALL_PARAMETER_LIST');
+
+ if (count($parameters->getChildren()) > 2) {
+ // If the `$allow_string` parameter is `true` then the `instanceof`
+ // operator cannot be used. Evaluating whether an expression is truthy
+ // or falsely is hard, and so we only check that the `$allow_string`
+ // parameter is either absent or literally `false`.
+ $allow_string = $parameters->getChildByIndex(2);
+
+ if (strtolower($allow_string->getConcreteString()) != 'false') {
+ continue;
+ }
+ }
+
+ $object = $parameters->getChildByIndex(0);
+ $class = $parameters->getChildByIndex(1);
+
+ switch ($class->getTypeName()) {
+ case 'n_STRING_SCALAR':
+ $replacement = stripslashes(
+ substr($class->getConcreteString(), 1, -1));
+ break;
+
+ case 'n_VARIABLE':
+ $replacement = $class->getConcreteString();
+ break;
+
+ default:
+ $replacement = null;
+ break;
+ }
+
+ $this->raiseLintAtNode(
+ $call,
+ pht(
+ 'Use `%s` instead of `%s`. The former is a language '.
+ 'construct whereas the latter is a function call, which '.
+ 'has additional overhead.',
+ 'instanceof',
+ 'is_a'),
+ ($replacement === null)
+ ? null
+ : sprintf(
+ '%s instanceof %s',
+ $object->getConcreteString(),
+ $replacement));
+ }
+ }
+
+}
diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php
@@ -0,0 +1,11 @@
+<?php
+
+final class ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase
+ extends ArcanistXHPASTLinterRuleTestCase {
+
+ public function testLinter() {
+ $this->executeTestsInDirectory(
+ dirname(__FILE__).'/is_a-should-be-instanceof/');
+ }
+
+}
diff --git a/src/lint/linter/xhpast/rules/__tests__/is_a-should-be-instanceof/allow_string.lint-test b/src/lint/linter/xhpast/rules/__tests__/is_a-should-be-instanceof/allow_string.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/is_a-should-be-instanceof/allow_string.lint-test
@@ -0,0 +1,3 @@
+<?php
+is_a('SubClass', 'BaseClass', true);
+~~~~~~~~~~
diff --git a/src/lint/linter/xhpast/rules/__tests__/is_a-should-be-instanceof/is_a.lint-test b/src/lint/linter/xhpast/rules/__tests__/is_a-should-be-instanceof/is_a.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/is_a-should-be-instanceof/is_a.lint-test
@@ -0,0 +1,16 @@
+<?php
+is_a($x, $y);
+is_a($x, 'SomeClass');
+is_a($x, '\\SomeClass');
+is_a($x, '\\'.'Some'.'Class');
+~~~~~~~~~~
+advice:2:1
+advice:3:1
+advice:4:1
+advice:5:1
+~~~~~~~~~~
+<?php
+$x instanceof $y;
+$x instanceof SomeClass;
+$x instanceof \SomeClass;
+is_a($x, '\\'.'Some'.'Class');

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 15, 4:22 AM (6 d, 16 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7388959
Default Alt Text
D14573.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility