From d860c3d61b9cc7c0ee9fccb927603561c75ef9e1 Mon Sep 17 00:00:00 2001 From: Landon Abney Date: Fri, 5 May 2017 15:46:48 -0700 Subject: [PATCH] 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 | 2 +- lib/helpers.js | 123 +++++++++++++++++++++++++++++++++++++ lib/main.js | 64 +++++++++++-------- package.json | 1 + spec/linter-jshint-spec.js | 48 ++++++++++++++- 5 files changed, 209 insertions(+), 29 deletions(-) create mode 100644 lib/helpers.js diff --git a/decls/atom-linter.js b/decls/atom-linter.js index 3c00b38..4296ec7 100644 --- a/decls/atom-linter.js +++ b/decls/atom-linter.js @@ -22,7 +22,7 @@ declare module 'atom-linter' { declare var findCachedAsync: (directory: string, names: string | Array) => Promise; declare var execNode: - (filePath: string, args: Array, options: ExecOptions) => Promise; + (filePath: string, args: Array, options?: ExecOptions) => Promise; declare var generateRange: (textEditor: TextEditor, lineNumber?: number, colStart?: number) => Array> diff --git a/lib/helpers.js b/lib/helpers.js new file mode 100644 index 0000000..f3bb4a7 --- /dev/null +++ b/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, + '', '', + '', + '', '', + '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. ` + + `Report this!`, + 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', + }, + ], + }; +} diff --git a/lib/main.js b/lib/main.js index 9312353..c822ef5 100644 --- a/lib/main.js +++ b/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,6 +93,16 @@ 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() { @@ -108,9 +110,6 @@ module.exports = { }, provideLinter() { - const Helpers = require('atom-linter'); - const Reporter = require('jshint-json'); - return { name: 'JSHint', grammarScopes: this.scopes, @@ -121,9 +120,12 @@ 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); @@ -131,12 +133,12 @@ module.exports = { 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; }, diff --git a/package.json b/package.json index ef96624..7fabc1d 100644 --- a/package.json +++ b/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" diff --git a/spec/linter-jshint-spec.js b/spec/linter-jshint-spec.js index 1c5d57f..d3c7d8c 100644 --- a/spec/linter-jshint-spec.js +++ b/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); + }); + }); });