Changeset View
Standalone View
src/lint/linter/ArcanistGoVetLinter.php
- This file was added.
<?php | |||||
final class ArcanistGoVetLinter extends ArcanistExternalLinter { | |||||
public function getInfoName() { | |||||
return 'Go Vet'; | |||||
} | |||||
joshuaspence: Maybe `Go Vet` | |||||
public function getInfoURI() { | |||||
return 'https://godoc.org/golang.org/x/tools/cmd/vet'; | |||||
} | |||||
public function getInfoDescription() { | |||||
return pht( | |||||
'Vet examines Go source code and reports suspicious constructs.'); | |||||
} | |||||
public function getLinterName() { | |||||
return 'GOVET'; | |||||
} | |||||
public function getLinterConfigurationName() { | |||||
return 'govet'; | |||||
} | |||||
public function getDefaultBinary() { | |||||
$binary = 'go'; | |||||
if (Filesystem::binaryExists($binary)) { | |||||
// Vet is only accessible through 'go vet' or 'go tool vet' | |||||
// Let's manually try to find out if it's installed. | |||||
list($err, $stdout, $stderr) = exec_manual('go tool vet'); | |||||
if ($err === 3) { | |||||
throw new ArcanistMissingLinterException( | |||||
sprintf( | |||||
"%s\n%s", | |||||
pht( | |||||
'Unable to locate "go vet" to run linter %s. You may need '. | |||||
'to install the binary, or adjust your linter configuration.', | |||||
get_class($this)), | |||||
pht( | |||||
'TO INSTALL: %s', | |||||
$this->getInstallInstructions()))); | |||||
} | |||||
} | |||||
return $binary; | |||||
joshuaspenceUnsubmitted Not Done Inline ActionsI installed golang-go on Ubuntu 15 (via apt-get) and it included go tool vet... when would it not be bundled? Is it common for it to not be bundled? In any case, what would happen if getDefaultBinary() returned go and go was installed but not go tool vet? How would it fail? joshuaspence: I installed `golang-go` on Ubuntu 15 (via `apt-get`) and it included `go tool vet`... when… | |||||
eMxyzptlkAuthorUnsubmitted Not Done Inline ActionsPerhaps it is included by default for Ubuntu (or whichever distro you're on) but AFAIK it is not bundled with the official distribution. If it's not found go tool vet ... will fail with an error message and an exit code 3 $ go tool vet /tmp/test.go; echo $? go tool: no such tool "vet"; to install: go get golang.org/x/tools/cmd/vet 3 The actual linter will not fail but it will print an error for every file: Warning (GOVET) >>> 1 package main 2 3 import ( 4 "flag" >>> Lint for model/source.go: Not sure where the 'package main' came from, it should be model, but in any case, it fails in an unwanted behavior. eMxyzptlk: Perhaps it is included by default for Ubuntu (or whichever distro you're on) but AFAIK it is… | |||||
} | |||||
Not Done Inline ActionsThere's no nice way of handling this... I'd just return go as the default binary. joshuaspence: There's no nice way of handling this... I'd just return `go` as the default binary. | |||||
Not Done Inline ActionsHowever the presence of the Go binary does not indicate if GoVet is installed or not. Unfortunately, ArcanistExternalLinter::checkBinaryConfiguration is final so I can't overload it. eMxyzptlk: However the presence of the Go binary does not indicate if GoVet is installed or not. | |||||
public function getInstallInstructions() { | |||||
return pht( | |||||
'Install Go vet using `%s`.', | |||||
Done Inline ActionsThis would be better written as pht('Install Go vet using `%s`.', 'go get golang.org/x/tools/cmd/vet'). joshuaspence: This would be better written as `pht('Install Go vet using ```%s```.', 'go get golang. | |||||
'go get golang.org/x/tools/cmd/vet'); | |||||
} | |||||
protected function getMandatoryFlags() { | |||||
return array('tool', 'vet'); | |||||
} | |||||
public function getVersion() { | |||||
list($stdout) = execx('%C version', $this->getExecutableCommand()); | |||||
Done Inline ActionsThese are the default values, just omit these methods. joshuaspence: These are the default values, just omit these methods. | |||||
$pattern = '/^go version go(?P<version>\d+\.\d+\.\d+).*$/'; | |||||
$matches = array(); | |||||
Done Inline ActionsThis is PHP 5.4 syntax but we need to be compatible with PHP 5.2, so you'll need to change this to array('tool', 'vet') joshuaspence: This is PHP 5.4 syntax but we need to be compatible with PHP 5.2, so you'll need to change this… | |||||
if (preg_match($pattern, $stdout, $matches)) { | |||||
return $matches['version']; | |||||
} else { | |||||
return false; | |||||
} | |||||
} | |||||
protected function parseLinterOutput($path, $err, $stdout, $stderr) { | |||||
$lines = phutil_split_lines($stderr, false); | |||||
$messages = array(); | |||||
foreach ($lines as $line) { | |||||
$matches = explode(':', $line, 3); | |||||
if (count($matches) === 3) { | |||||
joshuaspenceUnsubmitted Not Done Inline ActionsI think it should be configurable. What you think is a warning, others may say is advice. joshuaspence: I think it should be configurable. What you think is a warning, others may say is advice. | |||||
eMxyzptlkAuthorUnsubmitted Not Done Inline ActionsAlright I'll get it done tonight and update the differential. eMxyzptlk: Alright I'll get it done tonight and update the differential. | |||||
$message = new ArcanistLintMessage(); | |||||
Not Done Inline ActionsYou should maybe use $this->getLintMessageSeverity($code) here to allow the severity to be configurable. joshuaspence: You should maybe use `$this->getLintMessageSeverity($code)` here to allow the severity to be… | |||||
Not Done Inline ActionsSo GoVet does not report errors, it only report a possibility of a problem, for instance, running go tool vet on the include test src/lint/linter/__tests__/govet/fmt.lint-test reports /tmp/test.go:6: possible formatting directive in Println call which may or may not be a real problem. Given that, I think we will never report errors but only warnings. Should I still make it configurable? eMxyzptlk: So GoVet does not report errors, it only report a possibility of a problem, for instance… | |||||
$message->setPath($path); | |||||
$message->setLine($matches[1]); | |||||
$message->setCode($this->getLinterName()); | |||||
$message->setDescription(ucfirst(trim($matches[2]))); | |||||
$message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING); | |||||
$messages[] = $message; | |||||
} | |||||
} | |||||
return $messages; | |||||
} | |||||
} |
Maybe Go Vet