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`.
AtomLinter · Oct 10, 2016 · fcc3863 · fcc3863
1 parent b7884bd
commit fcc3863
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 = '<a href=http://eslint.org/docs/rules/no-unreachable ' +
+            'class="badge badge-flexible eslint">no-unreachable</a> ' +
+            '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
@@ -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
@@ -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})` +