diff --git a/scripts/arcanist.php b/scripts/arcanist.php index f29311f6..c888fae9 100755 --- a/scripts/arcanist.php +++ b/scripts/arcanist.php @@ -74,17 +74,27 @@ $config = null; $workflow = null; try { + if ($config_trace_mode) { + echo tsprintf( + "** %s ** %s\n", + pht('ARGV'), + csprintf('%Ls', $original_argv)); - $console->writeLog( - "%s\n", - pht( - "libphutil loaded from '%s'.", - phutil_get_library_root('phutil'))); - $console->writeLog( - "%s\n", - pht( - "arcanist loaded from '%s'.", - phutil_get_library_root('arcanist'))); + $libraries = array( + 'phutil', + 'arcanist', + ); + + foreach ($libraries as $library_name) { + echo tsprintf( + "** %s ** %s\n", + pht('LOAD'), + pht( + 'Loaded "%s" from "%s".', + $library_name, + phutil_get_library_root($library_name))); + } + } if (!$args) { if ($help) { diff --git a/scripts/repository/binary_safe_diff.sh b/scripts/repository/binary_safe_diff.sh index 1473e804..00b02680 100755 --- a/scripts/repository/binary_safe_diff.sh +++ b/scripts/repository/binary_safe_diff.sh @@ -1,6 +1,3 @@ #!/bin/sh -diff "$@" -if [ "$?" = "2" ]; then - exit 1 -fi +diff "$@" || exit 1 diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4d1a81c6..6012daf7 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -96,6 +96,8 @@ phutil_register_library_map(array( 'ArcanistCppcheckLinterTestCase' => 'lint/linter/__tests__/ArcanistCppcheckLinterTestCase.php', 'ArcanistCpplintLinter' => 'lint/linter/ArcanistCpplintLinter.php', 'ArcanistCpplintLinterTestCase' => 'lint/linter/__tests__/ArcanistCpplintLinterTestCase.php', + 'ArcanistCurlyBraceArrayIndexXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCurlyBraceArrayIndexXHPASTLinterRule.php', + 'ArcanistCurlyBraceArrayIndexXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistCurlyBraceArrayIndexXHPASTLinterRuleTestCase.php', 'ArcanistDeclarationParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php', 'ArcanistDeclarationParenthesesXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistDeclarationParenthesesXHPASTLinterRuleTestCase.php', 'ArcanistDefaultParametersXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistDefaultParametersXHPASTLinterRule.php', @@ -194,6 +196,8 @@ phutil_register_library_map(array( 'ArcanistInvalidModifiersXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidModifiersXHPASTLinterRuleTestCase.php', 'ArcanistInvalidOctalNumericScalarXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInvalidOctalNumericScalarXHPASTLinterRule.php', 'ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase.php', + 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php', + 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php', 'ArcanistJSHintLinter' => 'lint/linter/ArcanistJSHintLinter.php', 'ArcanistJSHintLinterTestCase' => 'lint/linter/__tests__/ArcanistJSHintLinterTestCase.php', 'ArcanistJSONLintLinter' => 'lint/linter/ArcanistJSONLintLinter.php', @@ -270,6 +274,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', @@ -315,6 +321,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', @@ -502,6 +510,8 @@ phutil_register_library_map(array( 'ArcanistCppcheckLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistCpplintLinter' => 'ArcanistExternalLinter', 'ArcanistCpplintLinterTestCase' => 'ArcanistExternalLinterTestCase', + 'ArcanistCurlyBraceArrayIndexXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistCurlyBraceArrayIndexXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistDeclarationParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistDeclarationParenthesesXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistDefaultParametersXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -600,6 +610,8 @@ phutil_register_library_map(array( 'ArcanistInvalidModifiersXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistInvalidOctalNumericScalarXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistJSHintLinter' => 'ArcanistExternalLinter', 'ArcanistJSHintLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistJSONLintLinter' => 'ArcanistExternalLinter', @@ -676,6 +688,8 @@ phutil_register_library_map(array( 'ArcanistPHPOpenTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistPHPShortTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPHPShortTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistPaamayimNekudotayimSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistParentMemberReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistParentMemberReferenceXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistParenthesesSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -721,6 +735,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/land/ArcanistGitLandEngine.php b/src/land/ArcanistGitLandEngine.php index 7256b050..068124d0 100644 --- a/src/land/ArcanistGitLandEngine.php +++ b/src/land/ArcanistGitLandEngine.php @@ -29,9 +29,7 @@ final class ArcanistGitLandEngine $this->updateWorkingCopy(); if ($this->getShouldHold()) { - $this->writeInfo( - pht('HOLD'), - pht('Holding change locally, it has not been pushed.')); + $this->didHoldChanges(); } else { $this->pushChange(); $this->reconcileLocalState(); @@ -542,4 +540,41 @@ final class ArcanistGitLandEngine ); } + private function didHoldChanges() { + $this->writeInfo( + pht('HOLD'), + pht( + 'Holding change locally, it has not been pushed.')); + + $push_command = csprintf( + '$ git push -- %R %R:%R', + $this->getTargetRemote(), + $this->mergedRef, + $this->getTargetOnto()); + + $restore_command = csprintf( + '$ git checkout %R --', + $this->localRef); + + echo tsprintf( + "\n%s\n\n". + "%s\n\n". + " %s\n\n". + "%s\n\n". + " %s\n\n". + "%s\n", + pht( + 'This local working copy now contains the merged changes in a '. + 'detached state.'), + pht('You can push the changes manually with this command:'), + $push_command, + pht( + 'You can go back to how things were before you ran `arc land` with '. + 'this command:'), + $restore_command, + pht( + 'Local branches have not been changed, and are still in exactly the '. + 'same state as before.')); + } + } diff --git a/src/lint/linter/xhpast/rules/ArcanistBraceFormattingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistBraceFormattingXHPASTLinterRule.php index 0a8c33b7..775c6f18 100644 --- a/src/lint/linter/xhpast/rules/ArcanistBraceFormattingXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistBraceFormattingXHPASTLinterRule.php @@ -24,14 +24,6 @@ final class ArcanistBraceFormattingXHPASTLinterRule if (!$before) { $first = head($tokens); - // Only insert the space if we're after a closing parenthesis. If - // we're in a construct like "else{}", other rules will insert space - // after the 'else' correctly. - $prev = $first->getPrevToken(); - if (!$prev || $prev->getValue() !== ')') { - continue; - } - $this->raiseLintAtToken( $first, pht( diff --git a/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php index 6c8c2f41..44eb9b6e 100644 --- a/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php @@ -46,7 +46,7 @@ final class ArcanistClassFilenameMismatchXHPASTLinterRule $this->raiseLintAtNode( $decl_name, pht( - "The name of this file differs from the name of the ". + 'The name of this file differs from the name of the '. 'class or interface it declares. Rename the file to `%s`.', $rename)); } diff --git a/src/lint/linter/xhpast/rules/ArcanistCurlyBraceArrayIndexXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistCurlyBraceArrayIndexXHPASTLinterRule.php new file mode 100644 index 00000000..99b3ab94 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistCurlyBraceArrayIndexXHPASTLinterRule.php @@ -0,0 +1,56 @@ +selectDescendantsOfType('n_INDEX_ACCESS'); + + foreach ($index_accesses as $index_access) { + $tokens = $index_access->getChildByIndex(1)->getTokens(); + + if (!$tokens) { + continue; + } + + $left_brace = head($tokens)->getPrevToken(); + while (!$left_brace->isSemantic()) { + $left_brace = $left_brace->getPrevToken(); + } + + $right_brace = last($tokens)->getNextToken(); + while (!$right_brace->isSemantic()) { + $right_brace = $right_brace->getNextToken(); + } + + if ($left_brace->getValue() == '{' || $right_brace->getValue() == '}') { + $replacement = null; + foreach ($index_access->getTokens() as $token) { + if ($token === $left_brace) { + $replacement .= '['; + } else if ($token === $right_brace) { + $replacement .= ']'; + } else { + $replacement .= $token->getValue(); + } + } + + $this->raiseLintAtNode( + $index_access, + pht('Use `%s` instead of `%s`.', "\$x['key']", "\$x{'key'}"), + $replacement); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php index 1fe7a2a7..7a86f2a1 100644 --- a/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php @@ -193,7 +193,7 @@ final class ArcanistImplicitFallthroughXHPASTLinterRule '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.", + 'comment to silence this warning.', 'case', 'default', 'break', diff --git a/src/lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php new file mode 100644 index 00000000..70561e41 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistIsAShouldBeInstanceOfXHPASTLinterRule.php @@ -0,0 +1,69 @@ +getFunctionCalls($root, array('is_a')); + + foreach ($calls as $call) { + $parameters = $call->getChildOfType(1, 'n_CALL_PARAMETER_LIST'); + + if (count($parameters->getChildren()) > 2) { + // If the `$allow_string` parameter is `true` then the `instanceof` + // operator cannot be used. Evaluating whether an expression is truthy + // or falsely is hard, and so we only check that the `$allow_string` + // parameter is either absent or literally `false`. + $allow_string = $parameters->getChildByIndex(2); + + if (strtolower($allow_string->getConcreteString()) != 'false') { + continue; + } + } + + $object = $parameters->getChildByIndex(0); + $class = $parameters->getChildByIndex(1); + + switch ($class->getTypeName()) { + case 'n_STRING_SCALAR': + $replacement = stripslashes( + substr($class->getConcreteString(), 1, -1)); + break; + + case 'n_VARIABLE': + $replacement = $class->getConcreteString(); + break; + + default: + $replacement = null; + break; + } + + $this->raiseLintAtNode( + $call, + pht( + 'Use `%s` instead of `%s`. The former is a language '. + 'construct whereas the latter is a function call, which '. + 'has additional overhead.', + 'instanceof', + 'is_a'), + ($replacement === null) + ? null + : sprintf( + '%s instanceof %s', + $object->getConcreteString(), + $replacement)); + } + } + +} 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__/ArcanistCurlyBraceArrayIndexXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistCurlyBraceArrayIndexXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..1426e555 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistCurlyBraceArrayIndexXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/curly-brace-array-index/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..7a463bf6 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistIsAShouldBeInstanceOfXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/is_a-should-be-instanceof/'); + } + +} 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__/brace-formatting/brace-formatting.lint-test b/src/lint/linter/xhpast/rules/__tests__/brace-formatting/brace-formatting.lint-test deleted file mode 100644 index 725773f3..00000000 --- a/src/lint/linter/xhpast/rules/__tests__/brace-formatting/brace-formatting.lint-test +++ /dev/null @@ -1,87 +0,0 @@ -api = $api; - return $this; } private function tokenizeBaseCommitSpecification($raw_spec) { diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php index 8d8181e3..8600674c 100644 --- a/src/repository/api/ArcanistGitAPI.php +++ b/src/repository/api/ArcanistGitAPI.php @@ -1216,8 +1216,20 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { // Ideally, we would use something like "for-each-ref --contains" // to get a filtered list of branches ready for script consumption. // Instead, try to get predictable output from "branch --contains". + + $flags = array(); + $flags[] = '--no-color'; + + // NOTE: The "--no-column" flag was introduced in Git 1.7.11, so + // don't pass it if we're running an older version. See T9953. + $version = $this->getGitVersion(); + if (version_compare($version, '1.7.11', '>=')) { + $flags[] = '--no-column'; + } + list($branches) = $this->execxLocal( - '-c column.ui=never -c color.ui=never branch --contains %s', + 'branch %Ls --contains %s', + $flags, $commit); $branches = array_filter(explode("\n", $branches)); diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php index 33105c20..6fa6c48a 100644 --- a/src/repository/api/ArcanistMercurialAPI.php +++ b/src/repository/api/ArcanistMercurialAPI.php @@ -360,7 +360,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { $working_status = ArcanistMercurialParser::parseMercurialStatus($stdout); foreach ($working_status as $path => $mask) { - if (!($mask & ArcanistRepositoryAPI::FLAG_UNTRACKED)) { + if (!($mask & parent::FLAG_UNTRACKED)) { // Mark tracked files as uncommitted. $mask |= self::FLAG_UNCOMMITTED; } diff --git a/src/repository/api/ArcanistSubversionAPI.php b/src/repository/api/ArcanistSubversionAPI.php index d8420a1b..f5768b14 100644 --- a/src/repository/api/ArcanistSubversionAPI.php +++ b/src/repository/api/ArcanistSubversionAPI.php @@ -139,7 +139,7 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI { $status = $this->svnStatus; if (!$with_externals) { foreach ($status as $path => $mask) { - if ($mask & ArcanistRepositoryAPI::FLAG_EXTERNALS) { + if ($mask & parent::FLAG_EXTERNALS) { unset($status[$path]); } } @@ -384,7 +384,7 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI { $status = $status[$path]; // Build meaningful diff text for "svn copy" operations. - if ($status & ArcanistRepositoryAPI::FLAG_ADDED) { + if ($status & parent::FLAG_ADDED) { $info = $this->getSVNInfo($path); if (!empty($info['Copied From URL'])) { return $this->buildSyntheticAdditionDiff( @@ -403,7 +403,7 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI { if (preg_match('/\.(gif|png|jpe?g|swf|pdf|ico)$/i', $path, $matches)) { // Check if the file is deleted first; SVN will complain if we try to // get properties of a deleted file. - if ($status & ArcanistRepositoryAPI::FLAG_DELETED) { + if ($status & parent::FLAG_DELETED) { return <<length; $ii++) { $line = $lines->item($ii); - $next_line = intval($line->getAttribute('number')); + $next_line = (int)$line->getAttribute('number'); for ($start_line; $start_line < $next_line; $start_line++) { $coverage .= 'N'; } - if (intval($line->getAttribute('hits')) == 0) { + if ((int)$line->getAttribute('hits') == 0) { $coverage .= 'U'; - } else if (intval($line->getAttribute('hits')) > 0) { + } else if ((int)$line->getAttribute('hits') > 0) { $coverage .= 'C'; } diff --git a/src/unit/engine/PytestTestEngine.php b/src/unit/engine/PytestTestEngine.php index 49ef3c48..1ebf98e6 100644 --- a/src/unit/engine/PytestTestEngine.php +++ b/src/unit/engine/PytestTestEngine.php @@ -116,14 +116,14 @@ final class PytestTestEngine extends ArcanistUnitTestEngine { for ($ii = 0; $ii < $lines->length; $ii++) { $line = $lines->item($ii); - $next_line = intval($line->getAttribute('number')); + $next_line = (int)$line->getAttribute('number'); for ($start_line; $start_line < $next_line; $start_line++) { $coverage .= 'N'; } - if (intval($line->getAttribute('hits')) == 0) { + if ((int)$line->getAttribute('hits') == 0) { $coverage .= 'U'; - } else if (intval($line->getAttribute('hits')) > 0) { + } else if ((int)$line->getAttribute('hits') > 0) { $coverage .= 'C'; } diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index aeae0526..b75de9cd 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -531,6 +531,7 @@ EOTEXT $this->updateLintDiffProperty(); $this->updateUnitDiffProperty(); $this->updateLocalDiffProperty(); + $this->updateOntoDiffProperty(); $this->resolveDiffPropertyUpdates(); $output_json = $this->getArgument('json'); @@ -2406,6 +2407,54 @@ EOTEXT $this->updateDiffProperty('local:commits', json_encode($local_info)); } + private function updateOntoDiffProperty() { + $onto = $this->getDiffOntoTargets(); + + if (!$onto) { + return; + } + + $this->updateDiffProperty('arc:onto', json_encode($onto)); + } + + private function getDiffOntoTargets() { + $api = $this->getRepositoryAPI(); + + if (!($api instanceof ArcanistGitAPI)) { + return null; + } + + // If we track an upstream branch either directly or indirectly, use that. + $branch = $api->getBranchName(); + if (strlen($branch)) { + $upstream_path = $api->getPathToUpstream($branch); + $remote_branch = $upstream_path->getRemoteBranchName(); + if (strlen($remote_branch)) { + return array( + array( + 'type' => 'branch', + 'name' => $remote_branch, + 'kind' => 'upstream', + ), + ); + } + } + + // If "arc.land.onto.default" is configured, use that. + $config_key = 'arc.land.onto.default'; + $onto = $this->getConfigFromAnySource($config_key); + if (strlen($onto)) { + return array( + array( + 'type' => 'branch', + 'name' => $onto, + 'kind' => 'arc.land.onto.default', + ), + ); + } + + return null; + } /** * Update an arbitrary diff property. diff --git a/src/workflow/ArcanistFlagWorkflow.php b/src/workflow/ArcanistFlagWorkflow.php index d8420728..9f1367b9 100644 --- a/src/workflow/ArcanistFlagWorkflow.php +++ b/src/workflow/ArcanistFlagWorkflow.php @@ -14,14 +14,30 @@ final class ArcanistFlagWorkflow extends ArcanistWorkflow { ); private static $colorSpec = array( - 'red' => 0, 'r' => 0, 0 => 0, - 'orange' => 1, 'o' => 1, 1 => 1, - 'yellow' => 2, 'y' => 2, 2 => 2, - 'green' => 3, 'g' => 3, 3 => 3, - 'blue' => 4, 'b' => 4, 4 => 4, - 'pink' => 5, 'p' => 5, 5 => 5, - 'purple' => 6, 'v' => 6, 6 => 6, - 'checkered' => 7, 'c' => 7, 7 => 7, + 'red' => 0, + 'r' => 0, + 0 => 0, + 'orange' => 1, + 'o' => 1, + 1 => 1, + 'yellow' => 2, + 'y' => 2, + 2 => 2, + 'green' => 3, + 'g' => 3, + 3 => 3, + 'blue' => 4, + 'b' => 4, + 4 => 4, + 'pink' => 5, + 'p' => 5, + 5 => 5, + 'purple' => 6, + 'v' => 6, + 6 => 6, + 'checkered' => 7, + 'c' => 7, + 7 => 7, ); public function getWorkflowName() { diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index 23b0b7b4..884f3a54 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -87,19 +87,23 @@ EOTEXT With **--preview**, execution stops here, before the change is merged. - The change is merged into the target branch, following these rules: + The change is merged with the changes in the target branch, + following these rules: - In mutable repositories or with **--squash**, this will perform a - squash merge (the entire branch will be represented as one commit on - the target branch). + In repositories with mutable history or with **--squash**, this will + perform a squash merge (the entire branch will be represented as one + commit after the merge). - In immutable repositories or with **--merge**, this will perform a - strict merge (a merge commit will always be created, and local - commits will be preserved). + In repositories with immutable history or with **--merge**, this will + perform a strict merge (a merge commit will always be created, and + local commits will be preserved). The resulting commit will be given an up-to-date commit message describing the final state of the revision in Differential. + In Git, the merge occurs in a detached HEAD. The local branch + reference (if one exists) is not updated yet. + With **--hold**, execution stops here, before the change is pushed. The change is pushed into the remote. diff --git a/src/workflow/ArcanistPatchWorkflow.php b/src/workflow/ArcanistPatchWorkflow.php index 7dd62cf2..e7c6760d 100644 --- a/src/workflow/ArcanistPatchWorkflow.php +++ b/src/workflow/ArcanistPatchWorkflow.php @@ -691,7 +691,7 @@ EOTEXT Filesystem::writeFile($patchfile, $bundle->toGitPatch()); $passthru = new PhutilExecPassthru( - 'git apply --index --reject -- %s', + 'git apply --whitespace nowarn --index --reject -- %s', $patchfile); $passthru->setCWD($repository_api->getPath()); $err = $passthru->execute();