diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -53,6 +53,7 @@ 'ArcanistCapabilityNotSupportedException' => 'workflow/exception/ArcanistCapabilityNotSupportedException.php', 'ArcanistCastSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCastSpacingXHPASTLinterRule.php', 'ArcanistCastSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistCastSpacingXHPASTLinterRuleTestCase.php', + 'ArcanistCheckstyleLinter' => 'lint/linter/ArcanistCheckstyleLinter.php', 'ArcanistCheckstyleXMLLintRenderer' => 'lint/renderer/ArcanistCheckstyleXMLLintRenderer.php', 'ArcanistChmodLinter' => 'lint/linter/ArcanistChmodLinter.php', 'ArcanistChmodLinterTestCase' => 'lint/linter/__tests__/ArcanistChmodLinterTestCase.php', @@ -451,6 +452,7 @@ 'ArcanistCapabilityNotSupportedException' => 'Exception', 'ArcanistCastSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistCastSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistCheckstyleLinter' => 'ArcanistExternalLinter', 'ArcanistCheckstyleXMLLintRenderer' => 'ArcanistLintRenderer', 'ArcanistChmodLinter' => 'ArcanistLinter', 'ArcanistChmodLinterTestCase' => 'ArcanistLinterTestCase', diff --git a/src/lint/linter/ArcanistCheckstyleLinter.php b/src/lint/linter/ArcanistCheckstyleLinter.php new file mode 100644 --- /dev/null +++ b/src/lint/linter/ArcanistCheckstyleLinter.php @@ -0,0 +1,176 @@ + array( + 'type' => 'optional bool', + 'help' => pht( + 'Lint messages reported by checkstyle indicate their source '. + 'as the fully-qualified classname of the respective module. '. + 'Set this value to true to simplify the classname, such as: '. + '`com.company.pkg.IndentationCheck` => `IndentationCheck`.'), + ), + ); + + return $options + parent::getLinterConfigurationOptions(); + } + + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'checkstyle.simplify-source-classname': + $this->setSimplifySourceClassname($value); + return; + } + + return parent::setLinterConfigurationValue($key, $value); + } + + public function setSimplifySourceClassname($simplify_source_classname) { + $this->simplifySourceClassname = $simplify_source_classname; + return $this; + } + + public function getVersion() { + list($stdout) = execx('%C -v', $this->getExecutableCommand()); + + $matches = array(); + $regex = '/^Checkstyle version: (?P\d+\.\d+)$/'; + if (preg_match($regex, $stdout, $matches)) { + return $matches['version']; + } + return false; + } + + public function getInstallInstructions() { + return pht('Ensure java is configured as interpreter and '. + 'the checkstyle jar is configured as the binary. If you need to pass '. + 'additional additional JVM arguments include them with the '. + '`interpreter` field. Use the `flags` field to configure checkstyle '. + 'arguments, including the `-c my_styles.xml` for the styles to verify.'); + } + + protected function getMandatoryFlags() { + return array('-f', 'xml'); + } + + public function shouldExpectCommandErrors() { + return false; + } + + public function shouldUseInterpreter() { + return true; + } + + public function getDefaultBinary() { + return 'checkstyle.jar'; + } + + public function getDefaultInterpreter() { + return 'java'; + } + + protected function getMandatoryInterpreterFlags() { + // since the binary is the checkstyle jar, this flag must + // always be passed to the interpreter, and is guaranteed to be provided + // as the first flag before the binary jar on the command line + return array('-jar'); + } + + protected function parseLinterOutput($path, $err, $stdout, $stderr) { + $dom = new DOMDocument(); + $ok = @$dom->loadXML($stdout); + + if (!$ok) { + return false; + } + + $files = $dom->getElementsByTagName('file'); + $messages = array(); + foreach ($files as $file) { + $errors = $file->getElementsByTagName('error'); + foreach ($errors as $error) { + $message = new ArcanistLintMessage(); + $message->setPath($file->getAttribute('name')); + $message->setLine($error->getAttribute('line')); + $message->setCode($this->getLinterName()); + + // source is the module's fully-qualified classname + // attempt to simplify it for readability + $source = $error->getAttribute('source'); + if ($this->simplifySourceClassname) { + $source = idx(array_slice(explode('.', $source), -1), 0); + } + $message->setName($source); + + // checkstyle's XMLLogger escapes these five characters + $description = $error->getAttribute('message'); + $description = str_replace( + ['<', '>', ''', '"', '&'], + ['<', '>', '\'', '"', '&'], + $description); + $message->setDescription($description); + + $column = $error->getAttribute('column'); + if ($column) { + $message->setChar($column); + } + + $severity = $error->getAttribute('severity'); + switch ($severity) { + case 'error': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); + break; + case 'warning': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING); + break; + case 'info': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE); + break; + case 'ignore': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_DISABLED); + break; + + // The above four are the only valid checkstyle severities, + // this is for completion as well as preparing for future severities + default: + $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING); + break; + } + + $messages[] = $message; + } + } + + return $messages; + } +} diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php --- a/src/lint/linter/ArcanistExternalLinter.php +++ b/src/lint/linter/ArcanistExternalLinter.php @@ -14,6 +14,7 @@ private $interpreter; private $flags; private $versionRequirement; + private $interpreterFlags; /* -( Interpreters, Binaries and Flags )----------------------------------- */ @@ -197,6 +198,52 @@ return $this; } + /** + * Return array of flags needed by the interpreter, such as "-jar" when + * using java as the interpreter. This method is only invoked if + * @{method:shouldUseInterpreter} has been overridden to return 'true'. + * + * @return array Flags to pass to interpreter + * @task bin + */ + protected function getDefaultInterpreterFlags() { + return array(); + } + + /** + * Provide mandatory, non-overridable flags to the linter interpreter. + * Generally these are arguments to configure invocation of the interpreter + * such as (assuming Java interpreter) `-Xmx1024m`, `-Dproperty=value`, or + * in the case of the binary being a jar file, the last one should be `-jar`. + * + * Due to the specific case of using java as interpreter and a jarfile as + * the binary, the `-jar` argument must appear last in the list of all + * flags passed to interpreter. For this, manadatory flags are ensured to + * always be listed before any default or user overridden flags. + * + * Flags which are not mandatory should be provided in + * @{method:getDefaultInterpreterFlags} instead. + * + * @return list Mandatory flags, like `"-jar"`. + * @task bin + */ + protected function getMandatoryInterpreterFlags() { + return array(); + } + + /** + * Override default interpreter flags with custom flags. If not overridden, + * flags provided by @{method:getDefaultInterpreterFlags} are used. + * + * @param list New flags for the interpreter. + * @return this + * @task bin + */ + final public function setInterpreterFlags(array $interpreter_flags) { + $this->interpreterFlags = $interpreter_flags; + return $this; + } + /* -( Parsing Linter Output )---------------------------------------------- */ @@ -351,14 +398,20 @@ $this->checkBinaryConfiguration(); $interpreter = null; + $interpreter_flags = array(); if ($this->shouldUseInterpreter()) { $interpreter = $this->getInterpreter(); + $interpreter_flags = $this->getInterpreterFlags(); } $binary = $this->getBinary(); if ($interpreter) { - $bin = csprintf('%s %s', $interpreter, $binary); + if (!empty($interpreter_flags)) { + $bin = csprintf('%s %Ls %s', $interpreter, $interpreter_flags, $binary); + } else { + $bin = csprintf('%s %s', $interpreter, $binary); + } } else { $bin = csprintf('%s', $binary); } @@ -379,6 +432,23 @@ nonempty($this->flags, $this->getDefaultFlags())); } + /** + * Get the composed flags for the interpreter, including both mandatory and + * configured flags. + * + * @return list Composed interpreter flags. + * @task exec + */ + final protected function getInterpreterFlags() { + // Note that this forces the mandatory flags to appear before any other + // flags provided through default or overridden. This is to handle the + // the case of java interpreter needing to combine the `-jar` flag followed + // immediately by the binary which would be the jar file. + return array_merge( + nonempty($this->interpreterFlags, $this->getDefaultInterpreterFlags()), + $this->getMandatoryInterpreterFlags()); + } + public function getCacheVersion() { try { $this->checkBinaryConfiguration(); @@ -495,6 +565,13 @@ 'a list of possible interpreters, the first one that exists '. 'will be used.'), ); + + $options['interpreter.flags'] = array( + 'type' => 'optional list', + 'help' => pht( + 'Provide a list of additional flags to pass to the interpreter on '. + 'the command line.'), + ); } return $options + parent::getLinterConfigurationOptions(); @@ -545,6 +622,9 @@ case 'flags': $this->setFlags($value); return; + case 'interpreter.flags': + $this->setInterpreterFlags($value); + return; case 'version': $this->setVersionRequirement($value); return;