1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 06:42:41 +01:00

Fix ArcanistSingleLintEngine for deleted paths

Summary:
Currently, ArcanisSingleLintEngine lints deleted paths and directories. These are sometimes appropriate, but SingleLintEngine is a less-sophisticated linter and should have more safe defaults.

Also fix an error where JSHint reported useless messages on failure.

Test Plan:
Reproduced the problem:

  $ git show
  commit d71efe2b13770c8861bcd3415c15503fc377339f
  Author: epriestley <git@epriestley.com>
  Date:   Fri Oct 19 12:22:50 2012 -0700

      WIP

  diff --git a/test.js b/test.js
  deleted file mode 100644
  index 8bd6648..0000000
  --- a/test.js
  +++ /dev/null
  @@ -1 +0,0 @@
  -asdf
  $ arc set-config --local lint.engine.single.linter ArcanistJSHintLinter
  Set key 'lint.engine.single.linter' = "ArcanistJSHintLinter" in local config (was null).
  $ arc lint --engine ArcanistSingleLintEngine --rev HEAD^
  Usage Exception: JSHint returned output we can't parse. Check that your JSHint installation.
  Output:

Applied the error message fix:

  $ arc lint --engine ArcanistSingleLintEngine --rev HEAD^
  Usage Exception: JSHint returned unparseable output.
  stdout:

  stderr:

  node.js:181
          throw e; // process.nextTick error, or 'error' event on first tick
          ^
  Error: ENOENT, No such file or directory '/INSECURE/repos/git-working-copy/test.js'
      at Object.statSync (fs.js:400:18)
      at _collect (/usr/lib/node_modules/jshint/lib/hint.js:77:12)
      at /usr/lib/node_modules/jshint/lib/hint.js:93:13
      at Array.forEach (native)
      at Object.hint (/usr/lib/node_modules/jshint/lib/hint.js:92:17)
      at Object.interpret (/usr/lib/node_modules/jshint/lib/cli.js:137:21)
      at Object.<anonymous> (/usr/lib/node_modules/jshint/bin/hint:2:25)
      at Module._compile (module.js:420:26)
      at Object..js (module.js:426:10)
      at Module.load (module.js:336:31)

Applied the remove paths fix:

  $ arc lint --engine ArcanistSingleLintEngine --rev HEAD^
  Usage Exception: No paths are lintable.

Reviewers: magazovski, btrahan, vrana

Reviewed By: btrahan

CC: aran, Korvin, vissi

Differential Revision: https://secure.phabricator.com/D3735
This commit is contained in:
epriestley 2012-10-20 06:12:24 -07:00
parent d09eb881a3
commit 3853483c64
2 changed files with 25 additions and 5 deletions

View file

@ -50,8 +50,28 @@ final class ArcanistSingleLintEngine extends ArcanistLintEngine {
"ArcanistLinter.");
}
// Filter the affected paths.
$paths = $this->getPaths();
foreach ($paths as $key => $path) {
if (!$this->pathExists($path)) {
// Don't lint removed files. In more complex linters it is sometimes
// appropriate to lint removed files so you can raise a warning like
// "you deleted X, but forgot to delete Y!", but most linters do not
// operate correctly on removed files.
unset($paths[$key]);
continue;
}
$disk = $this->getFilePathOnDisk($path);
if (is_dir($disk)) {
// Don't lint directories. (In SVN, they can be directly modified by
// changing properties on them, and may appear as modified paths.)
unset($paths[$key]);
continue;
}
}
$linter = newv($linter_name, array());
foreach ($this->getPaths() as $path) {
foreach ($paths as $path) {
$linter->addPath($path);
}

View file

@ -145,7 +145,7 @@ final class ArcanistJSHintLinter extends ArcanistLinter {
}
public function lintPath($path) {
list($rc, $stdout) = $this->results[$path];
list($rc, $stdout, $stderr) = $this->results[$path];
if ($rc === 0) {
return;
@ -155,9 +155,9 @@ final class ArcanistJSHintLinter extends ArcanistLinter {
if (!is_array($errors)) {
// Something went wrong and we can't decode the output. Exit abnormally.
throw new ArcanistUsageException(
"JSHint returned output we can't parse. Check that your JSHint installation.\n".
"Output:\n".
$stdout);
"JSHint returned unparseable output.\n".
"stdout:\n\n{$stdout}".
"stderr:\n\n{$stderr}");
}
foreach ($errors as $err) {