Page MenuHomePhabricator
Files F15455301

D21032.id50101.diff
No OneTemporary
Actions

D21032.id50101.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
@@ -357,8 +357,6 @@
'ArcanistPhutilXHPASTLinterStandard' => 'lint/linter/standards/phutil/ArcanistPhutilXHPASTLinterStandard.php',
'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php',
'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase.php',
- 'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php',
- 'ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase.php',
'ArcanistProjectConfigurationSource' => 'config/source/ArcanistProjectConfigurationSource.php',
'ArcanistPrompt' => 'toolset/ArcanistPrompt.php',
'ArcanistPublicPropertyXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPublicPropertyXHPASTLinterRule.php',
@@ -1299,8 +1297,6 @@
'ArcanistPhutilXHPASTLinterStandard' => 'ArcanistLinterStandard',
'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
- 'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
- 'ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistProjectConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource',
'ArcanistPrompt' => 'Phobject',
'ArcanistPublicPropertyXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
diff --git a/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php
--- a/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php
@@ -34,7 +34,20 @@
$replacement = '__CLASS__';
}
- $regex = '/\b'.preg_quote($class_name, '/').'\b/';
+ // NOTE: We're only warning when the entire string content is the
+ // class name. It's okay to hard-code the class name as part of a
+ // longer string, like an error or exception message.
+
+ // Sometimes the class name (like "Filesystem") is also a valid part
+ // of the message, which makes this warning a false positive.
+
+ // Even when we're generating a true positive by detecting a class
+ // name in part of a longer string, the cost of an error message
+ // being out-of-date is generally very small (mild confusion, but
+ // no incorrect beahavior) and using "__CLASS__" in errors is often
+ // clunky.
+
+ $regex = '(^'.preg_quote($class_name).'$)';
if (!preg_match($regex, $contents)) {
continue;
}
@@ -42,8 +55,7 @@
$this->raiseLintAtNode(
$string,
pht(
- "Don't hard-code class names, use `%s` instead.",
- '__CLASS__'),
+ 'Prefer "__CLASS__" over hard-coded class names.'),
$replacement);
}
}
diff --git a/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php
deleted file mode 100644
--- a/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php
+++ /dev/null
@@ -1,42 +0,0 @@
-<?php
-
-/**
- * @{function:preg_quote} takes two arguments, but the second one is optional
- * because it is possible to use `()`, `[]` or `{}` as regular expression
- * delimiters. If you don't pass a second argument, you're probably going to
- * get something wrong.
- */
-final class ArcanistPregQuoteMisuseXHPASTLinterRule
- extends ArcanistXHPASTLinterRule {
-
- const ID = 14;
-
- public function getLintName() {
- return pht('Misuse of `%s`', 'preg_quote');
- }
-
- public function getLintSeverity() {
- return ArcanistLintSeverity::SEVERITY_ADVICE;
- }
-
- public function process(XHPASTNode $root) {
- $function_calls = $this->getFunctionCalls($root, array('preg_quote'));
-
- foreach ($function_calls as $call) {
- $parameter_list = $call->getChildOfType(1, 'n_CALL_PARAMETER_LIST');
- if (count($parameter_list->getChildren()) !== 2) {
- $this->raiseLintAtNode(
- $call,
- pht(
- 'If you use pattern delimiters that require escaping '.
- '(such as `%s`, but not `%s`) then you should pass two '.
- 'arguments to %s, so that %s knows which delimiter to escape.',
- '//',
- '()',
- 'preg_quote',
- 'preg_quote'));
- }
- }
- }
-
-}
diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase.php
deleted file mode 100644
--- a/src/lint/linter/xhpast/rules/__tests__/ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase.php
+++ /dev/null
@@ -1,10 +0,0 @@
-<?php
-
-final class ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase
- extends ArcanistXHPASTLinterRuleTestCase {
-
- public function testLinter() {
- $this->executeTestsInDirectory(dirname(__FILE__).'/preg_quote-misuse/');
- }
-
-}
diff --git a/src/lint/linter/xhpast/rules/__tests__/class-name-literal/class-name-literal.lint-test b/src/lint/linter/xhpast/rules/__tests__/class-name-literal/class-name-literal.lint-test
--- a/src/lint/linter/xhpast/rules/__tests__/class-name-literal/class-name-literal.lint-test
+++ b/src/lint/linter/xhpast/rules/__tests__/class-name-literal/class-name-literal.lint-test
@@ -6,10 +6,15 @@
}
public function someOtherMethod() {
- $x = 'MyClass is awesome';
+ $x = 'MyClass';
$y = 'MyClassIsAwesome';
+ $z = 'MyClass is awesome';
return __CLASS__;
}
+
+ public function throwAnException() {
+ throw new Exception('MyClass is throwing an exception!');
+ }
}
$c = new class {
@@ -29,10 +34,15 @@
}
public function someOtherMethod() {
- $x = 'MyClass is awesome';
+ $x = __CLASS__;
$y = 'MyClassIsAwesome';
+ $z = 'MyClass is awesome';
return __CLASS__;
}
+
+ public function throwAnException() {
+ throw new Exception('MyClass is throwing an exception!');
+ }
}
$c = new class {
diff --git a/src/lint/linter/xhpast/rules/__tests__/preg_quote-misuse/preg_quote-misuse.lint-test b/src/lint/linter/xhpast/rules/__tests__/preg_quote-misuse/preg_quote-misuse.lint-test
deleted file mode 100644
--- a/src/lint/linter/xhpast/rules/__tests__/preg_quote-misuse/preg_quote-misuse.lint-test
+++ /dev/null
@@ -1,11 +0,0 @@
-<?php
-
-function foo($bar) {
- preg_quote($bar);
- preg_quote($bar, '/');
- preg_Quote('moo');
- preg_quote('moo', '/');
-}
-~~~~~~~~~~
-advice:4:3:XHP14:Misuse of `preg_quote`
-advice:6:3:XHP14:Misuse of `preg_quote`

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 30, 11:26 PM (1 w, 7 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7709161
Default Alt Text
D21032.id50101.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility