diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 680febaf..4783a8d6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -32,6 +32,7 @@ phutil_register_library_map(array( 'ArcanistCSSLintLinterTestCase' => 'lint/linter/__tests__/ArcanistCSSLintLinterTestCase.php', 'ArcanistCSharpLinter' => 'lint/linter/ArcanistCSharpLinter.php', 'ArcanistCallConduitWorkflow' => 'workflow/ArcanistCallConduitWorkflow.php', + 'ArcanistCallParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php', 'ArcanistCallTimePassByReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCallTimePassByReferenceXHPASTLinterRule.php', 'ArcanistCapabilityNotSupportedException' => 'workflow/exception/ArcanistCapabilityNotSupportedException.php', 'ArcanistCastSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCastSpacingXHPASTLinterRule.php', @@ -42,7 +43,6 @@ phutil_register_library_map(array( 'ArcanistClassNameLiteralXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php', 'ArcanistCloseRevisionWorkflow' => 'workflow/ArcanistCloseRevisionWorkflow.php', 'ArcanistCloseWorkflow' => 'workflow/ArcanistCloseWorkflow.php', - 'ArcanistClosingCallParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClosingCallParenthesesXHPASTLinterRule.php', 'ArcanistClosureLinter' => 'lint/linter/ArcanistClosureLinter.php', 'ArcanistClosureLinterTestCase' => 'lint/linter/__tests__/ArcanistClosureLinterTestCase.php', 'ArcanistCoffeeLintLinter' => 'lint/linter/ArcanistCoffeeLintLinter.php', @@ -171,6 +171,7 @@ phutil_register_library_map(array( 'ArcanistNoLintLinterTestCase' => 'lint/linter/__tests__/ArcanistNoLintLinterTestCase.php', 'ArcanistNoParentScopeXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistNoParentScopeXHPASTLinterRule.php', 'ArcanistNoneLintRenderer' => 'lint/renderer/ArcanistNoneLintRenderer.php', + 'ArcanistObjectOperatorSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistObjectOperatorSpacingXHPASTLinterRule.php', 'ArcanistPEP8Linter' => 'lint/linter/ArcanistPEP8Linter.php', 'ArcanistPEP8LinterTestCase' => 'lint/linter/__tests__/ArcanistPEP8LinterTestCase.php', 'ArcanistPHPCloseTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php', @@ -237,6 +238,8 @@ phutil_register_library_map(array( 'ArcanistTodoWorkflow' => 'workflow/ArcanistTodoWorkflow.php', 'ArcanistUSEnglishTranslation' => 'internationalization/ArcanistUSEnglishTranslation.php', 'ArcanistUnableToParseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnableToParseXHPASTLinterRule.php', + 'ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule.php', + 'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule.php', 'ArcanistUndeclaredVariableXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php', 'ArcanistUnitConsoleRenderer' => 'unit/renderer/ArcanistUnitConsoleRenderer.php', 'ArcanistUnitRenderer' => 'unit/renderer/ArcanistUnitRenderer.php', @@ -305,6 +308,7 @@ phutil_register_library_map(array( 'ArcanistCSSLintLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistCSharpLinter' => 'ArcanistLinter', 'ArcanistCallConduitWorkflow' => 'ArcanistWorkflow', + 'ArcanistCallParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistCallTimePassByReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistCapabilityNotSupportedException' => 'Exception', 'ArcanistCastSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -315,7 +319,6 @@ phutil_register_library_map(array( 'ArcanistClassNameLiteralXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistCloseRevisionWorkflow' => 'ArcanistWorkflow', 'ArcanistCloseWorkflow' => 'ArcanistWorkflow', - 'ArcanistClosingCallParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistClosureLinter' => 'ArcanistExternalLinter', 'ArcanistClosureLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistCoffeeLintLinter' => 'ArcanistExternalLinter', @@ -444,6 +447,7 @@ phutil_register_library_map(array( 'ArcanistNoLintLinterTestCase' => 'ArcanistLinterTestCase', 'ArcanistNoParentScopeXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistNoneLintRenderer' => 'ArcanistLintRenderer', + 'ArcanistObjectOperatorSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPEP8Linter' => 'ArcanistExternalLinter', 'ArcanistPEP8LinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistPHPCloseTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -510,6 +514,8 @@ phutil_register_library_map(array( 'ArcanistTodoWorkflow' => 'ArcanistWorkflow', 'ArcanistUSEnglishTranslation' => 'PhutilTranslation', 'ArcanistUnableToParseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistUndeclaredVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistUnitConsoleRenderer' => 'ArcanistUnitRenderer', 'ArcanistUnitRenderer' => 'Phobject', diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php index 40502020..d4de43b8 100644 --- a/src/lint/ArcanistLintMessage.php +++ b/src/lint/ArcanistLintMessage.php @@ -72,24 +72,7 @@ final class ArcanistLintMessage extends Phobject { } public function setLine($line) { - if ($line === null) { - // This just means that we don't have any line information. - } else { - // For compatibility, accept digit strings since a lot of linters pass - // line numbers that they have parsed from command output or XML, which - // won't be properly typed. - if (is_string($line) && preg_match('/^\d+\z/', $line)) { - $line = (int)$line; - } - - if (!is_int($line)) { - throw new Exception( - pht( - 'Parameter passed to setLine() must be an integer.')); - } - } - - $this->line = $line; + $this->line = $this->validateInteger($line, 'setLine'); return $this; } @@ -98,7 +81,7 @@ final class ArcanistLintMessage extends Phobject { } public function setChar($char) { - $this->char = $char; + $this->char = $this->validateInteger($char, 'setChar'); return $this; } @@ -242,4 +225,35 @@ final class ArcanistLintMessage extends Phobject { return $this->bypassChangedLineFiltering; } + /** + * Validate an integer-like value, returning a strict integer. + * + * Further on, the pipeline is strict about types. We want to be a little + * less strict in linters themselves, since they often parse command line + * output or XML and will end up with string representations of numbers. + * + * @param mixed Integer or digit string. + * @return int Integer. + */ + private function validateInteger($value, $caller) { + if ($value === null) { + // This just means that we don't have any information. + return null; + } + + // Strings like "234" are fine, coerce them to integers. + if (is_string($value) && preg_match('/^\d+\z/', $value)) { + $value = (int)$value; + } + + if (!is_int($value)) { + throw new Exception( + pht( + 'Parameter passed to "%s" must be an integer.', + $caller.'()')); + } + + return $value; + } + } diff --git a/src/lint/linter/__tests__/xhpast/call-parens-hug-closing.lint-test b/src/lint/linter/__tests__/xhpast/call-parens-hug-closing.lint-test index 81ad7a13..3ec2eaa5 100644 --- a/src/lint/linter/__tests__/xhpast/call-parens-hug-closing.lint-test +++ b/src/lint/linter/__tests__/xhpast/call-parens-hug-closing.lint-test @@ -16,9 +16,11 @@ The duck says, "Quack!" The robot says, "Beep beep boop boop!" EODOC ); +f (1); ~~~~~~~~~~ warning:4:4 warning:9:4 +warning:19:2 ~~~~~~~~~~ doSomething(); +id(new Something()) + ->doSomething(); +~~~~~~~~~~ +warning:2:3 +warning:2:6 +~~~~~~~~~~ +doSomething(); +id(new Something()) + ->doSomething(); diff --git a/src/lint/linter/__tests__/xhpast/php-tags-bad.lint-test b/src/lint/linter/__tests__/xhpast/php-tags-bad.lint-test index e749bfa6..7fb52e73 100644 --- a/src/lint/linter/__tests__/xhpast/php-tags-bad.lint-test +++ b/src/lint/linter/__tests__/xhpast/php-tags-bad.lint-test @@ -4,7 +4,6 @@ garbage garbage ?> ~~~~~~~~~~ error:1:1 -error:4:1 ~~~~~~~~~~ garbage garbage getChildOfType(1, 'n_CALL_PARAMETER_LIST'); + $tokens = $params->getTokens(); + $first = head($tokens); + + $leading = $first->getNonsemanticTokensBefore(); + $leading_text = implode('', mpull($leading, 'getValue')); + if (preg_match('/^\s+$/', $leading_text)) { + $this->raiseLintAtOffset( + $first->getOffset() - strlen($leading_text), + pht('Convention: no spaces before opening parenthesis in calls.'), + $leading_text, + ''); + } + } + foreach ($calls as $call) { // If the last parameter of a call is a HEREDOC, don't apply this rule. $params = $call diff --git a/src/lint/linter/xhpast/rules/ArcanistObjectOperatorSpacingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistObjectOperatorSpacingXHPASTLinterRule.php new file mode 100644 index 00000000..8fb7f4a4 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistObjectOperatorSpacingXHPASTLinterRule.php @@ -0,0 +1,47 @@ +selectTokensOfType('T_OBJECT_OPERATOR'); + + foreach ($operators as $operator) { + $before = $operator->getNonsemanticTokensBefore(); + $after = $operator->getNonsemanticTokensAfter(); + + if ($before) { + $value = implode('', mpull($before, 'getValue')); + + if (strpos($value, "\n") !== false) { + continue; + } + + $this->raiseLintAtOffset( + head($before)->getOffset(), + pht('There should be no whitespace before the object operator.'), + $value, + ''); + } + + if ($after) { + $this->raiseLintAtOffset( + head($after)->getOffset(), + pht('There should be no whitespace after the object operator.'), + implode('', mpull($before, 'getValue')), + ''); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php index 9a1a9db7..1b943b54 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php @@ -10,6 +10,12 @@ final class ArcanistPHPCloseTagXHPASTLinterRule } public function process(XHPASTNode $root) { + $inline_html = $root->selectDescendantsOfType('n_INLINE_HTML'); + + if ($inline_html) { + return; + } + foreach ($root->selectTokensOfType('T_CLOSE_TAG') as $token) { $this->raiseLintAtToken( $token, diff --git a/src/lint/linter/xhpast/rules/ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule.php new file mode 100644 index 00000000..175cf0d0 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistUnaryPostfixExpressionSpacingXHPASTLinterRule.php @@ -0,0 +1,36 @@ +selectDescendantsOfType('n_UNARY_POSTFIX_EXPRESSION'); + + foreach ($expressions as $expression) { + $operator = $expression->getChildOfType(1, 'n_OPERATOR'); + $operator_value = $operator->getConcreteString(); + list($before, $after) = $operator->getSurroundingNonsemanticTokens(); + + if (!empty($before)) { + $leading_text = implode('', mpull($before, 'getValue')); + + $this->raiseLintAtOffset( + $operator->getOffset() - strlen($leading_text), + pht('Unary postfix operators should not be prefixed by whitespace.'), + $leading_text, + ''); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule.php new file mode 100644 index 00000000..5fa91ffd --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRule.php @@ -0,0 +1,44 @@ +selectDescendantsOfType('n_UNARY_PREFIX_EXPRESSION'); + + foreach ($expressions as $expression) { + $operator = $expression->getChildOfType(0, 'n_OPERATOR'); + $operator_value = $operator->getConcreteString(); + list($before, $after) = $operator->getSurroundingNonsemanticTokens(); + + switch (strtolower($operator_value)) { + case 'clone': + case 'echo': + case 'print': + break; + + default: + if (!empty($after)) { + $this->raiseLintAtOffset( + $operator->getOffset() + strlen($operator->getConcreteString()), + pht( + 'Unary prefix operators should not be followed by whitespace.'), + implode('', mpull($after, 'getValue')), + ''); + } + break; + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php index afb07968..c60b216c 100644 --- a/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php @@ -164,6 +164,17 @@ final class ArcanistUndeclaredVariableXHPASTLinterRule $scope_destroyed_at = min($scope_destroyed_at, $call->getOffset()); } + $func_decls = $body->selectDescendantsOfType('n_FUNCTION_DECLARATION'); + foreach ($func_decls as $func_decl) { + if ($func_decl->getChildByIndex(2)->getTypeName() != 'n_EMPTY') { + continue; + } + + foreach ($func_decl->selectDescendantsOfType('n_VARIABLE') as $var) { + $exclude_tokens[$var->getID()] = true; + } + } + // Now we have every declaration except foreach(), handled below. Build // two maps, one which just keeps track of which tokens are part of // declarations ($declaration_tokens) and one which has the first offset diff --git a/src/unit/ArcanistUnitTestResult.php b/src/unit/ArcanistUnitTestResult.php index f570c072..039393f8 100644 --- a/src/unit/ArcanistUnitTestResult.php +++ b/src/unit/ArcanistUnitTestResult.php @@ -152,4 +152,68 @@ final class ArcanistUnitTestResult extends Phobject { ); } + public static function getAllResultCodes() { + return array( + self::RESULT_PASS, + self::RESULT_FAIL, + self::RESULT_SKIP, + self::RESULT_BROKEN, + self::RESULT_UNSOUND, + ); + } + + public static function getResultCodeName($result_code) { + $spec = self::getResultCodeSpec($result_code); + if (!$spec) { + return null; + } + return idx($spec, 'name'); + } + + public static function getResultCodeDescription($result_code) { + $spec = self::getResultCodeSpec($result_code); + if (!$spec) { + return null; + } + return idx($spec, 'description'); + } + + private static function getResultCodeSpec($result_code) { + $specs = self::getResultCodeSpecs(); + return idx($specs, $result_code); + } + + private static function getResultCodeSpecs() { + return array( + self::RESULT_PASS => array( + 'name' => pht('Pass'), + 'description' => pht( + 'The test passed.'), + ), + self::RESULT_FAIL => array( + 'name' => pht('Fail'), + 'description' => pht( + 'The test failed.'), + ), + self::RESULT_SKIP => array( + 'name' => pht('Skip'), + 'description' => pht( + 'The test was not executed.'), + ), + self::RESULT_BROKEN => array( + 'name' => pht('Broken'), + 'description' => pht( + 'The test failed in an abnormal or severe way. For example, the '. + 'harness crashed instead of reporting a failure.'), + ), + self::RESULT_UNSOUND => array( + 'name' => pht('Unsound'), + 'description' => pht( + 'The test failed, but this change is probably not what broke it. '. + 'For example, it might have already been failing.'), + ), + ); + } + + } diff --git a/src/unit/engine/NoseTestEngine.php b/src/unit/engine/NoseTestEngine.php index 5e5870e1..e81bcb63 100644 --- a/src/unit/engine/NoseTestEngine.php +++ b/src/unit/engine/NoseTestEngine.php @@ -7,6 +7,8 @@ */ final class NoseTestEngine extends ArcanistUnitTestEngine { + private $parser; + public function run() { $paths = $this->getPaths(); diff --git a/src/workflow/ArcanistBackoutWorkflow.php b/src/workflow/ArcanistBackoutWorkflow.php index 49ff007e..a9564dbb 100644 --- a/src/workflow/ArcanistBackoutWorkflow.php +++ b/src/workflow/ArcanistBackoutWorkflow.php @@ -161,8 +161,8 @@ EOTEXT $commit_hash = $commit['identifier']; // Convert commit hash from SVN to Git/HG (for FB case) if ($is_git_svn || $is_hg_svn) { - $commit_hash = $repository_api-> - getHashFromFromSVNRevisionNumber($commit_hash); + $commit_hash = $repository_api + ->getHashFromFromSVNRevisionNumber($commit_hash); } } else { // Assume input is a commit hash