diff --git a/arcanist/.phutil_module_cache b/arcanist/.phutil_module_cache --- a/arcanist/.phutil_module_cache +++ b/arcanist/.phutil_module_cache @@ -1 +1 @@ -{"__symbol_cache_version__":11,"90a8b110dc475955f15bb81d37268cb5":{"have":{"class":{"AutoPEP8FormatLinter":75}},"need":{"function":{"pht":297,"execx":769,"id":1903},"class":{"ArcanistExternalLinter":104,"ArcanistLintMessage":1910,"Filesystem":1754,"ArcanistLinter":2017,"ArcanistLintSeverity":2095}},"xmap":{"AutoPEP8FormatLinter":["ArcanistExternalLinter"]}},"38f0c676bff5192a344464142caaa253":{"have":{"class":{"CHeaderLinter":99}},"need":{"function":{"pht":611},"class":{"ArcanistLinter":121,"ArcanistLintSeverity":1060,"Filesystem":1307}},"xmap":{"CHeaderLinter":["ArcanistLinter"]}},"a30e4e25376ca05d4ae719915441be9e":{"have":{"class":{"CheckDocLinter":106}},"need":{"function":{"pht":323,"id":1848},"class":{"ArcanistExternalLinter":129,"ArcanistLintMessage":1855,"Filesystem":731,"ArcanistLinter":1903,"ArcanistLintSeverity":1989}},"xmap":{"CheckDocLinter":["ArcanistExternalLinter"]}},"6af7410cfea496ff1d4dcc2624b6b8ea":{"have":{"class":{"ClangFormatLinter":79}},"need":{"function":{"pht":302,"execx":781,"id":1653},"class":{"ArcanistExternalLinter":105,"ArcanistLintMessage":1660,"Filesystem":1504,"ArcanistLinter":1767,"ArcanistLintSeverity":1845}},"xmap":{"ClangFormatLinter":["ArcanistExternalLinter"]}},"9285ad9415f8ebe564f7119e5a72c559":{"have":{"class":{"FormatStringLinter":146}},"need":{"function":{"pht":377,"csprintf":1492,"id":1872},"class":{"ArcanistExternalLinter":173,"ArcanistLintMessage":1879,"Filesystem":827,"ArcanistLinter":1956,"ArcanistLintSeverity":2044}},"xmap":{"FormatStringLinter":["ArcanistExternalLinter"]}},"224d394856b17878058b4c14acb7178b":{"have":{"class":{"LocaleDependenceLinter":160}},"need":{"function":{"pht":5400},"class":{"ArcanistLinter":191,"ArcanistLintSeverity":5903,"Filesystem":6149}},"xmap":{"LocaleDependenceLinter":["ArcanistLinter"]}},"6f2f22dd0f259fb2eaa284b4fab3bc29":{"have":{"class":{"PythonFormatLinter":123}},"need":{"function":{"pht":353,"id":1838},"class":{"ArcanistExternalLinter":150,"ArcanistLintMessage":1845,"Filesystem":776,"ArcanistLinter":1970,"ArcanistLintSeverity":2053}},"xmap":{"PythonFormatLinter":["ArcanistExternalLinter"]}},"25781df78f6eebfb223296b8265e9d19":{"have":{"class":{"TestsLinter":103}},"need":{"function":{"pht":318,"id":2629},"class":{"ArcanistExternalLinter":123,"ArcanistLintMessage":2636,"Filesystem":776,"ArcanistLinter":2684,"ArcanistLintSeverity":2792}},"xmap":{"TestsLinter":["ArcanistExternalLinter"]}},"2809b09d2021203b43c57da33d1fe8bf":{"have":{"class":{"AssertWithSideEffectsLinter":210}},"need":{"function":{"pht":439},"class":{"ArcanistLinter":246,"ArcanistLintSeverity":926,"Filesystem":1170}},"xmap":{"AssertWithSideEffectsLinter":["ArcanistLinter"]}}} \ No newline at end of file +{"__symbol_cache_version__":11,"553814ef471cbdbb918ad5323661a3cd":{"have":{"class":{"ArcanistGlobalExternalLinter":665}},"need":{"function":{"coalesce":4481,"pht":7789,"csprintf":11178,"nonempty":11586},"class":{"ArcanistGlobalLinter":702,"PhutilMethodNotImplementedException":5438,"ArcanistMissingLinterException":7747,"ExecFuture":12269,"Filesystem":7687},"class\/interface":{"Future":12406,"ArcanistMissingLinterException":11744}},"xmap":{"ArcanistGlobalExternalLinter":["ArcanistGlobalLinter"]}},"2809b09d2021203b43c57da33d1fe8bf":{"have":{"class":{"AssertWithSideEffectsLinter":210}},"need":{"function":{"pht":439},"class":{"ArcanistLinter":246,"ArcanistLintSeverity":926,"Filesystem":1170}},"xmap":{"AssertWithSideEffectsLinter":["ArcanistLinter"]}},"90a8b110dc475955f15bb81d37268cb5":{"have":{"class":{"AutoPEP8FormatLinter":75}},"need":{"function":{"pht":297,"execx":769,"id":1903},"class":{"ArcanistExternalLinter":104,"ArcanistLintMessage":1910,"Filesystem":1754,"ArcanistLinter":2017,"ArcanistLintSeverity":2095}},"xmap":{"AutoPEP8FormatLinter":["ArcanistExternalLinter"]}},"38f0c676bff5192a344464142caaa253":{"have":{"class":{"CHeaderLinter":99}},"need":{"function":{"pht":611},"class":{"ArcanistLinter":121,"ArcanistLintSeverity":1060,"Filesystem":1307}},"xmap":{"CHeaderLinter":["ArcanistLinter"]}},"a430e0a2286be956cc4e9b5a5f4a9f71":{"have":{"class":{"CheckDocLinter":106}},"need":{"function":{"pht":329,"id":1847},"class":{"ArcanistGlobalExternalLinter":129,"ArcanistLintMessage":1854,"Filesystem":737,"ArcanistLinter":1902,"ArcanistLintSeverity":1988}},"xmap":{"CheckDocLinter":["ArcanistGlobalExternalLinter"]}},"6af7410cfea496ff1d4dcc2624b6b8ea":{"have":{"class":{"ClangFormatLinter":79}},"need":{"function":{"pht":302,"execx":781,"id":1653},"class":{"ArcanistExternalLinter":105,"ArcanistLintMessage":1660,"Filesystem":1504,"ArcanistLinter":1767,"ArcanistLintSeverity":1845}},"xmap":{"ClangFormatLinter":["ArcanistExternalLinter"]}},"9285ad9415f8ebe564f7119e5a72c559":{"have":{"class":{"FormatStringLinter":146}},"need":{"function":{"pht":377,"csprintf":1492,"id":1872},"class":{"ArcanistExternalLinter":173,"ArcanistLintMessage":1879,"Filesystem":827,"ArcanistLinter":1956,"ArcanistLintSeverity":2044}},"xmap":{"FormatStringLinter":["ArcanistExternalLinter"]}},"224d394856b17878058b4c14acb7178b":{"have":{"class":{"LocaleDependenceLinter":160}},"need":{"function":{"pht":5400},"class":{"ArcanistLinter":191,"ArcanistLintSeverity":5903,"Filesystem":6149}},"xmap":{"LocaleDependenceLinter":["ArcanistLinter"]}},"6f2f22dd0f259fb2eaa284b4fab3bc29":{"have":{"class":{"PythonFormatLinter":123}},"need":{"function":{"pht":353,"id":1838},"class":{"ArcanistExternalLinter":150,"ArcanistLintMessage":1845,"Filesystem":776,"ArcanistLinter":1970,"ArcanistLintSeverity":2053}},"xmap":{"PythonFormatLinter":["ArcanistExternalLinter"]}},"25781df78f6eebfb223296b8265e9d19":{"have":{"class":{"TestsLinter":103}},"need":{"function":{"pht":318,"id":2629},"class":{"ArcanistExternalLinter":123,"ArcanistLintMessage":2636,"Filesystem":776,"ArcanistLinter":2684,"ArcanistLintSeverity":2792}},"xmap":{"TestsLinter":["ArcanistExternalLinter"]}},"4e6945b31e189eab971e13c7129a1024":{"have":{"class":{"ArcanistGlobalLinter":88}},"need":{"class":{"ArcanistLinter":117},"class\/interface":{"Future":288}},"xmap":{"ArcanistGlobalLinter":["ArcanistLinter"]}}} \ No newline at end of file diff --git a/arcanist/__phutil_library_map__.php b/arcanist/__phutil_library_map__.php --- a/arcanist/__phutil_library_map__.php +++ b/arcanist/__phutil_library_map__.php @@ -9,6 +9,8 @@ phutil_register_library_map(array( '__library_version__' => 2, 'class' => array( + 'ArcanistGlobalExternalLinter' => 'linter/ArcanistGlobalExternalLinter.php', + 'ArcanistGlobalLinter' => 'linter/ArcanistGlobalLinter.php', 'AssertWithSideEffectsLinter' => 'linter/AssertWithSideEffectsLinter.php', 'AutoPEP8FormatLinter' => 'linter/AutoPEP8Linter.php', 'CHeaderLinter' => 'linter/CHeaderLinter.php', @@ -21,10 +23,12 @@ ), 'function' => array(), 'xmap' => array( + 'ArcanistGlobalExternalLinter' => 'ArcanistGlobalLinter', + 'ArcanistGlobalLinter' => 'ArcanistLinter', 'AssertWithSideEffectsLinter' => 'ArcanistLinter', 'AutoPEP8FormatLinter' => 'ArcanistExternalLinter', 'CHeaderLinter' => 'ArcanistLinter', - 'CheckDocLinter' => 'ArcanistExternalLinter', + 'CheckDocLinter' => 'ArcanistGlobalExternalLinter', 'ClangFormatLinter' => 'ArcanistExternalLinter', 'FormatStringLinter' => 'ArcanistExternalLinter', 'LocaleDependenceLinter' => 'ArcanistLinter', diff --git a/arcanist/linter/ArcanistGlobalExternalLinter.php b/arcanist/linter/ArcanistGlobalExternalLinter.php new file mode 100644 --- /dev/null +++ b/arcanist/linter/ArcanistGlobalExternalLinter.php @@ -0,0 +1,552 @@ + Mandatory flags, like `"--format=xml"`. + * @task bin + */ + protected function getMandatoryFlags() { + return array(); + } + + /** + * Provide default, overridable flags to the linter. Generally these are + * configuration flags which affect behavior but aren't critical. Flags + * which are required should be provided in @{method:getMandatoryFlags} + * instead. + * + * Default flags can be overridden with @{method:setFlags}. + * + * @return list Overridable default flags. + * @task bin + */ + protected function getDefaultFlags() { + return array(); + } + + /** + * Override default flags with custom flags. If not overridden, flags provided + * by @{method:getDefaultFlags} are used. + * + * @param list New flags. + * @return this + * @task bin + */ + final public function setFlags(array $flags) { + $this->flags = $flags; + return $this; + } + + /** + * Set the binary's version requirement. + * + * @param string Version requirement. + * @return this + * @task bin + */ + final public function setVersionRequirement($version) { + $this->versionRequirement = trim($version); + return $this; + } + + /** + * Return the binary or script to execute. This method synthesizes defaults + * and configuration. You can override the binary with @{method:setBinary}. + * + * @return string Binary to execute. + * @task bin + */ + final public function getBinary() { + return coalesce($this->bin, $this->getDefaultBinary()); + } + + /** + * Override the default binary with a new one. + * + * @param string New binary. + * @return this + * @task bin + */ + final public function setBinary($bin) { + $this->bin = $bin; + return $this; + } + + /** + * Return true if this linter should use an interpreter (like "python" or + * "node") in addition to the script. + * + * After overriding this method to return `true`, override + * @{method:getDefaultInterpreter} to set a default. + * + * @return bool True to use an interpreter. + * @task bin + */ + public function shouldUseInterpreter() { + return false; + } + + /** + * Return the default interpreter, like "python" or "node". This method is + * only invoked if @{method:shouldUseInterpreter} has been overridden to + * return `true`. + * + * @return string Default interpreter. + * @task bin + */ + public function getDefaultInterpreter() { + throw new PhutilMethodNotImplementedException(); + } + + /** + * Get the effective interpreter. This method synthesizes configuration and + * defaults. + * + * @return string Effective interpreter. + * @task bin + */ + final public function getInterpreter() { + return coalesce($this->interpreter, $this->getDefaultInterpreter()); + } + + /** + * Set the interpreter, overriding any default. + * + * @param string New interpreter. + * @return this + * @task bin + */ + final public function setInterpreter($interpreter) { + $this->interpreter = $interpreter; + return $this; + } + +/* -( Parsing Linter Output )---------------------------------------------- */ + + /** + * Parse the output of the external lint program into objects of class + * @{class:ArcanistLintMessage} which `arc` can consume. Generally, this + * means examining the output and converting each warning or error into a + * message. + * + * If parsing fails, returning `false` will cause the caller to throw an + * appropriate exception. (You can also throw a more specific exception if + * you're able to detect a more specific condition.) Otherwise, return a list + * of messages. + * + * @param int Exit code of the linter. + * @param string Stdout of the linter. + * @param string Stderr of the linter. + * @return list|false List of lint messages, or false + * to indicate parser failure. + * @task parse + */ + abstract protected function parseLinterOutput($err, $stdout, $stderr); + +/* -( Executing the Linter )----------------------------------------------- */ + + /** + * Check that the binary and interpreter (if applicable) exist, and throw + * an exception with a message about how to install them if they do not. + * + * @return void + */ + final public function checkBinaryConfiguration() { + $interpreter = null; + if ($this->shouldUseInterpreter()) { + $interpreter = $this->getInterpreter(); + } + + $binary = $this->getBinary(); + + // NOTE: If we have an interpreter, we don't require the script to be + // executable (so we just check that the path exists). Otherwise, the + // binary must be executable. + + if ($interpreter) { + if (!Filesystem::binaryExists($interpreter)) { + throw new ArcanistMissingLinterException( + pht( + 'Unable to locate interpreter "%s" to run linter %s. You may need '. + 'to install the interpreter, or adjust your linter configuration.', + $interpreter, + get_class($this))); + } + if (!Filesystem::pathExists($binary)) { + throw new ArcanistMissingLinterException( + sprintf( + "%s\n%s", + pht( + 'Unable to locate script "%s" to run linter %s. You may need '. + 'to install the script, or adjust your linter configuration.', + $binary, + get_class($this)), + pht( + 'TO INSTALL: %s', + $this->getInstallInstructions()))); + } + } else { + if (!Filesystem::binaryExists($binary)) { + throw new ArcanistMissingLinterException( + sprintf( + "%s\n%s", + pht( + 'Unable to locate binary "%s" to run linter %s. You may need '. + 'to install the binary, or adjust your linter configuration.', + $binary, + get_class($this)), + pht( + 'TO INSTALL: %s', + $this->getInstallInstructions()))); + } + } + } + + /** + * If a binary version requirement has been specified, compare the version + * of the configured binary to the required version, and if the binary's + * version is not supported, throw an exception. + * + * @param string Version string to check. + * @return void + */ + final protected function checkBinaryVersion($version) { + if (!$this->versionRequirement) { + return; + } + + if (!$version) { + $message = pht( + 'Linter %s requires %s version %s. Unable to determine the version '. + 'that you have installed.', + get_class($this), + $this->getBinary(), + $this->versionRequirement); + + $instructions = $this->getUpgradeInstructions(); + if ($instructions) { + $message .= "\n".pht('TO UPGRADE: %s', $instructions); + } + + throw new ArcanistMissingLinterException($message); + } + + $operator = '=='; + $compare_to = $this->versionRequirement; + + $matches = null; + if (preg_match('/^([<>]=?|=)\s*(.*)$/', $compare_to, $matches)) { + $operator = $matches[1]; + $compare_to = $matches[2]; + if ($operator === '=') { + $operator = '=='; + } + } + + if (!version_compare($version, $compare_to, $operator)) { + $message = pht( + 'Linter %s requires %s version %s. You have version %s.', + get_class($this), + $this->getBinary(), + $this->versionRequirement, + $version); + + $instructions = $this->getUpgradeInstructions(); + if ($instructions) { + $message .= "\n".pht('TO UPGRADE: %s', $instructions); + } + + throw new ArcanistMissingLinterException($message); + } + } + + /** + * Get the composed executable command, including the interpreter and binary + * but without flags or paths. This can be used to execute `--version` + * commands. + * + * @return string Command to execute the raw linter. + * @task exec + */ + final protected function getExecutableCommand() { + $this->checkBinaryConfiguration(); + + $interpreter = null; + if ($this->shouldUseInterpreter()) { + $interpreter = $this->getInterpreter(); + } + + $binary = $this->getBinary(); + + if ($interpreter) { + $bin = csprintf('%s %s', $interpreter, $binary); + } else { + $bin = csprintf('%s', $binary); + } + + return $bin; + } + + /** + * Get the composed flags for the executable, including both mandatory and + * configured flags. + * + * @return list Composed flags. + * @task exec + */ + final protected function getCommandFlags() { + return array_merge( + $this->getMandatoryFlags(), + nonempty($this->flags, $this->getDefaultFlags())); + } + + public function getCacheVersion() { + try { + $this->checkBinaryConfiguration(); + } catch (ArcanistMissingLinterException $e) { + return null; + } + + $version = $this->getVersion(); + + if ($version) { + $this->checkBinaryVersion($version); + return $version.'-'.json_encode($this->getCommandFlags()); + } else { + // Either we failed to parse the version number or the `getVersion` + // function hasn't been implemented. + return json_encode($this->getCommandFlags()); + } + } + + final protected function buildFuture() { + $bin = $this->getExecutableCommand(); + + $future = new ExecFuture('%C', $bin); + $future->setCWD($this->getProjectRoot()); + + return $future; + } + + final protected function resolveFuture(Future $future) { + list($err, $stdout, $stderr) = $future->resolve(); + if ($err && !$this->shouldExpectCommandErrors()) { + $future->resolvex(); + } + + $messages = $this->parseLinterOutput($err, $stdout, $stderr); + + if ($err && $this->shouldExpectCommandErrors() && !$messages) { + // We assume that if the future exits with a non-zero status and we + // failed to parse any linter messages, then something must've gone wrong + // during parsing. + $messages = false; + } + + if ($messages === false) { + if ($err) { + $future->resolvex(); + } else { + throw new Exception( + sprintf( + "%s\n\nSTDOUT\n%s\n\nSTDERR\n%s", + pht('Linter failed to parse output!'), + $stdout, + $stderr)); + } + } + + foreach ($messages as $message) { + $this->addLintMessage($message); + } + } + + public function getLinterConfigurationOptions() { + $options = array( + 'bin' => array( + 'type' => 'optional string | list', + 'help' => pht( + 'Specify a string (or list of strings) identifying the binary '. + 'which should be invoked to execute this linter. This overrides '. + 'the default binary. If you provide a list of possible binaries, '. + 'the first one which exists will be used.'), + ), + 'flags' => array( + 'type' => 'optional list', + 'help' => pht( + 'Provide a list of additional flags to pass to the linter on the '. + 'command line.'), + ), + 'version' => array( + 'type' => 'optional string', + 'help' => pht( + 'Specify a version requirement for the binary. The version number '. + 'may be prefixed with <, <=, >, >=, or = to specify the version '. + 'comparison operator (default: =).'), + ), + ); + + if ($this->shouldUseInterpreter()) { + $options['interpreter'] = array( + 'type' => 'optional string | list', + 'help' => pht( + 'Specify a string (or list of strings) identifying the interpreter '. + 'which should be used to invoke the linter binary. If you provide '. + 'a list of possible interpreters, the first one that exists '. + 'will be used.'), + ); + } + + return $options + parent::getLinterConfigurationOptions(); + } + + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'interpreter': + $root = $this->getProjectRoot(); + + foreach ((array)$value as $path) { + if (Filesystem::binaryExists($path)) { + $this->setInterpreter($path); + return; + } + + $path = Filesystem::resolvePath($path, $root); + + if (Filesystem::binaryExists($path)) { + $this->setInterpreter($path); + return; + } + } + + throw new Exception( + pht('None of the configured interpreters can be located.')); + case 'bin': + $is_script = $this->shouldUseInterpreter(); + + $root = $this->getProjectRoot(); + + foreach ((array)$value as $path) { + if (!$is_script && Filesystem::binaryExists($path)) { + $this->setBinary($path); + return; + } + + $path = Filesystem::resolvePath($path, $root); + if ((!$is_script && Filesystem::binaryExists($path)) || + ($is_script && Filesystem::pathExists($path))) { + $this->setBinary($path); + return; + } + } + + throw new Exception( + pht('None of the configured binaries can be located.')); + case 'flags': + $this->setFlags($value); + return; + case 'version': + $this->setVersionRequirement($value); + return; + } + + return parent::setLinterConfigurationValue($key, $value); + } + + /** + * Map a configuration lint code to an `arc` lint code. Primarily, this is + * intended for validation, but can also be used to normalize case or + * otherwise be more permissive in accepted inputs. + * + * If the code is not recognized, you should throw an exception. + * + * @param string Code specified in configuration. + * @return string Normalized code to use in severity map. + */ + protected function getLintCodeFromLinterConfigurationKey($code) { + return $code; + } + +} diff --git a/arcanist/linter/ArcanistGlobalLinter.php b/arcanist/linter/ArcanistGlobalLinter.php new file mode 100644 --- /dev/null +++ b/arcanist/linter/ArcanistGlobalLinter.php @@ -0,0 +1,48 @@ +didRun) { + return; + } + + $this->didRun = true; + + // These linters only run once, so build a single future on first path only. + $this->path = $paths[0]; + + $this->future = $this->buildFuture(); + } + + final public function lintPath($path) { + return; + } + + public function didLintPaths(array $paths) { + if (!$this->future) { + return; + } + +// $this->setActivePath($this->path); + $this->resolveFuture($this->future); + + // Prevent returning the messages multiple times + $this->future = null; + } +} diff --git a/arcanist/linter/CheckDocLinter.php b/arcanist/linter/CheckDocLinter.php --- a/arcanist/linter/CheckDocLinter.php +++ b/arcanist/linter/CheckDocLinter.php @@ -3,7 +3,7 @@ /** * Uses the check-doc.py script to enfore command line arguments documentation */ -final class CheckDocLinter extends ArcanistExternalLinter { +final class CheckDocLinter extends ArcanistGlobalExternalLinter { public function getInfoName() { return 'check-doc'; @@ -56,7 +56,7 @@ return array(); } - protected function parseLinterOutput($path, $err, $stdout, $stderr) { + protected function parseLinterOutput($err, $stdout, $stderr) { /* Split stdout: * 0 => Empty (before first 'Args' occurrence) * 1 => Args used: count