Page MenuHomePhabricator
Files F15425811

D8965.id21331.diff
No OneTemporary
Actions

D8965.id21331.diff
View Options

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
@@ -83,6 +83,7 @@
'ArcanistInlinesWorkflow' => 'workflow/ArcanistInlinesWorkflow.php',
'ArcanistInstallCertificateWorkflow' => 'workflow/ArcanistInstallCertificateWorkflow.php',
'ArcanistJSHintLinter' => 'lint/linter/ArcanistJSHintLinter.php',
+ 'ArcanistJSHintLinterTestCase' => 'lint/linter/__tests__/ArcanistJSHintLinterTestCase.php',
'ArcanistLandWorkflow' => 'workflow/ArcanistLandWorkflow.php',
'ArcanistLiberateWorkflow' => 'workflow/ArcanistLiberateWorkflow.php',
'ArcanistLintConsoleRenderer' => 'lint/renderer/ArcanistLintConsoleRenderer.php',
@@ -241,7 +242,8 @@
'ArcanistHgServerChannel' => 'PhutilProtocolChannel',
'ArcanistInlinesWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistInstallCertificateWorkflow' => 'ArcanistBaseWorkflow',
- 'ArcanistJSHintLinter' => 'ArcanistLinter',
+ 'ArcanistJSHintLinter' => 'ArcanistExternalLinter',
+ 'ArcanistJSHintLinterTestCase' => 'ArcanistArcanistLinterTestCase',
'ArcanistLandWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistLiberateWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistLintConsoleRenderer' => 'ArcanistLintRenderer',
diff --git a/src/lint/linter/ArcanistJSHintLinter.php b/src/lint/linter/ArcanistJSHintLinter.php
--- a/src/lint/linter/ArcanistJSHintLinter.php
+++ b/src/lint/linter/ArcanistJSHintLinter.php
@@ -1,44 +1,11 @@
<?php
/**
- * Uses "JSHint" to detect errors and potential problems in JavaScript code.
- * To use this linter, you must install jshint through NPM (Node Package
- * Manager). You can configure different JSHint options on a per-file basis.
- *
- * If you have NodeJS installed you should be able to install jshint with
- * ##npm install jshint -g## (don't forget the -g flag or NPM will install
- * the package locally). If your system is unusual, you can manually specify
- * the location of jshint and its dependencies by configuring these keys in
- * your .arcconfig:
- *
- * lint.jshint.prefix
- * lint.jshint.bin
- *
- * If you want to configure custom options for your project, create a JSON
- * file with these options and add the path to the file to your .arcconfig
- * by configuring this key:
- *
- * lint.jshint.config
- *
- * Example JSON file (config.json):
- *
- * {
- * "predef": [ // Custom globals
- * "myGlobalVariable",
- * "anotherGlobalVariable"
- * ],
- *
- * "es5": true, // Allow ES5 syntax
- * "strict": true // Require strict mode
- * }
- *
- * For more options see http://www.jshint.com/options/.
+ * Uses JSHint to detect errors and potential problems in JavaScript code.
*
* @group linter
*/
-final class ArcanistJSHintLinter extends ArcanistLinter {
-
- const JSHINT_ERROR = 1;
+final class ArcanistJSHintLinter extends ArcanistExternalLinter {
public function getLinterName() {
return 'JSHint';
@@ -48,116 +15,79 @@
return 'jshint';
}
- public function getLintSeverityMap() {
- return array(
- self::JSHINT_ERROR => ArcanistLintSeverity::SEVERITY_ERROR
- );
+ protected function getDefaultMessageSeverity($code) {
+ if (preg_match('/^W/', $code)) {
+ return ArcanistLintSeverity::SEVERITY_WARNING;
+ } else {
+ return ArcanistLintSeverity::SEVERITY_ERROR;
+ }
}
- // placeholder if/until we get a map code -> name map
- // jshint only offers code -> description right now (parsed as 'reason')
- public function getLintMessageName($code) {
- return "JSHint".$code;
- }
+ public function getDefaultBinary() {
+ $config = $this->getEngine()->getConfigurationManager();
+ $prefix = $config->getConfigFromAnySource('lint.jshint.prefix');
+ $bin = $config->getConfigFromAnySource('lint.jshint.bin', 'jshint');
- public function getLintNameMap() {
- return array(
- self::JSHINT_ERROR => "JSHint Error"
- );
+ if ($prefix) {
+ return $prefix.'/'.$bin;
+ } else {
+ return $bin;
+ }
}
- public function getJSHintOptions() {
- $config_manager = $this->getEngine()->getConfigurationManager();
- $options = '--reporter '.dirname(realpath(__FILE__)).'/reporter.js';
- $config = $config_manager->getConfigFromAnySource('lint.jshint.config');
-
- $working_copy = $this->getEngine()->getWorkingCopy();
- if ($config !== null) {
- $config = Filesystem::resolvePath(
- $config,
- $working_copy->getProjectRoot());
+ public function getCacheVersion() {
+ list($stdout) = execx('%C --version', $this->getExecutableCommand());
- if (!Filesystem::pathExists($config)) {
- throw new ArcanistUsageException(
- "Unable to find custom options file defined by ".
- "'lint.jshint.config'. Make sure that the path is correct.");
- }
-
- $options .= ' --config '.$config;
+ // Extract version number from standard output.
+ $matches = array();
+ if (preg_match('/^jshint v(\d+\.\d+\.\d+)$/', $stdout, $matches)) {
+ $version = $matches[1];
+ } else {
+ $version = md5($stdout);
}
- return $options;
+ return $version . '-' . md5(json_encode($this->getCommandFlags()));
}
- private function getJSHintPath() {
- $config_manager = $this->getEngine()->getConfigurationManager();
- $prefix = $config_manager->getConfigFromAnySource('lint.jshint.prefix');
- $bin = $config_manager->getConfigFromAnySource('lint.jshint.bin');
-
- if ($bin === null) {
- $bin = "jshint";
- }
-
- if ($prefix !== null) {
- $bin = $prefix."/".$bin;
+ public function getInstallInstructions() {
+ return pht('Install JSHint using `npm install -g jshint`.');
+ }
- if (!Filesystem::pathExists($bin)) {
- throw new ArcanistUsageException(
- "Unable to find JSHint binary in a specified directory. Make sure ".
- "that 'lint.jshint.prefix' and 'lint.jshint.bin' keys are set ".
- "correctly. If you'd rather use a copy of JSHint installed ".
- "globally, you can just remove these keys from your .arcconfig");
- }
+ public function shouldExpectCommandErrors() {
+ return true;
+ }
- return $bin;
- }
+ public function supportsReadDataFromStdin() {
+ return true;
+ }
- if (!Filesystem::binaryExists($bin)) {
- throw new ArcanistUsageException(
- "JSHint does not appear to be installed on this system. Install it ".
- "(e.g., with 'npm install jshint -g') or configure ".
- "'lint.jshint.prefix' in your .arcconfig to point to the directory ".
- "where it resides.");
- }
+ public function getReadDataFromStdinFilename() {
+ return '-';
+ }
- return $bin;
+ public function getMandatoryFlags() {
+ return array(
+ '--reporter='.dirname(realpath(__FILE__)).'/reporter.js',
+ );
}
- public function willLintPaths(array $paths) {
- if (!$this->isCodeEnabled(self::JSHINT_ERROR)) {
- return;
- }
+ public function getDefaultFlags() {
+ $config_manager = $this->getEngine()->getConfigurationManager();
+ $options = $config_manager->getConfigFromAnySource(
+ 'lint.jshint.options',
+ array());
- $jshint_bin = $this->getJSHintPath();
- $jshint_options = $this->getJSHintOptions();
- $futures = array();
-
- foreach ($paths as $path) {
- $filepath = $this->getEngine()->getFilePathOnDisk($path);
- $futures[$path] = new ExecFuture(
- "%s %s %C",
- $jshint_bin,
- $filepath,
- $jshint_options);
+ $config = $config_manager->getConfigFromAnySource('lint.jshint.config');
+ if ($config) {
+ $options[] = '--config='.$config;
}
- foreach (Futures($futures)->limit(8) as $path => $future) {
- $this->results[$path] = $future->resolve();
- }
+ return $options;
}
- public function lintPath($path) {
- if (!$this->isCodeEnabled(self::JSHINT_ERROR)) {
- return;
- }
-
- list($rc, $stdout, $stderr) = $this->results[$path];
-
- if ($rc === 0) {
- return;
- }
-
+ protected function parseLinterOutput($path, $err, $stdout, $stderr) {
$errors = json_decode($stdout);
+
if (!is_array($errors)) {
// Something went wrong and we can't decode the output. Exit abnormally.
throw new ArcanistUsageException(
@@ -166,12 +96,35 @@
"stderr:\n\n{$stderr}");
}
+ $messages = array();
foreach ($errors as $err) {
- $this->raiseLintAtLine(
- $err->line,
- $err->col,
- $err->code,
- $err->reason);
+ $message = new ArcanistLintMessage();
+ $message->setPath($path);
+ $message->setLine($err->line);
+ $message->setChar($err->col);
+ $message->setCode($err->code);
+ $message->setName('JSHint'.$err->code);
+ $message->setDescription($err->reason);
+ $message->setSeverity($this->getLintMessageSeverity($err->code));
+ $message->setOriginalText($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/__tests__/ArcanistJSHintLinterTestCase.php b/src/lint/linter/__tests__/ArcanistJSHintLinterTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/ArcanistJSHintLinterTestCase.php
@@ -0,0 +1,12 @@
+<?php
+
+final class ArcanistJSHintLinterTestCase
+ extends ArcanistArcanistLinterTestCase {
+
+ public function testJSHintLinter() {
+ $this->executeTestsInDirectory(
+ dirname(__FILE__).'/jshint/',
+ new ArcanistJSHintLinter());
+ }
+
+}
diff --git a/src/lint/linter/__tests__/jshint/dot-notation.lint-test b/src/lint/linter/__tests__/jshint/dot-notation.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/jshint/dot-notation.lint-test
@@ -0,0 +1,6 @@
+var args = {};
+args['foo'] = 'bar';
+args['bar'] = 'baz';
+~~~~~~~~~~
+warning:2:5
+warning:3:5
diff --git a/src/lint/linter/__tests__/jshint/expected-conditional.lint-test b/src/lint/linter/__tests__/jshint/expected-conditional.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/jshint/expected-conditional.lint-test
@@ -0,0 +1,6 @@
+var foo;
+if (foo = 'bar') {
+ return true;
+}
+~~~~~~~~~~
+warning:2:16
diff --git a/src/lint/linter/__tests__/jshint/missing-semicolon.lint-test b/src/lint/linter/__tests__/jshint/missing-semicolon.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/jshint/missing-semicolon.lint-test
@@ -0,0 +1,3 @@
+console.log('foobar')
+~~~~~~~~~~
+warning:1:22
diff --git a/src/lint/linter/__tests__/jshint/unnecessary-semicolon.lint-test b/src/lint/linter/__tests__/jshint/unnecessary-semicolon.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/jshint/unnecessary-semicolon.lint-test
@@ -0,0 +1,5 @@
+function main() {
+ return 'Hello, World!';
+};
+~~~~~~~~~~
+warning:3:2
diff --git a/src/lint/linter/reporter.js b/src/lint/linter/reporter.js
--- a/src/lint/linter/reporter.js
+++ b/src/lint/linter/reporter.js
@@ -5,11 +5,12 @@
results.forEach(function (result) {
var error = result.error;
report.push({
- 'file' : result.file,
- 'line' : error.line,
- 'col' : error.character,
- 'reason': error.reason,
- 'code' : error.code,
+ 'file' : result.file,
+ 'line' : error.line,
+ 'col' : error.character,
+ 'reason' : error.reason,
+ 'code' : error.code,
+ 'evidence': error.evidence,
});
});

File Metadata

Mime Type
text/plain
Expires
Mon, Mar 24, 6:15 AM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7636032
Default Alt Text
D8965.id21331.diff (11 KB)

Event Timeline

Phabricator · Built by Phacility