Add debug command and show nice position errors

This does two things:
* Add a debug command showing several bits of information about the
  current setup
* Handle invalid points from JSHint in a much better manner, translating
  them into a message on the current file with a link to report the
  issue.

decls/atom-linter.js

@@ -22,7 +22,7 @@ declare module 'atom-linter' {
   declare var findCachedAsync:
     (directory: string, names: string | Array<string>) => Promise<?string>;
   declare var execNode:
-    (filePath: string, args: Array<string>, options: ExecOptions) => Promise<string>;
+    (filePath: string, args: Array<string>, options?: ExecOptions) => Promise<string>;
   declare var generateRange:
     (textEditor: TextEditor, lineNumber?: number, colStart?: number) =>
       Array<Array<number>>

lib/helpers.js

@@ -0,0 +1,123 @@
+'use babel';
+
+import path from 'path';
+import * as atomlinter from 'atom-linter';
+import escapeHTML from 'escape-html';
+import { readFile as fsReadFile } from 'fs';
+// eslint-disable-next-line import/extensions, import/no-extraneous-dependencies
+import type { TextEditor } from 'atom';
+
+async function readFile(filePath) {
+  return new Promise((resolve, reject) => {
+    fsReadFile(filePath, 'utf8', (err, data) => {
+      if (err) {
+        reject(err);
+      }
+      resolve(data);
+    });
+  });
+}
+
+export async function readIgnoreList(ignorePath) {
+  return (await readFile(ignorePath)).split(/[\r\n]/);
+}
+
+export async function getDebugInfo() {
+  const textEditor = atom.workspace.getActiveTextEditor();
+  let editorScopes;
+  if (atom.workspace.isTextEditor(textEditor)) {
+    editorScopes = textEditor.getLastCursor().getScopeDescriptor().getScopesArray();
+  } else {
+    // Somehow this can be called with no active TextEditor, impossible I know...
+    editorScopes = ['unknown'];
+  }
+
+  const packagePath = atom.packages.resolvePackagePath('linter-jshint');
+  let linterJSHintMeta;
+  if (packagePath === undefined) {
+    // Apparently for some users the package path fails to resolve
+    linterJSHintMeta = { version: 'unknown!' };
+  } else {
+    // eslint-disable-next-line import/no-dynamic-require
+    const metaPath = path.join(packagePath, 'package.json');
+    linterJSHintMeta = JSON.parse(await readFile(metaPath));
+  }
+
+  const config = atom.config.get('linter-jshint');
+  const hoursSinceRestart = Math.round((process.uptime() / 3600) * 10) / 10;
+  // NOTE: Yes, `jshint --version` gets output on STDERR...
+  const jshintVersion = await atomlinter.execNode(
+      config.executablePath, ['--version'], { stream: 'stderr' });
+
+  const returnVal = {
+    atomVersion: atom.getVersion(),
+    linterJSHintVersion: linterJSHintMeta.version,
+    linterJSHintConfig: config,
+    // eslint-disable-next-line import/no-dynamic-require
+    jshintVersion,
+    hoursSinceRestart,
+    platform: process.platform,
+    editorScopes,
+  };
+  return returnVal;
+}
+
+export async function generateDebugString() {
+  const debug = await getDebugInfo();
+  const details = [
+    `Atom version: ${debug.atomVersion}`,
+    `linter-jshint version: ${debug.linterJSHintVersion}`,
+    `JSHint version: ${debug.jshintVersion}`,
+    `Hours since last Atom restart: ${debug.hoursSinceRestart}`,
+    `Platform: ${debug.platform}`,
+    `Current file's scopes: ${JSON.stringify(debug.editorScopes, null, 2)}`,
+    `linter-jshint configuration: ${JSON.stringify(debug.linterJSHintConfig, null, 2)}`,
+  ];
+  return details.join('\n');
+}
+
+export async function generateInvalidTrace(
+  msgLine: number, msgCol: number, filePath: string, textEditor: TextEditor,
+  error: Object,
+) {
+  const errMsgRange = `${msgLine + 1}:${msgCol}`;
+  const rangeText = `Requested start point: ${errMsgRange}`;
+  const issueURL = 'https://github.com/AtomLinter/linter-eslint/issues/new';
+  const titleText = `Invalid position given by '${error.code}'`;
+  const title = encodeURIComponent(titleText);
+  const body = encodeURIComponent([
+    'JSHint returned a point that did not exist in the document being edited.',
+    `Rule: \`${error.code}\``,
+    rangeText,
+    '', '',
+    '<!-- If at all possible, please include code to reproduce this issue! -->',
+    '', '',
+    'Debug information:',
+    '```json',
+    JSON.stringify(await getDebugInfo(), null, 2),
+    '```',
+  ].join('\n'));
+  const newIssueURL = `${issueURL}?title=${title}&body=${body}`;
+  return {
+    type: 'Error',
+    severity: 'error',
+    html: `${escapeHTML(titleText)}. See the trace for details. ` +
+      `<a href="${newIssueURL}">Report this!</a>`,
+    filePath,
+    range: atomlinter.generateRange(textEditor, 0),
+    trace: [
+      {
+        type: 'Trace',
+        text: `Original message: ${error.code} - ${error.reason}`,
+        filePath,
+        severity: 'info',
+      },
+      {
+        type: 'Trace',
+        text: rangeText,
+        filePath,
+        severity: 'info',
+      },
+    ],
+  };
+}

lib/main.js

@@ -3,23 +3,15 @@
 /* @flow */
 
 import Path from 'path';
-import { readFile } from 'fs';
 import minimatch from 'minimatch';
+import * as atomlinter from 'atom-linter';
 /* eslint-disable import/extensions, import/no-extraneous-dependencies */
 import { CompositeDisposable } from 'atom';
 import type { TextEditor } from 'atom';
 /* eslint-enable import/extensions, import/no-extraneous-dependencies */
+import * as helpers from './helpers';
 
-async function readIgnoreList(ignorePath) {
-  return new Promise((resolve, reject) => {
-    readFile(ignorePath, 'utf8', (err, data) => {
-      if (err) {
-        reject(err);
-      }
-      resolve(data.split(/[\r\n]/));
-    });
-  });
-}
+let Reporter;
 
 module.exports = {
   config: {
@@ -101,16 +93,23 @@ module.exports = {
         }
       }),
     );
+
+    this.subscriptions.add(
+      atom.commands.add('atom-text-editor', {
+        'linter-jshint:debug': async () => {
+          const debugString = await helpers.generateDebugString();
+          const notificationOptions = { detail: debugString, dismissable: true };
+          atom.notifications.addInfo('linter-jshint:: Debugging information', notificationOptions);
+        },
+      }),
+    );
   },
 
   deactivate() {
     this.subscriptions.dispose();
   },
 
   provideLinter() {
-    const Helpers = require('atom-linter');
-    const Reporter = require('jshint-json');
-
     return {
       name: 'JSHint',
       grammarScopes: this.scopes,
@@ -121,22 +120,25 @@ module.exports = {
         const filePath = textEditor.getPath();
         const fileDir = Path.dirname(filePath);
         const fileContents = textEditor.getText();
+        if (!Reporter) {
+          Reporter = require('jshint-json');
+        }
         const parameters = ['--reporter', Reporter, '--filename', filePath];
 
-        const configFile = await Helpers.findCachedAsync(fileDir, this.jshintFileName);
+        const configFile = await atomlinter.findCachedAsync(fileDir, this.jshintFileName);
 
         if (configFile) {
           parameters.push('--config', configFile);
         } else if (this.disableWhenNoJshintrcFileInPath) {
           return results;
         }
 
-        const ignoreFile = await Helpers.findCachedAsync(fileDir, this.jshintignoreFilename);
+        const ignoreFile = await atomlinter.findCachedAsync(fileDir, this.jshintignoreFilename);
 
         if (ignoreFile) {
           // JSHint completely ignores .jshintignore files for STDIN on it's own
           // so we must re-implement the functionality
-          const ignoreList = await readIgnoreList(ignoreFile);
+          const ignoreList = await helpers.readIgnoreList(ignoreFile);
           if (ignoreList.some(pattern => minimatch(filePath, pattern))) {
             // The file is ignored by one of the patterns
             return [];
@@ -155,7 +157,7 @@ module.exports = {
           ignoreExitCode: true,
           cwd: fileDir,
         };
-        const result = await Helpers.execNode(
+        const result = await atomlinter.execNode(
           this.executablePath, parameters, execOpts,
         );
 
@@ -176,7 +178,8 @@ module.exports = {
           return results;
         }
 
-        Object.keys(parsed.result).forEach((entryID) => {
+        Object.keys(parsed.result).forEach(async (entryID) => {
+          let message;
           const entry = parsed.result[entryID];
 
           if (!entry.error.id) {
@@ -193,14 +196,21 @@ module.exports = {
           }
           const line = error.line > 0 ? error.line - 1 : 0;
           const character = error.character > 0 ? error.character - 1 : 0;
-          const range = Helpers.generateRange(textEditor, line, character);
-
-          results.push({
-            type,
-            text: `${error.code} - ${error.reason}`,
-            filePath,
-            range,
-          });
+          let range;
+          try {
+            range = atomlinter.generateRange(textEditor, line, character);
+            message = {
+              type,
+              text: `${error.code} - ${error.reason}`,
+              filePath,
+              range,
+            };
+          } catch (e) {
+            message = await helpers.generateInvalidTrace(
+              line, character, filePath, textEditor, error);
+          }
+
+          results.push(message);
         });
         return results;
       },

package.json

@@ -15,6 +15,7 @@
   "dependencies": {
     "atom-linter": "^10.0.0",
     "atom-package-deps": "^4.0.1",
+    "escape-html": "^1.0.3",
     "jshint": "2.9.4",
     "jshint-json": "^1.0.0",
     "minimatch": "^3.0.3"

spec/linter-jshint-spec.js

@@ -5,8 +5,28 @@ import { it, fit, wait, beforeEach, afterEach } from 'jasmine-fix';
 import * as path from 'path';
 import linter from '../lib/main';
 
+const goodPath = path.join(__dirname, 'fixtures', 'good.js');
 const bitwisePath = path.join(__dirname, 'fixtures', 'bitwise', 'bitwise.js');
 
+async function getNotification(expectedMessage) {
+  return new Promise((resolve) => {
+    let notificationSub;
+    const newNotification = (notification) => {
+      if (notification.getMessage() !== expectedMessage) {
+        // As the specs execute asynchronously, it's possible a notification
+        // from a different spec was grabbed, if the message doesn't match what
+        // is expected simply return and keep waiting for the next message.
+        return;
+      }
+      // Dispose of the notificaiton subscription
+      notificationSub.dispose();
+      resolve(notification);
+    };
+    // Subscribe to Atom's notifications
+    notificationSub = atom.notifications.onDidAddNotification(newNotification);
+  });
+}
+
 describe('The JSHint provider for Linter', () => {
   const lint = linter.provideLinter().lint;
 
@@ -50,7 +70,6 @@ describe('The JSHint provider for Linter', () => {
   });
 
   it('finds nothing wrong with a valid file', async () => {
-    const goodPath = path.join(__dirname, 'fixtures', 'good.js');
     const editor = await atom.workspace.open(goodPath);
     const messages = await lint(editor);
     expect(messages.length).toBe(0);
@@ -97,4 +116,31 @@ describe('The JSHint provider for Linter', () => {
       expect(ignoreMessages.length).toBe(0);
     });
   });
+
+  describe('prints debugging information with the `debug` command', () => {
+    let editor;
+    const expectedMessage = 'linter-jshint:: Debugging information';
+    beforeEach(async () => {
+      editor = await atom.workspace.open(goodPath);
+    });
+
+    it('shows an info notification', async () => {
+      atom.commands.dispatch(atom.views.getView(editor), 'linter-jshint:debug');
+      const notification = await getNotification(expectedMessage);
+
+      expect(notification.getMessage()).toBe(expectedMessage);
+      expect(notification.getType()).toEqual('info');
+    });
+
+    it('includes debugging information in the details', async () => {
+      atom.commands.dispatch(atom.views.getView(editor), 'linter-jshint:debug');
+      const notification = await getNotification(expectedMessage);
+      const detail = notification.getDetail();
+
+      expect(detail.includes(`Atom version: ${atom.getVersion()}`)).toBe(true);
+      expect(detail.includes('linter-jshint version:')).toBe(true);
+      expect(detail.includes(`Platform: ${process.platform}`)).toBe(true);
+      expect(detail.includes('linter-jshint configuration:')).toBe(true);
+    });
+  });
 });