Skip to content

Commit 218e9fc

Browse files
yannickcryannickcr
committed
Merge pull request #301 from Khan/master
Improve jsx-closing-bracket-location error message
2 parents 03fda3f + 44f376e commit 218e9fc

File tree

2 files changed

+124
-24
lines changed

2 files changed

+124
-24
lines changed

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

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// ------------------------------------------------------------------------------
1010
module.exports = function(context) {
1111

12-
var MESSAGE = 'The closing bracket must be {{location}}';
12+
var MESSAGE = 'The closing bracket must be {{location}}{{details}}';
1313
var MESSAGE_LOCATION = {
1414
'after-props': 'placed after the last prop',
1515
'after-tag': 'placed after the opening tag',
@@ -65,6 +65,26 @@ module.exports = function(context) {
6565
return location;
6666
}
6767

68+
/**
69+
* Get the correct 0-indexed column for the closing bracket, given the
70+
* expected location.
71+
* @param {Object} tokens Locations of the opening bracket, closing bracket and last prop
72+
* @param {String} expectedLocation Expected location for the closing bracket
73+
* @return {?Number} The correct column for the closing bracket, or null
74+
*/
75+
function getCorrectColumn(tokens, expectedLocation) {
76+
switch (expectedLocation) {
77+
case 'props-aligned':
78+
return tokens.lastProp.column;
79+
case 'tag-aligned':
80+
return tokens.opening.column;
81+
case 'line-aligned':
82+
return tokens.openingStartOfLine.column;
83+
default:
84+
return null;
85+
}
86+
}
87+
6888
/**
6989
* Check if the closing bracket is correctly located
7090
* @param {Object} tokens Locations of the opening bracket, closing bracket and last prop
@@ -78,11 +98,10 @@ module.exports = function(context) {
7898
case 'after-props':
7999
return tokens.lastProp.line === tokens.closing.line;
80100
case 'props-aligned':
81-
return tokens.lastProp.column === tokens.closing.column;
82101
case 'tag-aligned':
83-
return tokens.opening.column === tokens.closing.column;
84102
case 'line-aligned':
85-
return tokens.openingStartOfLine.column === tokens.closing.column;
103+
var correctColumn = getCorrectColumn(tokens, expectedLocation);
104+
return correctColumn === tokens.closing.column;
86105
default:
87106
return true;
88107
}
@@ -129,8 +148,22 @@ module.exports = function(context) {
129148
if (hasCorrectLocation(tokens, expectedLocation)) {
130149
return;
131150
}
132-
context.report(node, MESSAGE, {
133-
location: MESSAGE_LOCATION[expectedLocation]
151+
152+
var data = {location: MESSAGE_LOCATION[expectedLocation], details: ''};
153+
var correctColumn = getCorrectColumn(tokens, expectedLocation);
154+
155+
if (correctColumn !== null) {
156+
var expectedNextLine = tokens.lastProp &&
157+
(tokens.lastProp.line === tokens.closing.line);
158+
data.details = ' (expected column ' + (correctColumn + 1) +
159+
(expectedNextLine ? ' on the next line)' : ')');
160+
}
161+
162+
context.report({
163+
node: node,
164+
loc: tokens.closing,
165+
message: MESSAGE,
166+
data: data
134167
});
135168
}
136169
};

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

Lines changed: 85 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,16 @@ var RuleTester = require('eslint').RuleTester;
1313

1414
var MESSAGE_AFTER_PROPS = [{message: 'The closing bracket must be placed after the last prop'}];
1515
var MESSAGE_AFTER_TAG = [{message: 'The closing bracket must be placed after the opening tag'}];
16-
var MESSAGE_PROPS_ALIGNED = [{message: 'The closing bracket must be aligned with the last prop'}];
17-
var MESSAGE_TAG_ALIGNED = [{message: 'The closing bracket must be aligned with the opening tag'}];
18-
var MESSAGE_LINE_ALIGNED = [{message: 'The closing bracket must be aligned with the line containing the opening tag'}];
16+
17+
var MESSAGE_PROPS_ALIGNED = 'The closing bracket must be aligned with the last prop';
18+
var MESSAGE_TAG_ALIGNED = 'The closing bracket must be aligned with the opening tag';
19+
var MESSAGE_LINE_ALIGNED = 'The closing bracket must be aligned with the line containing the opening tag';
20+
21+
var messageWithDetails = function(message, expectedColumn, expectedNextLine) {
22+
var details = ' (expected column ' + expectedColumn +
23+
(expectedNextLine ? ' on the next line)' : ')');
24+
return message + details;
25+
};
1926

2027
// ------------------------------------------------------------------------------
2128
// Tests
@@ -335,23 +342,35 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
335342
].join('\n'),
336343
options: [{location: 'props-aligned'}],
337344
ecmaFeatures: {jsx: true},
338-
errors: MESSAGE_PROPS_ALIGNED
345+
errors: [{
346+
message: messageWithDetails(MESSAGE_PROPS_ALIGNED, 3, true),
347+
line: 2,
348+
column: 7
349+
}]
339350
}, {
340351
code: [
341352
'<App ',
342353
' foo />'
343354
].join('\n'),
344355
options: [{location: 'tag-aligned'}],
345356
ecmaFeatures: {jsx: true},
346-
errors: MESSAGE_TAG_ALIGNED
357+
errors: [{
358+
message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, true),
359+
line: 2,
360+
column: 7
361+
}]
347362
}, {
348363
code: [
349364
'<App ',
350365
' foo />'
351366
].join('\n'),
352367
options: [{location: 'line-aligned'}],
353368
ecmaFeatures: {jsx: true},
354-
errors: MESSAGE_LINE_ALIGNED
369+
errors: [{
370+
message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, true),
371+
line: 2,
372+
column: 7
373+
}]
355374
}, {
356375
code: [
357376
'<App ',
@@ -369,7 +388,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
369388
].join('\n'),
370389
options: [{location: 'props-aligned'}],
371390
ecmaFeatures: {jsx: true},
372-
errors: MESSAGE_PROPS_ALIGNED
391+
errors: [{
392+
message: messageWithDetails(MESSAGE_PROPS_ALIGNED, 3, false),
393+
line: 3,
394+
column: 1
395+
}]
373396
}, {
374397
code: [
375398
'<App ',
@@ -387,7 +410,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
387410
].join('\n'),
388411
options: [{location: 'tag-aligned'}],
389412
ecmaFeatures: {jsx: true},
390-
errors: MESSAGE_TAG_ALIGNED
413+
errors: [{
414+
message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, false),
415+
line: 3,
416+
column: 3
417+
}]
391418
}, {
392419
code: [
393420
'<App ',
@@ -396,7 +423,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
396423
].join('\n'),
397424
options: [{location: 'line-aligned'}],
398425
ecmaFeatures: {jsx: true},
399-
errors: MESSAGE_LINE_ALIGNED
426+
errors: [{
427+
message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, false),
428+
line: 3,
429+
column: 3
430+
}]
400431
}, {
401432
code: [
402433
'<App',
@@ -414,7 +445,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
414445
].join('\n'),
415446
options: [{location: 'props-aligned'}],
416447
ecmaFeatures: {jsx: true},
417-
errors: MESSAGE_PROPS_ALIGNED
448+
errors: [{
449+
message: messageWithDetails(MESSAGE_PROPS_ALIGNED, 3, false),
450+
line: 3,
451+
column: 1
452+
}]
418453
}, {
419454
code: [
420455
'<App',
@@ -432,7 +467,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
432467
].join('\n'),
433468
options: [{location: 'tag-aligned'}],
434469
ecmaFeatures: {jsx: true},
435-
errors: MESSAGE_TAG_ALIGNED
470+
errors: [{
471+
message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, false),
472+
line: 3,
473+
column: 3
474+
}]
436475
}, {
437476
code: [
438477
'<App',
@@ -441,7 +480,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
441480
].join('\n'),
442481
options: [{location: 'line-aligned'}],
443482
ecmaFeatures: {jsx: true},
444-
errors: MESSAGE_LINE_ALIGNED
483+
errors: [{
484+
message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, false),
485+
line: 3,
486+
column: 3
487+
}]
445488
}, {
446489
code: [
447490
'<Provider ',
@@ -453,7 +496,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
453496
].join('\n'),
454497
options: [{selfClosing: 'props-aligned'}],
455498
ecmaFeatures: {jsx: true},
456-
errors: MESSAGE_TAG_ALIGNED
499+
errors: [{
500+
message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, true),
501+
line: 2,
502+
column: 8
503+
}]
457504
}, {
458505
code: [
459506
'<Provider',
@@ -466,7 +513,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
466513
].join('\n'),
467514
options: [{nonEmpty: 'props-aligned'}],
468515
ecmaFeatures: {jsx: true},
469-
errors: MESSAGE_TAG_ALIGNED
516+
errors: [{
517+
message: messageWithDetails(MESSAGE_TAG_ALIGNED, 3, false),
518+
line: 6,
519+
column: 5
520+
}]
470521
}, {
471522
code: [
472523
'<Provider ',
@@ -477,7 +528,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
477528
].join('\n'),
478529
options: [{selfClosing: 'after-props'}],
479530
ecmaFeatures: {jsx: true},
480-
errors: MESSAGE_TAG_ALIGNED
531+
errors: [{
532+
message: messageWithDetails(MESSAGE_TAG_ALIGNED, 1, true),
533+
line: 2,
534+
column: 8
535+
}]
481536
}, {
482537
code: [
483538
'<Provider ',
@@ -489,7 +544,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
489544
].join('\n'),
490545
options: [{nonEmpty: 'after-props'}],
491546
ecmaFeatures: {jsx: true},
492-
errors: MESSAGE_TAG_ALIGNED
547+
errors: [{
548+
message: messageWithDetails(MESSAGE_TAG_ALIGNED, 3, false),
549+
line: 5,
550+
column: 5
551+
}]
493552
}, {
494553
code: [
495554
'var x = function() {',
@@ -500,7 +559,11 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
500559
].join('\n'),
501560
options: [{location: 'line-aligned'}],
502561
ecmaFeatures: {jsx: true},
503-
errors: MESSAGE_LINE_ALIGNED
562+
errors: [{
563+
message: messageWithDetails(MESSAGE_LINE_ALIGNED, 3, false),
564+
line: 4,
565+
column: 10
566+
}]
504567
}, {
505568
code: [
506569
'var x = <App',
@@ -509,6 +572,10 @@ ruleTester.run('jsx-closing-bracket-location', rule, {
509572
].join('\n'),
510573
options: [{location: 'line-aligned'}],
511574
ecmaFeatures: {jsx: true},
512-
errors: MESSAGE_LINE_ALIGNED
575+
errors: [{
576+
message: messageWithDetails(MESSAGE_LINE_ALIGNED, 1, false),
577+
line: 3,
578+
column: 9
579+
}]
513580
}]
514581
});

0 commit comments

Comments
 (0)