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 @@ +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 @@ +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 @@ +