diff --git a/src/lint/linter/__tests__/phlxhp/array-formatting.lint-test b/src/lint/linter/__tests__/phlxhp/array-formatting.lint-test new file mode 100644 index 00000000..3dda317d --- /dev/null +++ b/src/lint/linter/__tests__/phlxhp/array-formatting.lint-test @@ -0,0 +1,16 @@ +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); $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.'), + 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); if ($last_param->getTypeName() === 'n_HEREDOC') { continue; } } - $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/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 @@ -1,69 +1,71 @@ 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) { $tokens = $group->getTokens(); $token_o = array_shift($tokens); $token_c = array_pop($tokens); if ($token_o->getTypeName() !== '(') { throw new Exception(pht('Expected open parentheses.')); } if ($token_c->getTypeName() !== ')') { throw new Exception(pht('Expected close parentheses.')); } $nonsem_o = $token_o->getNonsemanticTokensAfter(); $nonsem_c = $token_c->getNonsemanticTokensBefore(); if (!$nonsem_o) { continue; } $raise = array(); $string_o = implode('', mpull($nonsem_o, 'getValue')); if (preg_match('/^[ ]+$/', $string_o)) { $raise[] = array($nonsem_o, $string_o); } if ($nonsem_o !== $nonsem_c) { $string_c = implode('', mpull($nonsem_c, 'getValue')); if (preg_match('/^[ ]+$/', $string_c)) { $raise[] = array($nonsem_c, $string_c); } } foreach ($raise as $warning) { list($tokens, $string) = $warning; $this->raiseLintAtOffset( reset($tokens)->getOffset(), pht('Parentheses should hug their contents.'), $string, ''); } } } } diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php index 9c5ed8ab..e24b4d5f 100644 --- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php +++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php @@ -1,199 +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); } - $include = (array)idx($spec, 'include', array()); - $exclude = (array)idx($spec, 'exclude', array()); - $paths = $this->matchPaths( - $all_paths, - $include, - $exclude); - - $test_engine->setPaths($paths); + 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; } }