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 @@ -187,6 +187,7 @@ 'ArcanistPHPEchoTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPEchoTagXHPASTLinterRule.php', 'ArcanistPHPOpenTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPOpenTagXHPASTLinterRule.php', 'ArcanistPHPShortTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPShortTagXHPASTLinterRule.php', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule.php', 'ArcanistParenthesesSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php', 'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php', 'ArcanistPasteWorkflow' => 'workflow/ArcanistPasteWorkflow.php', @@ -220,6 +221,7 @@ 'ArcanistRubyLinter' => 'lint/linter/ArcanistRubyLinter.php', 'ArcanistRubyLinterTestCase' => 'lint/linter/__tests__/ArcanistRubyLinterTestCase.php', 'ArcanistScriptAndRegexLinter' => 'lint/linter/ArcanistScriptAndRegexLinter.php', + 'ArcanistSelfClassReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php', 'ArcanistSelfMemberReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php', 'ArcanistSemicolonSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSemicolonSpacingXHPASTLinterRule.php', 'ArcanistSetConfigWorkflow' => 'workflow/ArcanistSetConfigWorkflow.php', @@ -473,6 +475,7 @@ 'ArcanistPHPEchoTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPHPOpenTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPHPShortTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistParenthesesSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPasteWorkflow' => 'ArcanistWorkflow', @@ -506,6 +509,7 @@ 'ArcanistRubyLinter' => 'ArcanistExternalLinter', 'ArcanistRubyLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistScriptAndRegexLinter' => 'ArcanistLinter', + 'ArcanistSelfClassReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistSelfMemberReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistSemicolonSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistSetConfigWorkflow' => 'ArcanistWorkflow', diff --git a/src/lint/linter/__tests__/xhpast/paamayim-nekudotayim-spacing.lint-test b/src/lint/linter/__tests__/xhpast/paamayim-nekudotayim-spacing.lint-test new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/paamayim-nekudotayim-spacing.lint-test @@ -0,0 +1,34 @@ +<?php + +class Foo extends Bar { + public function bar() { + echo self::FOOBAR; + echo self :: FOOBAR; + } +} + +MyClass :: myMethod(); + +SomeReallyLongClassName + ::someMethod(); +~~~~~~~~~~ +error:3:7 +advice:6:10 +advice:6:14 +advice:6:17 +advice:10:8 +advice:10:11 +~~~~~~~~~~ +<?php + +class Foo extends Bar { + public function bar() { + echo self::FOOBAR; + echo Self::FOOBAR; + } +} + +MyClass::myMethod(); + +SomeReallyLongClassName + ::someMethod(); diff --git a/src/lint/linter/__tests__/xhpast/self-class-references.lint-test b/src/lint/linter/__tests__/xhpast/self-class-references.lint-test new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/self-class-references.lint-test @@ -0,0 +1,17 @@ +<?php + +class Foo extends Bar { + public static function newInstance() { + return new Foo(); + } +} +~~~~~~~~~~ +advice:5:6 +~~~~~~~~~~ +<?php + +class Foo extends Bar { + public static function newInstance() { + return new self(); + } +} diff --git a/src/lint/linter/__tests__/xhpast/self-member-references.lint-test b/src/lint/linter/__tests__/xhpast/self-member-references.lint-test --- a/src/lint/linter/__tests__/xhpast/self-member-references.lint-test +++ b/src/lint/linter/__tests__/xhpast/self-member-references.lint-test @@ -3,60 +3,22 @@ class Foo extends Bar { const FOOBAR = 'FOOBAR'; - public function __construct() { - PARENT::__construct(null); - } - - public function bar() { - echo self::FOOBAR; - echo Self :: FOOBAR; - } - public function baz(Foo $x) { - echo static::FOOBAR; + public function baz() { echo Foo::FOOBAR; - - $x::bar(); } } - -MyClass :: myMethod(); - -SomeReallyLongClassName - ::someMethod(); ~~~~~~~~~~ error:3:7 -advice:7:5 -advice:12:10 -advice:12:14 -advice:12:17 -advice:17:10 -advice:23:8 -advice:23:11 +advice:8:10 ~~~~~~~~~~ <?php class Foo extends Bar { const FOOBAR = 'FOOBAR'; - public function __construct() { - parent::__construct(null); - } - public function bar() { - echo self::FOOBAR; + public function baz() { echo self::FOOBAR; } - - public function baz(Foo $x) { - echo static::FOOBAR; - echo self::FOOBAR; - - $x::bar(); - } } - -MyClass::myMethod(); - -SomeReallyLongClassName - ::someMethod(); diff --git a/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php --- a/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php @@ -15,73 +15,95 @@ public function process(XHPASTNode $root) { $keywords = $root->selectTokensOfTypes(array( - 'T_REQUIRE_ONCE', - 'T_REQUIRE', - 'T_EVAL', - 'T_INCLUDE_ONCE', - 'T_INCLUDE', - 'T_LOGICAL_OR', - 'T_LOGICAL_XOR', - 'T_LOGICAL_AND', - 'T_PRINT', - 'T_INSTANCEOF', + 'T_ABSTRACT', + 'T_ARRAY', + 'T_AS', + 'T_BREAK', + 'T_CALLABLE', + 'T_CASE', + 'T_CATCH', + 'T_CLASS', 'T_CLONE', - 'T_NEW', - 'T_EXIT', - 'T_IF', - 'T_ELSEIF', + '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_ECHO', - 'T_DO', - 'T_WHILE', + 'T_ENDSWITCH', 'T_ENDWHILE', + 'T_EVAL', + 'T_EXIT', + 'T_EXTENDS', + 'T_FINAL', + 'T_FINALLY', '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_GOTO', 'T_HALT_COMPILER', - 'T_CLASS', - 'T_INTERFACE', - 'T_EXTENDS', + 'T_IF', 'T_IMPLEMENTS', + 'T_INCLUDE', + 'T_INCLUDE_ONCE', + 'T_INSTANCEOF', + 'T_INSTEADOF', + 'T_INTERFACE', + 'T_ISSET', 'T_LIST', - 'T_ARRAY', + 'T_LOGICAL_AND', + 'T_LOGICAL_OR', + 'T_LOGICAL_XOR', '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) { + break; + } + + $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 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule.php @@ -0,0 +1,38 @@ +<?php + +final class ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule + extends ArcanistXHPASTLinterRule { + + const ID = 84; + + public function getLintName() { + return pht('Paamayim Nekudotayim Spacing'); + } + + public function getLintSeverity() { + return ArcanistLintSeverity::SEVERITY_WARNING; + } + + public function process(XHPASTNode $root) { + $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/ArcanistSelfClassReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php new file mode 100644 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php @@ -0,0 +1,44 @@ +<?php + +final class ArcanistSelfClassReferenceXHPASTLinterRule + extends ArcanistXHPASTLinterRule { + + const ID = 83; + + public function getLintName() { + return pht('Self Class Reference'); + } + + public function getLintSeverity() { + return ArcanistLintSeverity::SEVERITY_ADVICE; + } + + public function process(XHPASTNode $root) { + $class_declarations = $root->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 --- a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php @@ -54,43 +54,6 @@ '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.'), - ''); - } } } }