diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2fbc0334..13ef8642 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -272,6 +272,8 @@ phutil_register_library_map(array( 'ArcanistPHPOpenTagXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPOpenTagXHPASTLinterRuleTestCase.php', 'ArcanistPHPShortTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPShortTagXHPASTLinterRule.php', 'ArcanistPHPShortTagXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPShortTagXHPASTLinterRuleTestCase.php', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule.php', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRuleTestCase.php', 'ArcanistParentMemberReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php', 'ArcanistParentMemberReferenceXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParentMemberReferenceXHPASTLinterRuleTestCase.php', 'ArcanistParenthesesSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php', @@ -317,6 +319,8 @@ phutil_register_library_map(array( 'ArcanistRubyLinter' => 'lint/linter/ArcanistRubyLinter.php', 'ArcanistRubyLinterTestCase' => 'lint/linter/__tests__/ArcanistRubyLinterTestCase.php', 'ArcanistScriptAndRegexLinter' => 'lint/linter/ArcanistScriptAndRegexLinter.php', + 'ArcanistSelfClassReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php', + 'ArcanistSelfClassReferenceXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistSelfClassReferenceXHPASTLinterRuleTestCase.php', 'ArcanistSelfMemberReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php', 'ArcanistSelfMemberReferenceXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistSelfMemberReferenceXHPASTLinterRuleTestCase.php', 'ArcanistSemicolonSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSemicolonSpacingXHPASTLinterRule.php', @@ -680,6 +684,8 @@ phutil_register_library_map(array( 'ArcanistPHPOpenTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistPHPShortTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPHPShortTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistParentMemberReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistParentMemberReferenceXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistParenthesesSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -725,6 +731,8 @@ phutil_register_library_map(array( 'ArcanistRubyLinter' => 'ArcanistExternalLinter', 'ArcanistRubyLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistScriptAndRegexLinter' => 'ArcanistLinter', + 'ArcanistSelfClassReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistSelfClassReferenceXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistSelfMemberReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistSelfMemberReferenceXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistSemicolonSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', diff --git a/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php index af0b5903..8af5a7c8 100644 --- a/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php @@ -15,73 +15,98 @@ final class ArcanistKeywordCasingXHPASTLinterRule public function process(XHPASTNode $root) { $keywords = $root->selectTokensOfTypes(array( - 'T_REQUIRE_ONCE', - 'T_REQUIRE', + 'T_ABSTRACT', + 'T_ARRAY', + 'T_AS', + 'T_BREAK', + 'T_CALLABLE', + 'T_CASE', + 'T_CATCH', + 'T_CLASS', + 'T_CLONE', + 'T_CONST', + 'T_CONTINUE', + 'T_DECLARE', + 'T_DEFAULT', + 'T_DO', + 'T_ECHO', + 'T_ELSE', + 'T_ELSEIF', + 'T_EMPTY', + 'T_ENDDECLARE', + 'T_ENDFOR', + 'T_ENDFOREACH', + 'T_ENDIF', + 'T_ENDSWITCH', + 'T_ENDWHILE', 'T_EVAL', - 'T_INCLUDE_ONCE', + 'T_EXIT', + 'T_EXTENDS', + 'T_FINAL', + 'T_FINALLY', + 'T_FOR', + 'T_FOREACH', + 'T_FUNCTION', + 'T_GLOBAL', + 'T_GOTO', + 'T_HALT_COMPILER', + 'T_IF', + 'T_IMPLEMENTS', 'T_INCLUDE', + 'T_INCLUDE_ONCE', + 'T_INSTANCEOF', + 'T_INSTEADOF', + 'T_INTERFACE', + 'T_ISSET', + 'T_LIST', + 'T_LOGICAL_AND', 'T_LOGICAL_OR', 'T_LOGICAL_XOR', - 'T_LOGICAL_AND', - 'T_PRINT', - 'T_INSTANCEOF', - 'T_CLONE', - 'T_NEW', - 'T_EXIT', - 'T_IF', - 'T_ELSEIF', - 'T_ELSE', - 'T_ENDIF', - 'T_ECHO', - 'T_DO', - 'T_WHILE', - 'T_ENDWHILE', - 'T_FOR', - 'T_ENDFOR', - 'T_FOREACH', - 'T_ENDFOREACH', - 'T_DECLARE', - 'T_ENDDECLARE', - 'T_AS', - 'T_SWITCH', - 'T_ENDSWITCH', - 'T_CASE', - 'T_DEFAULT', - 'T_BREAK', - 'T_CONTINUE', - 'T_GOTO', - 'T_FUNCTION', - 'T_CONST', - 'T_RETURN', - 'T_TRY', - 'T_CATCH', - 'T_THROW', - 'T_USE', - 'T_GLOBAL', - 'T_PUBLIC', - 'T_PROTECTED', - 'T_PRIVATE', - 'T_FINAL', - 'T_ABSTRACT', - 'T_STATIC', - 'T_VAR', - 'T_UNSET', - 'T_ISSET', - 'T_EMPTY', - 'T_HALT_COMPILER', - 'T_CLASS', - 'T_INTERFACE', - 'T_EXTENDS', - 'T_IMPLEMENTS', - 'T_LIST', - 'T_ARRAY', 'T_NAMESPACE', - 'T_INSTEADOF', - 'T_CALLABLE', + 'T_NEW', + 'T_PRINT', + 'T_PRIVATE', + 'T_PROTECTED', + 'T_PUBLIC', + 'T_REQUIRE', + 'T_REQUIRE_ONCE', + 'T_RETURN', + 'T_STATIC', + 'T_SWITCH', + 'T_THROW', 'T_TRAIT', + 'T_TRY', + 'T_UNSET', + 'T_USE', + 'T_VAR', + 'T_WHILE', 'T_YIELD', - 'T_FINALLY', )); + + // Because there is no `T_SELF` or `T_PARENT` token. + $class_static_accesses = $root + ->selectDescendantsOfType('n_CLASS_DECLARATION') + ->selectDescendantsOfType('n_CLASS_STATIC_ACCESS'); + foreach ($class_static_accesses as $class_static_access) { + $class_ref = $class_static_access->getChildByIndex(0); + + switch (strtolower($class_ref->getConcreteString())) { + case 'parent': + case 'self': + $tokens = $class_ref->getTokens(); + + if (count($tokens) > 1) { + throw new Exception( + pht( + 'Unexpected tokens whilst processing `%s`.', + __CLASS__)); + } + + $keywords[] = head($tokens); + break; + } + } + foreach ($keywords as $keyword) { $value = $keyword->getValue(); diff --git a/src/lint/linter/xhpast/rules/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule.php new file mode 100644 index 00000000..4be549ba --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule.php @@ -0,0 +1,40 @@ +selectTokensOfType('T_PAAMAYIM_NEKUDOTAYIM'); + + foreach ($double_colons as $double_colon) { + $tokens = $double_colon->getNonsemanticTokensBefore() + + $double_colon->getNonsemanticTokensAfter(); + + foreach ($tokens as $token) { + if ($token->isAnyWhitespace()) { + if (strpos($token->getValue(), "\n") !== false) { + continue; + } + + $this->raiseLintAtToken( + $token, + pht( + 'Unnecessary whitespace around paamayim nekudotayim '. + '(double colon) operator.'), + ''); + } + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php new file mode 100644 index 00000000..7ad7c238 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php @@ -0,0 +1,45 @@ +selectDescendantsOfType('n_CLASS_DECLARATION'); + + foreach ($class_declarations as $class_declaration) { + $class_name = $class_declaration + ->getChildOfType(1, 'n_CLASS_NAME') + ->getConcreteString(); + $instantiations = $class_declaration + ->selectDescendantsOfType('n_NEW'); + + foreach ($instantiations as $instantiation) { + $type = $instantiation->getChildByIndex(0); + + if ($type->getTypeName() != 'n_CLASS_NAME') { + continue; + } + + if (strtolower($type->getConcreteString()) == strtolower($class_name)) { + $this->raiseLintAtNode( + $type, + pht( + 'Use `%s` to instantiate the current class.', + 'self'), + 'self'); + } + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php index bfec780e..8db13ca7 100644 --- a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php @@ -10,7 +10,7 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule } public function getLintSeverity() { - return ArcanistLintSeverity::SEVERITY_ADVICE; + return ArcanistLintSeverity::SEVERITY_WARNING; } public function process(XHPASTNode $root) { @@ -20,14 +20,11 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule $class_name = $class_declaration ->getChildOfType(1, 'n_CLASS_NAME') ->getConcreteString(); - $class_static_accesses = $class_declaration ->selectDescendantsOfType('n_CLASS_STATIC_ACCESS'); $closures = $this->getAnonymousClosures($class_declaration); foreach ($class_static_accesses as $class_static_access) { - $double_colons = $class_static_access - ->selectTokensOfType('T_PAAMAYIM_NEKUDOTAYIM'); $class_ref = $class_static_access->getChildByIndex(0); if ($class_ref->getTypeName() != 'n_CLASS_NAME') { @@ -54,43 +51,6 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule 'self'); } } - - static $self_refs = array( - 'parent', - 'self', - 'static', - ); - - if (!in_array(strtolower($class_ref_name), $self_refs)) { - continue; - } - - if ($class_ref_name != strtolower($class_ref_name)) { - $this->raiseLintAtNode( - $class_ref, - pht('PHP keywords should be lowercase.'), - strtolower($class_ref_name)); - } - } - } - - $double_colons = $root->selectTokensOfType('T_PAAMAYIM_NEKUDOTAYIM'); - - foreach ($double_colons as $double_colon) { - $tokens = $double_colon->getNonsemanticTokensBefore() + - $double_colon->getNonsemanticTokensAfter(); - - foreach ($tokens as $token) { - if ($token->isAnyWhitespace()) { - if (strpos($token->getValue(), "\n") !== false) { - continue; - } - - $this->raiseLintAtToken( - $token, - pht('Unnecessary whitespace around double colon operator.'), - ''); - } } } } diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..d847cdf1 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/paamayim-nekudotayim-spacing/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistSelfClassReferenceXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistSelfClassReferenceXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..873abfd6 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistSelfClassReferenceXHPASTLinterRuleTestCase.php @@ -0,0 +1,10 @@ +executeTestsInDirectory(dirname(__FILE__).'/self-class-reference/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/keyword-casing/parent.lint-test b/src/lint/linter/xhpast/rules/__tests__/keyword-casing/parent.lint-test new file mode 100644 index 00000000..281f4148 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/keyword-casing/parent.lint-test @@ -0,0 +1,25 @@ +