diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f0f9a8fd..6d6b2f28 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -9,6 +9,10 @@ phutil_register_library_map(array( '__library_version__' => 2, 'class' => array( + 'ArcanistAbstractMethodBodyXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php', + 'ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php', + 'ArcanistAbstractPrivateMethodXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistAbstractPrivateMethodXHPASTLinterRule.php', + 'ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase.php', 'ArcanistAliasFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistAliasFunctionXHPASTLinterRule.php', 'ArcanistAliasFunctionXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistAliasFunctionXHPASTLinterRuleTestCase.php', 'ArcanistAliasWorkflow' => 'workflow/ArcanistAliasWorkflow.php', @@ -55,6 +59,8 @@ phutil_register_library_map(array( 'ArcanistClassExtendsObjectXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassExtendsObjectXHPASTLinterRule.php', 'ArcanistClassExtendsObjectXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistClassExtendsObjectXHPASTLinterRuleTestCase.php', 'ArcanistClassFilenameMismatchXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php', + 'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php', + 'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase.php', 'ArcanistClassNameLiteralXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php', 'ArcanistClassNameLiteralXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistClassNameLiteralXHPASTLinterRuleTestCase.php', 'ArcanistCloseRevisionWorkflow' => 'workflow/ArcanistCloseRevisionWorkflow.php', @@ -172,6 +178,10 @@ phutil_register_library_map(array( 'ArcanistInstallCertificateWorkflow' => 'workflow/ArcanistInstallCertificateWorkflow.php', 'ArcanistInstanceOfOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInstanceOfOperatorXHPASTLinterRule.php', 'ArcanistInstanceofOperatorXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInstanceofOperatorXHPASTLinterRuleTestCase.php', + 'ArcanistInterfaceAbstractMethodXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInterfaceAbstractMethodXHPASTLinterRule.php', + 'ArcanistInterfaceAbstractMethodXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInterfaceAbstractMethodXHPASTLinterRuleTestCase.php', + 'ArcanistInterfaceMethodBodyXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInterfaceMethodBodyXHPASTLinterRule.php', + 'ArcanistInterfaceMethodBodyXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInterfaceMethodBodyXHPASTLinterRuleTestCase.php', 'ArcanistInvalidDefaultParameterXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInvalidDefaultParameterXHPASTLinterRule.php', 'ArcanistInvalidDefaultParameterXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidDefaultParameterXHPASTLinterRuleTestCase.php', 'ArcanistInvalidModifiersXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php', @@ -224,6 +234,8 @@ phutil_register_library_map(array( 'ArcanistMissingLinterException' => 'lint/linter/exception/ArcanistMissingLinterException.php', 'ArcanistModifierOrderingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistModifierOrderingXHPASTLinterRule.php', 'ArcanistModifierOrderingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistModifierOrderingXHPASTLinterRuleTestCase.php', + 'ArcanistNamespaceFirstStatementXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistNamespaceFirstStatementXHPASTLinterRule.php', + 'ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase.php', 'ArcanistNamingConventionsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistNamingConventionsXHPASTLinterRule.php', 'ArcanistNamingConventionsXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistNamingConventionsXHPASTLinterRuleTestCase.php', 'ArcanistNestedNamespacesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistNestedNamespacesXHPASTLinterRule.php', @@ -347,11 +359,15 @@ phutil_register_library_map(array( 'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnnecessaryFinalModifierXHPASTLinterRuleTestCase.php', 'ArcanistUnnecessarySemicolonXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnnecessarySemicolonXHPASTLinterRule.php', + 'ArcanistUnnecessarySymbolAliasXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnnecessarySymbolAliasXHPASTLinterRule.php', + 'ArcanistUnnecessarySymbolAliasXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnnecessarySymbolAliasXHPASTLinterRuleTestCase.php', 'ArcanistUnsafeDynamicStringXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnsafeDynamicStringXHPASTLinterRule.php', 'ArcanistUnsafeDynamicStringXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnsafeDynamicStringXHPASTLinterRuleTestCase.php', 'ArcanistUpgradeWorkflow' => 'workflow/ArcanistUpgradeWorkflow.php', 'ArcanistUploadWorkflow' => 'workflow/ArcanistUploadWorkflow.php', 'ArcanistUsageException' => 'exception/ArcanistUsageException.php', + 'ArcanistUseStatementNamespacePrefixXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUseStatementNamespacePrefixXHPASTLinterRule.php', + 'ArcanistUseStatementNamespacePrefixXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUseStatementNamespacePrefixXHPASTLinterRuleTestCase.php', 'ArcanistUselessOverridingMethodXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php', 'ArcanistUselessOverridingMethodXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUselessOverridingMethodXHPASTLinterRuleTestCase.php', 'ArcanistUserAbortException' => 'exception/usage/ArcanistUserAbortException.php', @@ -387,6 +403,10 @@ phutil_register_library_map(array( ), 'function' => array(), 'xmap' => array( + 'ArcanistAbstractMethodBodyXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistAbstractPrivateMethodXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistAliasFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistAliasFunctionXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistAliasWorkflow' => 'ArcanistWorkflow', @@ -433,6 +453,8 @@ phutil_register_library_map(array( 'ArcanistClassExtendsObjectXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistClassExtendsObjectXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistClassFilenameMismatchXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistClassNameLiteralXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistClassNameLiteralXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistCloseRevisionWorkflow' => 'ArcanistWorkflow', @@ -550,6 +572,10 @@ phutil_register_library_map(array( 'ArcanistInstallCertificateWorkflow' => 'ArcanistWorkflow', 'ArcanistInstanceOfOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistInstanceofOperatorXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistInterfaceAbstractMethodXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistInterfaceAbstractMethodXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistInterfaceMethodBodyXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistInterfaceMethodBodyXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistInvalidDefaultParameterXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistInvalidDefaultParameterXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistInvalidModifiersXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -602,6 +628,8 @@ phutil_register_library_map(array( 'ArcanistMissingLinterException' => 'Exception', 'ArcanistModifierOrderingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistModifierOrderingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistNamespaceFirstStatementXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistNamingConventionsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistNamingConventionsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistNestedNamespacesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -725,11 +753,15 @@ phutil_register_library_map(array( 'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistUnnecessarySemicolonXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistUnnecessarySymbolAliasXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistUnnecessarySymbolAliasXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistUnsafeDynamicStringXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistUnsafeDynamicStringXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistUpgradeWorkflow' => 'ArcanistWorkflow', 'ArcanistUploadWorkflow' => 'ArcanistWorkflow', 'ArcanistUsageException' => 'Exception', + 'ArcanistUseStatementNamespacePrefixXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistUseStatementNamespacePrefixXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistUselessOverridingMethodXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistUselessOverridingMethodXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistUserAbortException' => 'ArcanistUsageException', diff --git a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php index 8c40e5c3..f780ec6c 100644 --- a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php @@ -225,6 +225,34 @@ abstract class ArcanistXHPASTLinterRule extends Phobject { return AASTNodeList::newFromTreeAndNodes($root->getTree(), $nodes); } + /** + * Get class/method modifiers. + * + * @param XHPASTNode A node of type `n_CLASS_DECLARATION` or + * `n_METHOD_DECLARATION`. + * @return map Class/method modifiers. + */ + final protected function getModifiers(XHPASTNode $node) { + $modifier_list = $node->getChildByIndex(0); + + switch ($modifier_list->getTypeName()) { + case 'n_CLASS_ATTRIBUTES': + case 'n_METHOD_MODIFIER_LIST': + break; + + default: + return array(); + } + + $modifiers = array(); + + foreach ($modifier_list->selectDescendantsOfType('n_STRING') as $modifier) { + $modifiers[strtolower($modifier->getConcreteString())] = true; + } + + return $modifiers; + } + /** * Get PHP superglobals. * diff --git a/src/lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php new file mode 100644 index 00000000..277e15e0 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php @@ -0,0 +1,30 @@ +selectDescendantsOfType('n_METHOD_DECLARATION'); + + foreach ($methods as $method) { + $modifiers = $this->getModifiers($method); + $body = $method->getChildByIndex(5); + + if (idx($modifiers, 'abstract') && $body->getTypeName() != 'n_EMPTY') { + $this->raiseLintAtNode( + $body, + pht( + '`%s` methods cannot contain a body. This construct will '. + 'cause a fatal error.', + 'abstract')); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistAbstractPrivateMethodXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistAbstractPrivateMethodXHPASTLinterRule.php new file mode 100644 index 00000000..b4a95abf --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistAbstractPrivateMethodXHPASTLinterRule.php @@ -0,0 +1,37 @@ +selectDescendantsOfType('n_METHOD_DECLARATION'); + + foreach ($methods as $method) { + $method_modifiers = $method + ->getChildOfType(0, 'n_METHOD_MODIFIER_LIST') + ->selectDescendantsOfType('n_STRING'); + $modifiers = array(); + + foreach ($method_modifiers as $modifier) { + $modifiers[strtolower($modifier->getConcreteString())] = true; + } + + if (idx($modifiers, 'abstract') && idx($modifiers, 'private')) { + $this->raiseLintAtNode( + $method, + pht( + '`%s` method cannot be declared `%s`. '. + 'This construct will cause a fatal error.', + 'abstract', + 'private')); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistArrayCombineXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistArrayCombineXHPASTLinterRule.php index 659bd30b..8d500c29 100644 --- a/src/lint/linter/xhpast/rules/ArcanistArrayCombineXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistArrayCombineXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistArrayCombineXHPASTLinterRule const ID = 84; public function getLintName() { - return pht('%s Unreliable', 'array_combine()'); + return pht('`%s` Unreliable', 'array_combine()'); } public function getLintSeverity() { @@ -35,8 +35,8 @@ final class ArcanistArrayCombineXHPASTLinterRule 'Prior to PHP 5.4, `%s` fails when given empty arrays. '. 'Prefer to write `%s` as `%s`.', 'array_combine()', - 'array_combine(x, x)', - 'array_fuse(x)')); + 'array_combine($x, $x)', + 'array_fuse($x)')); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistClassExtendsObjectXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistClassExtendsObjectXHPASTLinterRule.php index d712b3fe..c24dbc6c 100644 --- a/src/lint/linter/xhpast/rules/ArcanistClassExtendsObjectXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistClassExtendsObjectXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistClassExtendsObjectXHPASTLinterRule const ID = 88; public function getLintName() { - return pht('Class Not Extending %s', 'Phobject'); + return pht('Class Not Extending `%s`', 'Phobject'); } public function getLintSeverity() { @@ -29,8 +29,8 @@ final class ArcanistClassExtendsObjectXHPASTLinterRule $this->raiseLintAtNode( $class, pht( - 'Classes should extend from %s or from some other class. '. - 'All classes (except for %s itself) should have a base class.', + 'Classes should extend from `%s` or from some other class. '. + 'All classes (except for `%s` itself) should have a base class.', 'Phobject', 'Phobject')); } diff --git a/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php index 5b91f3ca..6c8c2f41 100644 --- a/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php @@ -47,7 +47,7 @@ final class ArcanistClassFilenameMismatchXHPASTLinterRule $decl_name, pht( "The name of this file differs from the name of the ". - "class or interface it declares. Rename the file to '%s'.", + 'class or interface it declares. Rename the file to `%s`.', $rename)); } diff --git a/src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php new file mode 100644 index 00000000..859dc319 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php @@ -0,0 +1,46 @@ +selectDescendantsOfType('n_CLASS_DECLARATION'); + + foreach ($classes as $class) { + $class_modifiers = $this->getModifiers($class); + + $abstract_methods = array(); + $methods = $class->selectDescendantsOfType('n_METHOD_DECLARATION'); + + foreach ($methods as $method) { + $method_modifiers = $this->getModifiers($method); + + if (idx($method_modifiers, 'abstract')) { + $abstract_methods[] = $method; + } + } + + if (!idx($class_modifiers, 'abstract') && $abstract_methods) { + $this->raiseLintAtNode( + $class, + pht( + 'Class contains %s %s method(s) and must therefore '. + 'be declared `%s`.', + phutil_count($abstract_methods), + 'abstract', + 'abstract')); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php index cfa0bf93..a22a9859 100644 --- a/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php @@ -39,7 +39,7 @@ final class ArcanistClassNameLiteralXHPASTLinterRule $this->raiseLintAtNode( $string, pht( - "Don't hard-code class names, use %s instead.", + "Don't hard-code class names, use `%s` instead.", '__CLASS__'), $replacement); } diff --git a/src/lint/linter/xhpast/rules/ArcanistCommentStyleXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistCommentStyleXHPASTLinterRule.php index a5fe569c..3a493e2b 100644 --- a/src/lint/linter/xhpast/rules/ArcanistCommentStyleXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistCommentStyleXHPASTLinterRule.php @@ -19,7 +19,10 @@ final class ArcanistCommentStyleXHPASTLinterRule $this->raiseLintAtOffset( $comment->getOffset(), - pht('Use "%s" single-line comments, not "%s".', '//', '#'), + pht( + 'Use `%s` single-line comments, not `%s`.', + '//', + '#'), '#', preg_match('/^#\S/', $value) ? '// ' : '//'); } diff --git a/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php index 6606b0b3..1f5de931 100644 --- a/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php @@ -78,7 +78,9 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule if (!$before) { $this->raiseLintAtOffset( $use->getOffset(), - pht('Convention: space before `%s` token.', 'use'), + pht( + 'Convention: space before `%s` token.', + 'use'), '', ' '); } @@ -86,7 +88,9 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule if (!$after) { $this->raiseLintAtOffset( $use->getOffset() + strlen($use->getValue()), - pht('Convention: space after `%s` token.', 'use'), + pht( + 'Convention: space after `%s` token.', + 'use'), '', ' '); } diff --git a/src/lint/linter/xhpast/rules/ArcanistDynamicDefineXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistDynamicDefineXHPASTLinterRule.php index 4148df3b..be144625 100644 --- a/src/lint/linter/xhpast/rules/ArcanistDynamicDefineXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistDynamicDefineXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistDynamicDefineXHPASTLinterRule const ID = 12; public function getLintName() { - return pht('Dynamic %s', 'define()'); + return pht('Dynamic `%s`', 'define'); } public function process(XHPASTNode $root) { @@ -20,8 +20,8 @@ final class ArcanistDynamicDefineXHPASTLinterRule $this->raiseLintAtNode( $defined, pht( - 'First argument to %s must be a string literal.', - 'define()')); + 'First argument to `%s` must be a string literal.', + 'define')); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistElseIfUsageXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistElseIfUsageXHPASTLinterRule.php index 986859f8..246e8048 100644 --- a/src/lint/linter/xhpast/rules/ArcanistElseIfUsageXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistElseIfUsageXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistElseIfUsageXHPASTLinterRule const ID = 42; public function getLintName() { - return pht('ElseIf Usage'); + return pht('`elseif` Usage'); } public function getLintSeverity() { @@ -19,7 +19,10 @@ final class ArcanistElseIfUsageXHPASTLinterRule foreach ($tokens as $token) { $this->raiseLintAtToken( $token, - pht('Usage of `%s` is preferred over `%s`.', 'else if', 'elseif'), + pht( + 'Usage of `%s` is preferred over `%s`.', + 'else if', + 'elseif'), 'else if'); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistExitExpressionXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistExitExpressionXHPASTLinterRule.php index 60ce126c..ceac22c3 100644 --- a/src/lint/linter/xhpast/rules/ArcanistExitExpressionXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistExitExpressionXHPASTLinterRule.php @@ -21,7 +21,7 @@ final class ArcanistExitExpressionXHPASTLinterRule const ID = 17; public function getLintName() { - return pht('Exit Used as Expression'); + return pht('`%s` Used as Expression', 'exit'); } public function process(XHPASTNode $root) { @@ -34,7 +34,9 @@ final class ArcanistExitExpressionXHPASTLinterRule if ($unary->getParentNode()->getTypeName() !== 'n_STATEMENT') { $this->raiseLintAtNode( $unary, - pht('Use `%s` as a statement, not an expression.', 'exit')); + pht( + 'Use `%s` as a statement, not an expression.', + 'exit')); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistExtractUseXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistExtractUseXHPASTLinterRule.php index 88a655cc..1c6976de 100644 --- a/src/lint/linter/xhpast/rules/ArcanistExtractUseXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistExtractUseXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistExtractUseXHPASTLinterRule const ID = 4; public function getLintName() { - return pht('Use of %s', 'extract()'); + return pht('Use of `%s`', 'extract'); } public function process(XHPASTNode $root) { @@ -16,8 +16,8 @@ final class ArcanistExtractUseXHPASTLinterRule $this->raiseLintAtNode( $call, pht( - 'Avoid %s. It is confusing and hinders static analysis.', - 'extract()')); + 'Avoid `%s`. It is confusing and hinders static analysis.', + 'extract')); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php index 2f25dccc..333d7ab1 100644 --- a/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php @@ -16,7 +16,7 @@ final class ArcanistFormattedStringXHPASTLinterRule 'xhpast.printf-functions' => array( 'type' => 'optional map', 'help' => pht( - '%s-style functions which take a format string and list of values '. + '`%s`-style functions which take a format string and list of values '. 'as arguments. The value for the mapping is the start index of the '. 'function parameters (the index of the format string parameter).', 'printf()'), diff --git a/src/lint/linter/xhpast/rules/ArcanistImplicitConstructorXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistImplicitConstructorXHPASTLinterRule.php index 466f74fe..cf3d567a 100644 --- a/src/lint/linter/xhpast/rules/ArcanistImplicitConstructorXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistImplicitConstructorXHPASTLinterRule.php @@ -24,9 +24,10 @@ final class ArcanistImplicitConstructorXHPASTLinterRule $this->raiseLintAtNode( $method_name_token, pht( - 'Name constructors %s explicitly. This method is a constructor '. - ' because it has the same name as the class it is defined in.', - '__construct()')); + 'Name constructors `%s` explicitly. This method is a '. + 'constructor because it has the same name as the class '. + 'it is defined in.', + '__construct')); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php index 74c7d57b..1fe7a2a7 100644 --- a/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php @@ -20,10 +20,10 @@ final class ArcanistImplicitFallthroughXHPASTLinterRule 'xhpast.switchhook' => array( 'type' => 'optional string', 'help' => pht( - 'Name of a concrete subclass of %s which tunes the '. - 'analysis of %s statements for this linter.', + 'Name of a concrete subclass of `%s` which tunes the '. + 'analysis of `%s` statements for this linter.', 'ArcanistXHPASTLintSwitchHook', - 'switch()'), + 'switch'), ), ); } @@ -190,9 +190,9 @@ final class ArcanistImplicitFallthroughXHPASTLinterRule $this->raiseLintAtToken( head($tokens), pht( - "This '%s' or '%s' has a nonempty block which does not end ". - "with '%s', '%s', '%s', '%s' or '%s'. Did you forget to add ". - "one of those? If you intend to fall through, add a '%s' ". + 'This `%s` or `%s` has a nonempty block which does not end '. + 'with `%s`, `%s`, `%s`, `%s` or `%s`. Did you forget to add '. + 'one of those? If you intend to fall through, add a `%s` '. "comment to silence this warning.", 'case', 'default', diff --git a/src/lint/linter/xhpast/rules/ArcanistInstanceOfOperatorXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistInstanceOfOperatorXHPASTLinterRule.php index 519d5645..47cb9ab5 100644 --- a/src/lint/linter/xhpast/rules/ArcanistInstanceOfOperatorXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistInstanceOfOperatorXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistInstanceOfOperatorXHPASTLinterRule const ID = 69; public function getLintName() { - return pht('%s Operator', 'instanceof'); + return pht('`%s` Operator', 'instanceof'); } public function process(XHPASTNode $root) { @@ -26,7 +26,7 @@ final class ArcanistInstanceOfOperatorXHPASTLinterRule $this->raiseLintAtNode( $object, pht( - '%s expects an object instance, constant given.', + '`%s` expects an object instance, constant given.', 'instanceof')); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistInterfaceAbstractMethodXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistInterfaceAbstractMethodXHPASTLinterRule.php new file mode 100644 index 00000000..039f0e2a --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistInterfaceAbstractMethodXHPASTLinterRule.php @@ -0,0 +1,34 @@ +selectDescendantsOfType('n_INTERFACE_DECLARATION'); + + foreach ($interfaces as $interface) { + $methods = $interface->selectDescendantsOfType('n_METHOD_DECLARATION'); + + foreach ($methods as $method) { + $modifiers = $this->getModifiers($method); + + if (idx($modifiers, 'abstract')) { + $this->raiseLintAtNode( + $method, + pht( + '`%s` methods cannot be marked as `%s`. This construct will '. + 'cause a fatal error.', + 'interface', + 'abstract')); + } + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistInterfaceMethodBodyXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistInterfaceMethodBodyXHPASTLinterRule.php new file mode 100644 index 00000000..f4835d0b --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistInterfaceMethodBodyXHPASTLinterRule.php @@ -0,0 +1,33 @@ +selectDescendantsOfType('n_INTERFACE_DECLARATION'); + + foreach ($interfaces as $interface) { + $methods = $interface->selectDescendantsOfType('n_METHOD_DECLARATION'); + + foreach ($methods as $method) { + $body = $method->getChildByIndex(5); + + if ($body->getTypeName() != 'n_EMPTY') { + $this->raiseLintAtNode( + $body, + pht( + '`%s` methods cannot contain a body. This construct will '. + 'cause a fatal error.', + 'interface')); + } + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistInvalidDefaultParameterXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistInvalidDefaultParameterXHPASTLinterRule.php index 12bdcf17..482fb396 100644 --- a/src/lint/linter/xhpast/rules/ArcanistInvalidDefaultParameterXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistInvalidDefaultParameterXHPASTLinterRule.php @@ -39,8 +39,8 @@ final class ArcanistInvalidDefaultParameterXHPASTLinterRule $this->raiseLintAtNode( $default, pht( - 'Default value for parameters with %s type hint '. - 'can only be an %s or %s.', + 'Default value for parameters with `%s` type hint '. + 'can only be an `%s` or `%s`.', 'array', 'array', 'null')); @@ -54,7 +54,8 @@ final class ArcanistInvalidDefaultParameterXHPASTLinterRule $this->raiseLintAtNode( $default, pht( - 'Default value for parameters with %s type hint can only be %s.', + 'Default value for parameters with `%s` type hint '. + 'can only be `%s`.', 'callable', 'null')); break; @@ -69,7 +70,7 @@ final class ArcanistInvalidDefaultParameterXHPASTLinterRule $default, pht( 'Default value for parameters with a class type hint '. - 'can only be %s.', + 'can only be `%s`.', 'null')); break; } diff --git a/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php index e8c87a3a..c1e6cc6f 100644 --- a/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php @@ -30,7 +30,7 @@ final class ArcanistInvalidModifiersXHPASTLinterRule $this->raiseLintAtNode( $modifier, pht( - 'Properties cannot be declared %s.', + 'Properties cannot be declared `%s`.', 'abstract')); } @@ -38,7 +38,7 @@ final class ArcanistInvalidModifiersXHPASTLinterRule $this->raiseLintAtNode( $modifier, pht( - 'Multiple %s modifiers are not allowed.', + 'Multiple `%s` modifiers are not allowed.', 'abstract')); } @@ -46,7 +46,7 @@ final class ArcanistInvalidModifiersXHPASTLinterRule $this->raiseLintAtNode( $modifier, pht( - 'Cannot use the %s modifier on an %s class member', + 'Cannot use the `%s` modifier on an `%s` class member', 'final', 'abstract')); } @@ -59,7 +59,7 @@ final class ArcanistInvalidModifiersXHPASTLinterRule $this->raiseLintAtNode( $modifier, pht( - 'Cannot use the %s modifier on an %s class member', + 'Cannot use the `%s` modifier on an `%s` class member', 'final', 'abstract')); } @@ -68,7 +68,7 @@ final class ArcanistInvalidModifiersXHPASTLinterRule $this->raiseLintAtNode( $modifier, pht( - 'Multiple %s modifiers are not allowed.', + 'Multiple `%s` modifiers are not allowed.', 'final')); } @@ -91,7 +91,7 @@ final class ArcanistInvalidModifiersXHPASTLinterRule $this->raiseLintAtNode( $modifier, pht( - 'Multiple %s modifiers are not allowed.', + 'Multiple `%s` modifiers are not allowed.', 'static')); } break; diff --git a/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php index bc7c6bb6..af0b5903 100644 --- a/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php @@ -89,7 +89,7 @@ final class ArcanistKeywordCasingXHPASTLinterRule $this->raiseLintAtToken( $keyword, pht( - "Convention: spell keyword '%s' as '%s'.", + 'Convention: spell keyword `%s` as `%s`.', $value, strtolower($value)), strtolower($value)); @@ -115,7 +115,7 @@ final class ArcanistKeywordCasingXHPASTLinterRule $this->raiseLintAtNode( $symbol, pht( - "Convention: spell keyword '%s' as '%s'.", + 'Convention: spell keyword `%s` as `%s`.', $symbol_name, strtolower($symbol_name)), strtolower($symbol_name)); diff --git a/src/lint/linter/xhpast/rules/ArcanistLambdaFuncFunctionXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistLambdaFuncFunctionXHPASTLinterRule.php index 34be9baf..2ddeb722 100644 --- a/src/lint/linter/xhpast/rules/ArcanistLambdaFuncFunctionXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistLambdaFuncFunctionXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistLambdaFuncFunctionXHPASTLinterRule const ID = 68; public function getLintName() { - return pht('%s Function', '__lambda_func'); + return pht('`%s` Function', '__lambda_func'); } public function process(XHPASTNode $root) { @@ -28,10 +28,10 @@ final class ArcanistLambdaFuncFunctionXHPASTLinterRule $this->raiseLintAtNode( $function_declaration, pht( - 'Declaring a function named %s causes any call to %s to fail. '. - 'This is because %s eval-declares the function %s, then '. + 'Declaring a function named `%s` causes any call to %s to fail. '. + 'This is because `%s` eval-declares the function `%s`, then '. 'modifies the symbol table so that the function is instead '. - 'named %s, and returns that name.', + 'named `%s`, and returns that name.', '__lambda_func', 'create_function', 'create_function', diff --git a/src/lint/linter/xhpast/rules/ArcanistNamespaceFirstStatementXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistNamespaceFirstStatementXHPASTLinterRule.php new file mode 100644 index 00000000..729c0de4 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistNamespaceFirstStatementXHPASTLinterRule.php @@ -0,0 +1,36 @@ +selectDescendantsOfType('n_NAMESPACE'); + + if (!count($namespaces)) { + return; + } + + $statements = $root->getChildOfType(0, 'n_STATEMENT_LIST'); + + // Ignore the first statement, which should be `n_OPEN_TAG`. + $second_statement = $statements->getChildByIndex(1)->getChildByIndex(0); + + if ($second_statement->getTypeName() != 'n_NAMESPACE') { + $this->raiseLintAtNode( + $second_statement, + pht( + 'A script which contains a `%s` statement expects the very first '. + 'statement to be a `%s` statement. Otherwise, a PHP fatal error '. + 'will occur. %s', + 'namespace', + 'namespace', $second_statement->getTypeName())); + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistNamingConventionsXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistNamingConventionsXHPASTLinterRule.php index 24852abf..3d00ed08 100644 --- a/src/lint/linter/xhpast/rules/ArcanistNamingConventionsXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistNamingConventionsXHPASTLinterRule.php @@ -20,7 +20,7 @@ final class ArcanistNamingConventionsXHPASTLinterRule 'xhpast.naminghook' => array( 'type' => 'optional string', 'help' => pht( - 'Name of a concrete subclass of %s which enforces more '. + 'Name of a concrete subclass of `%s` which enforces more '. 'granular naming convention rules for symbols.', 'ArcanistXHPASTLintNamingHook'), ), @@ -56,8 +56,8 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::isUpperCamelCase($name_string) ? null : pht( - 'Follow naming conventions: classes should be named using '. - 'UpperCamelCase.'), + 'Follow naming conventions: classes should be named using `%s`.', + 'UpperCamelCase'), ); } @@ -72,8 +72,8 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::isUpperCamelCase($name_string) ? null : pht( - 'Follow naming conventions: interfaces should be named using '. - 'UpperCamelCase.'), + 'Follow naming conventions: interfaces should be named using `%s`.', + 'UpperCamelCase'), ); } @@ -94,8 +94,8 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::stripPHPFunction($name_string)) ? null : pht( - 'Follow naming conventions: functions should be named using '. - 'lowercase_with_underscores.'), + 'Follow naming conventions: functions should be named using `%s`.', + 'lowercase_with_underscores'), ); } @@ -112,8 +112,8 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::stripPHPFunction($name_string)) ? null : pht( - 'Follow naming conventions: methods should be named using '. - 'lowerCamelCase.'), + 'Follow naming conventions: methods should be named using `%s`.', + 'lowerCamelCase'), ); } @@ -137,8 +137,9 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::stripPHPVariable($name_string)) ? null : pht( - 'Follow naming conventions: parameters should be named using '. - 'lowercase_with_underscores.'), + 'Follow naming conventions: parameters '. + 'should be named using `%s`', + 'lowercase_with_underscores'), ); } } @@ -157,8 +158,9 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::isUppercaseWithUnderscores($name_string) ? null : pht( - 'Follow naming conventions: class constants should be named '. - 'using UPPERCASE_WITH_UNDERSCORES.'), + 'Follow naming conventions: class constants '. + 'should be named using `%s`', + 'UPPERCASE_WITH_UNDERSCORES'), ); } } @@ -184,8 +186,9 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::stripPHPVariable($name_string)) ? null : pht( - 'Follow naming conventions: class properties should be named '. - 'using lowerCamelCase.'), + 'Follow naming conventions: class properties '. + 'should be named using `%s`.', + 'lowerCamelCase'), ); } } @@ -269,8 +272,9 @@ final class ArcanistNamingConventionsXHPASTLinterRule ArcanistXHPASTLintNamingHook::stripPHPVariable($var_string)) ? null : pht( - 'Follow naming conventions: variables should be named using '. - 'lowercase_with_underscores.'), + 'Follow naming conventions: variables '. + 'should be named using `%s`.', + 'lowercase_with_underscores'), ); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistNewlineAfterOpenTagXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistNewlineAfterOpenTagXHPASTLinterRule.php index 63bed628..4baa4053 100644 --- a/src/lint/linter/xhpast/rules/ArcanistNewlineAfterOpenTagXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistNewlineAfterOpenTagXHPASTLinterRule.php @@ -38,7 +38,9 @@ final class ArcanistNewlineAfterOpenTagXHPASTLinterRule $next = $token->getNextToken(); $this->raiseLintAtToken( $next, - pht('`%s` should be separated from code by an empty line.', 'getValue()); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistNoParentScopeXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistNoParentScopeXHPASTLinterRule.php index 1574a013..3aa1c4f9 100644 --- a/src/lint/linter/xhpast/rules/ArcanistNoParentScopeXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistNoParentScopeXHPASTLinterRule.php @@ -34,7 +34,7 @@ final class ArcanistNoParentScopeXHPASTLinterRule $this->raiseLintAtNode( $static_access, pht( - 'Cannot access %s when current class scope has no parent.', + 'Cannot access `%s` when current class scope has no parent.', 'parent::')); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php index 1b943b54..95a9918f 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistPHPCloseTagXHPASTLinterRule const ID = 8; public function getLintName() { - return pht('Use of Close Tag "%s"', '?>'); + return pht('Use of Close Tag `%s`', '?>'); } public function process(XHPASTNode $root) { @@ -19,7 +19,9 @@ final class ArcanistPHPCloseTagXHPASTLinterRule foreach ($root->selectTokensOfType('T_CLOSE_TAG') as $token) { $this->raiseLintAtToken( $token, - pht('Do not use the PHP closing tag, "%s".', '?>')); + pht( + 'Do not use the PHP closing tag, `%s`.', + '?>')); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPEchoTagXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPEchoTagXHPASTLinterRule.php index 5c863fbb..6862d571 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPHPEchoTagXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPHPEchoTagXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistPHPEchoTagXHPASTLinterRule const ID = 7; public function getLintName() { - return pht('Use of Echo Tag "%s"', 'getTypeName() === 'T_OPEN_TAG_WITH_ECHO') { $this->raiseLintAtToken( $token, - pht('Avoid the PHP echo short form, "%s".', 'raiseLintAtToken( $token, pht( - 'PHP files should start with "%s", which may be preceded by '. - 'a "%s" line for scripts.', + 'PHP files should start with `%s`, which may be preceded by '. + 'a `%s` line for scripts.', 'raiseLintAtToken( $token, pht( - 'Use the full form of the PHP open tag, "%s".', + 'Use the full form of the PHP open tag, `%s`.', 'raiseLintAtNode( $call, pht( - 'Avoid %s unless the second parameter is specified. '. + 'Avoid `%s` unless the second parameter is specified. '. 'It is confusing and hinders static analysis.', - 'parse_str()')); + 'parse_str')); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php index 15ee8300..1f8d9ba0 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php @@ -26,9 +26,10 @@ final class ArcanistPlusOperatorOnStringsXHPASTLinterRule $this->raiseLintAtNode( $binop, pht( - "In PHP, '%s' is the string concatenation operator, not '%s'. ". - "This expression uses '+' with a string literal as an operand.", + 'In PHP, `%s` is the string concatenation operator, not `%s`. '. + 'This expression uses `%s` with a string literal as an operand.', '.', + '+', '+')); } } diff --git a/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php index 37b3422d..de5b522f 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php @@ -12,7 +12,7 @@ final class ArcanistPregQuoteMisuseXHPASTLinterRule const ID = 14; public function getLintName() { - return pht('Misuse of %s', 'preg_quote()'); + return pht('Misuse of `%s`', 'preg_quote'); } public function getLintSeverity() { @@ -33,8 +33,8 @@ final class ArcanistPregQuoteMisuseXHPASTLinterRule 'arguments to %s, so that %s knows which delimiter to escape.', '//', '()', - 'preg_quote()', - 'preg_quote()')); + 'preg_quote', + 'preg_quote')); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistRaggedClassTreeEdgeXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistRaggedClassTreeEdgeXHPASTLinterRule.php index 64bfd87f..864d1f6e 100644 --- a/src/lint/linter/xhpast/rules/ArcanistRaggedClassTreeEdgeXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistRaggedClassTreeEdgeXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistRaggedClassTreeEdgeXHPASTLinterRule const ID = 87; public function getLintName() { - return pht('Class Not %s Or %s', 'abstract', 'final'); + return pht('Class Not `%s` Or `%s`', 'abstract', 'final'); } public function getLintSeverity() { @@ -42,8 +42,8 @@ final class ArcanistRaggedClassTreeEdgeXHPASTLinterRule $this->raiseLintAtNode( $class->getChildOfType(1, 'n_CLASS_NAME'), pht( - "This class is neither '%s' nor '%s', and does not have ". - "a docblock marking it '%s'.", + 'This class is neither `%s` nor `%s`, and does not have '. + 'a docblock marking it `%s`.', 'final', 'abstract', '@concrete-extensible')); diff --git a/src/lint/linter/xhpast/rules/ArcanistReusedIteratorReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistReusedIteratorReferenceXHPASTLinterRule.php index f3373332..65851435 100644 --- a/src/lint/linter/xhpast/rules/ArcanistReusedIteratorReferenceXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistReusedIteratorReferenceXHPASTLinterRule.php @@ -173,7 +173,7 @@ final class ArcanistReusedIteratorReferenceXHPASTLinterRule $var, pht( 'This variable was used already as a by-reference iterator '. - 'variable. Such variables survive outside the %s loop, '. + 'variable. Such variables survive outside the `%s` loop, '. 'do not reuse.', 'foreach')); } diff --git a/src/lint/linter/xhpast/rules/ArcanistSlownessXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSlownessXHPASTLinterRule.php index 79d33e0a..268ca5d0 100644 --- a/src/lint/linter/xhpast/rules/ArcanistSlownessXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistSlownessXHPASTLinterRule.php @@ -51,14 +51,14 @@ final class ArcanistSlownessXHPASTLinterRule $this->raiseLintAtNode( $strstr, pht( - 'Use %s for checking if the string contains something.', - 'strpos()')); + 'Use `%s` for checking if the string contains something.', + 'strpos')); } else if ($name === 'stristr') { $this->raiseLintAtNode( $strstr, pht( - 'Use %s for checking if the string contains something.', - 'stripos()')); + 'Use `%s` for checking if the string contains something.', + 'stripos')); } } } @@ -96,14 +96,14 @@ final class ArcanistSlownessXHPASTLinterRule $this->raiseLintAtNode( $strpos, pht( - 'Use %s for checking if the string starts with something.', - 'strncmp()')); + 'Use `%s` for checking if the string starts with something.', + 'strncmp')); } else if ($name === 'stripos') { $this->raiseLintAtNode( $strpos, pht( - 'Use %s for checking if the string starts with something.', - 'strncasecmp()')); + 'Use `%s` for checking if the string starts with something.', + 'strncasecmp')); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistStaticThisXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistStaticThisXHPASTLinterRule.php index 1750a6df..695d0858 100644 --- a/src/lint/linter/xhpast/rules/ArcanistStaticThisXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistStaticThisXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistStaticThisXHPASTLinterRule const ID = 13; public function getLintName() { - return pht('Use of %s in Static Context', '$this'); + return pht('Use of `%s` in Static Context', '$this'); } public function process(XHPASTNode $root) { diff --git a/src/lint/linter/xhpast/rules/ArcanistToStringExceptionXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistToStringExceptionXHPASTLinterRule.php index c6a92314..ab0eba56 100644 --- a/src/lint/linter/xhpast/rules/ArcanistToStringExceptionXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistToStringExceptionXHPASTLinterRule.php @@ -6,7 +6,7 @@ final class ArcanistToStringExceptionXHPASTLinterRule const ID = 67; public function getLintName() { - return pht('Throwing Exception in %s Method', '__toString'); + return pht('Throwing Exception in `%s` Method', '__toString'); } public function process(XHPASTNode $root) { @@ -33,7 +33,7 @@ final class ArcanistToStringExceptionXHPASTLinterRule $this->raiseLintAtNode( $throw, pht( - 'It is not possible to throw an %s from within the %s method.', + 'It is not possible to throw an `%s` from within the `%s` method.', 'Exception', '__toString')); } diff --git a/src/lint/linter/xhpast/rules/ArcanistUnexpectedReturnValueXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUnexpectedReturnValueXHPASTLinterRule.php index b73ea0a5..10ffda44 100644 --- a/src/lint/linter/xhpast/rules/ArcanistUnexpectedReturnValueXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistUnexpectedReturnValueXHPASTLinterRule.php @@ -33,6 +33,10 @@ final class ArcanistUnexpectedReturnValueXHPASTLinterRule continue; } + if ($this->isInAnonymousFunction($return)) { + continue; + } + $this->raiseLintAtNode( $return, pht( @@ -45,4 +49,15 @@ final class ArcanistUnexpectedReturnValueXHPASTLinterRule } } + private function isInAnonymousFunction(XHPASTNode $node) { + while ($node) { + if ($node->getTypeName() == 'n_FUNCTION_DECLARATION' && + $node->getChildByIndex(2)->getTypeName() == 'n_EMPTY') { + return true; + } + + $node = $node->getParentNode(); + } + } + } diff --git a/src/lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php index 67021b47..1abbaa01 100644 --- a/src/lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php @@ -40,7 +40,7 @@ final class ArcanistUnnecessaryFinalModifierXHPASTLinterRule $this->raiseLintAtNode( $attribute, pht( - 'Unnecessary %s modifier in %s class.', + 'Unnecessary `%s` modifier in `%s` class.', 'final', 'final')); } diff --git a/src/lint/linter/xhpast/rules/ArcanistUnnecessarySymbolAliasXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUnnecessarySymbolAliasXHPASTLinterRule.php new file mode 100644 index 00000000..f2b1fb09 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistUnnecessarySymbolAliasXHPASTLinterRule.php @@ -0,0 +1,43 @@ +selectDescendantsOfType('n_USE'); + + foreach ($uses as $use) { + $symbol = $use->getChildOfType(0, 'n_SYMBOL_NAME'); + $alias = $use->getChildByIndex(1); + + if ($alias->getTypeName() == 'n_EMPTY') { + continue; + } + + $symbol_name = last(explode('\\', $symbol->getConcreteString())); + $alias_name = $alias->getConcreteString(); + + if ($symbol_name == $alias_name) { + $this->raiseLintAtNode( + $use, + pht( + 'Importing `%s` with `%s` is unnecessary because the aliased '. + 'name is identical to the imported symbol name.', + $symbol->getConcreteString(), + sprintf('as %s', $alias_name)), + $symbol->getConcreteString()); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistUnsafeDynamicStringXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUnsafeDynamicStringXHPASTLinterRule.php index be483f3c..bf3c8e51 100644 --- a/src/lint/linter/xhpast/rules/ArcanistUnsafeDynamicStringXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistUnsafeDynamicStringXHPASTLinterRule.php @@ -70,9 +70,11 @@ final class ArcanistUnsafeDynamicStringXHPASTLinterRule AASTNodeList $calls, array $safe) { - $safe = array_combine( - array_map('strtolower', array_keys($safe)), - $safe); + if ($safe) { + $safe = array_combine( + array_map('strtolower', array_keys($safe)), + $safe); + } foreach ($calls as $call) { $name = $call->getChildByIndex(0)->getConcreteString(); @@ -92,10 +94,10 @@ final class ArcanistUnsafeDynamicStringXHPASTLinterRule $this->raiseLintAtNode( $call, pht( - "Parameter %d of %s should be a scalar string, ". + "Parameter %d of `%s` should be a scalar string, ". "otherwise it's not safe.", $param + 1, - $name.'()')); + $name)); } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistUseStatementNamespacePrefixXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUseStatementNamespacePrefixXHPASTLinterRule.php new file mode 100644 index 00000000..d9c10892 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistUseStatementNamespacePrefixXHPASTLinterRule.php @@ -0,0 +1,38 @@ +selectDescendantsOfType('n_USE_LIST'); + + foreach ($use_lists as $use_list) { + $uses = $use_list->selectDescendantsOfType('n_USE'); + + foreach ($uses as $use) { + $symbol = $use->getChildOfType(0, 'n_SYMBOL_NAME'); + $symbol_name = $symbol->getConcreteString(); + + if ($symbol_name[0] == '\\') { + $this->raiseLintAtNode( + $symbol, + pht( + 'Imported symbols should not be prefixed with `%s`.', + '\\'), + substr($symbol_name, 1)); + } + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..133e6b9a --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php @@ -0,0 +1,10 @@ +executeTestsInDirectory(dirname(__FILE__).'/abstract-method-body/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..127149dc --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/abstract-private-method/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..ae34b95f --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/class-must-be-declared-abstract/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistInterfaceAbstractMethodXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistInterfaceAbstractMethodXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..3c305c21 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistInterfaceAbstractMethodXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/interface-abstract-method/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistInterfaceMethodBodyXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistInterfaceMethodBodyXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..7af7379b --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistInterfaceMethodBodyXHPASTLinterRuleTestCase.php @@ -0,0 +1,10 @@ +executeTestsInDirectory(dirname(__FILE__).'/interface-method-body/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..514c0ba9 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/namespace-first-statement/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistUnnecessarySymbolAliasXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistUnnecessarySymbolAliasXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..21ce2687 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistUnnecessarySymbolAliasXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/unnecessary-symbol-alias/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistUseStatementNamespacePrefixXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistUseStatementNamespacePrefixXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..f8341687 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistUseStatementNamespacePrefixXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/use-statement-namespace-prefix/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/abstract-method-body/body.lint-test b/src/lint/linter/xhpast/rules/__tests__/abstract-method-body/body.lint-test new file mode 100644 index 00000000..0ffc86e1 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/abstract-method-body/body.lint-test @@ -0,0 +1,6 @@ +closure = function() { + return null; + }; + } +} +~~~~~~~~~~ diff --git a/src/lint/linter/xhpast/rules/__tests__/unnecessary-symbol-alias/use.lint-test b/src/lint/linter/xhpast/rules/__tests__/unnecessary-symbol-alias/use.lint-test new file mode 100644 index 00000000..a9a25d5c --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/unnecessary-symbol-alias/use.lint-test @@ -0,0 +1,16 @@ +getBranchName(); + if ($branch) { + $path = $this->getPathToUpstream($branch); + if ($path->isConnectedToRemote()) { + $remote = $path->getRemoteRemoteName(); + } + } + // "git ls-remote --get-url" is the appropriate plumbing to get the remote // URI. "git config remote.origin.url", on the other hand, may not be as // accurate (for example, it does not take into account possible URL @@ -528,9 +538,9 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { // the --get-url flag requires git 1.7.5. $version = $this->getGitVersion(); if (version_compare($version, '1.7.5', '>=')) { - list($stdout) = $this->execxLocal('ls-remote --get-url origin'); + list($stdout) = $this->execxLocal('ls-remote --get-url %s', $remote); } else { - list($stdout) = $this->execxLocal('config remote.origin.url'); + list($stdout) = $this->execxLocal('config %s', "remote.{$remote}.url"); } $uri = rtrim($stdout); diff --git a/src/workflow/ArcanistInstallCertificateWorkflow.php b/src/workflow/ArcanistInstallCertificateWorkflow.php index 41f0726e..41a98519 100644 --- a/src/workflow/ArcanistInstallCertificateWorkflow.php +++ b/src/workflow/ArcanistInstallCertificateWorkflow.php @@ -56,9 +56,11 @@ EOTEXT $config = $configuration_manager->readUserConfigurationFile(); - $console->writeOut( - "%s\n", - pht('Trying to connect to server...')); + $this->writeInfo( + pht('CONNECT'), + pht( + 'Connecting to "%s"...', + $uri)); $conduit = $this->establishConduit()->getConduit(); try { @@ -194,10 +196,19 @@ EOTEXT $uri = $conduit_uri; } - $uri = new PhutilURI($uri); - $uri->setPath('/api/'); + $uri_object = new PhutilURI($uri); + if (!$uri_object->getProtocol() || !$uri_object->getDomain()) { + throw new ArcanistUsageException( + pht( + 'Server URI "%s" must include a protocol and domain. It should be '. + 'in the form "%s".', + $uri, + 'https://phabricator.example.com/')); + } - return (string)$uri; + $uri_object->setPath('/api/'); + + return (string)$uri_object; } } diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index 4525eeb4..23b0b7b4 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -1282,10 +1282,16 @@ EOTEXT $err = $repository_api->execPassthru('push %s', $this->remote); $cmd = 'hg push'; } else if ($this->isHg) { - $err = $repository_api->execPassthru( - 'push -r %s %s', - $this->onto, - $this->remote); + if (strlen($this->remote)) { + $err = $repository_api->execPassthru( + 'push -r %s %s', + $this->onto, + $this->remote); + } else { + $err = $repository_api->execPassthru( + 'push -r %s', + $this->onto); + } $cmd = 'hg push'; }