Skip to content

Commit dc0b68e

Browse files
yeonjuanyeonjuan
authored
fix: bugs in element-newline (#290)
* fix: bugs in element-newline * fix messages * add tests
1 parent 8f7a4e8 commit dc0b68e

File tree

5 files changed

+353
-197
lines changed

5 files changed

+353
-197
lines changed

‎packages/eslint-plugin/lib/rules/element-newline.js‎

Lines changed: 158 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,24 @@
66
* @typedef { import("../types").ScriptTag } ScriptTag
77
* @typedef { import("../types").StyleTag } StyleTag
88
* @typedef { import("../types").Text } Text
9-
* @typedef { Tag | Doctype | ScriptTag | StyleTag | Text } NewlineNode
10-
* @typedef {{
11-
* childFirst: NewlineNode | null;
12-
* childLast: NewlineNode | null;
13-
* shouldBeNewline: boolean;
14-
* }} NodeMeta
9+
* @typedef { import("../types").AnyNode } AnyNode
10+
* @typedef { import("../types").OpenTagEnd } OpenTagEnd
11+
* @typedef { import("../types").CloseTag } CloseTag
1512
*/
1613

1714
const { RULE_CATEGORY } = require("../constants");
18-
const { isTag, isComment, isText } = require("./utils/node");
15+
const {
16+
isTag,
17+
isComment,
18+
isText,
19+
splitToLineNodes,
20+
isLine,
21+
isScript,
22+
isStyle,
23+
} = require("./utils/node");
1924
const { createVisitors } = require("./utils/visitors");
2025
const MESSAGE_IDS = {
2126
EXPECT_NEW_LINE_AFTER: "expectAfter",
22-
EXPECT_NEW_LINE_AFTER_OPEN: "expectAfterOpen",
23-
EXPECT_NEW_LINE_BEFORE: "expectBefore",
24-
EXPECT_NEW_LINE_BEFORE_CLOSE: "expectBeforeClose",
2527
};
2628

2729
/**
@@ -100,174 +102,194 @@ module.exports = {
100102
],
101103
messages: {
102104
[MESSAGE_IDS.EXPECT_NEW_LINE_AFTER]:
103-
"There should be a linebreak after {{tag}} element.",
104-
[MESSAGE_IDS.EXPECT_NEW_LINE_AFTER_OPEN]:
105-
"There should be a linebreak after {{tag}} open.",
106-
[MESSAGE_IDS.EXPECT_NEW_LINE_BEFORE]:
107-
"There should be a linebreak before {{tag}} element.",
108-
[MESSAGE_IDS.EXPECT_NEW_LINE_BEFORE_CLOSE]:
109-
"There should be a linebreak before {{tag}} close.",
105+
"There should be a linebreak after {{name}}.",
110106
},
111107
},
112108

113109
create(context) {
114110
const option = context.options[0] || {};
115-
const skipTags = option.skip || [];
111+
/**
112+
* @type {string[]}
113+
*/
114+
const skipTags = option.skip || ["pre", "code"];
116115
const inlineTags = optionsOrPresets(option.inline || []);
117116

118117
/**
119-
* @param {Array<NewlineNode>} siblings
120-
* @returns {NodeMeta} meta
118+
* @param {AnyNode[]} children
119+
* @returns {Exclude<AnyNode, Text>[]}
121120
*/
122-
function checkSiblings(siblings) {
121+
function getChildrenToCheck(children) {
123122
/**
124-
* @type {NodeMeta}
123+
* @type {Exclude<AnyNode, Text>[]}
125124
*/
126-
const meta = {
127-
childFirst: null,
128-
childLast: null,
129-
shouldBeNewline: false,
130-
};
131-
132-
const nodesWithContent = [];
133-
for (
134-
let length = siblings.length, index = 0;
135-
index < length;
136-
index += 1
137-
) {
138-
const node = siblings[index];
125+
const childrenToCheck = [];
139126

140-
if (isEmptyText(node) === false) {
141-
nodesWithContent.push(node);
127+
for (const child of children) {
128+
if (isText(child)) {
129+
const lines = splitToLineNodes(child);
130+
childrenToCheck.push(...lines);
131+
continue;
142132
}
133+
childrenToCheck.push(child);
143134
}
135+
return childrenToCheck.filter((child) => !isEmptyText(child));
136+
}
144137

145-
for (
146-
let length = nodesWithContent.length, index = 0;
147-
index < length;
148-
index += 1
149-
) {
150-
const node = nodesWithContent[index];
151-
const nodeNext = nodesWithContent[index + 1];
138+
/**
139+
* @param {AnyNode} before
140+
* @param {AnyNode} after
141+
* @returns {boolean}
142+
*/
143+
function isOnTheSameLine(before, after) {
144+
return before.loc.end.line === after.loc.start.line;
145+
}
152146

153-
if (meta.childFirst === null) {
154-
meta.childFirst = node;
155-
}
147+
/**
148+
* @param {AnyNode} node
149+
* @returns {boolean}
150+
*/
151+
function shouldSkipChildren(node) {
152+
if (isTag(node) && skipTags.includes(node.name.toLowerCase())) {
153+
return true;
154+
}
155+
return false;
156+
}
156157

157-
meta.childLast = node;
158+
/**
159+
* @param {AnyNode} node
160+
* @returns {boolean}
161+
*/
162+
function isInline(node) {
163+
return (
164+
isLine(node) ||
165+
(isTag(node) && inlineTags.includes(node.name.toLowerCase()))
166+
);
167+
}
168+
169+
/**
170+
* @param {AnyNode[]} children
171+
* @param {AnyNode} parent
172+
* @param {[OpenTagEnd, CloseTag]} [wrapper]
173+
*/
174+
function checkChildren(children, parent, wrapper) {
175+
if (shouldSkipChildren(parent)) {
176+
return;
177+
}
158178

159-
const nodeShouldBeNewline = shouldBeNewline(node);
179+
const childrenToCheck = getChildrenToCheck(children);
180+
const firstChild = childrenToCheck[0];
181+
if (
182+
wrapper &&
183+
firstChild &&
184+
childrenToCheck.some((child) => !isInline(child))
185+
) {
186+
const open = wrapper[0];
187+
if (isOnTheSameLine(open, firstChild)) {
188+
context.report({
189+
node: open,
190+
messageId: MESSAGE_IDS.EXPECT_NEW_LINE_AFTER,
191+
data: { name: getName(parent) },
192+
fix(fixer) {
193+
return fixer.insertTextAfter(open, `\n`);
194+
},
195+
});
196+
}
197+
}
160198

161-
if (isTag(node) && skipTags.includes(node.name) === false) {
162-
const nodeMeta = checkSiblings(node.children);
163-
const nodeChildShouldBeNewline = nodeMeta.shouldBeNewline;
199+
childrenToCheck.forEach((current, index) => {
200+
const next = childrenToCheck[index + 1];
164201

165-
if (nodeShouldBeNewline || nodeChildShouldBeNewline) {
166-
meta.shouldBeNewline = true;
167-
}
202+
if (
203+
!next ||
204+
!isOnTheSameLine(current, next) ||
205+
(isInline(current) && isInline(next))
206+
) {
207+
return;
208+
}
168209

169-
if (
170-
nodeShouldBeNewline &&
171-
nodeChildShouldBeNewline &&
172-
nodeMeta.childFirst &&
173-
nodeMeta.childLast
174-
) {
175-
if (
176-
node.openEnd.loc.end.line === nodeMeta.childFirst.loc.start.line
177-
) {
178-
if (isNotNewlineStart(nodeMeta.childFirst)) {
179-
context.report({
180-
node: node,
181-
messageId: MESSAGE_IDS.EXPECT_NEW_LINE_AFTER_OPEN,
182-
data: { tag: label(node) },
183-
fix(fixer) {
184-
return fixer.insertTextAfter(node.openEnd, `\n`);
185-
},
186-
});
187-
}
188-
}
210+
context.report({
211+
node: current,
212+
messageId: MESSAGE_IDS.EXPECT_NEW_LINE_AFTER,
213+
data: { name: getName(current, { isClose: true }) },
214+
fix(fixer) {
215+
return fixer.insertTextAfter(current, `\n`);
216+
},
217+
});
218+
});
189219

190-
if (
191-
node.close &&
192-
nodeMeta.childLast.loc.end.line === node.close.loc.start.line
193-
) {
194-
if (isNotNewlineEnd(nodeMeta.childLast)) {
195-
context.report({
196-
node: node,
197-
messageId: MESSAGE_IDS.EXPECT_NEW_LINE_BEFORE_CLOSE,
198-
data: { tag: label(node, { isClose: true }) },
199-
fix(fixer) {
200-
return fixer.insertTextBefore(node.close, `\n`);
201-
},
202-
});
203-
}
204-
}
205-
}
220+
childrenToCheck.forEach((child) => {
221+
if (isTag(child)) {
222+
/**
223+
* @type {[OpenTagEnd, CloseTag] | undefined}
224+
*/
225+
const wrapper = child.close
226+
? [child.openEnd, child.close]
227+
: undefined;
228+
checkChildren(child.children, child, wrapper);
206229
}
230+
});
207231

208-
if (nodeNext && node.loc.end.line === nodeNext.loc.start.line) {
209-
if (nodeShouldBeNewline) {
210-
if (isNotNewlineStart(nodeNext)) {
211-
context.report({
212-
node: nodeNext,
213-
messageId: MESSAGE_IDS.EXPECT_NEW_LINE_AFTER,
214-
data: { tag: label(node) },
215-
fix(fixer) {
216-
return fixer.insertTextAfter(node, `\n`);
217-
},
218-
});
219-
}
220-
} else if (shouldBeNewline(nodeNext)) {
221-
if (isNotNewlineEnd(node)) {
222-
context.report({
223-
node: nodeNext,
224-
messageId: MESSAGE_IDS.EXPECT_NEW_LINE_BEFORE,
225-
data: { tag: label(nodeNext) },
226-
fix(fixer) {
227-
return fixer.insertTextBefore(nodeNext, `\n`);
228-
},
229-
});
230-
}
231-
}
232+
const lastChild = childrenToCheck[childrenToCheck.length - 1];
233+
if (
234+
wrapper &&
235+
lastChild &&
236+
childrenToCheck.some((child) => !isInline(child))
237+
) {
238+
const close = wrapper[1];
239+
if (isOnTheSameLine(close, lastChild)) {
240+
context.report({
241+
node: lastChild,
242+
messageId: MESSAGE_IDS.EXPECT_NEW_LINE_AFTER,
243+
data: { name: getName(lastChild, { isClose: true }) },
244+
fix(fixer) {
245+
return fixer.insertTextAfter(lastChild, `\n`);
246+
},
247+
});
232248
}
233249
}
234-
235-
return meta;
236250
}
237251

238252
/**
239-
* @param {NewlineNode} node
253+
* @param {AnyNode} node
254+
* @returns {boolean}
240255
*/
241256
function isEmptyText(node) {
242-
return node.type === `Text` && node.value.trim().length === 0;
243-
}
244-
245-
/**
246-
* @param {NewlineNode} node
247-
*/
248-
function isNotNewlineEnd(node) {
249-
return node.type !== `Text` || /(\n|\r\n)\s*$/.test(node.value) === false;
250-
}
251-
252-
/**
253-
* @param {NewlineNode} node
254-
*/
255-
function isNotNewlineStart(node) {
256-
return node.type !== `Text` || /^(\n|\r\n)/.test(node.value) === false;
257+
return (
258+
(isText(node) && node.value.trim().length === 0) ||
259+
(isLine(node) && node.value.trim().length === 0)
260+
);
257261
}
258262

259263
/**
260-
* @param {NewlineNode} node
264+
* @param {AnyNode} node
261265
* @param {{ isClose?: boolean }} options
262266
*/
263-
function label(node, options = {}) {
267+
function getName(node, options = {}) {
264268
const isClose = options.isClose || false;
265269
if (isTag(node)) {
266270
if (isClose) {
267271
return `</${node.name}>`;
268272
}
269273
return `<${node.name}>`;
270274
}
275+
if (isLine(node)) {
276+
return "text";
277+
}
278+
if (isComment(node)) {
279+
return "comment";
280+
}
281+
if (isScript(node)) {
282+
if (isClose) {
283+
return `</script>`;
284+
}
285+
return "<script>";
286+
}
287+
if (isStyle(node)) {
288+
if (isClose) {
289+
return `</style>`;
290+
}
291+
return "<style>";
292+
}
271293
return `<${node.type}>`;
272294
}
273295

@@ -287,26 +309,9 @@ module.exports = {
287309
return result;
288310
}
289311

290-
/**
291-
* @param {NewlineNode} node
292-
*/
293-
function shouldBeNewline(node) {
294-
if (isComment(node)) {
295-
return /[\n\r]+/.test(node.value.value.trim());
296-
}
297-
if (isTag(node)) {
298-
return inlineTags.includes(node.name.toLowerCase()) === false;
299-
}
300-
if (isText(node)) {
301-
return /[\n\r]+/.test(node.value.trim());
302-
}
303-
return true;
304-
}
305-
306312
return createVisitors(context, {
307313
Document(node) {
308-
// @ts-ignore
309-
checkSiblings(node.children);
314+
checkChildren(node.children, node);
310315
},
311316
});
312317
},

0 commit comments

Comments
 (0)