diff --git a/src/lint/linter/ArcanistCSSLintLinter.php b/src/lint/linter/ArcanistCSSLintLinter.php index 1f07a4b9..a69929cf 100644 --- a/src/lint/linter/ArcanistCSSLintLinter.php +++ b/src/lint/linter/ArcanistCSSLintLinter.php @@ -1,104 +1,104 @@ getEngine()->getConfigurationManager(); - return $config->getConfigFromAnySource('lint.csslint.options'); + return $config->getConfigFromAnySource('lint.csslint.options', array()); } public function getDefaultBinary() { // TODO: Deprecation warning. $config = $this->getEngine()->getConfigurationManager(); return $config->getConfigFromAnySource('lint.csslint.bin', 'csslint'); } public function getInstallInstructions() { return pht('Install CSSLint using `npm install -g csslint`.'); } public function shouldExpectCommandErrors() { return true; } protected function parseLinterOutput($path, $err, $stdout, $stderr) { $report_dom = new DOMDocument(); $ok = @$report_dom->loadXML($stdout); if (!$ok) { return false; } $files = $report_dom->getElementsByTagName('file'); $messages = array(); foreach ($files as $file) { foreach ($file->childNodes as $child) { if (!($child instanceof DOMElement)) { continue; } $data = $this->getData($path); $lines = explode("\n", $data); $name = $child->getAttribute('reason'); $severity = ($child->getAttribute('severity') == 'warning') ? ArcanistLintSeverity::SEVERITY_WARNING : ArcanistLintSeverity::SEVERITY_ERROR; $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($child->getAttribute('line')); $message->setChar($child->getAttribute('char')); $message->setCode('CSSLint'); $message->setDescription($child->getAttribute('reason')); $message->setSeverity($severity); if ($child->hasAttribute('line') && $child->getAttribute('line') > 0) { $line = $lines[$child->getAttribute('line') - 1]; $text = substr($line, $child->getAttribute('char') - 1); $message->setOriginalText($text); } $messages[] = $message; } } return $messages; } protected function getLintCodeFromLinterConfigurationKey($code) { // NOTE: We can't figure out which rule generated each message, so we // can not customize severities. I opened a pull request to add this // ability; see: // // https://github.com/stubbornella/csslint/pull/409 throw new Exception( pht( "CSSLint does not currently support custom severity levels, because ". "rules can't be identified from messages in output. ". "See Pull Request #409.")); } } diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php index d8fde402..4a952359 100644 --- a/src/lint/linter/ArcanistExternalLinter.php +++ b/src/lint/linter/ArcanistExternalLinter.php @@ -1,492 +1,507 @@ Mandatory flags, like `"--format=xml"`. * @task bin */ protected function getMandatoryFlags() { - return null; + 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 string|null Overridable default flags. + * @return list Overridable default flags. * @task bin */ protected function getDefaultFlags() { - return null; + return array(); } /** * Override default flags with custom flags. If not overridden, flags provided * by @{method:getDefaultFlags} are used. * - * @param string New flags. + * @param list New flags. * @return this * @task bin */ final public function setFlags($flags) { - $this->flags = $flags; + $this->flags = (array) $flags; 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 Exception("Incomplete implementation!"); } /** * 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 string Path to the file being linted. * @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($path, $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 ArcanistUsageException( pht( 'Unable to locate interpreter "%s" to run linter %s. You may '. 'need to install the intepreter, or adjust your linter '. 'configuration.'. "\nTO INSTALL: %s", $interpreter, get_class($this), $this->getInstallInstructions())); } if (!Filesystem::pathExists($binary)) { throw new ArcanistUsageException( pht( 'Unable to locate script "%s" to run linter %s. You may need '. 'to install the script, or adjust your linter configuration. '. "\nTO INSTALL: %s", $binary, get_class($this), $this->getInstallInstructions())); } } else { if (!Filesystem::binaryExists($binary)) { throw new ArcanistUsageException( pht( 'Unable to locate binary "%s" to run linter %s. You may need '. 'to install the binary, or adjust your linter configuration. '. "\nTO INSTALL: %s", $binary, get_class($this), $this->getInstallInstructions())); } } } /** * 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 */ 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 string Composed flags. + * @return list Composed flags. * @task exec */ protected function getCommandFlags() { - return csprintf( - '%C %C', - $this->getMandatoryFlags(), - coalesce($this->flags, $this->getDefaultFlags())); + $mandatory_flags = $this->getMandatoryFlags(); + if (!is_array($mandatory_flags)) { + phutil_deprecated( + 'String support for flags.', 'You should use list instead.'); + $mandatory_flags = (array) $mandatory_flags; + } + + $flags = nonempty($this->flags, $this->getDefaultFlags()); + if (!is_array($flags)) { + phutil_deprecated( + 'String support for flags.', 'You should use list instead.'); + $flags = (array) $flags; + } + + return array_merge($mandatory_flags, $flags); } protected function buildFutures(array $paths) { $executable = $this->getExecutableCommand(); - $bin = csprintf('%C %C', $executable, $this->getCommandFlags()); + $bin = csprintf('%C %Ls', $executable, $this->getCommandFlags()); $futures = array(); foreach ($paths as $path) { if ($this->supportsReadDataFromStdin()) { $future = new ExecFuture( '%C %C', $bin, $this->getReadDataFromStdinFilename()); $future->write($this->getEngine()->loadData($path)); } else { // TODO: In commit hook mode, we need to do more handling here. $disk_path = $this->getEngine()->getFilePathOnDisk($path); $future = new ExecFuture('%C %s', $bin, $disk_path); } $futures[$path] = $future; } return $futures; } protected function resolveFuture($path, Future $future) { list($err, $stdout, $stderr) = $future->resolve(); if ($err && !$this->shouldExpectCommandErrors()) { $future->resolvex(); } $messages = $this->parseLinterOutput($path, $err, $stdout, $stderr); if ($messages === false) { if ($err) { $future->resolvex(); } else { throw new Exception( "Linter failed to parse output!\n\n{$stdout}\n\n{$stderr}"); } } foreach ($messages as $message) { $this->addLintMessage($message); } } public function getLinterConfigurationOptions() { $options = array( 'bin' => 'optional string | list', - 'flags' => 'optional string', + 'flags' => 'optional list', ); if ($this->shouldUseInterpreter()) { $options['interpreter'] = 'optional string | list'; } return $options + parent::getLinterConfigurationOptions(); } public function setLinterConfigurationValue($key, $value) { switch ($key) { case 'interpreter': $working_copy = $this->getEngine()->getWorkingCopy(); $root = $working_copy->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(); $working_copy = $this->getEngine()->getWorkingCopy(); $root = $working_copy->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': - if (strlen($value)) { - $this->setFlags($value); + if (!is_array($value)) { + phutil_deprecated( + 'String support for flags.', + 'You should use list instead.'); + $value = (array) $value; } + $this->setFlags($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/src/lint/linter/ArcanistFlake8Linter.php b/src/lint/linter/ArcanistFlake8Linter.php index 01ecd49c..36ecbd70 100644 --- a/src/lint/linter/ArcanistFlake8Linter.php +++ b/src/lint/linter/ArcanistFlake8Linter.php @@ -1,120 +1,120 @@ getEngine()->getConfigurationManager(); - return $config->getConfigFromAnySource('lint.flake8.options', ''); + return $config->getConfigFromAnySource('lint.flake8.options', array()); } public function getDefaultBinary() { $config = $this->getEngine()->getConfigurationManager(); $prefix = $config->getConfigFromAnySource('lint.flake8.prefix'); $bin = $config->getConfigFromAnySource('lint.flake8.bin', 'flake8'); if ($prefix || ($bin != 'flake8')) { return $prefix.'/'.$bin; } return 'flake8'; } public function getInstallInstructions() { return pht('Install flake8 using `easy_install flake8`.'); } public function supportsReadDataFromStdin() { return true; } public function getReadDataFromStdinFilename() { return '-'; } public function shouldExpectCommandErrors() { return true; } protected function parseLinterOutput($path, $err, $stdout, $stderr) { $lines = phutil_split_lines($stdout, $retain_endings = false); $messages = array(); foreach ($lines as $line) { $matches = null; // stdin:2: W802 undefined name 'foo' # pyflakes // stdin:3:1: E302 expected 2 blank lines, found 1 # pep8 $regexp = '/^(.*?):(\d+):(?:(\d+):)? (\S+) (.*)$/'; if (!preg_match($regexp, $line, $matches)) { continue; } foreach ($matches as $key => $match) { $matches[$key] = trim($match); } $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($matches[2]); if (!empty($matches[3])) { $message->setChar($matches[3]); } $message->setCode($matches[4]); $message->setName($this->getLinterName().' '.$matches[3]); $message->setDescription($matches[5]); $message->setSeverity($this->getLintMessageSeverity($matches[4])); $messages[] = $message; } if ($err && !$messages) { return false; } return $messages; } protected function getDefaultMessageSeverity($code) { if (preg_match('/^C/', $code)) { // "C": Cyclomatic complexity return ArcanistLintSeverity::SEVERITY_ADVICE; } else if (preg_match('/^W/', $code)) { // "W": PEP8 Warning return ArcanistLintSeverity::SEVERITY_WARNING; } else { // "E": PEP8 Error // "F": PyFlakes Error return ArcanistLintSeverity::SEVERITY_ERROR; } } protected function getLintCodeFromLinterConfigurationKey($code) { if (!preg_match('/^(E|W|C|F)\d+$/', $code)) { throw new Exception( pht( 'Unrecognized lint message code "%s". Expected a valid flake8 '. 'lint code like "%s", or "%s", or "%s", or "%s".', $code, "E225", "W291", "F811", "C901")); } return $code; } } diff --git a/src/lint/linter/ArcanistPEP8Linter.php b/src/lint/linter/ArcanistPEP8Linter.php index 2f67a859..1058c8f1 100644 --- a/src/lint/linter/ArcanistPEP8Linter.php +++ b/src/lint/linter/ArcanistPEP8Linter.php @@ -1,124 +1,124 @@ getExecutableCommand()); - return $stdout.$this->getCommandFlags(); + return $stdout.implode(' ', $this->getCommandFlags()); } public function getDefaultFlags() { // TODO: Warn that all of this is deprecated. $config = $this->getEngine()->getConfigurationManager(); return $config->getConfigFromAnySource( 'lint.pep8.options', - $this->getConfig('options')); + $this->getConfig('options', array())); } public function shouldUseInterpreter() { return ($this->getDefaultBinary() !== 'pep8'); } public function getDefaultInterpreter() { return 'python2.6'; } public function getDefaultBinary() { if (Filesystem::binaryExists('pep8')) { return 'pep8'; } $config = $this->getEngine()->getConfigurationManager(); $old_prefix = $config->getConfigFromAnySource('lint.pep8.prefix'); $old_bin = $config->getConfigFromAnySource('lint.pep8.bin'); if ($old_prefix || $old_bin) { // TODO: Deprecation warning. $old_bin = nonempty($old_bin, 'pep8'); return $old_prefix.'/'.$old_bin; } $arc_root = dirname(phutil_get_library_root('arcanist')); return $arc_root.'/externals/pep8/pep8.py'; } public function getInstallInstructions() { return pht('Install PEP8 using `easy_install pep8`.'); } public function shouldExpectCommandErrors() { return true; } protected function parseLinterOutput($path, $err, $stdout, $stderr) { $lines = phutil_split_lines($stdout, $retain_endings = false); $messages = array(); foreach ($lines as $line) { $matches = null; if (!preg_match('/^(.*?):(\d+):(\d+): (\S+) (.*)$/', $line, $matches)) { continue; } foreach ($matches as $key => $match) { $matches[$key] = trim($match); } $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($matches[2]); $message->setChar($matches[3]); $message->setCode($matches[4]); $message->setName('PEP8 '.$matches[4]); $message->setDescription($matches[5]); $message->setSeverity($this->getLintMessageSeverity($matches[4])); $messages[] = $message; } if ($err && !$messages) { return false; } return $messages; } protected function getDefaultMessageSeverity($code) { if (preg_match('/^W/', $code)) { return ArcanistLintSeverity::SEVERITY_WARNING; } else { // TODO: Once severities/.arclint are more usable, restore this to // "ERROR". // return ArcanistLintSeverity::SEVERITY_ERROR; return ArcanistLintSeverity::SEVERITY_WARNING; } } protected function getLintCodeFromLinterConfigurationKey($code) { if (!preg_match('/^(E|W)\d+$/', $code)) { throw new Exception( pht( 'Unrecognized lint message code "%s". Expected a valid PEP8 '. 'lint code like "%s" or "%s".', $code, "E101", "W291")); } return $code; } } diff --git a/src/lint/linter/ArcanistPhpcsLinter.php b/src/lint/linter/ArcanistPhpcsLinter.php index efbb3fca..a5c292b2 100644 --- a/src/lint/linter/ArcanistPhpcsLinter.php +++ b/src/lint/linter/ArcanistPhpcsLinter.php @@ -1,120 +1,126 @@ getEngine()->getConfigurationManager(); - $options = $config->getConfigFromAnySource('lint.phpcs.options'); + $options = $config->getConfigFromAnySource('lint.phpcs.options', array()); $standard = $config->getConfigFromAnySource('lint.phpcs.standard'); - $options .= !empty($standard) ? ' --standard=' . $standard : ''; + if (!empty($standard)) { + if (is_array($options)) { + $options[] = '--standard='.$standard; + } else { + $options .= ' --standard='.$standard; + } + } return $options; } public function getDefaultBinary() { // TODO: Deprecation warnings. $config = $this->getEngine()->getConfigurationManager(); return $config->getConfigFromAnySource('lint.phpcs.bin', 'phpcs'); } public function shouldExpectCommandErrors() { return true; } protected function parseLinterOutput($path, $err, $stdout, $stderr) { // NOTE: Some version of PHPCS after 1.4.6 stopped printing a valid, empty // XML document to stdout in the case of no errors. If PHPCS exits with // error 0, just ignore output. if (!$err) { return array(); } $report_dom = new DOMDocument(); $ok = @$report_dom->loadXML($stdout); if (!$ok) { return false; } $files = $report_dom->getElementsByTagName('file'); $messages = array(); foreach ($files as $file) { foreach ($file->childNodes as $child) { if (!($child instanceof DOMElement)) { continue; } if ($child->tagName == 'error') { $prefix = 'E'; } else { $prefix = 'W'; } $code = 'PHPCS.'.$prefix.'.'.$child->getAttribute('source'); $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($child->getAttribute('line')); $message->setChar($child->getAttribute('column')); $message->setCode($code); $message->setDescription($child->nodeValue); $message->setSeverity($this->getLintMessageSeverity($code)); $messages[] = $message; } } return $messages; } protected function getDefaultMessageSeverity($code) { if (preg_match('/^PHPCS\\.W\\./', $code)) { return ArcanistLintSeverity::SEVERITY_WARNING; } else { return ArcanistLintSeverity::SEVERITY_ERROR; } } protected function getLintCodeFromLinterConfigurationKey($code) { if (!preg_match('/^PHPCS\\.(E|W)\\./', $code)) { throw new Exception( "Invalid severity code '{$code}', should begin with 'PHPCS.'."); } return $code; } } diff --git a/src/lint/linter/ArcanistRubyLinter.php b/src/lint/linter/ArcanistRubyLinter.php index 08619c79..2921f5f4 100644 --- a/src/lint/linter/ArcanistRubyLinter.php +++ b/src/lint/linter/ArcanistRubyLinter.php @@ -1,86 +1,86 @@ getEngine()->getConfigurationManager(); $prefix = $config->getConfigFromAnySource('lint.ruby.prefix'); if ($prefix !== null) { $ruby_bin = $prefix.'ruby'; } return 'ruby'; } public function getInstallInstructions() { return pht('Install `ruby` from .'); } public function supportsReadDataFromStdin() { return true; } public function shouldExpectCommandErrors() { return true; } protected function getMandatoryFlags() { // -w: turn on warnings // -c: check syntax - return '-w -c'; + return array('-w', '-c'); } protected function getDefaultMessageSeverity($code) { return ArcanistLintSeverity::SEVERITY_ERROR; } protected function parseLinterOutput($path, $err, $stdout, $stderr) { $lines = phutil_split_lines($stderr, $retain_endings = false); $messages = array(); foreach ($lines as $line) { $matches = null; if (!preg_match("/(.*?):(\d+): (.*?)$/", $line, $matches)) { continue; } foreach ($matches as $key => $match) { $matches[$key] = trim($match); } $code = head(explode(',', $matches[3])); $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($matches[2]); $message->setCode($this->getLinterName()); $message->setName(pht('Syntax Error')); $message->setDescription($matches[3]); $message->setSeverity($this->getLintMessageSeverity($code)); $messages[] = $message; } if ($err && !$messages) { return false; } return $messages; } }