Skip to content

Commit cb5f447

Browse files
committed
handle certain cases in one pass
1 parent 7bdc856 commit cb5f447

File tree

2 files changed

+79
-37
lines changed

2 files changed

+79
-37
lines changed

lib/rules/jsx-one-element-per-line.js

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ module.exports = {
2323
create: function (context) {
2424
const sourceCode = context.getSourceCode();
2525

26+
function nodeKey (node) {
27+
return `${node.loc.start.line},${node.loc.start.column}`;
28+
}
29+
30+
function nodeDescriptor (n) {
31+
return n.openingElement ? n.openingElement.name.name : sourceCode.getText(n).replace(/\n/g, '');
32+
}
33+
2634
return {
2735
JSXElement: function (node) {
2836
const children = node.children;
@@ -37,6 +45,7 @@ module.exports = {
3745
const closingElementStartLine = closingElement.loc.start.line;
3846

3947
const childrenGroupedByLine = {};
48+
const fixDetailsByNode = {};
4049

4150
children.forEach(child => {
4251
let countNewLinesBeforeContent = 0;
@@ -105,28 +114,60 @@ module.exports = {
105114
(child.type === 'Literal' && child.raw.match(/ $/)) ||
106115
sourceCode.isSpaceBetweenTokens(child, nextChild);
107116
}
108-
const leadingSpace = prevChild && spaceBetweenPrev() ? '\n{\' \'}' : '';
109-
const trailingSpace = nextChild && spaceBetweenNext() ? '{\' \'}\n' : '';
110-
const leadingNewLine = prevChild ? '\n' : '';
111-
const trailingNewLine = nextChild ? '\n' : '';
112117

113118
if (!prevChild && !nextChild) {
114119
return;
115120
}
116121

117122
const source = sourceCode.getText(child);
123+
const leadingSpace = !!(prevChild && spaceBetweenPrev());
124+
const trailingSpace = !!(nextChild && spaceBetweenNext());
125+
const leadingNewLine = !!(prevChild);
126+
const trailingNewLine = !!(nextChild);
127+
128+
const key = nodeKey(child);
129+
130+
if (!fixDetailsByNode[key]) {
131+
fixDetailsByNode[key] = {
132+
node: child,
133+
source: source,
134+
descriptor: nodeDescriptor(child)
135+
};
136+
}
118137

119-
function nodeDescriptor (n) {
120-
return n.openingElement ? n.openingElement.name.name : source.replace(/\n/g, '');
138+
if (leadingSpace) {
139+
fixDetailsByNode[key].leadingSpace = true;
140+
}
141+
if (leadingNewLine) {
142+
fixDetailsByNode[key].leadingNewLine = true;
121143
}
144+
if (trailingNewLine) {
145+
fixDetailsByNode[key].trailingNewLine = true;
146+
}
147+
if (trailingSpace) {
148+
fixDetailsByNode[key].trailingSpace = true;
149+
}
150+
});
151+
});
122152

123-
context.report({
124-
node: child,
125-
message: `\`${nodeDescriptor(child)}\` must be placed on a new line`,
126-
fix: function (fixer) {
127-
return fixer.replaceText(child, `${leadingSpace}${leadingNewLine}${source}${trailingNewLine}${trailingSpace}`);
128-
}
129-
});
153+
Object.keys(fixDetailsByNode).forEach(key => {
154+
const details = fixDetailsByNode[key];
155+
156+
const nodeToReport = details.node;
157+
const descriptor = details.descriptor;
158+
const source = details.source;
159+
160+
const leadingSpaceString = details.leadingSpace ? '\n{\' \'}' : '';
161+
const trailingSpaceString = details.trailingSpace ? '{\' \'}\n' : '';
162+
const leadingNewLineString = details.leadingNewLine ? '\n' : '';
163+
const trailingNewLineString = details.trailingNewLine ? '\n' : '';
164+
165+
context.report({
166+
node: nodeToReport,
167+
message: `\`${descriptor}\` must be placed on a new line`,
168+
fix: function (fixer) {
169+
return fixer.replaceText(nodeToReport, `${leadingSpaceString}${leadingNewLineString}${source}${trailingNewLineString}${trailingSpaceString}`);
170+
}
130171
});
131172
});
132173
}

tests/lib/rules/jsx-one-element-per-line.js

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,28 @@ ruleTester.run('jsx-one-element-per-line', rule, {
707707
{message: '` baz` must be placed on a new line'}
708708
],
709709
parserOptions: parserOptions
710+
}, {
711+
// Would be nice to handle in one pass, but multipass works fine.
712+
code: [
713+
'<App>',
714+
' foo ',
715+
'{\' \'}',
716+
'{"bar"} baz',
717+
'</App>'
718+
].join('\n'),
719+
output: [
720+
'<App>',
721+
' foo ',
722+
'{\' \'}',
723+
'{"bar"}',
724+
'{\' \'}',
725+
' baz',
726+
'</App>'
727+
].join('\n'),
728+
errors: [
729+
{message: '` baz` must be placed on a new line'}
730+
],
731+
parserOptions: parserOptions
710732
}, {
711733
// Would be nice to handle in one pass, but multipass works fine.
712734
code: [
@@ -757,31 +779,11 @@ ruleTester.run('jsx-one-element-per-line', rule, {
757779
],
758780
parserOptions: parserOptions
759781
}, {
760-
// Would be nice to handle in one pass, but multipass works fine.
761782
code: [
762783
'<App>{',
763784
' foo',
764785
'}</App>'
765786
].join('\n'),
766-
output: [
767-
'<App>',
768-
'{',
769-
' foo',
770-
'}</App>'
771-
].join('\n'),
772-
errors: [
773-
{message: '`{ foo}` must be placed on a new line'},
774-
{message: '`{ foo}` must be placed on a new line'}
775-
],
776-
parserOptions: parserOptions
777-
}, {
778-
// Would be nice to handle in one pass, but multipass works fine.
779-
code: [
780-
'<App>',
781-
'{',
782-
' foo',
783-
'}</App>'
784-
].join('\n'),
785787
output: [
786788
'<App>',
787789
'{',
@@ -794,7 +796,6 @@ ruleTester.run('jsx-one-element-per-line', rule, {
794796
],
795797
parserOptions: parserOptions
796798
}, {
797-
// Would be nice to handle in one pass, but multipass works fine.
798799
code: [
799800
'<App> {',
800801
' foo',
@@ -805,15 +806,15 @@ ruleTester.run('jsx-one-element-per-line', rule, {
805806
'{\' \'}',
806807
'{',
807808
' foo',
808-
'} </App>'
809+
'}',
810+
'{\' \'}',
811+
' </App>'
809812
].join('\n'),
810813
errors: [
811-
{message: '`{ foo}` must be placed on a new line'},
812814
{message: '`{ foo}` must be placed on a new line'}
813815
],
814816
parserOptions: parserOptions
815817
}, {
816-
// Would be nice to handle in one pass, but multipass works fine.
817818
code: [
818819
'<App> ',
819820
'{\' \'}',

0 commit comments

Comments
 (0)