diff --git a/.arcconfig b/.arcconfig index 28823294..66841c44 100644 --- a/.arcconfig +++ b/.arcconfig @@ -1,6 +1,5 @@ { "phabricator.uri": "https://secure.phabricator.com/", - "unit.engine": "PhutilUnitTestEngine", "load": [ "src/" ] diff --git a/.arcunit b/.arcunit new file mode 100644 index 00000000..860ee1ae --- /dev/null +++ b/.arcunit @@ -0,0 +1,8 @@ +{ + "engines": { + "phutil": { + "type": "phutil", + "include": "(\\.php$)" + } + } +} diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4783a8d6..ca5061e4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -15,6 +15,7 @@ phutil_register_library_map(array( 'ArcanistAnoidWorkflow' => 'workflow/ArcanistAnoidWorkflow.php', 'ArcanistArrayIndexSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArrayIndexSpacingXHPASTLinterRule.php', 'ArcanistArraySeparatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArraySeparatorXHPASTLinterRule.php', + 'ArcanistArrayValueXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php', 'ArcanistBackoutWorkflow' => 'workflow/ArcanistBackoutWorkflow.php', 'ArcanistBaseCommitParser' => 'parser/ArcanistBaseCommitParser.php', 'ArcanistBaseCommitParserTestCase' => 'parser/__tests__/ArcanistBaseCommitParserTestCase.php', @@ -57,6 +58,7 @@ phutil_register_library_map(array( 'ArcanistConcatenationOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConcatenationOperatorXHPASTLinterRule.php', 'ArcanistConfiguration' => 'configuration/ArcanistConfiguration.php', 'ArcanistConfigurationDrivenLintEngine' => 'lint/engine/ArcanistConfigurationDrivenLintEngine.php', + 'ArcanistConfigurationDrivenUnitTestEngine' => 'unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php', 'ArcanistConfigurationManager' => 'configuration/ArcanistConfigurationManager.php', 'ArcanistConsoleLintRenderer' => 'lint/renderer/ArcanistConsoleLintRenderer.php', 'ArcanistConstructorParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConstructorParenthesesXHPASTLinterRule.php', @@ -108,6 +110,7 @@ phutil_register_library_map(array( 'ArcanistGeneratedLinterTestCase' => 'lint/linter/__tests__/ArcanistGeneratedLinterTestCase.php', 'ArcanistGetConfigWorkflow' => 'workflow/ArcanistGetConfigWorkflow.php', 'ArcanistGitAPI' => 'repository/api/ArcanistGitAPI.php', + 'ArcanistGlobalVariableXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php', 'ArcanistGoLintLinter' => 'lint/linter/ArcanistGoLintLinter.php', 'ArcanistGoLintLinterTestCase' => 'lint/linter/__tests__/ArcanistGoLintLinterTestCase.php', 'ArcanistGoTestResultParser' => 'unit/parser/ArcanistGoTestResultParser.php', @@ -122,6 +125,7 @@ phutil_register_library_map(array( 'ArcanistImplicitConstructorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitConstructorXHPASTLinterRule.php', 'ArcanistImplicitFallthroughXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php', 'ArcanistImplicitVisibilityXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitVisibilityXHPASTLinterRule.php', + 'ArcanistInlineHTMLXHPASTLinterRule' => 'lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php', 'ArcanistInnerFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInnerFunctionXHPASTLinterRule.php', 'ArcanistInstallCertificateWorkflow' => 'workflow/ArcanistInstallCertificateWorkflow.php', 'ArcanistInstanceOfOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInstanceOfOperatorXHPASTLinterRule.php', @@ -154,6 +158,7 @@ phutil_register_library_map(array( 'ArcanistLinter' => 'lint/linter/ArcanistLinter.php', 'ArcanistLinterTestCase' => 'lint/linter/__tests__/ArcanistLinterTestCase.php', 'ArcanistLintersWorkflow' => 'workflow/ArcanistLintersWorkflow.php', + 'ArcanistListAssignmentXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php', 'ArcanistListWorkflow' => 'workflow/ArcanistListWorkflow.php', 'ArcanistLogicalOperatorsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLogicalOperatorsXHPASTLinterRule.php', 'ArcanistLowercaseFunctionsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLowercaseFunctionsXHPASTLinterRule.php', @@ -180,6 +185,7 @@ phutil_register_library_map(array( 'ArcanistPHPOpenTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPOpenTagXHPASTLinterRule.php', 'ArcanistPHPShortTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPShortTagXHPASTLinterRule.php', 'ArcanistParenthesesSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php', + 'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php', 'ArcanistPasteWorkflow' => 'workflow/ArcanistPasteWorkflow.php', 'ArcanistPatchWorkflow' => 'workflow/ArcanistPatchWorkflow.php', 'ArcanistPhpLinter' => 'lint/linter/ArcanistPhpLinter.php', @@ -245,6 +251,7 @@ phutil_register_library_map(array( 'ArcanistUnitRenderer' => 'unit/renderer/ArcanistUnitRenderer.php', 'ArcanistUnitTestEngine' => 'unit/engine/ArcanistUnitTestEngine.php', 'ArcanistUnitTestResult' => 'unit/ArcanistUnitTestResult.php', + 'ArcanistUnitTestResultTestCase' => 'unit/__tests__/ArcanistUnitTestResultTestCase.php', 'ArcanistUnitTestableLintEngine' => 'lint/engine/ArcanistUnitTestableLintEngine.php', 'ArcanistUnitWorkflow' => 'workflow/ArcanistUnitWorkflow.php', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php', @@ -291,6 +298,7 @@ phutil_register_library_map(array( 'ArcanistAnoidWorkflow' => 'ArcanistWorkflow', 'ArcanistArrayIndexSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistArraySeparatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistArrayValueXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistBackoutWorkflow' => 'ArcanistWorkflow', 'ArcanistBaseCommitParser' => 'Phobject', 'ArcanistBaseCommitParserTestCase' => 'PhutilTestCase', @@ -333,6 +341,7 @@ phutil_register_library_map(array( 'ArcanistConcatenationOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistConfiguration' => 'Phobject', 'ArcanistConfigurationDrivenLintEngine' => 'ArcanistLintEngine', + 'ArcanistConfigurationDrivenUnitTestEngine' => 'ArcanistUnitTestEngine', 'ArcanistConfigurationManager' => 'Phobject', 'ArcanistConsoleLintRenderer' => 'ArcanistLintRenderer', 'ArcanistConstructorParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -384,6 +393,7 @@ phutil_register_library_map(array( 'ArcanistGeneratedLinterTestCase' => 'ArcanistLinterTestCase', 'ArcanistGetConfigWorkflow' => 'ArcanistWorkflow', 'ArcanistGitAPI' => 'ArcanistRepositoryAPI', + 'ArcanistGlobalVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistGoLintLinter' => 'ArcanistExternalLinter', 'ArcanistGoLintLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistGoTestResultParser' => 'ArcanistTestResultParser', @@ -398,6 +408,7 @@ phutil_register_library_map(array( 'ArcanistImplicitConstructorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistImplicitFallthroughXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistImplicitVisibilityXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistInlineHTMLXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistInnerFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistInstallCertificateWorkflow' => 'ArcanistWorkflow', 'ArcanistInstanceOfOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -430,6 +441,7 @@ phutil_register_library_map(array( 'ArcanistLinter' => 'Phobject', 'ArcanistLinterTestCase' => 'PhutilTestCase', 'ArcanistLintersWorkflow' => 'ArcanistWorkflow', + 'ArcanistListAssignmentXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistListWorkflow' => 'ArcanistWorkflow', 'ArcanistLogicalOperatorsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistLowercaseFunctionsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -456,6 +468,7 @@ phutil_register_library_map(array( 'ArcanistPHPOpenTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPHPShortTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistParenthesesSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPasteWorkflow' => 'ArcanistWorkflow', 'ArcanistPatchWorkflow' => 'ArcanistWorkflow', 'ArcanistPhpLinter' => 'ArcanistExternalLinter', @@ -521,6 +534,7 @@ phutil_register_library_map(array( 'ArcanistUnitRenderer' => 'Phobject', 'ArcanistUnitTestEngine' => 'Phobject', 'ArcanistUnitTestResult' => 'Phobject', + 'ArcanistUnitTestResultTestCase' => 'PhutilTestCase', 'ArcanistUnitTestableLintEngine' => 'ArcanistLintEngine', 'ArcanistUnitWorkflow' => 'ArcanistWorkflow', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', diff --git a/src/configuration/ArcanistConfiguration.php b/src/configuration/ArcanistConfiguration.php index 71152587..91d0dc65 100644 --- a/src/configuration/ArcanistConfiguration.php +++ b/src/configuration/ArcanistConfiguration.php @@ -35,28 +35,10 @@ class ArcanistConfiguration extends Phobject { } public function buildAllWorkflows() { - $workflows_by_name = array(); - - $workflows_by_class_name = id(new PhutilSymbolLoader()) + return id(new PhutilClassMapQuery()) ->setAncestorClass('ArcanistWorkflow') - ->loadObjects(); - foreach ($workflows_by_class_name as $class => $workflow) { - $name = $workflow->getWorkflowName(); - - if (isset($workflows_by_name[$name])) { - $other = get_class($workflows_by_name[$name]); - throw new Exception( - pht( - 'Workflows %s and %s both implement workflows named %s.', - $class, - $other, - $name)); - } - - $workflows_by_name[$name] = $workflow; - } - - return $workflows_by_name; + ->setUniqueMethod('getWorkflowName') + ->execute(); } final public function isValidWorkflow($workflow) { diff --git a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php b/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php index 531ccf6c..9d8693af 100644 --- a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php +++ b/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php @@ -137,36 +137,10 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine { } private function loadAvailableLinters() { - $linters = id(new PhutilSymbolLoader()) + return id(new PhutilClassMapQuery()) ->setAncestorClass('ArcanistLinter') - ->loadObjects(); - - $map = array(); - foreach ($linters as $linter) { - $name = $linter->getLinterConfigurationName(); - - // This linter isn't selectable through configuration. - if ($name === null) { - continue; - } - - if (empty($map[$name])) { - $map[$name] = $linter; - continue; - } - - $orig_class = get_class($map[$name]); - $this_class = get_class($linter); - throw new Exception( - pht( - "Two linters (`%s`, `%s`) both have the same configuration ". - "name ('%s'). Linters must have unique configuration names.", - $orig_class, - $this_class, - $name)); - } - - return $map; + ->setUniqueMethod('getLinterConfigurationName', true) + ->execute(); } private function matchPaths( diff --git a/src/lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php b/src/lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php new file mode 100644 index 00000000..3cb758ad --- /dev/null +++ b/src/lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php @@ -0,0 +1,31 @@ +selectTokensOfType('T_INLINE_HTML'); + + foreach ($inline_html as $html) { + if (substr($html->getValue(), 0, 2) == '#!') { + // Ignore shebang lines. + continue; + } + + $this->raiseLintAtToken( + $html, + pht('PHP files must only contain PHP code.')); + } + } + +} diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php index 3a036f2f..985f0950 100644 --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -67,14 +67,21 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { } public function setLinterConfigurationValue($key, $value) { + $matched = false; + foreach ($this->rules as $rule) { foreach ($rule->getLinterConfigurationOptions() as $k => $spec) { if ($k == $key) { - return $rule->setLinterConfigurationValue($key, $value); + $matched = true; + $rule->setLinterConfigurationValue($key, $value); } } } + if ($matched) { + return; + } + return parent::setLinterConfigurationValue($key, $value); } diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php index c023efd1..d4007d2e 100644 --- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php @@ -65,7 +65,7 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { '~~~~~~~~~~')); } - list ($data, $expect, $xform, $config) = array_merge( + list($data, $expect, $xform, $config) = array_merge( $contents, array(null, null)); diff --git a/src/lint/linter/__tests__/jshint/jshint.lint-test b/src/lint/linter/__tests__/jshint/jshint.lint-test index fa6d2249..37c13892 100644 --- a/src/lint/linter/__tests__/jshint/jshint.lint-test +++ b/src/lint/linter/__tests__/jshint/jshint.lint-test @@ -9,3 +9,4 @@ function f() { ~~~~~~~~~~ warning:3:8 error:7:1 +error:9: diff --git a/src/lint/linter/__tests__/xhpast/array-formatting.lint-test b/src/lint/linter/__tests__/xhpast/array-formatting.lint-test new file mode 100644 index 00000000..3dda317d --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/array-formatting.lint-test @@ -0,0 +1,16 @@ + 'bar', 'bar' => 'baz', +); + +array( + 1, /* quack */ 2, /* quack */3, +); + +array( + /* OPEN */ 1, + /* CLOSED */ 2, +); + +array('quack', +); +~~~~~~~~~~ +warning:4:5 +warning:4:8 +warning:8:18 +warning:12:17 +warning:12:32 +warning:20:7 +~~~~~~~~~~ + 'bar', + 'bar' => 'baz', +); + +array( + 1, /* quack */ + 2, /* quack */ + 3, +); + +array( + /* OPEN */ 1, + /* CLOSED */ 2, +); + +array( +'quack', +); diff --git a/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test b/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test index 6aacffdb..0f03a774 100644 --- a/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test +++ b/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test @@ -1,8 +1,8 @@ +~~~~~~~~~~ +disabled:2:1 diff --git a/src/lint/linter/__tests__/xhpast/inner-function.lint-test b/src/lint/linter/__tests__/xhpast/inner-function.lint-test index 38d0ca15..6294eb0a 100644 --- a/src/lint/linter/__tests__/xhpast/inner-function.lint-test +++ b/src/lint/linter/__tests__/xhpast/inner-function.lint-test @@ -8,7 +8,7 @@ function outer() { // Closures are allowed. function my_func($foo) { - function() {}; + function () {}; } ~~~~~~~~~~ warning:5:5 diff --git a/src/lint/linter/__tests__/xhpast/list-assignment.lint-test b/src/lint/linter/__tests__/xhpast/list-assignment.lint-test new file mode 100644 index 00000000..b9871b03 --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/list-assignment.lint-test @@ -0,0 +1,11 @@ +m()[0]; +final class SomeClass extends Phobject { + public function someMethod() { + return function () { + $this->someOtherMethod(); + }; + } + + public static function someStaticMethod() { + return function () { + self::someOtherMethod(); + }; + } +} + ~~~~~~~~~~ error:3:5 error:4:9 +error:9:13 +error:12:7 +error:18:7 ~~~~~~~~~~ ~~~~~~~~~~ { diff --git a/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test b/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test index cbfa13b1..ac65ff57 100644 --- a/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test +++ b/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test @@ -90,13 +90,13 @@ function variable_variable() { function closure1() { $ar = array(); foreach ($ar as &$a) {} - function($a) { + function ($a) { $a++; }; } function closure2() { - function() { + function () { $ar = array(); foreach ($ar as &$a) {} }; @@ -104,7 +104,7 @@ function closure2() { } function closure3() { - function() { + function () { $ar = array(); foreach ($ar as &$a) {} $a++; // Reuse of $a @@ -114,7 +114,7 @@ function closure3() { function closure4() { $ar = array(); foreach ($ar as &$a) {} - function($a) { + function ($a) { unset($a); }; $a++; // Reuse of $a diff --git a/src/lint/linter/__tests__/xhpast/self-member-references-php53.lint-test b/src/lint/linter/__tests__/xhpast/self-member-references-php53.lint-test new file mode 100644 index 00000000..5d06f638 --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/self-member-references-php53.lint-test @@ -0,0 +1,19 @@ +setAncestorClass(__CLASS__) @@ -47,10 +50,29 @@ abstract class ArcanistXHPASTLinterRule extends Phobject { } public function getLinterConfigurationOptions() { - return array(); + return array( + 'xhpast.php-version' => array( + 'type' => 'optional string', + 'help' => pht('PHP version to target.'), + ), + 'xhpast.php-version.windows' => array( + 'type' => 'optional string', + 'help' => pht('PHP version to target on Windows.'), + ), + ); } - public function setLinterConfigurationValue($key, $value) {} + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'xhpast.php-version': + $this->version = $value; + return; + + case 'xhpast.php-version.windows': + $this->windowsVersion = $value; + return; + } + } abstract public function process(XHPASTNode $root); @@ -138,6 +160,28 @@ abstract class ArcanistXHPASTLinterRule extends Phobject { /* -( Utility )------------------------------------------------------------ */ + /** + * Retrieve all anonymous closure(s). + * + * Returns all descendant nodes which represent an anonymous function + * declaration. + * + * @param XHPASTNode Root node. + * @return AASTNodeList + */ + protected function getAnonymousClosures(XHPASTNode $root) { + $func_decls = $root->selectDescendantsOfType('n_FUNCTION_DECLARATION'); + $nodes = array(); + + foreach ($func_decls as $func_decl) { + if ($func_decl->getChildByIndex(2)->getTypeName() == 'n_EMPTY') { + $nodes[] = $func_decl; + } + } + + return AASTNodeList::newFromTreeAndNodes($root->getTree(), $nodes); + } + /** * Retrieve all calls to some specified function(s). * diff --git a/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php new file mode 100644 index 00000000..6b960d74 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php @@ -0,0 +1,56 @@ +selectDescendantsOfType('n_ARRAY_LITERAL'); + + foreach ($arrays as $array) { + $array_values = $array + ->getChildOfType(0, 'n_ARRAY_VALUE_LIST') + ->getChildrenOfType('n_ARRAY_VALUE'); + + if (!$array_values) { + // There is no need to check an empty array. + continue; + } + + $multiline = $array->getLineNumber() != $array->getEndLineNumber(); + + if (!$multiline) { + continue; + } + + foreach ($array_values as $value) { + list($before, $after) = $value->getSurroundingNonsemanticTokens(); + + if (strpos(implode('', mpull($before, 'getValue')), "\n") === false) { + if (last($before) && last($before)->isAnyWhitespace()) { + $token = last($before); + $replacement = "\n".$value->getIndentation(); + } else { + $token = head($value->getTokens()); + $replacement = "\n".$value->getIndentation().$token->getValue(); + } + + $this->raiseLintAtToken( + $token, + pht('Array elements should each occupy a single line.'), + $replacement); + } + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php index 5d6bc2cf..67cb3676 100644 --- a/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php @@ -14,13 +14,33 @@ final class ArcanistCallParenthesesXHPASTLinterRule } public function process(XHPASTNode $root) { - $calls = $root->selectDescendantsOfTypes(array( + $nodes = $root->selectDescendantsOfTypes(array( + 'n_ARRAY_LITERAL', 'n_FUNCTION_CALL', 'n_METHOD_CALL', + 'n_LIST', )); - foreach ($calls as $call) { - $params = $call->getChildOfType(1, 'n_CALL_PARAMETER_LIST'); + foreach ($nodes as $node) { + switch ($node->getTypeName()) { + case 'n_ARRAY_LITERAL': + $params = $node->getChildOfType(0, 'n_ARRAY_VALUE_LIST'); + break; + + case 'n_FUNCTION_CALL': + case 'n_METHOD_CALL': + $params = $node->getChildOfType(1, 'n_CALL_PARAMETER_LIST'); + break; + + case 'n_LIST': + $params = $node->getChildOfType(0, 'n_ASSIGNMENT_LIST'); + break; + + default: + throw new Exception( + pht("Unexpected node of type '%s'!", $node->getTypeName())); + } + $tokens = $params->getTokens(); $first = head($tokens); @@ -29,17 +49,13 @@ final class ArcanistCallParenthesesXHPASTLinterRule if (preg_match('/^\s+$/', $leading_text)) { $this->raiseLintAtOffset( $first->getOffset() - strlen($leading_text), - pht('Convention: no spaces before opening parenthesis in calls.'), + pht('Convention: no spaces before opening parentheses.'), $leading_text, ''); } - } - foreach ($calls as $call) { // If the last parameter of a call is a HEREDOC, don't apply this rule. - $params = $call - ->getChildOfType(1, 'n_CALL_PARAMETER_LIST') - ->getChildren(); + $params = $params->getChildren(); if ($params) { $last_param = last($params); @@ -48,15 +64,19 @@ final class ArcanistCallParenthesesXHPASTLinterRule } } - $tokens = $call->getTokens(); + $tokens = $node->getTokens(); $last = array_pop($tokens); + if ($node->getTypeName() == 'n_ARRAY_LITERAL') { + continue; + } + $trailing = $last->getNonsemanticTokensBefore(); $trailing_text = implode('', mpull($trailing, 'getValue')); if (preg_match('/^\s+$/', $trailing_text)) { $this->raiseLintAtOffset( $last->getOffset() - strlen($trailing_text), - pht('Convention: no spaces before closing parenthesis in calls.'), + pht('Convention: no spaces before closing parentheses.'), $trailing_text, ''); } diff --git a/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php index 357ac77f..6606b0b3 100644 --- a/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php @@ -24,7 +24,7 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule $tokens = $params->getTokens(); $first = head($tokens); - $last = last($tokens); + $last = last($tokens); $leading = $first->getNonsemanticTokensBefore(); $leading_text = implode('', mpull($leading, 'getValue')); @@ -32,14 +32,27 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule $trailing = $last->getNonsemanticTokensBefore(); $trailing_text = implode('', mpull($trailing, 'getValue')); - if (preg_match('/^\s+$/', $leading_text)) { - $this->raiseLintAtOffset( - $first->getOffset() - strlen($leading_text), - pht( - 'Convention: no spaces before opening parenthesis in '. - 'function and method declarations.'), - $leading_text, - ''); + if ($dec->getChildByIndex(2)->getTypeName() == 'n_EMPTY') { + // Anonymous functions. + if ($leading_text != ' ') { + $this->raiseLintAtOffset( + $first->getOffset() - strlen($leading_text), + pht( + 'Convention: space before opening parenthesis in '. + 'anonymous function declarations.'), + $leading_text, + ' '); + } + } else { + if (preg_match('/^\s+$/', $leading_text)) { + $this->raiseLintAtOffset( + $first->getOffset() - strlen($leading_text), + pht( + 'Convention: no spaces before opening parenthesis in '. + 'function and method declarations.'), + $leading_text, + ''); + } } if (preg_match('/^\s+$/', $trailing_text)) { @@ -51,6 +64,33 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule $trailing_text, ''); } + + $use_list = $dec->getChildByIndex(4); + if ($use_list->getTypeName() == 'n_EMPTY') { + continue; + } + $use_token = $use_list->selectTokensOfType('T_USE'); + + foreach ($use_token as $use) { + $before = $use->getNonsemanticTokensBefore(); + $after = $use->getNonsemanticTokensAfter(); + + if (!$before) { + $this->raiseLintAtOffset( + $use->getOffset(), + pht('Convention: space before `%s` token.', 'use'), + '', + ' '); + } + + if (!$after) { + $this->raiseLintAtOffset( + $use->getOffset() + strlen($use->getValue()), + pht('Convention: space after `%s` token.', 'use'), + '', + ' '); + } + } } } diff --git a/src/lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php new file mode 100644 index 00000000..7a68bf17 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php @@ -0,0 +1,28 @@ +selectDescendantsOfType('n_GLOBAL_DECLARATION_LIST'); + + foreach ($nodes as $node) { + $this->raiseLintAtNode( + $node, + pht( + 'Limit the use of global variables. Global variables are '. + 'generally a bad idea and should be avoided when possible.')); + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php new file mode 100644 index 00000000..152c09a6 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php @@ -0,0 +1,38 @@ +selectDescendantsOfType('n_ASSIGNMENT_LIST'); + + foreach ($assignment_lists as $assignment_list) { + $tokens = array_slice($assignment_list->getTokens(), 1, -1); + + foreach (array_reverse($tokens) as $token) { + if ($token->getTypeName() == ',') { + $this->raiseLintAtToken( + $token, + pht('Unnecessary comma in list assignment.'), + ''); + continue; + } + + if ($token->isSemantic()) { + break; + } + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php index 3f118b43..5ee6f3fe 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php @@ -5,41 +5,10 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule const ID = 45; - private $version; - private $windowsVersion; - public function getLintName() { return pht('PHP Compatibility'); } - public function getLinterConfigurationOptions() { - return parent::getLinterConfigurationOptions() + array( - 'xhpast.php-version' => array( - 'type' => 'optional string', - 'help' => pht('PHP version to target.'), - ), - 'xhpast.php-version.windows' => array( - 'type' => 'optional string', - 'help' => pht('PHP version to target on Windows.'), - ), - ); - } - - public function setLinterConfigurationValue($key, $value) { - switch ($key) { - case 'xhpast.php-version': - $this->version = $value; - return; - - case 'xhpast.php-version.windows': - $this->windowsVersion = $value; - return; - - default: - return parent::setLinterConfigurationValue($key, $value); - } - } - public function process(XHPASTNode $root) { static $compat_info; @@ -402,6 +371,52 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule break; } } + + $closures = $this->getAnonymousClosures($root); + foreach ($closures as $closure) { + $static_accesses = $closure + ->selectDescendantsOfType('n_CLASS_STATIC_ACCESS'); + + foreach ($static_accesses as $static_access) { + $class = $static_access->getChildByIndex(0); + + if ($class->getTypeName() != 'n_CLASS_NAME') { + continue; + } + + if (strtolower($class->getConcreteString()) != 'self') { + continue; + } + + $this->raiseLintAtNode( + $class, + pht( + 'The use of `%s` in an anonymous closure is not '. + 'available before PHP 5.4.', + 'self')); + } + + $property_accesses = $closure + ->selectDescendantsOfType('n_OBJECT_PROPERTY_ACCESS'); + foreach ($property_accesses as $property_access) { + $variable = $property_access->getChildByIndex(0); + + if ($variable->getTypeName() != 'n_VARIABLE') { + continue; + } + + if ($variable->getConcreteString() != '$this') { + continue; + } + + $this->raiseLintAtNode( + $variable, + pht( + 'The use of `%s` in an anonymous closure is not '. + 'available before PHP 5.4.', + '$this')); + } + } } private function lintPHP54Incompatibilities(XHPASTNode $root) { diff --git a/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php index b0bd8340..7ad16a44 100644 --- a/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php @@ -15,11 +15,13 @@ final class ArcanistParenthesesSpacingXHPASTLinterRule public function process(XHPASTNode $root) { $all_paren_groups = $root->selectDescendantsOfTypes(array( + 'n_ARRAY_VALUE_LIST', + 'n_ASSIGNMENT_LIST', 'n_CALL_PARAMETER_LIST', + 'n_DECLARATION_PARAMETER_LIST', 'n_CONTROL_CONDITION', 'n_FOR_EXPRESSION', 'n_FOREACH_EXPRESSION', - 'n_DECLARATION_PARAMETER_LIST', )); foreach ($all_paren_groups as $group) { diff --git a/src/lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php new file mode 100644 index 00000000..9656e4a7 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php @@ -0,0 +1,29 @@ +getFunctionCalls($root, array('parse_str')); + + foreach ($calls as $call) { + $call_params = $call->getChildOfType(1, 'n_CALL_PARAMETER_LIST'); + + if (count($call_params->getChildren()) < 2) { + $this->raiseLintAtNode( + $call, + pht( + 'Avoid %s unless the second parameter is specified. '. + 'It is confusing and hinders static analysis.', + 'parse_str()')); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php index 4c3399b7..bfec780e 100644 --- a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php @@ -23,6 +23,7 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule $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 @@ -35,12 +36,23 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule $class_ref_name = $class_ref->getConcreteString(); if (strtolower($class_name) == strtolower($class_ref_name)) { - $this->raiseLintAtNode( - $class_ref, - pht( - 'Use `%s` for local static member references.', - 'self::'), - 'self'); + $in_closure = false; + + foreach ($closures as $closure) { + if ($class_ref->isDescendantOf($closure)) { + $in_closure = true; + break; + } + } + + if (version_compare($this->version, '5.4.0', '>=') || !$in_closure) { + $this->raiseLintAtNode( + $class_ref, + pht( + 'Use `%s` for local static member references.', + 'self::'), + 'self'); + } } static $self_refs = array( diff --git a/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php index c60b216c..05464d41 100644 --- a/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php @@ -48,6 +48,8 @@ final class ArcanistUndeclaredVariableXHPASTLinterRule // // TODO: Support functions defined inside other functions which is commonly // used with anonymous functions. + // + // TODO: parse_str() also makes lexical scope unknowable, see D13857. $defs = $root->selectDescendantsOfTypes(array( 'n_FUNCTION_DECLARATION', diff --git a/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php index 8a4859e5..475ed25f 100644 --- a/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php @@ -26,12 +26,17 @@ final class ArcanistUselessOverridingMethodXHPASTLinterRule $parameters = array(); foreach ($parameter_list->getChildren() as $parameter) { + $default = $parameter->getChildByIndex(2); $parameter = $parameter->getChildByIndex(1); if ($parameter->getTypeName() == 'n_VARIABLE_REFERENCE') { $parameter = $parameter->getChildOfType(0, 'n_VARIABLE'); } + if ($default->getTypeName() != 'n_EMPTY') { + continue 2; + } + $parameters[] = $parameter->getConcreteString(); } diff --git a/src/parser/ArcanistBundle.php b/src/parser/ArcanistBundle.php index a4d85581..8586e281 100644 --- a/src/parser/ArcanistBundle.php +++ b/src/parser/ArcanistBundle.php @@ -819,16 +819,91 @@ final class ArcanistBundle extends Phobject { // additional casting.) static $map = array( - '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', - 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', - 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', - 'U', 'V', 'W', 'X', 'Y', 'Z', - 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', - 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', - 'u', 'v', 'w', 'x', 'y', 'z', - '!', '#', '$', '%', '&', '(', ')', '*', '+', '-', - ';', '<', '=', '>', '?', '@', '^', '_', '`', '{', - '|', '}', '~', + '0', + '1', + '2', + '3', + '4', + '5', + '6', + '7', + '8', + '9', + 'A', + 'B', + 'C', + 'D', + 'E', + 'F', + 'G', + 'H', + 'I', + 'J', + 'K', + 'L', + 'M', + 'N', + 'O', + 'P', + 'Q', + 'R', + 'S', + 'T', + 'U', + 'V', + 'W', + 'X', + 'Y', + 'Z', + 'a', + 'b', + 'c', + 'd', + 'e', + 'f', + 'g', + 'h', + 'i', + 'j', + 'k', + 'l', + 'm', + 'n', + 'o', + 'p', + 'q', + 'r', + 's', + 't', + 'u', + 'v', + 'w', + 'x', + 'y', + 'z', + '!', + '#', + '$', + '%', + '&', + '(', + ')', + '*', + '+', + '-', + ';', + '<', + '=', + '>', + '?', + '@', + '^', + '_', + '`', + '{', + '|', + '}', + '~', ); $buf = ''; diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php index 4abcea04..33105c20 100644 --- a/src/repository/api/ArcanistMercurialAPI.php +++ b/src/repository/api/ArcanistMercurialAPI.php @@ -250,7 +250,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { list($node, $rev, $full_author, $date, $branch, $tag, $parents, $desc) = explode("\1", $log, 9); - list ($author, $author_email) = $this->parseFullAuthor($full_author); + list($author, $author_email) = $this->parseFullAuthor($full_author); // NOTE: If a commit has only one parent, {parents} returns empty. // If it has two parents, {parents} returns revs and short hashes, not @@ -535,7 +535,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { public function supportsRebase() { if ($this->supportsRebase === null) { - list ($err) = $this->execManualLocal('help rebase'); + list($err) = $this->execManualLocal('help rebase'); $this->supportsRebase = $err === 0; } @@ -544,7 +544,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { public function supportsPhases() { if ($this->supportsPhases === null) { - list ($err) = $this->execManualLocal('help phase'); + list($err) = $this->execManualLocal('help phase'); $this->supportsPhases = $err === 0; } diff --git a/src/unit/ArcanistUnitTestResult.php b/src/unit/ArcanistUnitTestResult.php index 039393f8..a0dc2f72 100644 --- a/src/unit/ArcanistUnitTestResult.php +++ b/src/unit/ArcanistUnitTestResult.php @@ -129,13 +129,22 @@ final class ArcanistUnitTestResult extends Phobject { $base = reset($coverage); foreach ($coverage as $more_coverage) { - $len = min(strlen($base), strlen($more_coverage)); + $base_len = strlen($base); + $more_len = strlen($more_coverage); + + $len = min($base_len, $more_len); for ($ii = 0; $ii < $len; $ii++) { if ($more_coverage[$ii] == 'C') { $base[$ii] = 'C'; } } + + // If a secondary report has more data, copy all of it over. + if ($more_len > $base_len) { + $base .= substr($more_coverage, $base_len); + } } + return $base; } diff --git a/src/unit/__tests__/ArcanistUnitTestResultTestCase.php b/src/unit/__tests__/ArcanistUnitTestResultTestCase.php new file mode 100644 index 00000000..7c4d43dd --- /dev/null +++ b/src/unit/__tests__/ArcanistUnitTestResultTestCase.php @@ -0,0 +1,43 @@ + array(), + 'expect' => null, + ), + array( + 'coverage' => array( + 'UUUNCNC', + ), + 'expect' => 'UUUNCNC', + ), + array( + 'coverage' => array( + 'UUCUUU', + 'UUUUCU', + ), + 'expect' => 'UUCUCU', + ), + array( + 'coverage' => array( + 'UUCCCU', + 'UUUCCCNNNC', + ), + 'expect' => 'UUCCCCNNNC', + ), + ); + + foreach ($cases as $case) { + $input = $case['coverage']; + $expect = $case['expect']; + + $actual = ArcanistUnitTestResult::mergeCoverage($input); + + $this->assertEqual($expect, $actual); + } + } + +} diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php new file mode 100644 index 00000000..e24b4d5f --- /dev/null +++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php @@ -0,0 +1,202 @@ +buildTestEngines(); + + foreach ($engines as $engine) { + if ($engine->supportsRunAllTests()) { + return true; + } + } + + return false; + } + + public function buildTestEngines() { + $working_copy = $this->getWorkingCopy(); + $config_path = $working_copy->getProjectPath('.arcunit'); + + if (!Filesystem::pathExists($config_path)) { + throw new ArcanistUsageException( + pht( + "Unable to find '%s' file to configure test engines. Create an ". + "'%s' file in the root directory of the working copy.", + '.arcunit', + '.arcunit')); + } + + $data = Filesystem::readFile($config_path); + $config = null; + try { + $config = phutil_json_decode($data); + } catch (PhutilJSONParserException $ex) { + throw new PhutilProxyException( + pht( + "Expected '%s' file to be a valid JSON file, but ". + "failed to decode '%s'.", + '.arcunit', + $config_path), + $ex); + } + + $test_engines = $this->loadAvailableTestEngines(); + + try { + PhutilTypeSpec::checkMap( + $config, + array( + 'engines' => 'map>', + )); + } catch (PhutilTypeCheckException $ex) { + throw new PhutilProxyException( + pht("Error in parsing '%s' file.", $config_path), + $ex); + } + + $built_test_engines = array(); + $all_paths = $this->getPaths(); + + foreach ($config['engines'] as $name => $spec) { + $type = idx($spec, 'type'); + + if ($type !== null) { + if (empty($test_engines[$type])) { + throw new ArcanistUsageException( + pht( + "Test engine '%s' specifies invalid type '%s'. ". + "Available test engines are: %s.", + $name, + $type, + implode(', ', array_keys($test_engines)))); + } + + $test_engine = clone $test_engines[$type]; + } else { + // We'll raise an error below about the invalid "type" key. + // TODO: Can we just do the type check first, and simplify this a bit? + $test_engine = null; + } + + try { + PhutilTypeSpec::checkMap( + $spec, + array( + 'type' => 'string', + 'include' => 'optional regex | list', + 'exclude' => 'optional regex | list', + )); + } catch (PhutilTypeCheckException $ex) { + throw new PhutilProxyException( + pht( + "Error in parsing '%s' file, for test engine '%s'.", + '.arcunit', + $name), + $ex); + } + + if ($all_paths) { + $include = (array)idx($spec, 'include', array()); + $exclude = (array)idx($spec, 'exclude', array()); + $paths = $this->matchPaths( + $all_paths, + $include, + $exclude); + + $test_engine->setPaths($paths); + } + + $built_test_engines[] = $test_engine; + } + + return $built_test_engines; + } + + public function run() { + $paths = $this->getPaths(); + + // If we are running with `--everything` then `$paths` will be `null`. + if (!$paths) { + $paths = array(); + } + + $engines = $this->buildTestEngines(); + $results = array(); + $exceptions = array(); + + foreach ($engines as $engine) { + $engine + ->setWorkingCopy($this->getWorkingCopy()) + ->setEnableCoverage($this->getEnableCoverage()); + + // TODO: At some point, maybe we should emit a warning here if an engine + // doesn't support `--everything`, to reduce surprise when `--everything` + // does not really mean `--everything`. + if ($engine->supportsRunAllTests()) { + $engine->setRunAllTests($this->getRunAllTests()); + } + + try { + // TODO: Type check the results. + $results[] = $engine->run(); + } catch (ArcanistNoEffectException $ex) { + $exceptions[] = $ex; + } + } + + if (!$results) { + // If all engines throw an `ArcanistNoEffectException`, then we should + // preserve this behavior. + throw new ArcanistNoEffectException(pht('No tests to run.')); + } + + return array_mergev($results); + } + + private function loadAvailableTestEngines() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass('ArcanistUnitTestEngine') + ->setUniqueMethod('getEngineConfigurationName', true) + ->execute(); + } + + /** + * TODO: This is copied from @{class:ArcanistConfigurationDrivenLintEngine}. + */ + private function matchPaths(array $paths, array $include, array $exclude) { + $match = array(); + + foreach ($paths as $path) { + $keep = false; + if (!$include) { + $keep = true; + } else { + foreach ($include as $rule) { + if (preg_match($rule, $path)) { + $keep = true; + break; + } + } + } + + if (!$keep) { + continue; + } + + if ($exclude) { + foreach ($exclude as $rule) { + if (preg_match($rule, $path)) { + continue 2; + } + } + } + + $match[] = $path; + } + + return $match; + } + +} diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php index 9783ce26..ad66bb90 100644 --- a/src/unit/engine/ArcanistUnitTestEngine.php +++ b/src/unit/engine/ArcanistUnitTestEngine.php @@ -16,6 +16,10 @@ abstract class ArcanistUnitTestEngine extends Phobject { final public function __construct() {} + public function getEngineConfigurationName() { + return null; + } + final public function setRunAllTests($run_all_tests) { if (!$this->supportsRunAllTests() && $run_all_tests) { throw new Exception( diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php index 7509b300..110f8e35 100644 --- a/src/unit/engine/PhutilUnitTestEngine.php +++ b/src/unit/engine/PhutilUnitTestEngine.php @@ -5,6 +5,10 @@ */ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine { + public function getEngineConfigurationName() { + return 'phutil'; + } + protected function supportsRunAllTests() { return true; } diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php index f1578c37..27db427d 100644 --- a/src/workflow/ArcanistLintWorkflow.php +++ b/src/workflow/ArcanistLintWorkflow.php @@ -526,7 +526,7 @@ EOTEXT $prompt = pht( 'Apply this patch to %s?', phutil_console_format('__%s__', $result->getPath())); - if (!$console->confirm($prompt, $default_no = false)) { + if (!$console->confirm($prompt, $default = true)) { continue; } } diff --git a/src/workflow/ArcanistLintersWorkflow.php b/src/workflow/ArcanistLintersWorkflow.php index b872e300..6d70b4d6 100644 --- a/src/workflow/ArcanistLintersWorkflow.php +++ b/src/workflow/ArcanistLintersWorkflow.php @@ -36,9 +36,9 @@ EOTEXT public function run() { $console = PhutilConsole::getConsole(); - $linters = id(new PhutilSymbolLoader()) + $linters = id(new PhutilClassMapQuery()) ->setAncestorClass('ArcanistLinter') - ->loadObjects(); + ->execute(); try { $built = $this->newLintEngine()->buildLinters(); diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php index 567b772e..a97bbdeb 100644 --- a/src/workflow/ArcanistUnitWorkflow.php +++ b/src/workflow/ArcanistUnitWorkflow.php @@ -113,21 +113,8 @@ EOTEXT } public function run() { - $working_copy = $this->getWorkingCopy(); - $engine_class = $this->getArgument( - 'engine', - $this->getConfigurationManager()->getConfigFromAnySource('unit.engine')); - - if (!$engine_class) { - throw new ArcanistNoEngineException( - pht( - 'No unit test engine is configured for this project. Edit %s '. - 'to specify a unit test engine.', - '.arcconfig')); - } - $paths = $this->getArgument('paths'); $rev = $this->getArgument('rev'); $everything = $this->getArgument('everything'); @@ -145,18 +132,7 @@ EOTEXT $paths = $this->selectPathsForWorkflow($paths, $rev); } - if (!class_exists($engine_class) || - !is_subclass_of($engine_class, 'ArcanistUnitTestEngine')) { - throw new ArcanistUsageException( - pht( - "Configured unit test engine '%s' is not a subclass of '%s'.", - $engine_class, - 'ArcanistUnitTestEngine')); - } - - $this->engine = newv($engine_class, array()); - $this->engine->setWorkingCopy($working_copy); - $this->engine->setConfigurationManager($this->getConfigurationManager()); + $this->engine = $this->newUnitTestEngine($this->getArgument('engine')); if ($everything) { $this->engine->setRunAllTests(true); } else { diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index dc4913e2..87b37605 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -1878,6 +1878,59 @@ abstract class ArcanistWorkflow extends Phobject { return $engine; } + /** + * Build a new unit test engine for the current working copy. + * + * Optionally, you can pass an explicit engine class name to build an engine + * of a particular class. Normally this is used to implement an `--engine` + * flag from the CLI. + * + * @param string Optional explicit engine class name. + * @return ArcanistUnitTestEngine Constructed engine. + */ + protected function newUnitTestEngine($engine_class = null) { + $working_copy = $this->getWorkingCopy(); + $config = $this->getConfigurationManager(); + + if (!$engine_class) { + $engine_class = $config->getConfigFromAnySource('unit.engine'); + } + + if (!$engine_class) { + if (Filesystem::pathExists($working_copy->getProjectPath('.arcunit'))) { + $engine_class = 'ArcanistConfigurationDrivenUnitTestEngine'; + } + } + + if (!$engine_class) { + throw new ArcanistNoEngineException( + pht( + "No unit test engine is configured for this project. Create an ". + "'%s' file, or configure an advanced engine with '%s' in '%s'.", + '.arcunit', + 'unit.engine', + '.arcconfig')); + } + + $base_class = 'ArcanistUnitTestEngine'; + if (!class_exists($engine_class) || + !is_subclass_of($engine_class, $base_class)) { + throw new ArcanistUsageException( + pht( + 'Configured unit test engine "%s" is not a subclass of "%s", '. + 'but must be.', + $engine_class, + $base_class)); + } + + $engine = newv($engine_class, array()) + ->setWorkingCopy($working_copy) + ->setConfigurationManager($config); + + return $engine; + } + + protected function openURIsInBrowser(array $uris) { $browser = $this->getBrowserCommand(); foreach ($uris as $uri) {