From b1e95867c8f39c7b3fff1379ad33de5ce83c6a49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Mon, 20 Dec 2021 09:32:20 +0100 Subject: [PATCH 1/4] fix: fix validateBlockStart to validate all the lines --- lib/rules/block-indentation.js | 52 +++++++++++++---------- test/unit/rules/block-indentation-test.js | 43 +++++++++++++++++-- 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index 59ac80e00..6e43b0a77 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -192,18 +192,14 @@ module.exports = class BlockIndentation extends Rule { this.seen = new Set(); return { - Template(node) { - if (this.mode === 'fix') { - if ( - node.body[0] && - node.body[0].type === 'TextNode' && - !removeWhitespaceEnd(node.body[0].chars) - ) { - // We need to remove the text node in this case - node.body.shift(); - } - } - return node; + Template: { + enter(node) { + this.template = node; + return node; + }, + exit() { + this.template = null; + }, }, BlockStatement(node) { @@ -281,7 +277,7 @@ module.exports = class BlockIndentation extends Rule { } // Recreate the node only if the source has changed, otherwise this would mean useless computation and possible mistake - if (sourceBeforeFix !== this.sourceEdited) { + if (sourceBeforeFix !== this.sourceEdited && this.sourceEdited) { fixedNode = this.fixLine( this.sourceEdited.split('\n'), 0, @@ -302,20 +298,30 @@ module.exports = class BlockIndentation extends Rule { return node; } + const indexOfNodeInTemplate = this.template.body.indexOf(node); + const isDirectChildOfTemplate = indexOfNodeInTemplate !== -1; let startColumn = node.loc.start.column; let startLine = node.loc.start.line; + const shouldValidate = startLine === 1 || isDirectChildOfTemplate; - if (startLine === 1 && startColumn !== 0) { + if (shouldValidate && startColumn !== 0) { if (this.mode === 'fix') { - // If we are in a child (path set) we want to edit directly the source and not the source of the node - const elementSource = path - ? this.sourceEdited || this.source.join('') - : sourceForLoc(this.sourceEdited || this.source, { - end: node.loc.end, - start: { line: node.loc.start.line, column: 0 }, - }); - const fixedNode = this.fixLine(elementSource.split('\n'), 0, startColumn, 0, path); - return fixedNode; + if (isDirectChildOfTemplate) { + const previousNode = this.template.body[indexOfNodeInTemplate - 1]; + if (previousNode.type === 'TextNode') { + previousNode.chars = removeWhitespaceEnd(previousNode.chars); + } + } else { + // If we are in a child (path set) we want to edit directly the source and not the source of the node + const elementSource = path + ? this.sourceEdited || this.source.join('') + : sourceForLoc(this.sourceEdited || this.source, { + end: node.loc.end, + start: { line: node.loc.start.line, column: 0 }, + }); + const fixedNode = this.fixLine(elementSource.split('\n'), 0, startColumn, 0, path); + return fixedNode; + } } else { let isElementNode = node && node.type === 'ElementNode'; let displayName = isElementNode ? node.tag : node.path.original; diff --git a/test/unit/rules/block-indentation-test.js b/test/unit/rules/block-indentation-test.js index 5297f6dab..dbffd2234 100644 --- a/test/unit/rules/block-indentation-test.js +++ b/test/unit/rules/block-indentation-test.js @@ -26,7 +26,7 @@ generateRuleTests({ ' ', '{{/if}}', ].join('\n'), - '\n {{#each cats as |dog|}}\n {{/each}}', + '\n{{#each cats as |dog|}}\n{{/each}}', '

Stuff

', '
\n

Stuff Here

\n
', '{{#if isMorning}}\n' + @@ -227,17 +227,30 @@ generateRuleTests({ '{{/if}}', ].join('\n'), }, - ], + ].filter((value, index) => index >= 0), bad: [ { // start and end must be the same indentation template: '\n {{#each cats as |dog|}}\n {{/each}}', - fixedTemplate: '\n {{#each cats as |dog|}}\n {{/each}}', + fixedTemplate: '\n{{#each cats as |dog|}}\n{{/each}}', verifyResults(results) { expect(results).toMatchInlineSnapshot(` Array [ + Object { + "column": 2, + "endColumn": 17, + "endLine": 3, + "filePath": "layout.hbs", + "isFixable": true, + "line": 2, + "message": "Incorrect indentation for \`{{#each}}\` beginning at L2:C2. Expected \`{{#each}}\` to be at an indentation of 0, but was found at 2.", + "rule": "block-indentation", + "severity": 2, + "source": "{{#each cats as |dog|}} + {{/each}}", + }, Object { "column": 17, "endColumn": 17, @@ -1182,5 +1195,29 @@ generateRuleTests({ `); }, }, + { + template: '
\n
\n \n ', + fixedTemplate: '
\n
\n\n', + + verifyResults(results) { + expect(results).toMatchInlineSnapshot(` + Array [ + Object { + "column": 2, + "endColumn": 9, + "endLine": 4, + "filePath": "layout.hbs", + "isFixable": true, + "line": 3, + "message": "Incorrect indentation for \`\` beginning at L3:C2. Expected \`\` to be at an indentation of 0, but was found at 2.", + "rule": "block-indentation", + "severity": 2, + "source": " + ", + }, + ] + `); + }, + }, ], }); From 1bb74e91bff16f338d08bf1459605fc4416358c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Wed, 5 Jan 2022 11:58:52 +0100 Subject: [PATCH 2/4] fix: fix indentation of first block when that blokc is on a single line --- lib/rules/block-indentation.js | 14 ++++---- test/unit/rules/block-indentation-test.js | 42 +++++++++++++++++++++-- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index 6e43b0a77..34ffc6cec 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -226,14 +226,15 @@ module.exports = class BlockIndentation extends Rule { * @returns {Node} */ process(node, path) { + let fixedNode = this.validateBlockStart(node, path); + // Nodes that start and end on the same line cannot have any indentation // issues (since the column of the start block was already checked in the // parent's validateBlockChildren()) if (node.loc.start.line === node.loc.end.line || this.seen.has(node)) { - return node; + return fixedNode; } - let fixedNode = this.validateBlockStart(node, path); fixedNode = this.validateBlockElse(fixedNode, path); fixedNode = this.validateBlockEnd(fixedNode, path); fixedNode = this.validateBlockChildren(fixedNode, path); @@ -301,10 +302,9 @@ module.exports = class BlockIndentation extends Rule { const indexOfNodeInTemplate = this.template.body.indexOf(node); const isDirectChildOfTemplate = indexOfNodeInTemplate !== -1; let startColumn = node.loc.start.column; - let startLine = node.loc.start.line; - const shouldValidate = startLine === 1 || isDirectChildOfTemplate; - if (shouldValidate && startColumn !== 0) { + // If it's not a direct child of the template the block start is already fixed by the validateBlockChildren function + if (isDirectChildOfTemplate && startColumn !== 0) { if (this.mode === 'fix') { if (isDirectChildOfTemplate) { const previousNode = this.template.body[indexOfNodeInTemplate - 1]; @@ -687,7 +687,7 @@ module.exports = class BlockIndentation extends Rule { return AstNodeInfo.hasChildren(node); } - let source = this.sourceForNode(node); + let source = sourceForLoc(this.sourceEdited || this.source, node.loc); let endingToken = `/${node.path.original}`; let indexOfEnding = source.lastIndexOf(endingToken); @@ -706,7 +706,7 @@ module.exports = class BlockIndentation extends Rule { return 3; } - let source = this.sourceForNode(node); + let source = sourceForLoc(this.sourceEdited || this.source, node.loc); let endingToken = `/${node.path.original}`; let indexOfEnding = source.lastIndexOf(endingToken); diff --git a/test/unit/rules/block-indentation-test.js b/test/unit/rules/block-indentation-test.js index dbffd2234..9b5b48d6b 100644 --- a/test/unit/rules/block-indentation-test.js +++ b/test/unit/rules/block-indentation-test.js @@ -145,7 +145,7 @@ generateRuleTests({ config: { ignoreComments: true, }, - template: '{{! Comment }}
foo
', + template: ' {{! Comment }}\n
foo
', }, { config: { @@ -169,7 +169,7 @@ generateRuleTests({ config: { ignoreComments: true, }, - template: '
foo
', + template: ' \n
foo
', }, { config: { @@ -227,7 +227,7 @@ generateRuleTests({ '{{/if}}', ].join('\n'), }, - ].filter((value, index) => index >= 0), + ], bad: [ { @@ -1219,5 +1219,41 @@ generateRuleTests({ `); }, }, + { + template: ' Title\n \n ', + fixedTemplate: 'Title\n\n', + + verifyResults(results) { + expect(results).toMatchInlineSnapshot(` + Array [ + Object { + "column": 2, + "endColumn": 22, + "endLine": 1, + "filePath": "layout.hbs", + "isFixable": true, + "line": 1, + "message": "Incorrect indentation for \`\` beginning at L1:C2. Expected \`<title>\` to be at an indentation of 0, but was found at 2.", + "rule": "block-indentation", + "severity": 2, + "source": "<title>Title", + }, + Object { + "column": 2, + "endColumn": 6, + "endLine": 3, + "filePath": "layout.hbs", + "isFixable": true, + "line": 2, + "message": "Incorrect indentation for \`\` beginning at L2:C2. Expected \`\` to be at an indentation of 0, but was found at 2.", + "rule": "block-indentation", + "severity": 2, + "source": " + ", + }, + ] + `); + }, + }, ], }); From 55e42947be71f6b4adc3e274bf4f457ff607a84e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Wed, 5 Jan 2022 13:01:44 +0100 Subject: [PATCH 3/4] fix: fix some issues --- lib/rules/block-indentation.js | 77 ++++++++++++++--------- test/unit/rules/block-indentation-test.js | 66 +++++++++++++++++++ 2 files changed, 112 insertions(+), 31 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index 34ffc6cec..c2a7232d0 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -232,6 +232,7 @@ module.exports = class BlockIndentation extends Rule { // issues (since the column of the start block was already checked in the // parent's validateBlockChildren()) if (node.loc.start.line === node.loc.end.line || this.seen.has(node)) { + this.seen.add(fixedNode); return fixedNode; } @@ -294,8 +295,8 @@ module.exports = class BlockIndentation extends Rule { return fixedNode; } - validateBlockStart(node, path) { - if (!this.shouldValidateBlockEnd(node)) { + validateBlockStart(node) { + if (!this.shouldValidateBlockStart(node)) { return node; } @@ -306,21 +307,9 @@ module.exports = class BlockIndentation extends Rule { // If it's not a direct child of the template the block start is already fixed by the validateBlockChildren function if (isDirectChildOfTemplate && startColumn !== 0) { if (this.mode === 'fix') { - if (isDirectChildOfTemplate) { - const previousNode = this.template.body[indexOfNodeInTemplate - 1]; - if (previousNode.type === 'TextNode') { - previousNode.chars = removeWhitespaceEnd(previousNode.chars); - } - } else { - // If we are in a child (path set) we want to edit directly the source and not the source of the node - const elementSource = path - ? this.sourceEdited || this.source.join('') - : sourceForLoc(this.sourceEdited || this.source, { - end: node.loc.end, - start: { line: node.loc.start.line, column: 0 }, - }); - const fixedNode = this.fixLine(elementSource.split('\n'), 0, startColumn, 0, path); - return fixedNode; + const previousNode = this.template.body[indexOfNodeInTemplate - 1]; + if (previousNode.type === 'TextNode') { + previousNode.chars = removeWhitespaceEnd(previousNode.chars); } } else { let isElementNode = node && node.type === 'ElementNode'; @@ -624,7 +613,7 @@ module.exports = class BlockIndentation extends Rule { const line = lines[lineNumber]; // If line starts with only white spaces... - if (line.startsWith(' '.repeat(actualColumn))) { + if (line.startsWith(' '.repeat(actualColumn)) || actualColumn === expectedColumn) { // ... then it means we just need to fix the number of whitespace lines[lineNumber] = actualColumn ? line.replace(/^ +/, ' '.repeat(expectedColumn)) @@ -636,7 +625,9 @@ module.exports = class BlockIndentation extends Rule { )}${line.slice(Math.max(0, actualColumn))}`; } - let node = parse(lines.join('\n')).body.find((node) => node.type !== 'TextNode'); + this.sourceEdited = lines.join('\n'); + + let node = parse(this.sourceEdited).body.find((node) => node.type !== 'TextNode'); node = get(node, path); // As we recreate a new node the _isElseIfBlock is not set anymore so we need to recompute it @@ -646,8 +637,6 @@ module.exports = class BlockIndentation extends Rule { elseBlockStatement._isElseIfBlock = true; } - this.sourceEdited = lines.join('\n'); - return node; } @@ -668,14 +657,28 @@ module.exports = class BlockIndentation extends Rule { return false; } - shouldValidateBlockEnd(node) { + sourceDoesNotMatchNode(node, shouldIncludeEndingToken) { + let source = sourceForLoc(this.sourceEdited || this.source, node.loc); + let endingToken = `/${node.path.original}`; + let indexOfEnding = source.lastIndexOf(endingToken); + + // Do not validate if source doesn't match node (if vs iframe) + let charAfterEnding = source[indexOfEnding + endingToken.length]; + if (indexOfEnding !== -1 && !VALID_FOLLOWING_CHARS.has(charAfterEnding)) { + return true; + } + + return !shouldIncludeEndingToken || indexOfEnding !== -1; + } + + shouldValidateBlockStart(node) { if (node._isElseIfBlock) { return false; } // do not validate indentation on VOID_TAG's if (VOID_TAGS[node.tag]) { - return false; + return true; } if (this.isWithinIgnoredElement()) { @@ -684,20 +687,32 @@ module.exports = class BlockIndentation extends Rule { // do not validate nodes without children (whitespace will count as TextNodes) if (node && node.type === 'ElementNode') { - return AstNodeInfo.hasChildren(node); + return true; } - let source = sourceForLoc(this.sourceEdited || this.source, node.loc); - let endingToken = `/${node.path.original}`; - let indexOfEnding = source.lastIndexOf(endingToken); + return this.sourceDoesNotMatchNode(node, false); + } - // Do not validate if source doesn't match node (if vs iframe) - let charAfterEnding = source[indexOfEnding + endingToken.length]; - if (indexOfEnding !== -1 && !VALID_FOLLOWING_CHARS.has(charAfterEnding)) { + shouldValidateBlockEnd(node) { + if (node._isElseIfBlock) { + return false; + } + + // do not validate indentation on VOID_TAG's + if (VOID_TAGS[node.tag]) { + return false; + } + + if (this.isWithinIgnoredElement()) { return false; } - return indexOfEnding !== -1; + // do not validate nodes without children (whitespace will count as TextNodes) + if (node && node.type === 'ElementNode') { + return AstNodeInfo.hasChildren(node); + } + + return this.sourceDoesNotMatchNode(node, true); } endingControlCharCount(node) { diff --git a/test/unit/rules/block-indentation-test.js b/test/unit/rules/block-indentation-test.js index 9b5b48d6b..e72a8eec0 100644 --- a/test/unit/rules/block-indentation-test.js +++ b/test/unit/rules/block-indentation-test.js @@ -1255,5 +1255,71 @@ generateRuleTests({ `); }, }, + { + template: '
\n
\n {{#if foo}}\n {{#if bar}}\n {{/if}}\n {{/if}}', + fixedTemplate: '
\n
\n{{#if foo}}\n {{#if bar}}\n {{/if}}\n{{/if}}', + + verifyResults(results) { + expect(results).toMatchInlineSnapshot(` + Array [ + Object { + "column": 2, + "endColumn": 9, + "endLine": 6, + "filePath": "layout.hbs", + "isFixable": true, + "line": 3, + "message": "Incorrect indentation for \`{{#if}}\` beginning at L3:C2. Expected \`{{#if}}\` to be at an indentation of 0, but was found at 2.", + "rule": "block-indentation", + "severity": 2, + "source": "{{#if foo}} + {{#if bar}} + {{/if}} + {{/if}}", + }, + ] + `); + }, + }, + { + template: '

\n
\n
\n
\n
\n
\n
', + fixedTemplate: '

\n
\n
\n
\n
\n
\n
', + + verifyResults(results) { + expect(results).toMatchInlineSnapshot(` + Array [ + Object { + "column": 2, + "endColumn": 11, + "endLine": 1, + "filePath": "layout.hbs", + "isFixable": true, + "line": 1, + "message": "Incorrect indentation for \`

\` beginning at L1:C2. Expected \`

\` to be at an indentation of 0, but was found at 2.", + "rule": "block-indentation", + "severity": 2, + "source": "

", + }, + Object { + "column": 2, + "endColumn": 8, + "endLine": 7, + "filePath": "layout.hbs", + "isFixable": true, + "line": 2, + "message": "Incorrect indentation for \`
\` beginning at L2:C2. Expected \`
\` to be at an indentation of 0, but was found at 2.", + "rule": "block-indentation", + "severity": 2, + "source": "
+
+
+
+
+
", + }, + ] + `); + }, + }, ], }); From 7088cf08372bbe71db7365c4807e07af3d2be80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Wed, 5 Jan 2022 16:54:29 +0100 Subject: [PATCH 4/4] fix: fix indentation when tag has a leading content --- lib/rules/block-indentation.js | 72 +++++++++++++---------- test/unit/rules/block-indentation-test.js | 11 +++- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index c2a7232d0..1ec63b49b 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -302,10 +302,12 @@ module.exports = class BlockIndentation extends Rule { const indexOfNodeInTemplate = this.template.body.indexOf(node); const isDirectChildOfTemplate = indexOfNodeInTemplate !== -1; - let startColumn = node.loc.start.column; + const hasLeadingContent = + !isDirectChildOfTemplate || this.hasLeadingContent(node, this.template.body); + const startColumn = node.loc.start.column; // If it's not a direct child of the template the block start is already fixed by the validateBlockChildren function - if (isDirectChildOfTemplate && startColumn !== 0) { + if (isDirectChildOfTemplate && startColumn !== 0 && !hasLeadingContent) { if (this.mode === 'fix') { const previousNode = this.template.body[indexOfNodeInTemplate - 1]; if (previousNode.type === 'TextNode') { @@ -421,36 +423,7 @@ module.exports = class BlockIndentation extends Rule { break; } - // We might not actually be the first thing on the line. We might be - // preceded by another element or statement, or by some text. So walk - // backwards looking for something else on this line. - let hasLeadingContent = false; - for (let j = i - 2; j >= 0; j--) { - let sibling = children[j]; - if (sibling.loc && sibling.type !== 'TextNode') { - // Found an element or statement. If it's on this line, then we - // have leading content, so set the flag and break. If it's not - // on this line, then we've scanned back to a previous line, so - // we can also break. - if (sibling.loc.end.line === child.loc.start.line) { - hasLeadingContent = true; - } - break; - } else { - let lines = sibling.chars.split(/[\n\r]/); - let lastLine = lines[lines.length - 1]; - if (lastLine.trim()) { - // The last line in this text node has non-whitespace content, so - // set the flag. - hasLeadingContent = true; - } - if (lines.length > 1) { - // There are multiple lines meaning we've now scanned back to a - // previous line, so we can break. - break; - } - } - } + const hasLeadingContent = this.hasLeadingContent(child, children); if (hasLeadingContent) { // There's content before us on the same line, so we don't care about @@ -584,6 +557,41 @@ module.exports = class BlockIndentation extends Rule { return node; } + hasLeadingContent(node, parentChildren) { + const currentIndex = parentChildren.indexOf(node); + // We might not actually be the first thing on the line. We might be + // preceded by another element or statement, or by some text. So walk + // backwards looking for something else on this line. + for (let j = currentIndex - 1; j >= 0; j--) { + let sibling = parentChildren[j]; + if (sibling.loc && sibling.type !== 'TextNode') { + // Found an element or statement. If it's on this line, then we + // have leading content, so set the flag and break. If it's not + // on this line, then we've scanned back to a previous line, so + // we can also break. + if (sibling.loc.end.line === node.loc.start.line) { + return true; + } + break; + } else { + let lines = sibling.chars.split(/[\n\r]/); + let lastLine = lines[lines.length - 1]; + if (lastLine.trim()) { + // The last line in this text node has non-whitespace content, so + // set the flag. + return true; + } + if (lines.length > 1) { + // There are multiple lines meaning we've now scanned back to a + // previous line, so we can break. + break; + } + } + } + + return false; + } + getDisplayNameForNode(node) { switch (node.type) { case 'ElementNode': diff --git a/test/unit/rules/block-indentation-test.js b/test/unit/rules/block-indentation-test.js index e72a8eec0..b80183c87 100644 --- a/test/unit/rules/block-indentation-test.js +++ b/test/unit/rules/block-indentation-test.js @@ -145,7 +145,7 @@ generateRuleTests({ config: { ignoreComments: true, }, - template: ' {{! Comment }}\n
foo
', + template: ' {{! Comment }}
foo
', }, { config: { @@ -169,7 +169,7 @@ generateRuleTests({ config: { ignoreComments: true, }, - template: ' \n
foo
', + template: '
foo
', }, { config: { @@ -227,6 +227,13 @@ generateRuleTests({ '{{/if}}', ].join('\n'), }, + { + template: + '{{relativeDate}} ({{absoluteDate}})', + }, + { + template: '\n', + }, ], bad: [