mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-12-23 05:50:54 +01:00
Upgrade C# linter to support linting multiple files at a time
Summary: This upgrades `cslint` to support linting multiple files at a time. As this required a backwards-incompatible to `cslint`, I've added a SUPPORTED_VERSION constant which can be used to detect these kinds of breaking changes in the future (and prompt users to upgrade the `cslint` they have in their repository). The reason for this upgrade is mainly around running `arc lint --everything`, where there are significant performance benefits gained when bulk linting lots of files per command execution. Test Plan: Upgraded `cslint` in the Tychaia repository and ran `arc lint --everything --trace`. Saw a substantially less number of executions happening for linting and all of the results came through as expected. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley CC: Korvin, epriestley, aran, waynea Differential Revision: https://secure.phabricator.com/D7599
This commit is contained in:
James Rhodes
epriestley
parent
19edbbb46e
commit
61753b6770
1 changed files with 116 additions and 52 deletions
|
|
@ -5,13 +5,16 @@
|
|||
*
|
||||
* @group linter
|
||||
*/
|
||||
final class ArcanistCSharpLinter extends ArcanistFutureLinter {
|
||||
final class ArcanistCSharpLinter extends ArcanistLinter {
|
||||
|
||||
private $runtimeEngine;
|
||||
private $cslintEngine;
|
||||
private $cslintHintPath;
|
||||
private $loaded;
|
||||
private $discoveryMap;
|
||||
private $futures;
|
||||
|
||||
const SUPPORTED_VERSION = 1;
|
||||
|
||||
public function getLinterName() {
|
||||
return 'C#';
|
||||
|
|
@ -101,74 +104,135 @@ final class ArcanistCSharpLinter extends ArcanistFutureLinter {
|
|||
throw new Exception("Unable to locate cslint.");
|
||||
}
|
||||
|
||||
// Determine cslint version.
|
||||
$ver_future = new ExecFuture(
|
||||
"%C -v",
|
||||
$this->runtimeEngine.$this->cslintEngine);
|
||||
list($err, $stdout, $stderr) = $ver_future->resolve();
|
||||
if ($err !== 0) {
|
||||
throw new Exception(
|
||||
"You are running an old version of cslint. Please ".
|
||||
"upgrade to version ".self::SUPPORTED_VERSION.".");
|
||||
}
|
||||
$ver = (int)$stdout;
|
||||
if ($ver < self::SUPPORTED_VERSION) {
|
||||
throw new Exception(
|
||||
"You are running an old version of cslint. Please ".
|
||||
"upgrade to version ".self::SUPPORTED_VERSION.".");
|
||||
} elseif ($ver > self::SUPPORTED_VERSION) {
|
||||
throw new Exception(
|
||||
"Arcanist does not support this version of cslint (it is ".
|
||||
"newer). You can try upgrading Arcanist with `arc upgrade`.");
|
||||
}
|
||||
|
||||
$this->loaded = true;
|
||||
}
|
||||
|
||||
protected function buildFutures(array $paths) {
|
||||
public function lintPath($path) {
|
||||
}
|
||||
|
||||
public function willLintPaths(array $paths) {
|
||||
$this->loadEnvironment();
|
||||
|
||||
$futures = array();
|
||||
|
||||
// Bulk linting up into futures, where the number of files
|
||||
// is based on how long the command is.
|
||||
$current_paths = array();
|
||||
foreach ($paths as $path) {
|
||||
// %s won't pass through the JSON correctly
|
||||
// under Windows. This is probably because not only
|
||||
// does the JSON have quotation marks in the content,
|
||||
// but because there'll be a lot of escaping and
|
||||
// double escaping because the JSON also contains
|
||||
// regular expressions. cslint supports passing the
|
||||
// settings JSON through base64-encoded to mitigate
|
||||
// this issue.
|
||||
$futures[$path] = new ExecFuture(
|
||||
"%C --settings-base64=%s -r=. %s",
|
||||
// If the current paths for the command, plus the next path
|
||||
// is greater than 6000 characters (less than the Windows
|
||||
// command line limit), then finalize this future and add it.
|
||||
$total = 0;
|
||||
foreach ($current_paths as $current_path) {
|
||||
$total += strlen($current_path) + 3; // Quotes and space.
|
||||
}
|
||||
if ($total + strlen($path) > 6000) {
|
||||
// %s won't pass through the JSON correctly
|
||||
// under Windows. This is probably because not only
|
||||
// does the JSON have quotation marks in the content,
|
||||
// but because there'll be a lot of escaping and
|
||||
// double escaping because the JSON also contains
|
||||
// regular expressions. cslint supports passing the
|
||||
// settings JSON through base64-encoded to mitigate
|
||||
// this issue.
|
||||
$futures[] = new ExecFuture(
|
||||
"%C --settings-base64=%s -r=. %Ls",
|
||||
$this->runtimeEngine.$this->cslintEngine,
|
||||
base64_encode(json_encode($this->discoveryMap)),
|
||||
$current_paths);
|
||||
$current_paths = array();
|
||||
}
|
||||
|
||||
// Append the path to the current paths array.
|
||||
$current_paths[] = $this->getEngine()->getFilePathOnDisk($path);
|
||||
}
|
||||
|
||||
// If we still have paths left in current paths, then we need to create
|
||||
// a future for those too.
|
||||
if (count($current_paths) > 0) {
|
||||
$futures[] = new ExecFuture(
|
||||
"%C --settings-base64=%s -r=. %Ls",
|
||||
$this->runtimeEngine.$this->cslintEngine,
|
||||
base64_encode(json_encode($this->discoveryMap)),
|
||||
$this->getEngine()->getFilePathOnDisk($path));
|
||||
$current_paths);
|
||||
$current_paths = array();
|
||||
}
|
||||
|
||||
return $futures;
|
||||
$this->futures = $futures;
|
||||
}
|
||||
|
||||
protected function resolveFuture($path, Future $future) {
|
||||
list($rc, $stdout) = $future->resolve();
|
||||
$results = json_decode($stdout);
|
||||
if ($results === null || $results->Issues === null) {
|
||||
return;
|
||||
public function didRunLinters() {
|
||||
if ($this->futures) {
|
||||
foreach (Futures($this->futures) as $future) {
|
||||
$this->resolveFuture($future);
|
||||
}
|
||||
}
|
||||
foreach ($results->Issues as $issue) {
|
||||
$message = new ArcanistLintMessage();
|
||||
$message->setPath($path);
|
||||
$message->setLine($issue->LineNumber);
|
||||
$message->setCode($issue->Index->Code);
|
||||
$message->setName($issue->Index->Name);
|
||||
$message->setChar($issue->Column);
|
||||
$message->setOriginalText($issue->OriginalText);
|
||||
$message->setReplacementText($issue->ReplacementText);
|
||||
$message->setDescription(
|
||||
vsprintf($issue->Index->Message, $issue->Parameters));
|
||||
$severity = ArcanistLintSeverity::SEVERITY_ADVICE;
|
||||
switch ($issue->Index->Severity) {
|
||||
case 0:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_ADVICE;
|
||||
break;
|
||||
case 1:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_AUTOFIX;
|
||||
break;
|
||||
case 2:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_WARNING;
|
||||
break;
|
||||
case 3:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_ERROR;
|
||||
break;
|
||||
case 4:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_DISABLED;
|
||||
break;
|
||||
}
|
||||
|
||||
protected function resolveFuture(Future $future) {
|
||||
list($stdout) = $future->resolvex();
|
||||
$all_results = json_decode($stdout);
|
||||
foreach ($all_results as $results) {
|
||||
if ($results === null || $results->Issues === null) {
|
||||
return;
|
||||
}
|
||||
$severity_override = $this->getLintMessageSeverity($issue->Index->Code);
|
||||
if ($severity_override !== null) {
|
||||
$severity = $severity_override;
|
||||
foreach ($results->Issues as $issue) {
|
||||
$message = new ArcanistLintMessage();
|
||||
$message->setPath($results->FileName);
|
||||
$message->setLine($issue->LineNumber);
|
||||
$message->setCode($issue->Index->Code);
|
||||
$message->setName($issue->Index->Name);
|
||||
$message->setChar($issue->Column);
|
||||
$message->setOriginalText($issue->OriginalText);
|
||||
$message->setReplacementText($issue->ReplacementText);
|
||||
$message->setDescription(
|
||||
vsprintf($issue->Index->Message, $issue->Parameters));
|
||||
$severity = ArcanistLintSeverity::SEVERITY_ADVICE;
|
||||
switch ($issue->Index->Severity) {
|
||||
case 0:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_ADVICE;
|
||||
break;
|
||||
case 1:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_AUTOFIX;
|
||||
break;
|
||||
case 2:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_WARNING;
|
||||
break;
|
||||
case 3:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_ERROR;
|
||||
break;
|
||||
case 4:
|
||||
$severity = ArcanistLintSeverity::SEVERITY_DISABLED;
|
||||
break;
|
||||
}
|
||||
$severity_override = $this->getLintMessageSeverity($issue->Index->Code);
|
||||
if ($severity_override !== null) {
|
||||
$severity = $severity_override;
|
||||
}
|
||||
$message->setSeverity($severity);
|
||||
$this->addLintMessage($message);
|
||||
}
|
||||
$message->setSeverity($severity);
|
||||
$this->addLintMessage($message);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue