diff --git a/src/lint/engine/ComprehensiveLintEngine.php b/src/lint/engine/ComprehensiveLintEngine.php index 06555014..9d46cfb4 100644 --- a/src/lint/engine/ComprehensiveLintEngine.php +++ b/src/lint/engine/ComprehensiveLintEngine.php @@ -1,51 +1,51 @@ getPaths(); foreach ($paths as $key => $path) { if (preg_match('@^externals/@', $path)) { // Third-party stuff lives in /externals/; don't run lint engines // against it. unset($paths[$key]); } } $text_paths = preg_grep('/\.(php|css|hpp|cpp|l|y|py|pl)$/', $paths); $linters[] = id(new ArcanistGeneratedLinter())->setPaths($text_paths); $linters[] = id(new ArcanistNoLintLinter())->setPaths($text_paths); $linters[] = id(new ArcanistTextLinter())->setPaths($text_paths); $linters[] = id(new ArcanistFilenameLinter())->setPaths($paths); $linters[] = id(new ArcanistXHPASTLinter()) ->setPaths(preg_grep('/\.php$/', $paths)); $py_paths = preg_grep('/\.py$/', $paths); $linters[] = id(new ArcanistPyFlakesLinter())->setPaths($py_paths); $linters[] = id(new ArcanistPEP8Linter()) - ->setConfig(array('options' => $this->getPEP8WithTextOptions())) + ->setConfig(array('flags' => array($this->getPEP8WithTextOptions()))) ->setPaths($py_paths); $linters[] = id(new ArcanistRubyLinter()) ->setPaths(preg_grep('/\.rb$/', $paths)); $linters[] = id(new ArcanistScalaSBTLinter()) ->setPaths(preg_grep('/\.scala$/', $paths)); $linters[] = id(new ArcanistJSHintLinter()) ->setPaths(preg_grep('/\.js$/', $paths)); return $linters; } } diff --git a/src/lint/linter/ArcanistCSSLintLinter.php b/src/lint/linter/ArcanistCSSLintLinter.php index 0eb1f0b8..a488ed34 100644 --- a/src/lint/linter/ArcanistCSSLintLinter.php +++ b/src/lint/linter/ArcanistCSSLintLinter.php @@ -1,126 +1,122 @@ getEngine()->getConfigurationManager(); - return $config->getConfigFromAnySource('lint.csslint.options', array()); + return $this->getDeprecatedConfiguration('lint.csslint.options', array()); } public function getDefaultBinary() { - // TODO: Deprecation warning. - $config = $this->getEngine()->getConfigurationManager(); - return $config->getConfigFromAnySource('lint.csslint.bin', 'csslint'); + return $this->getDeprecatedConfiguration('lint.csslint.bin', 'csslint'); } public function getVersion() { list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); if (preg_match('/^v(?P\d+\.\d+\.\d+)$/', $stdout, $matches)) { return $matches['version']; } else { return false; } } 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/ArcanistCpplintLinter.php b/src/lint/linter/ArcanistCpplintLinter.php index 41974eed..3f660066 100644 --- a/src/lint/linter/ArcanistCpplintLinter.php +++ b/src/lint/linter/ArcanistCpplintLinter.php @@ -1,97 +1,94 @@ getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.cpplint.prefix'); - $bin = $config->getConfigFromAnySource('lint.cpplint.bin', 'cpplint.py'); + $prefix = $this->getDeprecatedConfiguration('lint.cpplint.prefix'); + $bin = $this->getDeprecatedConfiguration('lint.cpplint.bin', 'cpplint.py'); if ($prefix) { return $prefix.'/'.$bin; } else { return $bin; } } public function getInstallInstructions() { return pht('Install cpplint.py using `wget http://google-styleguide.'. 'googlecode.com/svn/trunk/cpplint/cpplint.py`.'); } public function shouldExpectCommandErrors() { return true; } public function supportsReadDataFromStdin() { return true; } public function getReadDataFromStdinFilename() { return '-'; } protected function getDefaultFlags() { - $config = $this->getEngine()->getConfigurationManager(); - return $config->getConfigFromAnySource('lint.cpplint.options', array()); + return $this->getDeprecatedConfiguration('lint.cpplint.options', array()); } protected function parseLinterOutput($path, $err, $stdout, $stderr) { $lines = explode("\n", $stderr); $messages = array(); foreach ($lines as $line) { $line = trim($line); $matches = null; $regex = '/^-:(\d+):\s*(.*)\s*\[(.*)\] \[(\d+)\]$/'; if (!preg_match($regex, $line, $matches)) { continue; } foreach ($matches as $key => $match) { $matches[$key] = trim($match); } $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($matches[1]); $message->setCode($matches[3]); $message->setName($matches[3]); $message->setDescription($matches[2]); $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING); $messages[] = $message; } if ($err && !$messages) { return false; } return $messages; } protected function getLintCodeFromLinterConfigurationKey($code) { if (!preg_match('@^[a-z_]+/[a-z_]+$@', $code)) { throw new Exception( pht( 'Unrecognized lint message code "%s". Expected a valid cpplint '. 'lint code like "%s" or "%s".', $code, 'build/include_order', 'whitespace/braces')); } return $code; } } diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php index 70612d5a..d05fa219 100644 --- a/src/lint/linter/ArcanistExternalLinter.php +++ b/src/lint/linter/ArcanistExternalLinter.php @@ -1,532 +1,532 @@ 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($flags) { - $this->flags = (array) $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 list Composed flags. * @task exec */ protected function getCommandFlags() { $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); } public function getCacheVersion() { $version = $this->getVersion(); if ($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()); } } /** * Prepare the path to be added to the command string. * * This method is expected to return an already escaped string. * * @param string Path to the file being linted * @return string The command-ready file argument */ protected function getPathArgumentForLinterFuture($path) { return csprintf('%s', $path); } protected function buildFutures(array $paths) { $executable = $this->getExecutableCommand(); $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); $path_argument = $this->getPathArgumentForLinterFuture($disk_path); $future = new ExecFuture('%C %C', $bin, $path_argument); } $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 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 (!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 cfac19c7..ef810efb 100644 --- a/src/lint/linter/ArcanistFlake8Linter.php +++ b/src/lint/linter/ArcanistFlake8Linter.php @@ -1,143 +1,140 @@ getEngine()->getConfigurationManager(); - return $config->getConfigFromAnySource('lint.flake8.options', array()); + return $this->getDeprecatedConfiguration('lint.flake8.options', array()); } public function getDefaultBinary() { - $config = $this->getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.flake8.prefix'); - $bin = $config->getConfigFromAnySource('lint.flake8.bin', 'flake8'); + $prefix = $this->getDeprecatedConfiguration('lint.flake8.prefix'); + $bin = $this->getDeprecatedConfiguration('lint.flake8.bin', 'flake8'); - if ($prefix || ($bin != 'flake8')) { + if ($prefix) { return $prefix.'/'.$bin; + } else { + return $bin; } - - return 'flake8'; } public function getVersion() { list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); if (preg_match('/^(?P\d+\.\d+\.\d+)\b/', $stdout, $matches)) { return $matches['version']; } else { return false; } } 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/ArcanistJSHintLinter.php b/src/lint/linter/ArcanistJSHintLinter.php index ef6ec9db..d05bcecc 100644 --- a/src/lint/linter/ArcanistJSHintLinter.php +++ b/src/lint/linter/ArcanistJSHintLinter.php @@ -1,139 +1,137 @@ getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.jshint.prefix'); - $bin = $config->getConfigFromAnySource('lint.jshint.bin', 'jshint'); + $prefix = $this->getDeprecatedConfiguration('lint.jshint.prefix'); + $bin = $this->getDeprecatedConfiguration('lint.jshint.bin', 'jshint'); if ($prefix) { return $prefix.'/'.$bin; } else { return $bin; } } public function getVersion() { list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); $regex = '/^jshint v(?P\d+\.\d+\.\d+)$/'; if (preg_match($regex, $stdout, $matches)) { return $matches['version']; } else { return false; } } public function getInstallInstructions() { return pht('Install JSHint using `npm install -g jshint`.'); } public function shouldExpectCommandErrors() { return true; } public function supportsReadDataFromStdin() { return true; } public function getReadDataFromStdinFilename() { return '-'; } protected function getMandatoryFlags() { return array( '--reporter='.dirname(realpath(__FILE__)).'/reporter.js', ); } protected function getDefaultFlags() { - $config_manager = $this->getEngine()->getConfigurationManager(); - $options = $config_manager->getConfigFromAnySource( + $options = $this->getDeprecatedConfiguration( 'lint.jshint.options', array()); - $config = $config_manager->getConfigFromAnySource('lint.jshint.config'); + $config = $this->getDeprecatedConfiguration('lint.jshint.config'); if ($config) { $options[] = '--config='.$config; } return $options; } protected function parseLinterOutput($path, $err, $stdout, $stderr) { $errors = json_decode($stdout, true); if (!is_array($errors)) { // Something went wrong and we can't decode the output. Exit abnormally. throw new ArcanistUsageException( "JSHint returned unparseable output.\n". "stdout:\n\n{$stdout}". "stderr:\n\n{$stderr}"); } $messages = array(); foreach ($errors as $err) { $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine(idx($err, 'line')); $message->setChar(idx($err, 'col')); $message->setCode(idx($err, 'code')); $message->setName('JSHint'.idx($err, 'code')); $message->setDescription(idx($err, 'reason')); $message->setSeverity($this->getLintMessageSeverity(idx($err, 'code'))); $message->setOriginalText(idx($err, 'evidence')); $messages[] = $message; } return $messages; } protected function getLintCodeFromLinterConfigurationKey($code) { if (!preg_match('/^(E|W)\d+$/', $code)) { throw new Exception( pht( 'Unrecognized lint message code "%s". Expected a valid JSHint '. 'lint code like "%s" or "%s".', $code, 'E033', 'W093')); } return $code; } } diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php index caae477d..e8847512 100644 --- a/src/lint/linter/ArcanistLinter.php +++ b/src/lint/linter/ArcanistLinter.php @@ -1,441 +1,486 @@ getLinterName(), $this->getLinterConfigurationName(), get_class($this)); } public function getLinterPriority() { return 1.0; } public function setCustomSeverityMap(array $map) { $this->customSeverityMap = $map; return $this; } public function setCustomSeverityRules(array $rules) { $this->customSeverityRules = $rules; return $this; } public function setConfig(array $config) { $this->config = $config; return $this; } protected function getConfig($key, $default = null) { return idx($this->config, $key, $default); } public function getActivePath() { return $this->activePath; } public function getOtherLocation($offset, $path = null) { if ($path === null) { $path = $this->getActivePath(); } list($line, $char) = $this->getEngine()->getLineAndCharFromOffset( $path, $offset); return array( 'path' => $path, 'line' => $line + 1, 'char' => $char, ); } public function stopAllLinters() { $this->stopAllLinters = true; return $this; } public function didStopAllLinters() { return $this->stopAllLinters; } public function addPath($path) { $this->paths[$path] = $path; return $this; } public function setPaths(array $paths) { $this->paths = $paths; return $this; } /** * Filter out paths which this linter doesn't act on (for example, because * they are binaries and the linter doesn't apply to binaries). */ private function filterPaths($paths) { $engine = $this->getEngine(); $keep = array(); foreach ($paths as $path) { if (!$this->shouldLintDeletedFiles() && !$engine->pathExists($path)) { continue; } if (!$this->shouldLintDirectories() && $engine->isDirectory($path)) { continue; } if (!$this->shouldLintBinaryFiles() && $engine->isBinaryFile($path)) { continue; } $keep[] = $path; } return $keep; } public function getPaths() { return $this->filterPaths(array_values($this->paths)); } public function addData($path, $data) { $this->data[$path] = $data; return $this; } protected function getData($path) { if (!array_key_exists($path, $this->data)) { $this->data[$path] = $this->getEngine()->loadData($path); } return $this->data[$path]; } public function setEngine(ArcanistLintEngine $engine) { $this->engine = $engine; return $this; } protected function getEngine() { return $this->engine; } public function getCacheVersion() { return 0; } public function getLintMessageFullCode($short_code) { return $this->getLinterName().$short_code; } public function getLintMessageSeverity($code) { $map = $this->customSeverityMap; if (isset($map[$code])) { return $map[$code]; } $map = $this->getLintSeverityMap(); if (isset($map[$code])) { return $map[$code]; } foreach ($this->customSeverityRules as $rule => $severity) { if (preg_match($rule, $code)) { return $severity; } } return $this->getDefaultMessageSeverity($code); } protected function getDefaultMessageSeverity($code) { return ArcanistLintSeverity::SEVERITY_ERROR; } public function isMessageEnabled($code) { return ($this->getLintMessageSeverity($code) !== ArcanistLintSeverity::SEVERITY_DISABLED); } public function getLintMessageName($code) { $map = $this->getLintNameMap(); if (isset($map[$code])) { return $map[$code]; } return "Unknown lint message!"; } protected function addLintMessage(ArcanistLintMessage $message) { if (!$this->getEngine()->getCommitHookMode()) { $root = $this->getEngine()->getWorkingCopy()->getProjectRoot(); $path = Filesystem::resolvePath($message->getPath(), $root); $message->setPath(Filesystem::readablePath($path, $root)); } $this->messages[] = $message; return $message; } public function getLintMessages() { return $this->messages; } protected function raiseLintAtLine( $line, $char, $code, $desc, $original = null, $replacement = null) { $message = id(new ArcanistLintMessage()) ->setPath($this->getActivePath()) ->setLine($line) ->setChar($char) ->setCode($this->getLintMessageFullCode($code)) ->setSeverity($this->getLintMessageSeverity($code)) ->setName($this->getLintMessageName($code)) ->setDescription($desc) ->setOriginalText($original) ->setReplacementText($replacement); return $this->addLintMessage($message); } protected function raiseLintAtPath( $code, $desc) { return $this->raiseLintAtLine(null, null, $code, $desc, null, null); } protected function raiseLintAtOffset( $offset, $code, $desc, $original = null, $replacement = null) { $path = $this->getActivePath(); $engine = $this->getEngine(); if ($offset === null) { $line = null; $char = null; } else { list($line, $char) = $engine->getLineAndCharFromOffset($path, $offset); } return $this->raiseLintAtLine( $line + 1, $char + 1, $code, $desc, $original, $replacement); } public function willLintPath($path) { $this->stopAllLinters = false; $this->activePath = $path; } public function canRun() { return true; } public function willLintPaths(array $paths) { return; } abstract public function lintPath($path); abstract public function getLinterName(); public function getVersion() { return null; } public function didRunLinters() { // This is a hook. } protected function isCodeEnabled($code) { $severity = $this->getLintMessageSeverity($code); return $this->getEngine()->isSeverityEnabled($severity); } public function getLintSeverityMap() { return array(); } public function getLintNameMap() { return array(); } public function getCacheGranularity() { return self::GRANULARITY_FILE; } /** * If this linter is selectable via `.arclint` configuration files, return * a short, human-readable name to identify it. For example, `"jshint"` or * `"pep8"`. * * If you do not implement this method, the linter will not be selectable * through `.arclint` files. */ public function getLinterConfigurationName() { return null; } public function getLinterConfigurationOptions() { return array( 'severity' => 'optional map', 'severity.rules' => 'optional map', ); } public function setLinterConfigurationValue($key, $value) { $sev_map = array( 'error' => ArcanistLintSeverity::SEVERITY_ERROR, 'warning' => ArcanistLintSeverity::SEVERITY_WARNING, 'autofix' => ArcanistLintSeverity::SEVERITY_AUTOFIX, 'advice' => ArcanistLintSeverity::SEVERITY_ADVICE, 'disabled' => ArcanistLintSeverity::SEVERITY_DISABLED, ); switch ($key) { case 'severity': $custom = array(); foreach ($value as $code => $severity) { if (empty($sev_map[$severity])) { $valid = implode(', ', array_keys($sev_map)); throw new Exception( pht( 'Unknown lint severity "%s". Valid severities are: %s.', $severity, $valid)); } $code = $this->getLintCodeFromLinterConfigurationKey($code); $custom[$code] = $severity; } $this->setCustomSeverityMap($custom); return; case 'severity.rules': foreach ($value as $rule => $severity) { if (@preg_match($rule, '') === false) { throw new Exception( pht( 'Severity rule "%s" is not a valid regular expression.', $rule)); } if (empty($sev_map[$severity])) { $valid = implode(', ', array_keys($sev_map)); throw new Exception( pht( 'Unknown lint severity "%s". Valid severities are: %s.', $severity, $valid)); } } $this->setCustomSeverityRules($value); return; } throw new Exception("Incomplete implementation: {$key}!"); } protected function shouldLintBinaryFiles() { return false; } protected function shouldLintDeletedFiles() { return false; } protected function shouldLintDirectories() { return false; } /** * 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; } + + /** + * Retrieve an old lint configuration value from `.arcconfig` or a similar + * source. + * + * Modern linters should use @{method:getConfig} to read configuration from + * `.arclint`. + * + * @param string Configuration key to retrieve. + * @param wild Default value to return if key is not present in config. + * @return wild Configured value, or default if no configuration exists. + */ + protected function getDeprecatedConfiguration($key, $default = null) { + + // If we're being called in a context without an engine (probably from + // `arc linters`), just return the default value. + if (!$this->engine) { + return $default; + } + + $config = $this->getEngine()->getConfigurationManager(); + + // Construct a sentinel object so we can tell if we're reading config + // or not. + $sentinel = (object)array(); + $result = $config->getConfigFromAnySource($key, $sentinel); + + // If we read config, warn the user that this mechanism is deprecated and + // discouraged. + if ($result !== $sentinel) { + $console = PhutilConsole::getConsole(); + $console->writeErr( + "**%s**: %s\n", + pht('Deprecation Warning'), + pht( + 'Configuration option "%s" is deprecated. Generally, linters should '. + 'now be configured using an `.arclint` file. See "Arcanist User '. + 'Guide: Lint" in the documentation for more information.', + $key)); + return $result; + } + + return $default; + } + } diff --git a/src/lint/linter/ArcanistPEP8Linter.php b/src/lint/linter/ArcanistPEP8Linter.php index 463bb44d..e02134d1 100644 --- a/src/lint/linter/ArcanistPEP8Linter.php +++ b/src/lint/linter/ArcanistPEP8Linter.php @@ -1,142 +1,135 @@ getEngine()->getConfigurationManager(); - return $config->getConfigFromAnySource( - 'lint.pep8.options', - $this->getConfig('options', array())); + return $this->getDeprecatedConfiguration('lint.pep8.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'); - + $old_prefix = $this->getDeprecatedConfiguration('lint.pep8.prefix'); + $old_bin = $this->getDeprecatedConfiguration('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 getVersion() { list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); if (preg_match('/^(?P\d+\.\d+\.\d+)$/', $stdout, $matches)) { return $matches['version']; } else { return false; } } 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 92631251..1dbb10a7 100644 --- a/src/lint/linter/ArcanistPhpcsLinter.php +++ b/src/lint/linter/ArcanistPhpcsLinter.php @@ -1,156 +1,149 @@ getEngine()->getConfigurationManager(); - - $options = $config->getConfigFromAnySource('lint.phpcs.options', array()); - - $standard = $config->getConfigFromAnySource('lint.phpcs.standard'); + $options = $this->getDeprecatedConfiguration('lint.phpcs.options', array()); + $standard = $this->getDeprecatedConfiguration('lint.phpcs.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'); + return $this->getDeprecatedConfiguration('lint.phpcs.bin', 'phpcs'); } public function getVersion() { list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); $regex = '/^PHP_CodeSniffer version (?P\d+\.\d+\.\d+)\b/'; if (preg_match($regex, $stdout, $matches)) { return $matches['version']; } else { return false; } } public function shouldExpectCommandErrors() { return true; } public function supportsReadDataFromStdin() { 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/ArcanistPyFlakesLinter.php b/src/lint/linter/ArcanistPyFlakesLinter.php index 7617bf16..19740a1e 100644 --- a/src/lint/linter/ArcanistPyFlakesLinter.php +++ b/src/lint/linter/ArcanistPyFlakesLinter.php @@ -1,103 +1,102 @@ getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.pyflakes.prefix'); - $bin = $config->getConfigFromAnySource('lint.pyflakes.bin', 'pyflakes'); + $prefix = $this->getDeprecatedConfiguration('lint.pyflakes.prefix'); + $bin = $this->getDeprecatedConfiguration('lint.pyflakes.bin', 'pyflakes'); if ($prefix) { return $prefix.'/'.$bin; } else { return $bin; } } public function getVersion() { list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); if (preg_match('/^(?P\d+\.\d+\.\d+)$/', $stdout, $matches)) { return $matches['version']; } else { return false; } } public function getInstallInstructions() { return pht('Install pyflakes with `pip install pyflakes`.'); } public function shouldExpectCommandErrors() { return true; } public function supportsReadDataFromStdin() { return true; } protected function parseLinterOutput($path, $err, $stdout, $stderr) { $lines = phutil_split_lines($stdout, 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); } $severity = ArcanistLintSeverity::SEVERITY_WARNING; $description = $matches[3]; $error_regexp = '/(^undefined|^duplicate|before assignment$)/'; if (preg_match($error_regexp, $description)) { $severity = ArcanistLintSeverity::SEVERITY_ERROR; } $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($matches[2]); $message->setCode($this->getLinterName()); $message->setDescription($description); $message->setSeverity($severity); $messages[] = $message; } if ($err && !$messages) { return false; } return $messages; } } diff --git a/src/lint/linter/ArcanistRubyLinter.php b/src/lint/linter/ArcanistRubyLinter.php index 739585d5..38c60702 100644 --- a/src/lint/linter/ArcanistRubyLinter.php +++ b/src/lint/linter/ArcanistRubyLinter.php @@ -1,108 +1,106 @@ getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.ruby.prefix'); + $prefix = $this->getDeprecatedConfiguration('lint.ruby.prefix'); if ($prefix !== null) { $ruby_bin = $prefix.'ruby'; } return 'ruby'; } public function getVersion() { list($stdout) = execx('%C --version', $this->getExecutableCommand()); $matches = array(); $regex = '/^ruby (?P\d+\.\d+\.\d+)p\d+/'; if (preg_match($regex, $stdout, $matches)) { return $matches['version']; } else { return false; } } 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 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; } } diff --git a/src/lint/linter/ArcanistScalaSBTLinter.php b/src/lint/linter/ArcanistScalaSBTLinter.php index 4963373a..5e24f358 100644 --- a/src/lint/linter/ArcanistScalaSBTLinter.php +++ b/src/lint/linter/ArcanistScalaSBTLinter.php @@ -1,91 +1,76 @@ getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.scala_sbt.prefix'); + $prefix = $this->getDeprecatedConfiguration('lint.scala_sbt.prefix'); if ($prefix !== null) { $sbt_bin = $prefix . $sbt_bin; } - if (!Filesystem::pathExists($sbt_bin)) { - - list($err) = exec_manual('which %s', $sbt_bin); - if ($err) { - throw new ArcanistUsageException( - "SBT does not appear to be installed on this system. Install it or ". - "add 'lint.scala_sbt.prefix' in your .arcconfig to point to ". - "the directory where it resides."); - } - } - return $sbt_bin; } private function getMessageCodeSeverity($type_of_error) { switch ($type_of_error) { case 'warn': return ArcanistLintSeverity::SEVERITY_WARNING; case 'error': return ArcanistLintSeverity::SEVERITY_ERROR; } } public function lintPath($path) { $sbt = $this->getSBTPath(); // Tell SBT to not use color codes so our regex life is easy. // TODO: Should this be "clean compile" instead of "compile"? $f = new ExecFuture("%s -Dsbt.log.noformat=true compile", $sbt); list($err, $stdout, $stderr) = $f->resolve(); $lines = explode("\n", $stdout); $messages = array(); foreach ($lines as $line) { $matches = null; if (!preg_match( "/\[(warn|error)\] (.*?):(\d+): (.*?)$/", $line, $matches)) { continue; } foreach ($matches as $key => $match) { $matches[$key] = trim($match); } $message = new ArcanistLintMessage(); $message->setPath($matches[2]); $message->setLine($matches[3]); $message->setCode($this->getLinterName()); $message->setDescription($matches[4]); $message->setSeverity($this->getMessageCodeSeverity($matches[1])); $this->addLintMessage($message); } } }