Page MenuHomePhabricator
Files F13193533

D14572.id35313.diff
No OneTemporary
Actions

D14572.id35313.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
@@ -145,6 +145,8 @@
'ArcanistFlake8LinterTestCase' => 'lint/linter/__tests__/ArcanistFlake8LinterTestCase.php',
'ArcanistFormattedStringXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php',
'ArcanistFormattedStringXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistFormattedStringXHPASTLinterRuleTestCase.php',
+ 'ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRule.php',
+ 'ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRuleTestCase.php',
'ArcanistFutureLinter' => 'lint/linter/ArcanistFutureLinter.php',
'ArcanistGeneratedLinter' => 'lint/linter/ArcanistGeneratedLinter.php',
'ArcanistGeneratedLinterTestCase' => 'lint/linter/__tests__/ArcanistGeneratedLinterTestCase.php',
@@ -539,6 +541,8 @@
'ArcanistFlake8LinterTestCase' => 'ArcanistExternalLinterTestCase',
'ArcanistFormattedStringXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistFormattedStringXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
+ 'ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+ 'ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistFutureLinter' => 'ArcanistLinter',
'ArcanistGeneratedLinter' => 'ArcanistLinter',
'ArcanistGeneratedLinterTestCase' => 'ArcanistLinterTestCase',
diff --git a/src/lint/linter/xhpast/rules/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRule.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRule.php
@@ -0,0 +1,54 @@
+<?php
+
+final class ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRule
+ extends ArcanistXHPASTLinterRule {
+
+ const ID = 105;
+
+ public function getLintName() {
+ return pht('Function Call Should Be Type Cast');
+ }
+
+ public function getLintSeverity() {
+ return ArcanistLintSeverity::SEVERITY_ADVICE;
+ }
+
+ public function process(XHPASTNode $root) {
+ static $cast_functions = array(
+ 'boolval' => 'bool',
+ 'doubleval' => 'double',
+ 'floatval' => 'double',
+ 'intval' => 'int',
+ 'strval' => 'string',
+ );
+
+ $casts = $this->getFunctionCalls($root, array_keys($cast_functions));
+
+ foreach ($casts as $cast) {
+ $function_name = $cast
+ ->getChildOfType(0, 'n_SYMBOL_NAME')
+ ->getConcreteString();
+ $cast_name = $cast_functions[$function_name];
+
+ $parameters = $cast->getChildOfType(1, 'n_CALL_PARAMETER_LIST');
+ $replacement = null;
+
+ // Only suggest a replacement if the function call has exactly
+ // one parameter.
+ if (count($parameters->getChildren()) == 1) {
+ $parameter = $parameters->getChildByIndex(0);
+ $replacement = '('.$cast_name.')'.$parameter->getConcreteString();
+ }
+
+ $this->raiseLintAtNode(
+ $cast,
+ pht(
+ 'For consistency, use `%s` (a type cast) instead of `%s` '.
+ '(a function call). Function calls impose additional overhead.',
+ '('.$cast_name.')',
+ $function_name),
+ $replacement);
+ }
+ }
+
+}
diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRuleTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRuleTestCase.php
@@ -0,0 +1,11 @@
+<?php
+
+final class ArcanistFunctionCallShouldBeTypeCastXHPASTLinterRuleTestCase
+ extends ArcanistXHPASTLinterRuleTestCase {
+
+ public function testLinter() {
+ $this->executeTestsInDirectory(
+ dirname(__FILE__).'/function-call-should-be-type-cast/');
+ }
+
+}
diff --git a/src/lint/linter/xhpast/rules/__tests__/function-call-should-be-type-cast/cast-functions.lint-test b/src/lint/linter/xhpast/rules/__tests__/function-call-should-be-type-cast/cast-functions.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/function-call-should-be-type-cast/cast-functions.lint-test
@@ -0,0 +1,19 @@
+<?php
+boolval($x);
+doubleval($x);
+floatval($x);
+intval($x);
+strval($x);
+~~~~~~~~~~
+advice:2:1
+advice:3:1
+advice:4:1
+advice:5:1
+advice:6:1
+~~~~~~~~~~
+<?php
+(bool)$x;
+(double)$x;
+(double)$x;
+(int)$x;
+(string)$x;
diff --git a/src/lint/linter/xhpast/rules/__tests__/function-call-should-be-type-cast/parameter-mismatch.lint-test b/src/lint/linter/xhpast/rules/__tests__/function-call-should-be-type-cast/parameter-mismatch.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/function-call-should-be-type-cast/parameter-mismatch.lint-test
@@ -0,0 +1,11 @@
+<?php
+// If the function call doesn't have exactly one parameter,
+// we don't provide an autofix.
+strval($x, $y);
+~~~~~~~~~~
+advice:4:1
+~~~~~~~~~~
+<?php
+// If the function call doesn't have exactly one parameter,
+// we don't provide an autofix.
+strval($x, $y);

File Metadata

Mime Type
text/plain
Expires
Mon, May 13, 2:35 PM (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6291044
Default Alt Text
D14572.id35313.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility