Improve jsx-closing-bracket-location error message

Summary:
By adding expected column information for the closing bracket, as well
as reporting the error at the closing bracket instead of the opening
bracket. Parsing the error message should now be sufficient to allow
writing an external autofixer.

Test Plan:
npm install
npm run test

Reviewers: csilvers

Reviewed By: csilvers

Differential Revision: https://phabricator.khanacademy.org/D23050

Author: Alex Lopatin

lib/rules/jsx-closing-bracket-location.js

@@ -9,7 +9,7 @@
 // ------------------------------------------------------------------------------
 module.exports = function(context) {
 
-  var MESSAGE = 'The closing bracket must be {{location}}';
+  var MESSAGE = 'The closing bracket must be {{location}}{{details}}';
   var MESSAGE_LOCATION = {
     'after-props': 'placed after the last prop',
     'after-tag': 'placed after the opening tag',
@@ -65,6 +65,26 @@ module.exports = function(context) {
     return location;
   }
 
+  /**
+   * Get the correct 0-indexed column for the closing bracket, given the
+   * expected location.
+   * @param {Object} tokens Locations of the opening bracket, closing bracket and last prop
+   * @param {String} expectedLocation Expected location for the closing bracket
+   * @return {?Number} The correct column for the closing bracket, or null
+   */
+  function getCorrectColumn(tokens, expectedLocation) {
+    switch (expectedLocation) {
+      case 'props-aligned':
+        return tokens.lastProp.column;
+      case 'tag-aligned':
+        return tokens.opening.column;
+      case 'line-aligned':
+        return tokens.openingStartOfLine.column;
+      default:
+        return null;
+    }
+  }
+
   /**
    * Check if the closing bracket is correctly located
    * @param {Object} tokens Locations of the opening bracket, closing bracket and last prop
@@ -78,11 +98,10 @@ module.exports = function(context) {
       case 'after-props':
         return tokens.lastProp.line === tokens.closing.line;
       case 'props-aligned':
-        return tokens.lastProp.column === tokens.closing.column;
       case 'tag-aligned':
-        return tokens.opening.column === tokens.closing.column;
       case 'line-aligned':
-        return tokens.openingStartOfLine.column === tokens.closing.column;
+        var correctColumn = getCorrectColumn(tokens, expectedLocation);
+        return correctColumn === tokens.closing.column;
       default:
         return true;
     }
@@ -129,8 +148,22 @@ module.exports = function(context) {
       if (hasCorrectLocation(tokens, expectedLocation)) {
         return;
       }
-      context.report(node, MESSAGE, {
-        location: MESSAGE_LOCATION[expectedLocation]
+
+      var data = {location: MESSAGE_LOCATION[expectedLocation], details: ''};
+      var correctColumn = getCorrectColumn(tokens, expectedLocation);
+
+      if (correctColumn !== null) {
+        var expectedNextLine = tokens.lastProp &&
+          (tokens.lastProp.line === tokens.closing.line);
+        data.details = ' (expected column ' + (correctColumn + 1) +
+          (expectedNextLine ? ' on the next line)' : ')');
+      }
+
+      context.report({
+        node: node,
+        loc: tokens.closing,
+        message: MESSAGE,
+        data: data
       });
     }
   };

tests/lib/rules/jsx-closing-bracket-location.js

@@ -13,9 +13,16 @@ var RuleTester = require('eslint').RuleTester;
 
 var MESSAGE_AFTER_PROPS = [{message: 'The closing bracket must be placed after the last prop'}];
 var MESSAGE_AFTER_TAG = [{message: 'The closing bracket must be placed after the opening tag'}];
-var MESSAGE_PROPS_ALIGNED = [{message: 'The closing bracket must be aligned with the last prop'}];
-var MESSAGE_TAG_ALIGNED = [{message: 'The closing bracket must be aligned with the opening tag'}];
-var MESSAGE_LINE_ALIGNED = [{message: 'The closing bracket must be aligned with the line containing the opening tag'}];
+
+var MESSAGE_PROPS_ALIGNED = 'The closing bracket must be aligned with the last prop';
+var MESSAGE_TAG_ALIGNED = 'The closing bracket must be aligned with the opening tag';
+var MESSAGE_LINE_ALIGNED = 'The closing bracket must be aligned with the line containing the opening tag';
+
+var messageWithDetails = function(message, expectedColumn, expectedNextLine) {
+  var details = ' (expected column ' + expectedColumn +
+    (expectedNextLine ? ' on the next line)' : ')');
+  return message + details;
+};
 
 // ------------------------------------------------------------------------------
 // Tests
@@ -335,23 +342,35 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'props-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_PROPS_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_PROPS_ALIGNED, 3, true),
+      line: 2,
+      column: 7
+    }]
   }, {
     code: [
       '<App ',
       '  foo />'
     ].join('\n'),
     options: [{location: 'tag-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_TAG_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, true),
+      line: 2,
+      column: 7
+    }]
   }, {
     code: [
       '<App ',
       '  foo />'
     ].join('\n'),
     options: [{location: 'line-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_LINE_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, true),
+      line: 2,
+      column: 7
+    }]
   }, {
     code: [
       '<App ',
@@ -369,7 +388,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'props-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_PROPS_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_PROPS_ALIGNED, 3, false),
+      line: 3,
+      column: 1
+    }]
   }, {
     code: [
       '<App ',
@@ -387,7 +410,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'tag-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_TAG_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, false),
+      line: 3,
+      column: 3
+    }]
   }, {
     code: [
       '<App ',
@@ -396,7 +423,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'line-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_LINE_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, false),
+      line: 3,
+      column: 3
+    }]
   }, {
     code: [
       '<App',
@@ -414,7 +445,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'props-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_PROPS_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_PROPS_ALIGNED, 3, false),
+      line: 3,
+      column: 1
+    }]
   }, {
     code: [
       '<App',
@@ -432,7 +467,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'tag-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_TAG_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, false),
+      line: 3,
+      column: 3
+    }]
   }, {
     code: [
       '<App',
@@ -441,7 +480,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'line-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_LINE_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, false),
+      line: 3,
+      column: 3
+    }]
   }, {
     code: [
       '<Provider ',
@@ -453,7 +496,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{selfClosing: 'props-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_TAG_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, true),
+      line: 2,
+      column: 8
+    }]
   }, {
     code: [
       '<Provider',
@@ -466,7 +513,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{nonEmpty: 'props-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_TAG_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_TAG_ALIGNED, 3, false),
+      line: 6,
+      column: 5
+    }]
   }, {
     code: [
       '<Provider ',
@@ -477,7 +528,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{selfClosing: 'after-props'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_TAG_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, true),
+      line: 2,
+      column: 8
+    }]
   }, {
     code: [
       '<Provider ',
@@ -489,7 +544,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{nonEmpty: 'after-props'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_TAG_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_TAG_ALIGNED, 3, false),
+      line: 5,
+      column: 5
+    }]
   }, {
     code: [
       'var x = function() {',
@@ -500,7 +559,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'line-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_LINE_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_LINE_ALIGNED, 3, false),
+      line: 4,
+      column: 10
+    }]
   }, {
     code: [
       'var x = <App',
@@ -509,6 +572,10 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
     ].join('\n'),
     options: [{location: 'line-aligned'}],
     ecmaFeatures: {jsx: true},
-    errors: MESSAGE_LINE_ALIGNED
+    errors: [{
+      message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, false),
+      line: 3,
+      column: 9
+    }]
   }]
 });