Skip to content

Commit adf5d81

Browse files
lukpsaxoljharb
authored andcommitted
[Fix] jsx-curly-spacing:
- Fix comment nodes being deleted when fixed - Fix problem when last token spans multiple lines
1 parent 7aaec03 commit adf5d81

File tree

2 files changed

+93
-11
lines changed

2 files changed

+93
-11
lines changed

lib/rules/jsx-curly-spacing.js

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,33 @@ module.exports = {
148148
* @returns {boolean} Whether or not there is a newline between the tokens.
149149
*/
150150
function isMultiline(left, right) {
151-
return left.loc.start.line !== right.loc.start.line;
151+
return left.loc.end.line !== right.loc.start.line;
152+
}
153+
154+
/**
155+
* Trims text of whitespace between two ranges
156+
* @param {Fixer} fixer - the eslint fixer object
157+
* @param {Location} fromLoc - the start location
158+
* @param {Location} toLoc - the end location
159+
* @param {string} mode - either 'start' or 'end'
160+
* @param {string=} spacing - a spacing value that will optionally add a space to the removed text
161+
* @returns {Object|*|{range, text}}
162+
*/
163+
function fixByTrimmingWhitespace(fixer, fromLoc, toLoc, mode, spacing) {
164+
let replacementText = sourceCode.text.slice(fromLoc, toLoc);
165+
if (mode === 'start') {
166+
replacementText = replacementText.replace(/^\s+/gm, '');
167+
} else {
168+
replacementText = replacementText.replace(/\s+$/gm, '');
169+
}
170+
if (spacing === SPACING.always) {
171+
if (mode === 'start') {
172+
replacementText += ' ';
173+
} else {
174+
replacementText = ` ${replacementText}`;
175+
}
176+
}
177+
return fixer.replaceTextRange([fromLoc, toLoc], replacementText);
152178
}
153179

154180
/**
@@ -164,7 +190,7 @@ module.exports = {
164190
message: `There should be no newline after '${token.value}'`,
165191
fix: function(fixer) {
166192
const nextToken = sourceCode.getTokenAfter(token);
167-
return fixer.replaceTextRange([token.range[1], nextToken.range[0]], spacing === SPACING.always ? ' ' : '');
193+
return fixByTrimmingWhitespace(fixer, token.range[1], nextToken.range[0], 'start', spacing);
168194
}
169195
});
170196
}
@@ -182,7 +208,7 @@ module.exports = {
182208
message: `There should be no newline before '${token.value}'`,
183209
fix: function(fixer) {
184210
const previousToken = sourceCode.getTokenBefore(token);
185-
return fixer.replaceTextRange([previousToken.range[1], token.range[0]], spacing === SPACING.always ? ' ' : '');
211+
return fixByTrimmingWhitespace(fixer, previousToken.range[1], token.range[0], 'end', spacing);
186212
}
187213
});
188214
}
@@ -200,10 +226,7 @@ module.exports = {
200226
message: `There should be no space after '${token.value}'`,
201227
fix: function(fixer) {
202228
const nextToken = sourceCode.getTokenAfter(token);
203-
const nextNode = sourceCode.getNodeByRangeIndex(nextToken.range[0]);
204-
const leadingComments = sourceCode.getComments(nextNode).leading;
205-
const rangeEndRef = leadingComments.length ? leadingComments[0] : nextToken;
206-
return fixer.removeRange([token.range[1], rangeEndRef.range[0]]);
229+
return fixByTrimmingWhitespace(fixer, token.range[1], nextToken.range[0], 'start');
207230
}
208231
});
209232
}
@@ -221,10 +244,7 @@ module.exports = {
221244
message: `There should be no space before '${token.value}'`,
222245
fix: function(fixer) {
223246
const previousToken = sourceCode.getTokenBefore(token);
224-
const previousNode = sourceCode.getNodeByRangeIndex(previousToken.range[0]);
225-
const trailingComments = sourceCode.getComments(previousNode).trailing;
226-
const rangeStartRef = trailingComments.length ? trailingComments[trailingComments.length - 1] : previousToken;
227-
return fixer.removeRange([rangeStartRef.range[1], token.range[0]]);
247+
return fixByTrimmingWhitespace(fixer, previousToken.range[1], token.range[0], 'end');
228248
}
229249
});
230250
}

tests/lib/rules/jsx-curly-spacing.js

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,13 @@ ruleTester.run('jsx-curly-spacing', rule, {
671671
}, {
672672
code: '<App foo={ bar }>{bar}</App>',
673673
options: ['always']
674+
}, {
675+
code: [
676+
'<App>{`',
677+
'text',
678+
'`}</App>'
679+
].join('\n'),
680+
options: [{children: {when: 'never', allowMultiline: false}}]
674681
}],
675682

676683
invalid: [{
@@ -2118,5 +2125,60 @@ ruleTester.run('jsx-curly-spacing', rule, {
21182125
}, {
21192126
message: 'A space is required before \'}\''
21202127
}]
2128+
}, {
2129+
code: '<App>{/*comment*/ }</App>',
2130+
output: '<App>{/*comment*/}</App>',
2131+
options: [{children: {when: 'never'}}],
2132+
errors: [{
2133+
message: 'There should be no space before \'}\''
2134+
}]
2135+
}, {
2136+
code: '<App>{ /*comment*/}</App>',
2137+
output: '<App>{/*comment*/}</App>',
2138+
options: [{children: {when: 'never'}}],
2139+
errors: [{
2140+
message: 'There should be no space after \'{\''
2141+
}]
2142+
}, {
2143+
code: '<App>{/*comment*/}</App>',
2144+
output: '<App>{ /*comment*/ }</App>',
2145+
options: [{children: {when: 'always'}}],
2146+
errors: [{
2147+
message: 'A space is required after \'{\''
2148+
}, {
2149+
message: 'A space is required before \'}\''
2150+
}]
2151+
}, {
2152+
code: [
2153+
'<App>',
2154+
'{/*comment*/',
2155+
'}',
2156+
'</App>'
2157+
].join('\n'),
2158+
output: [
2159+
'<App>',
2160+
'{/*comment*/}',
2161+
'</App>'
2162+
].join('\n'),
2163+
options: [{children: {when: 'never', allowMultiline: false}}],
2164+
errors: [{
2165+
message: 'There should be no newline before \'}\''
2166+
}]
2167+
}, {
2168+
code: [
2169+
'<App>',
2170+
'{',
2171+
'/*comment*/}',
2172+
'</App>'
2173+
].join('\n'),
2174+
output: [
2175+
'<App>',
2176+
'{/*comment*/}',
2177+
'</App>'
2178+
].join('\n'),
2179+
options: [{children: {when: 'never', allowMultiline: false}}],
2180+
errors: [{
2181+
message: 'There should be no newline after \'{\''
2182+
}]
21212183
}]
21222184
});

0 commit comments

Comments
 (0)