From fcc38630d1f84ce42600a60f562213dfa3235083 Mon Sep 17 00:00:00 2001 From: Landon Abney Date: Mon, 10 Oct 2016 13:20:13 -0700 Subject: [PATCH] Add support for endLine and endColumn (#709) * Add support for endLine and endColumn Add support for creating a full range from an ESLint message if it specifies an `endLine` and `endColumn`. Also adds a `validatePoint` helper function to check that these coordinates given from ESLint are valid since we no longer have the checking built into `helpers.rangeFromLineNumber`. --- lib/helpers.js | 13 +++++-- lib/main.js | 21 ++++++++++-- spec/fixtures/end-range/.eslintrc.js | 6 ++++ spec/fixtures/end-range/no-unreachable.js | 8 +++++ spec/linter-eslint-spec.js | 41 +++++++++++++++++------ src/helpers.js | 8 +++++ src/main.js | 24 ++++++++++--- 7 files changed, 101 insertions(+), 20 deletions(-) create mode 100644 spec/fixtures/end-range/.eslintrc.js create mode 100644 spec/fixtures/end-range/no-unreachable.js diff --git a/lib/helpers.js b/lib/helpers.js index 931fe8de..39aec3ad 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -7,6 +7,7 @@ Object.defineProperty(exports, "__esModule", { exports.spawnWorker = spawnWorker; exports.showError = showError; exports.idsToIgnoredRules = idsToIgnoredRules; +exports.validatePoint = validatePoint; var _child_process = require('child_process'); @@ -49,7 +50,7 @@ function spawnWorker() { } function showError(givenMessage) { - var givenDetail = arguments.length <= 1 || arguments[1] === undefined ? null : arguments[1]; + var givenDetail = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : null; var detail = void 0; var message = void 0; @@ -67,10 +68,18 @@ function showError(givenMessage) { } function idsToIgnoredRules() { - var ruleIds = arguments.length <= 0 || arguments[0] === undefined ? [] : arguments[0]; + var ruleIds = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : []; return ruleIds.reduce(function (ids, id) { ids[id] = RULE_OFF_SEVERITY; return ids; }, {}); } + +function validatePoint(textEditor, line, col) { + var buffer = textEditor.getBuffer(); + // Clip the given point to a valid one, and check if it equals the original + if (!buffer.clipPosition([line, col]).isEqual([line, col])) { + throw new Error(line + ':' + col + ' isn\'t a valid point!'); + } +} diff --git a/lib/main.js b/lib/main.js index fbde4a62..cd4c98e4 100644 --- a/lib/main.js +++ b/lib/main.js @@ -75,6 +75,7 @@ module.exports = { 'linter-eslint:debug': function linterEslintDebug() { var textEditor = atom.workspace.getActiveTextEditor(); var filePath = textEditor.getPath(); + // eslint-disable-next-line import/no-dynamic-require var linterEslintMeta = require(_path2.default.join(atom.packages.resolvePackagePath('linter-eslint'), 'package.json')); var config = atom.config.get('linter-eslint'); var configString = JSON.stringify(config, null, 2); @@ -84,7 +85,9 @@ module.exports = { config: config, filePath: filePath }).then(function (response) { - var detail = ['atom version: ' + atom.getVersion(), 'linter-eslint version: ' + linterEslintMeta.version, 'eslint version: ' + require(_path2.default.join(response.path, 'package.json')).version, 'hours since last atom restart: ' + Math.round(hoursSinceRestart * 10) / 10, 'platform: ' + process.platform, 'Using ' + response.type + ' eslint from ' + response.path, 'linter-eslint configuration: ' + configString].join('\n'); + var detail = ['atom version: ' + atom.getVersion(), 'linter-eslint version: ' + linterEslintMeta.version, + // eslint-disable-next-line import/no-dynamic-require + 'eslint version: ' + require(_path2.default.join(response.path, 'package.json')).version, 'hours since last atom restart: ' + Math.round(hoursSinceRestart * 10) / 10, 'platform: ' + process.platform, 'Using ' + response.type + ' eslint from ' + response.path, 'linter-eslint configuration: ' + configString].join('\n'); var notificationOptions = { detail: detail, dismissable: true }; atom.notifications.addInfo('linter-eslint debugging information', notificationOptions); }).catch(function (response) { @@ -193,6 +196,8 @@ module.exports = { var ruleId = _ref.ruleId; var column = _ref.column; var fix = _ref.fix; + var endLine = _ref.endLine; + var endColumn = _ref.endColumn; var textBuffer = textEditor.getBuffer(); var linterFix = null; @@ -204,8 +209,20 @@ module.exports = { }; } var range = void 0; + var msgLine = line - 1; try { - range = Helpers.rangeFromLineNumber(textEditor, line - 1, column ? column - 1 : column); + if (typeof endColumn !== 'undefined' && typeof endLine !== 'undefined') { + // Here we always want the column to be a number + var msgCol = Math.max(0, column - 1); + (0, _helpers.validatePoint)(textEditor, msgLine, msgCol); + (0, _helpers.validatePoint)(textEditor, endLine - 1, endColumn - 1); + range = [[msgLine, msgCol], [endLine - 1, endColumn - 1]]; + } else { + // We want msgCol to remain undefined if it was initially so + // `rangeFromLineNumber` will give us a range over the entire line + var _msgCol = typeof column !== 'undefined' ? column - 1 : column; + range = Helpers.rangeFromLineNumber(textEditor, msgLine, _msgCol); + } } catch (err) { throw new Error('Cannot mark location in editor for (' + ruleId + ') - (' + message + ')' + (' at line (' + line + ') column (' + column + ')')); } diff --git a/spec/fixtures/end-range/.eslintrc.js b/spec/fixtures/end-range/.eslintrc.js new file mode 100644 index 00000000..c4e73b57 --- /dev/null +++ b/spec/fixtures/end-range/.eslintrc.js @@ -0,0 +1,6 @@ +module.exports = { + root: true, + rules: { + 'no-unreachable': 'error' + } +}; diff --git a/spec/fixtures/end-range/no-unreachable.js b/spec/fixtures/end-range/no-unreachable.js new file mode 100644 index 00000000..1af8df64 --- /dev/null +++ b/spec/fixtures/end-range/no-unreachable.js @@ -0,0 +1,8 @@ +'use strict'; + +function foo() { + return 42; + + var a = 4; + return a + 2; +} diff --git a/spec/linter-eslint-spec.js b/spec/linter-eslint-spec.js index 47ade60b..877f3c48 100644 --- a/spec/linter-eslint-spec.js +++ b/spec/linter-eslint-spec.js @@ -6,21 +6,23 @@ import { tmpdir } from 'os' import rimraf from 'rimraf' import linter from '../lib/main' -const goodPath = path.join(__dirname, 'fixtures', 'files', 'good.js') -const badPath = path.join(__dirname, 'fixtures', 'files', 'bad.js') -const emptyPath = path.join(__dirname, 'fixtures', 'files', 'empty.js') -const fixPath = path.join(__dirname, 'fixtures', 'files', 'fix.js') -const configPath = path.join(__dirname, 'fixtures', 'configs', '.eslintrc.yml') -const importingpath = path.join(__dirname, 'fixtures', +const fixturesDir = path.join(__dirname, 'fixtures') + +const goodPath = path.join(fixturesDir, 'files', 'good.js') +const badPath = path.join(fixturesDir, 'files', 'bad.js') +const emptyPath = path.join(fixturesDir, 'files', 'empty.js') +const fixPath = path.join(fixturesDir, 'files', 'fix.js') +const configPath = path.join(fixturesDir, 'configs', '.eslintrc.yml') +const importingpath = path.join(fixturesDir, 'import-resolution', 'nested', 'importing.js') -const badImportPath = path.join(__dirname, 'fixtures', +const badImportPath = path.join(fixturesDir, 'import-resolution', 'nested', 'badImport.js') -const ignoredPath = path.join(__dirname, 'fixtures', - 'eslintignore', 'ignored.js') -const modifiedIgnorePath = path.join(__dirname, 'fixtures', +const ignoredPath = path.join(fixturesDir, 'eslintignore', 'ignored.js') +const modifiedIgnorePath = path.join(fixturesDir, 'modified-ignore-rule', 'foo.js') -const modifiedIgnoreSpacePath = path.join(__dirname, 'fixtures', +const modifiedIgnoreSpacePath = path.join(fixturesDir, 'modified-ignore-rule', 'foo-space.js') +const endRangePath = path.join(fixturesDir, 'end-range', 'no-unreachable.js') describe('The eslint provider for Linter', () => { const { spawnWorker } = require('../lib/helpers') @@ -342,4 +344,21 @@ describe('The eslint provider for Linter', () => { ) }) }) + + it('handles ranges in messages', () => + waitsForPromise(() => + atom.workspace.open(endRangePath).then(editor => + lint(editor).then((messages) => { + const expected = 'no-unreachable ' + + 'Unreachable code.' + expect(messages[0].type).toBe('Error') + expect(messages[0].text).not.toBeDefined() + expect(messages[0].html).toBe(expected) + expect(messages[0].filePath).toBe(endRangePath) + expect(messages[0].range).toEqual([[5, 2], [6, 15]]) + }) + ) + ) + ) }) diff --git a/src/helpers.js b/src/helpers.js index c6e26ec7..c3e89b50 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -56,3 +56,11 @@ export function idsToIgnoredRules(ruleIds = []) { return ids }, {}) } + +export function validatePoint(textEditor, line, col) { + const buffer = textEditor.getBuffer() + // Clip the given point to a valid one, and check if it equals the original + if (!buffer.clipPosition([line, col]).isEqual([line, col])) { + throw new Error(`${line}:${col} isn't a valid point!`) + } +} diff --git a/src/main.js b/src/main.js index 41d327cc..f120032d 100644 --- a/src/main.js +++ b/src/main.js @@ -7,7 +7,7 @@ import ruleURI from 'eslint-rule-documentation' // eslint-disable-next-line import/no-extraneous-dependencies, import/extensions import { CompositeDisposable, Range } from 'atom' -import { spawnWorker, showError, idsToIgnoredRules } from './helpers' +import { spawnWorker, showError, idsToIgnoredRules, validatePoint } from './helpers' // Configuration const scopes = [] @@ -65,6 +65,7 @@ module.exports = { 'linter-eslint:debug': () => { const textEditor = atom.workspace.getActiveTextEditor() const filePath = textEditor.getPath() + // eslint-disable-next-line import/no-dynamic-require const linterEslintMeta = require(Path.join(atom.packages.resolvePackagePath('linter-eslint'), 'package.json')) const config = atom.config.get('linter-eslint') const configString = JSON.stringify(config, null, 2) @@ -77,6 +78,7 @@ module.exports = { const detail = [ `atom version: ${atom.getVersion()}`, `linter-eslint version: ${linterEslintMeta.version}`, + // eslint-disable-next-line import/no-dynamic-require `eslint version: ${require(Path.join(response.path, 'package.json')).version}`, `hours since last atom restart: ${Math.round(hoursSinceRestart * 10) / 10}`, `platform: ${process.platform}`, @@ -181,7 +183,9 @@ module.exports = { */ return null } - return response.map(({ message, line, severity, ruleId, column, fix }) => { + return response.map(({ + message, line, severity, ruleId, column, fix, endLine, endColumn } + ) => { const textBuffer = textEditor.getBuffer() let linterFix = null if (fix) { @@ -195,10 +199,20 @@ module.exports = { } } let range + const msgLine = line - 1 try { - range = Helpers.rangeFromLineNumber( - textEditor, line - 1, column ? column - 1 : column - ) + if (typeof endColumn !== 'undefined' && typeof endLine !== 'undefined') { + // Here we always want the column to be a number + const msgCol = Math.max(0, column - 1) + validatePoint(textEditor, msgLine, msgCol) + validatePoint(textEditor, endLine - 1, endColumn - 1) + range = [[msgLine, msgCol], [endLine - 1, endColumn - 1]] + } else { + // We want msgCol to remain undefined if it was initially so + // `rangeFromLineNumber` will give us a range over the entire line + const msgCol = typeof column !== 'undefined' ? column - 1 : column + range = Helpers.rangeFromLineNumber(textEditor, msgLine, msgCol) + } } catch (err) { throw new Error( `Cannot mark location in editor for (${ruleId}) - (${message})` +