diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6b1bc7e8..b3a3e5e7 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -357,8 +357,6 @@ phutil_register_library_map(array( '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 @@ phutil_register_library_map(array( '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 index 288fc05c..6d9da91b 100644 --- a/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php @@ -34,7 +34,20 @@ final class ArcanistClassNameLiteralXHPASTLinterRule $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 @@ final class ArcanistClassNameLiteralXHPASTLinterRule $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 index de5b522f..00000000 --- a/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php +++ /dev/null @@ -1,42 +0,0 @@ -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 index 47838fdd..00000000 --- a/src/lint/linter/xhpast/rules/__tests__/ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase.php +++ /dev/null @@ -1,10 +0,0 @@ -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 index 66bd140b..0182d151 100644 --- 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 @@ class MyClass { } 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 @@ class MyClass { } 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 index 4df4f718..00000000 --- a/src/lint/linter/xhpast/rules/__tests__/preg_quote-misuse/preg_quote-misuse.lint-test +++ /dev/null @@ -1,11 +0,0 @@ -