diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ebd21156..a7ca5c09 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -104,6 +104,7 @@ phutil_register_library_map(array( 'ArcanistLintWorkflow' => 'workflow/ArcanistLintWorkflow.php', 'ArcanistLinter' => 'lint/linter/ArcanistLinter.php', 'ArcanistLinterTestCase' => 'lint/linter/__tests__/ArcanistLinterTestCase.php', + 'ArcanistLintersWorkflow' => 'workflow/ArcanistLintersWorkflow.php', 'ArcanistListWorkflow' => 'workflow/ArcanistListWorkflow.php', 'ArcanistMarkCommittedWorkflow' => 'workflow/ArcanistMarkCommittedWorkflow.php', 'ArcanistMercurialAPI' => 'repository/api/ArcanistMercurialAPI.php', @@ -266,6 +267,7 @@ phutil_register_library_map(array( 'ArcanistLintSummaryRenderer' => 'ArcanistLintRenderer', 'ArcanistLintWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistLinterTestCase' => 'ArcanistPhutilTestCase', + 'ArcanistLintersWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistListWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistMarkCommittedWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistMercurialAPI' => 'ArcanistRepositoryAPI', diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php index a858a54e..fe05f0c5 100644 --- a/src/lint/linter/ArcanistExternalLinter.php +++ b/src/lint/linter/ArcanistExternalLinter.php @@ -381,10 +381,6 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter { } } - public function getVersion() { - return null; - } - protected function buildFutures(array $paths) { $executable = $this->getExecutableCommand(); diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php index fd8700af..caae477d 100644 --- a/src/lint/linter/ArcanistLinter.php +++ b/src/lint/linter/ArcanistLinter.php @@ -3,7 +3,8 @@ /** * Implements lint rules, like syntax checks for a specific language. * - * @group linter + * @task info Human Readable Information + * * @stable */ abstract class ArcanistLinter { @@ -25,6 +26,54 @@ abstract class ArcanistLinter { private $customSeverityRules = array(); private $config = array(); + +/* -( Human Readable Information )---------------------------------------- */ + + + /** + * Return an optional informative URI where humans can learn more about this + * linter. + * + * For most linters, this should return a link to the project home page. This + * is shown on `arc linters`. + * + * @return string|null Optionally, return an informative URI. + * @task info + */ + public function getInfoURI() { + return null; + } + + + /** + * Return a brief human-readable description of the linter. + * + * These should be a line or two, and are shown on `arc linters`. + * + * @return string|null Optionally, return a brief human-readable description. + * @task info + */ + public function getInfoDescription() { + return null; + } + + + /** + * Return a human-readable linter name. + * + * These are used by `arc linters`, and can let you give a linter a more + * presentable name. + * + * @return string Human-readable linter name. + * @task info + */ + public function getInfoName() { + return nonempty( + $this->getLinterName(), + $this->getLinterConfigurationName(), + get_class($this)); + } + public function getLinterPriority() { return 1.0; } @@ -268,6 +317,10 @@ abstract class ArcanistLinter { abstract public function lintPath($path); abstract public function getLinterName(); + public function getVersion() { + return null; + } + public function didRunLinters() { // This is a hook. } diff --git a/src/lint/linter/ArcanistNoLintLinter.php b/src/lint/linter/ArcanistNoLintLinter.php index 04fac3d3..67ef6922 100644 --- a/src/lint/linter/ArcanistNoLintLinter.php +++ b/src/lint/linter/ArcanistNoLintLinter.php @@ -1,13 +1,21 @@ severity = $severity; $this->wholeWordRules = ArcanistSpellingDefaultData::getFullWordRules(); diff --git a/src/lint/linter/ArcanistTextLinter.php b/src/lint/linter/ArcanistTextLinter.php index 3b6ee659..1599a750 100644 --- a/src/lint/linter/ArcanistTextLinter.php +++ b/src/lint/linter/ArcanistTextLinter.php @@ -2,8 +2,6 @@ /** * Enforces basic text file rules. - * - * @group linter */ final class ArcanistTextLinter extends ArcanistLinter { @@ -19,6 +17,16 @@ final class ArcanistTextLinter extends ArcanistLinter { private $maxLineLength = 80; + public function getInfoName() { + return pht('Basic Text Linter'); + } + + public function getInfoDescription() { + return pht( + 'Enforces basic text rules like line length, character encoding, '. + 'and trailing whitespace.'); + } + public function getLinterPriority() { return 0.5; } diff --git a/src/lint/linter/ArcanistXMLLinter.php b/src/lint/linter/ArcanistXMLLinter.php index 4ce1a1da..59e05feb 100644 --- a/src/lint/linter/ArcanistXMLLinter.php +++ b/src/lint/linter/ArcanistXMLLinter.php @@ -5,6 +5,15 @@ * errors and potential problems in XML files. */ final class ArcanistXMLLinter extends ArcanistLinter { + + public function getInfoName() { + return pht('SimpleXML Linter'); + } + + public function getInfoDescription() { + return pht('Uses SimpleXML to detect formatting errors in XML files.'); + } + public function getLinterName() { return 'XML'; } diff --git a/src/workflow/ArcanistBaseWorkflow.php b/src/workflow/ArcanistBaseWorkflow.php index e4fd4e2f..9ebe9d20 100644 --- a/src/workflow/ArcanistBaseWorkflow.php +++ b/src/workflow/ArcanistBaseWorkflow.php @@ -1741,4 +1741,54 @@ abstract class ArcanistBaseWorkflow extends Phobject { } + /** + * Build a new lint 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 ArcanistLintEngine Constructed engine. + */ + protected function newLintEngine($engine_class = null) { + $working_copy = $this->getWorkingCopy(); + $config = $this->getConfigurationManager(); + + if (!$engine_class) { + $engine_class = $config->getConfigFromAnySource('lint.engine'); + } + + if (!$engine_class) { + if (Filesystem::pathExists($working_copy->getProjectPath('.arclint'))) { + $engine_class = 'ArcanistConfigurationDrivenLintEngine'; + } + } + + if (!$engine_class) { + throw new ArcanistNoEngineException( + pht( + "No lint engine is configured for this project. ". + "Create an '.arclint' file, or configure an advanced engine ". + "with 'lint.engine' in '.arcconfig'.")); + } + + $base_class = 'ArcanistLintEngine'; + if (!class_exists($engine_class) || + !is_subclass_of($engine_class, $base_class)) { + throw new ArcanistUsageException( + pht( + 'Configured lint 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; + } + } diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php index de8266c2..df515585 100644 --- a/src/workflow/ArcanistLintWorkflow.php +++ b/src/workflow/ArcanistLintWorkflow.php @@ -184,22 +184,7 @@ EOTEXT $working_copy = $this->getWorkingCopy(); $configuration_manager = $this->getConfigurationManager(); - $engine = $this->getArgument('engine'); - if (!$engine) { - $engine = $configuration_manager->getConfigFromAnySource('lint.engine'); - } - - if (!$engine) { - if (Filesystem::pathExists($working_copy->getProjectPath('.arclint'))) { - $engine = 'ArcanistConfigurationDrivenLintEngine'; - } - } - - if (!$engine) { - throw new ArcanistNoEngineException( - "No lint engine configured for this project. Edit '.arcconfig' to ". - "specify a lint engine, or create an '.arclint' file."); - } + $engine = $this->newLintEngine($this->getArgument('engine')); $rev = $this->getArgument('rev'); $paths = $this->getArgument('paths'); @@ -252,17 +237,8 @@ EOTEXT $paths = $this->selectPathsForWorkflow($paths, $rev); } - if (!class_exists($engine) || - !is_subclass_of($engine, 'ArcanistLintEngine')) { - throw new ArcanistUsageException( - "Configured lint engine '{$engine}' is not a subclass of ". - "'ArcanistLintEngine'."); - } - - $engine = newv($engine, array()); $this->engine = $engine; - $engine->setWorkingCopy($working_copy); - $engine->setConfigurationManager($configuration_manager); + $engine->setMinimumSeverity( $this->getArgument('severity', self::DEFAULT_SEVERITY)); diff --git a/src/workflow/ArcanistLintersWorkflow.php b/src/workflow/ArcanistLintersWorkflow.php new file mode 100644 index 00000000..d96488c3 --- /dev/null +++ b/src/workflow/ArcanistLintersWorkflow.php @@ -0,0 +1,176 @@ +setAncestorClass('ArcanistLinter') + ->loadObjects(); + + try { + $built = $this->newLintEngine()->buildLinters(); + } catch (ArcanistNoEngineException $ex) { + $built = $engine->buildLinters(); + } + + // Note that an engine can emit multiple linters of the same class to run + // different rulesets on different groups of files, so these linters do not + // necessarily have unique classes or types. + $groups = array(); + foreach ($built as $linter) { + $groups[get_class($linter)][] = $linter; + } + + $linter_info = array(); + foreach ($linters as $key => $linter) { + $installed = idx($groups, $key, array()); + $exception = null; + + if ($installed) { + $status = 'configured'; + try { + $version = head($installed)->getVersion(); + } catch (Exception $ex) { + $status = 'error'; + $exception = $ex; + } + } else { + $status = 'available'; + $version = null; + } + + $linter_info[$key] = array( + 'short' => $linter->getLinterConfigurationName(), + 'class' => get_class($linter), + 'status' => $status, + 'version' => $version, + 'name' => $linter->getInfoName(), + 'uri' => $linter->getInfoURI(), + 'description' => $linter->getInfoDescription(), + 'exception' => $exception, + ); + } + + $linter_info = isort($linter_info, 'short'); + + $status_map = $this->getStatusMap(); + $pad = ' '; + + $color_map = array( + 'configured' => 'green', + 'available' => 'yellow', + 'error' => 'red', + ); + + foreach ($linter_info as $key => $linter) { + $status = $linter['status']; + $color = $color_map[$status]; + $text = $status_map[$status]; + $print_tail = false; + + $console->writeOut( + "** %s ** **%s** (%s)\n", + $text, + nonempty($linter['short'], '-'), + $linter['name']); + + if ($linter['exception']) { + $console->writeOut( + "\n%s**%s**\n%s\n", + $pad, + get_class($linter['exception']), + phutil_console_wrap( + $linter['exception']->getMessage(), + strlen($pad))); + $print_tail = true; + } + + $version = $linter['version']; + $uri = $linter['uri']; + if ($version || $uri) { + $console->writeOut("\n"); + $print_tail = true; + } + + if ($version) { + $console->writeOut("%s%s **%s**\n", $pad, pht('Version'), $version); + } + + if ($uri) { + $console->writeOut("%s__%s__\n", $pad, $linter['uri']); + } + + $description = $linter['description']; + if ($description) { + $console->writeOut( + "\n%s\n", + phutil_console_wrap($linter['description'], strlen($pad))); + $print_tail = true; + } + + if ($print_tail) { + $console->writeOut("\n"); + } + } + } + + + /** + * Get human-readable linter statuses, padded to fixed width. + * + * @return map Human-readable linter status names. + */ + private function getStatusMap() { + $text_map = array( + 'configured' => pht('CONFIGURED'), + 'available' => pht('AVAILABLE'), + 'error' => pht('ERROR'), + ); + + $sizes = array(); + foreach ($text_map as $key => $string) { + $sizes[$key] = phutil_utf8_console_strlen($string); + } + + $longest = max($sizes); + foreach ($text_map as $key => $string) { + if ($sizes[$key] < $longest) { + $text_map[$key] .= str_repeat(' ', $longest - $sizes[$key]); + } + } + + $text_map['padding'] = str_repeat(' ', $longest); + + return $text_map; + } + +}