From b1ecfffd44acfc8a89a9f4ba43540a46744d3e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Thu, 16 Dec 2021 17:48:59 +0100 Subject: [PATCH 1/8] feat(block-indentation): add autofix to the block-indentation rule --- README.md | 2 +- docs/rule/block-indentation.md | 2 + lib/rules/block-indentation.js | 415 ++++++++++++++++------ test/acceptance/public-api-test.js | 2 + test/unit/rules/block-indentation-test.js | 127 ++++++- 5 files changed, 435 insertions(+), 113 deletions(-) diff --git a/README.md b/README.md index d543d8bd8..e7ee376af 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ Each rule has emojis denoting: | Name | ✅ | 💅 | ⌨️ | 🔧 | | :-------------------------------------------------------------------------------------------------------- | :-- | :-- | :-- | --- | -| [attribute-indentation](./docs/rule/attribute-indentation.md) | | | | | +| [attribute-indentation](./docs/rule/attribute-indentation.md) | | | | 🔧 | | [block-indentation](./docs/rule/block-indentation.md) | | 💅 | | | | [builtin-component-arguments](./docs/rule/builtin-component-arguments.md) | ✅ | | | | | [deprecated-each-syntax](./docs/rule/deprecated-each-syntax.md) | | | | | diff --git a/docs/rule/block-indentation.md b/docs/rule/block-indentation.md index d028486a7..7c30cbb81 100644 --- a/docs/rule/block-indentation.md +++ b/docs/rule/block-indentation.md @@ -2,6 +2,8 @@ 💅 The `extends: 'stylistic'` property in a configuration file enables this rule. +🔧 The `--fix` option on the command line can automatically fix some of the problems reported by this rule. + Good indentation is crucial for long-term maintenance of templates. For example, having blocks misaligned is a common cause of logic errors. ## Examples diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index 0bfe98e94..e0a342b5c 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -21,7 +21,7 @@

{{t "greeting"}}

-
+ ``` 2. Forces children of all blocks to start at a single indentation level deeper. @@ -52,6 +52,8 @@ const assert = require('assert'); +const { parse, sourceForLoc } = require('ember-template-recast'); + const AstNodeInfo = require('../helpers/ast-node-info'); const createErrorMessage = require('../helpers/create-error-message'); const Rule = require('./_base'); @@ -81,6 +83,18 @@ function isControlChar(char) { return char === '~' || char === '{' || char === '}'; } +function get(node, path) { + let value = node; + + if (path) { + for (const property of path) { + value = value[property]; + } + } + + return value; +} + module.exports = class BlockIndentation extends Rule { parseConfig(config) { let configType = typeof config; @@ -161,17 +175,33 @@ module.exports = class BlockIndentation extends Rule { visitor() { this._elementStack = []; + this.seen = new Set(); + this.nodeToSource = new WeakMap(); return { + Template(node) { + if (this.mode === 'fix') { + if ( + node.body[0] && + node.body[0].type === 'TextNode' && + !node.body[0].chars.replace(/ +/, '') + ) { + // We need to remove the text node in this case + node.body.shift(); + } + } + return node; + }, + BlockStatement(node) { - this.process(node); + return this.process(node); }, ElementNode: { enter(node) { this._elementStack.push(node); - this.process(node); + return this.process(node); }, exit() { this._elementStack.pop(); @@ -180,56 +210,132 @@ module.exports = class BlockIndentation extends Rule { }; } - process(node) { + process(node, nestedOptions) { // 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) { - return; + if (node.loc.start.line === node.loc.end.line || this.seen.has(node)) { + return nestedOptions ? nestedOptions.sourceToEdit : node; + } + + let fixedNode = this.validateBlockStart(node, nestedOptions); + fixedNode = this.validateBlockElse(fixedNode, nestedOptions); + fixedNode = this.validateBlockEnd(fixedNode, nestedOptions); + fixedNode = this.validateBlockChildren(fixedNode, nestedOptions); + + if (this.mode === 'fix') { + const { sourceToEdit, path } = nestedOptions || { path: [] }; + const sourceToEditBeforeFix = + sourceToEdit || this.nodeToSource.get(fixedNode) || this.source.join(''); + let sourceToEditFixed = sourceToEditBeforeFix; + + const childrenWithPath = []; + + if (fixedNode.inverse) { + const inverseChildPath = [...path, 'inverse', 'body']; + for (const [index, inverseNestedNode] of fixedNode.inverse.body.entries()) { + childrenWithPath.push({ + path: [...inverseChildPath, index], + child: inverseNestedNode, + }); + } + } + + const childPath = + fixedNode.type === 'BlockStatement' ? [...path, 'program', 'body'] : [...path, 'children']; + const children = + fixedNode.type === 'BlockStatement' ? fixedNode.program.body : fixedNode.children; + for (const [index, nestedNode] of children.entries()) { + childrenWithPath.push({ + path: [...childPath, index], + child: nestedNode, + }); + } + + for (const { path, child } of childrenWithPath) { + if (['BlockStatement', 'ElementNode'].includes(child.type)) { + sourceToEditFixed = this.process(child, { + sourceToEdit: sourceToEditFixed, + path, + }); + if (nestedOptions) { + nestedOptions.sourceToEdit = sourceToEditFixed; + } + } + } + + if (sourceToEditBeforeFix !== sourceToEditFixed) { + fixedNode = this.fixLine( + sourceToEditFixed.split('\n'), + 0, + fixedNode.loc.start.column, + fixedNode.loc.start.column, + nestedOptions && nestedOptions.path + ); + } } - this.validateBlockStart(node); - this.validateBlockElse(node); - this.validateBlockEnd(node); - this.validateBlockChildren(node); + this.seen.add(fixedNode); + + return nestedOptions ? nestedOptions.sourceToEdit : fixedNode; } - validateBlockStart(node) { + validateBlockStart(node, nestedOptions = {}) { if (!this.shouldValidateBlockEnd(node)) { - return; + return node; } if (this.isWithinIgnoredElement()) { - return; + return node; } let startColumn = node.loc.start.column; let startLine = node.loc.start.line; if (startLine === 1 && startColumn !== 0) { - let isElementNode = node && node.type === 'ElementNode'; - let displayName = isElementNode ? node.tag : node.path.original; - let display = isElementNode ? `<${displayName}>` : `{{#${displayName}}}`; - let startLocation = `L${node.loc.start.line}:C${node.loc.start.column}`; - - let warning = - `Incorrect indentation for \`${display}\` beginning at ${startLocation}. ` + - `Expected \`${display}\` to be at an indentation of 0, but was found at ${startColumn}.`; - - this.log({ - message: warning, - node, - }); + if (this.mode === 'fix') { + const elementSource = + nestedOptions.sourceToEdit || + sourceForLoc(this.nodeToSource.get(node) || this.source, { + end: node.loc.end, + start: { line: node.loc.start.line, column: 0 }, + }); + const fixedNode = this.fixLine( + elementSource.split('\n'), + 0, + startColumn, + 0, + nestedOptions.path + ); + nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); + return fixedNode; + } else { + let isElementNode = node && node.type === 'ElementNode'; + let displayName = isElementNode ? node.tag : node.path.original; + let display = isElementNode ? `<${displayName}>` : `{{#${displayName}}}`; + let startLocation = `L${node.loc.start.line}:C${node.loc.start.column}`; + + let warning = + `Incorrect indentation for \`${display}\` beginning at ${startLocation}. ` + + `Expected \`${display}\` to be at an indentation of 0, but was found at ${startColumn}.`; + + this.log({ + message: warning, + node, + isFixable: true, + }); + } } + return node; } - validateBlockEnd(node) { + validateBlockEnd(node, nestedOptions = {}) { if (!this.shouldValidateBlockEnd(node)) { - return; + return node; } if (this.isWithinIgnoredElement()) { - return; + return node; } let isElementNode = node && node.type === 'ElementNode'; @@ -242,44 +348,73 @@ module.exports = class BlockIndentation extends Rule { let correctedEndColumn = endColumn - displayName.length - controlCharCount; if (correctedEndColumn !== startColumn) { - let startLocation = `L${node.loc.start.line}:C${node.loc.start.column}`; - let endLocation = `L${node.loc.end.line}:C${node.loc.end.column}`; - - let warning = - `Incorrect indentation for \`${displayName}\` beginning at ${startLocation}` + - `. Expected \`${display}\` ending at ${endLocation} to be at an indentation of ${startColumn} but ` + - `was found at ${correctedEndColumn}.`; - - this.log({ - message: warning, - node, - line: node.loc.end.line, - column: node.loc.end.column, - }); + if (this.mode === 'fix') { + const elementSource = + nestedOptions.sourceToEdit || + `${' '.repeat(node.loc.start.column)}${sourceForLoc( + this.nodeToSource.get(node) || this.source, + { + end: node.loc.end, + start: { line: node.loc.start.line, column: node.loc.start.column }, + } + )}`; + const lines = elementSource.split('\n'); + const fixedNode = this.fixLine( + lines, + node.loc.end.line - (nestedOptions.sourceToEdit ? 1 : node.loc.start.line), + correctedEndColumn, + startColumn, + nestedOptions.path + ); + nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); + return fixedNode; + } else { + let startLocation = `L${node.loc.start.line}:C${node.loc.start.column}`; + let endLocation = `L${node.loc.end.line}:C${node.loc.end.column}`; + + let warning = + `Incorrect indentation for \`${displayName}\` beginning at ${startLocation}` + + `. Expected \`${display}\` ending at ${endLocation} to be at an indentation of ${startColumn} but ` + + `was found at ${correctedEndColumn}.`; + + this.log({ + message: warning, + node, + line: node.loc.end.line, + column: node.loc.end.column, + isFixable: true, + }); + } } + + return node; } - validateBlockChildren(node) { + validateBlockChildren(node, nestedOptions = {}) { if (this.isWithinIgnoredElement()) { - return; + return node; } let children = AstNodeInfo.childrenFor(node).filter((x) => !x._isElseIfBlock); if (!AstNodeInfo.hasChildren(node)) { - return; + return node; } // HTML elements that start and end on the same line are fine if (node.loc.start.line === node.loc.end.line) { - return; + return node; } let startColumn = node.loc.start.column; let expectedStartColumn = startColumn + this.config.indentation; + let fixedNode = node; - for (let i = 0; i < children.length; i++) { + let i = 0; + while (i < children.length) { let child = children[i]; + i += 1; + if (!child.loc) { continue; } @@ -295,7 +430,7 @@ module.exports = class BlockIndentation extends Rule { // 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 - 1; j >= 0; j--) { + 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 @@ -347,7 +482,7 @@ module.exports = class BlockIndentation extends Rule { if (/^(\r\n|\n)/.test(child.chars)) { childStartColumn = 0; let newLineLength = child.chars.length - withoutLeadingNewLines.length; - let leadingNewLines = child.chars.slice(newLineLength); + let leadingNewLines = child.chars.slice(0, newLineLength); childStartLine += (leadingNewLines.match(/\n/g) || []).length; } @@ -361,92 +496,154 @@ module.exports = class BlockIndentation extends Rule { } if (expectedStartColumn !== childStartColumn) { - let isElementNode = child.type === 'ElementNode'; - let display; + if (this.mode === 'fix') { + const sourceToFix = + nestedOptions.sourceToEdit || + sourceForLoc(this.nodeToSource.get(fixedNode) || this.source, { + end: fixedNode.loc.end, + start: { line: fixedNode.loc.start.line, column: 0 }, + }); + const lines = sourceToFix.split('\n'); + + fixedNode = this.fixLine( + lines, + childStartLine - 1, + childStartColumn, + expectedStartColumn, + nestedOptions.path + ); + nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); - if (isElementNode) { - display = `<${child.tag}>`; + children = AstNodeInfo.childrenFor(fixedNode).filter((x) => !x._isElseIfBlock); } else { - switch (child.type) { - case 'BlockStatement': { - display = `{{#${child.path.original}}}`; + const display = this.getDisplayNameForNode(child); + + let startLocation = `L${childStartLine}:C${childStartColumn}`; + let warning = + `Incorrect indentation for \`${display}\` beginning at ${startLocation}` + + `. Expected \`${display}\` to be at an indentation of ${expectedStartColumn} but ` + + `was found at ${childStartColumn}.`; + + this.log({ + message: warning, + node, + line: childStartLine, + column: childStartColumn, + isFixable: true, + }); + } + } + } - break; - } - case 'MustacheStatement': { - display = `{{${child.path.original}}}`; + return fixedNode; + } - break; - } - case 'TextNode': { - display = child.chars.replace(/^\s*/, ''); + validateBlockElse(node, nestedOptions = {}) { + if (node.type !== 'BlockStatement' || !node.inverse) { + return node; + } - break; - } - case 'CommentStatement': { - display = ``; + if (this.detectNestedElseIfBlock(node)) { + let elseBlockStatement = node.inverse.body[0]; - break; - } - case 'MustacheCommentStatement': { - display = `{{!${child.value}}}`; + elseBlockStatement._isElseIfBlock = true; + } - break; - } - default: { - display = child.path.original; - } - } - } + let inverse = node.inverse; + let startColumn = node.loc.start.column; + let elseStartColumn = node.program.loc.end.column; + + if (elseStartColumn !== startColumn) { + if (this.mode === 'fix') { + const sourceToFix = + nestedOptions.sourceToEdit || + sourceForLoc(this.nodeToSource.get(node) || this.source, { + end: node.loc.end, + start: { line: node.loc.start.line, column: 0 }, + }); + const lines = sourceToFix.split('\n'); + const elseLine = node.program.loc.end.line; + const fixedNode = this.fixLine( + lines, + elseLine - 1, + elseStartColumn, + startColumn, + nestedOptions.path + ); + nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); + return fixedNode; + } else { + let displayName = node.path.original; + let startLocation = `L${node.loc.start.line}:C${node.loc.start.column}`; + let elseLocation = `L${inverse.loc.start.line}:C${elseStartColumn}`; - let startLocation = `L${childStartLine}:C${childStartColumn}`; let warning = - `Incorrect indentation for \`${display}\` beginning at ${startLocation}` + - `. Expected \`${display}\` to be at an indentation of ${expectedStartColumn} but ` + - `was found at ${childStartColumn}.`; + `Incorrect indentation for inverse block of \`{{#${displayName}}}\` beginning at ${startLocation}` + + `. Expected \`{{else}}\` starting at ${elseLocation} to be at an indentation of ${startColumn} but ` + + `was found at ${elseStartColumn}.`; this.log({ message: warning, node, - line: childStartLine, - column: childStartColumn, + line: inverse.loc.start.line, + column: elseStartColumn, + isFixable: true, }); } } + + return node; } - validateBlockElse(node) { - if (node.type !== 'BlockStatement' || !node.inverse) { - return; + getDisplayNameForNode(node) { + switch (node.type) { + case 'ElementNode': + return `<${node.tag}>`; + case 'BlockStatement': { + return `{{#${node.path.original}}}`; + } + case 'MustacheStatement': { + return `{{${node.path.original}}}`; + } + case 'TextNode': { + return node.chars.replace(/^\s*/, ''); + } + case 'CommentStatement': { + return ``; + } + case 'MustacheCommentStatement': { + return `{{!${node.value}}}`; + } + default: { + return node.path.original; + } + } + } + + fixLine(lines, lineNumber, actualColumn, expectedColumn, path) { + const line = lines[lineNumber]; + if (!line.startsWith(' '.repeat(actualColumn))) { + lines[lineNumber] = `${line.slice(0, Math.max(0, actualColumn)).trimEnd()}\n${' '.repeat( + expectedColumn + )}${line.slice(Math.max(0, actualColumn))}`; + } else { + lines[lineNumber] = actualColumn + ? line.replace(/^ +/, ' '.repeat(expectedColumn)) + : `${' '.repeat(expectedColumn)}${line}`; } + let node = parse(lines.join('\n')).body.find((node) => node.type !== 'TextNode'); + node = get(node, path); + if (this.detectNestedElseIfBlock(node)) { let elseBlockStatement = node.inverse.body[0]; elseBlockStatement._isElseIfBlock = true; } - let inverse = node.inverse; - let startColumn = node.loc.start.column; - let elseStartColumn = node.program.loc.end.column; + this.nodeToSource.set(node, lines.join('\n')); - if (elseStartColumn !== startColumn) { - let displayName = node.path.original; - let startLocation = `L${node.loc.start.line}:C${node.loc.start.column}`; - let elseLocation = `L${inverse.loc.start.line}:C${elseStartColumn}`; - - let warning = - `Incorrect indentation for inverse block of \`{{#${displayName}}}\` beginning at ${startLocation}` + - `. Expected \`{{else}}\` starting at ${elseLocation} to be at an indentation of ${startColumn} but ` + - `was found at ${elseStartColumn}.`; - - this.log({ - message: warning, - node, - line: inverse.loc.start.line, - column: elseStartColumn, - }); - } + return node; } detectNestedElseIfBlock(node) { diff --git a/test/acceptance/public-api-test.js b/test/acceptance/public-api-test.js index 01d7338a8..b36201767 100644 --- a/test/acceptance/public-api-test.js +++ b/test/acceptance/public-api-test.js @@ -489,6 +489,7 @@ describe('public api', function () { message: 'Incorrect indentation for `

` beginning at L2:C0. Expected `

` to be at an indentation of 2 but was found at 0.', filePath: 'some/path/here.hbs', + isFixable: true, line: 2, column: 0, endColumn: 6, @@ -826,6 +827,7 @@ describe('public api', function () { message: 'Incorrect indentation for `

` beginning at L2:C0. Expected `

` to be at an indentation of 2 but was found at 0.', filePath: 'some/path/here.hbs', + isFixable: true, line: 2, column: 0, endColumn: 6, diff --git a/test/unit/rules/block-indentation-test.js b/test/unit/rules/block-indentation-test.js index 7c1c2b4d7..5297f6dab 100644 --- a/test/unit/rules/block-indentation-test.js +++ b/test/unit/rules/block-indentation-test.js @@ -233,6 +233,7 @@ generateRuleTests({ { // start and end must be the same indentation template: '\n {{#each cats as |dog|}}\n {{/each}}', + fixedTemplate: '\n {{#each cats as |dog|}}\n {{/each}}', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -242,6 +243,7 @@ generateRuleTests({ "endColumn": 17, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 3, "message": "Incorrect indentation for \`each\` beginning at L2:C2. Expected \`{{/each}}\` ending at L3:C17 to be at an indentation of 2 but was found at 8.", "rule": "block-indentation", @@ -255,6 +257,7 @@ generateRuleTests({ }, { template: '

\n
', + fixedTemplate: '
\n
', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -264,6 +267,7 @@ generateRuleTests({ "endColumn": 8, "endLine": 2, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`div\` beginning at L1:C0. Expected \`\` ending at L2:C8 to be at an indentation of 0 but was found at 2.", "rule": "block-indentation", @@ -277,6 +281,7 @@ generateRuleTests({ }, { template: '
\n

Stuff goes here

', + fixedTemplate: '
\n

Stuff goes here

\n
', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -286,6 +291,7 @@ generateRuleTests({ "endColumn": 30, "endLine": 2, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`div\` beginning at L1:C0. Expected \`\` ending at L2:C30 to be at an indentation of 0 but was found at 24.", "rule": "block-indentation", @@ -308,6 +314,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`

\` beginning at L2:C0. Expected \`

\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -322,6 +329,7 @@ generateRuleTests({ }, { template: '{{#if}}\n

Stuff goes here

\n{{/if}}', + fixedTemplate: '{{#if}}\n

Stuff goes here

\n{{/if}}', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -331,6 +339,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`

\` beginning at L2:C0. Expected \`

\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -351,6 +360,13 @@ generateRuleTests({ ' Good night\n' + ' {{/if}}\n' + '{{/if}}', + fixedTemplate: + '{{#if isMorning}}\n' + + '{{else}}\n' + + ' {{#if something}}\n' + + ' Good night\n' + + ' {{/if}}\n' + + '{{/if}}', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -360,6 +376,7 @@ generateRuleTests({ "endColumn": 11, "endLine": 5, "filePath": "layout.hbs", + "isFixable": true, "line": 5, "message": "Incorrect indentation for \`if\` beginning at L3:C2. Expected \`{{/if}}\` ending at L5:C11 to be at an indentation of 2 but was found at 4.", "rule": "block-indentation", @@ -375,7 +392,8 @@ generateRuleTests({ { config: 4, - template: '' + '

\n' + '

Hi!

\n' + '
', + template: '
\n' + '

Hi!

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

Hi!

\n' + '
', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -385,6 +403,7 @@ generateRuleTests({ "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 4 but was found at 2.", "rule": "block-indentation", @@ -399,6 +418,7 @@ generateRuleTests({ }, { template: '

\n' + ' {{foo}}\n' + '{{bar}}\n' + '
', + fixedTemplate: '
\n' + ' {{foo}}\n' + ' {{bar}}\n' + '
', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -408,6 +428,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 4, "filePath": "layout.hbs", + "isFixable": true, "line": 3, "message": "Incorrect indentation for \`{{bar}}\` beginning at L3:C0. Expected \`{{bar}}\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -423,6 +444,7 @@ generateRuleTests({ }, { template: '
\n' + ' Foo:\n' + '{{bar}}\n' + '
', + fixedTemplate: '
\n' + ' Foo:\n' + ' {{bar}}\n' + '
', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -432,6 +454,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 4, "filePath": "layout.hbs", + "isFixable": true, "line": 3, "message": "Incorrect indentation for \`{{bar}}\` beginning at L3:C0. Expected \`{{bar}}\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -450,6 +473,11 @@ generateRuleTests({ // has other content preceding it on its line template: '
\n' + ' Foo{{#some-thing}}\n' + ' {{/some-thing}}\n' + '
', + fixedTemplate: + '
\n' + + ' Foo{{#some-thing}}\n' + + ' {{/some-thing}}\n' + + '
', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -459,6 +487,7 @@ generateRuleTests({ "endColumn": 17, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 3, "message": "Incorrect indentation for \`some-thing\` beginning at L2:C18. Expected \`{{/some-thing}}\` ending at L3:C17 to be at an indentation of 18 but was found at 2.", "rule": "block-indentation", @@ -474,6 +503,8 @@ generateRuleTests({ // Start and end of multi-line element must be aligned, even when start // has other content preceding it on its line template: '{{#if foo}}\n' + ' {{foo}}

\n' + ' Bar\n' + '

\n' + '{{/if}}', + fixedTemplate: + '{{#if foo}}\n' + ' {{foo}}

\n' + ' Bar\n' + '

\n' + '{{/if}}', verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -483,6 +514,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 4, "filePath": "layout.hbs", + "isFixable": true, "line": 4, "message": "Incorrect indentation for \`p\` beginning at L2:C10. Expected \`

\` ending at L4:C6 to be at an indentation of 10 but was found at 2.", "rule": "block-indentation", @@ -498,6 +530,7 @@ generateRuleTests({ { template: ['
', '', '
'].join('\n'), + fixedTemplate: ['
', ' ', '
'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -507,6 +540,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`\` beginning at L2:C0. Expected \`\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -521,6 +555,7 @@ generateRuleTests({ }, { template: ['{{#if foo}}', ' {{else}}', '{{/if}}'].join('\n'), + fixedTemplate: ['{{#if foo}}', '{{else}}', '{{/if}}'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -530,6 +565,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for inverse block of \`{{#if}}\` beginning at L1:C0. Expected \`{{else}}\` starting at L2:C2 to be at an indentation of 0 but was found at 2.", "rule": "block-indentation", @@ -552,6 +588,14 @@ generateRuleTests({ ' {{/if~}}', ' {{/if}}', ].join('\n'), + fixedTemplate: [ + '{{#if foo}}', + '{{else if bar}}', + '{{else}}', + ' {{#if baz}}', + ' {{/if~}}', + '{{/if}}', + ].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -561,6 +605,7 @@ generateRuleTests({ "endColumn": 9, "endLine": 6, "filePath": "layout.hbs", + "isFixable": true, "line": 6, "message": "Incorrect indentation for \`if\` beginning at L1:C0. Expected \`{{/if}}\` ending at L6:C9 to be at an indentation of 0 but was found at 2.", "rule": "block-indentation", @@ -579,6 +624,7 @@ generateRuleTests({ { template: ['{{#each foo as |bar|}}', ' {{else}}', '{{/each}}'].join('\n'), + fixedTemplate: ['{{#each foo as |bar|}}', '{{else}}', '{{/each}}'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -588,6 +634,7 @@ generateRuleTests({ "endColumn": 9, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for inverse block of \`{{#each}}\` beginning at L1:C0. Expected \`{{else}}\` starting at L2:C2 to be at an indentation of 0 but was found at 2.", "rule": "block-indentation", @@ -609,6 +656,13 @@ generateRuleTests({ ' {{! comment with incorrect indentation }}', '', ].join('\n'), + fixedTemplate: [ + '
', + ' {{#if foo}}', + ' {{/if}}', + ' {{! comment with incorrect indentation }}', + '
', + ].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -618,6 +672,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 5, "filePath": "layout.hbs", + "isFixable": true, "line": 4, "message": "Incorrect indentation for \`{{! comment with incorrect indentation }}\` beginning at L4:C4. Expected \`{{! comment with incorrect indentation }}\` to be at an indentation of 2 but was found at 4.", "rule": "block-indentation", @@ -643,6 +698,15 @@ generateRuleTests({ ' Good night\n' + '{{/if}}', ].join('\n'), + fixedTemplate: [ + '{{#if isMorning}}\n' + + ' Good morning\n' + + '{{else if isAfternoon}}\n' + + ' Good afternoon\n' + + '{{else}}\n' + + ' Good night\n' + + '{{/if}}', + ].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -652,6 +716,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 6, "filePath": "layout.hbs", + "isFixable": true, "line": 1, "message": "Incorrect indentation for \`Good morning \` beginning at L1:C19. Expected \`Good morning @@ -678,6 +743,13 @@ generateRuleTests({ ' Good afternoon\n' + '{{/if}}', ].join('\n'), + fixedTemplate: [ + '{{#if isMorning}}\n' + + ' Good morning\n' + + '{{else if isAfternoon~}}\n' + + ' Good afternoon\n' + + '{{/if}}', + ].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -687,6 +759,7 @@ generateRuleTests({ "endColumn": 0, "endLine": 5, "filePath": "layout.hbs", + "isFixable": true, "line": 4, "message": "Incorrect indentation for \`Good afternoon \` beginning at L4:C4. Expected \`Good afternoon @@ -703,6 +776,7 @@ generateRuleTests({ }, { template: ['
', '{{! What a comment }}', ' {{foo-bar}}', '
'].join('\n'), + fixedTemplate: ['
', ' {{! What a comment }}', ' {{foo-bar}}', '
'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -712,6 +786,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 4, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`{{! What a comment }}\` beginning at L2:C0. Expected \`{{! What a comment }}\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -727,6 +802,7 @@ generateRuleTests({ }, { template: ['
{{! bad comment }}', ' {{foo-bar}}', '
'].join('\n'), + fixedTemplate: ['
', ' {{! bad comment }}', ' {{foo-bar}}', '
'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -736,6 +812,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 1, "message": "Incorrect indentation for \`{{! bad comment }}\` beginning at L1:C6. Expected \`{{! bad comment }}\` to be at an indentation of 2 but was found at 6.", "rule": "block-indentation", @@ -750,6 +827,13 @@ generateRuleTests({ }, { template: ['{{#if media.isMobile}}', '{{else}}', '', '', '{{/if}}'].join('\n'), + fixedTemplate: [ + '{{#if media.isMobile}}', + '{{else}}', + ' ', + ' ', + '{{/if}}', + ].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -759,6 +843,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 5, "filePath": "layout.hbs", + "isFixable": true, "line": 3, "message": "Incorrect indentation for \`\` beginning at L3:C0. Expected \`\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -775,6 +860,7 @@ generateRuleTests({ }, { template: ['\uFEFF {{#if foo}}', '{{/if}}'].join('\n'), + fixedTemplate: ['\uFEFF{{#if foo}}', '{{/if}}'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -784,6 +870,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 2, "filePath": "layout.hbs", + "isFixable": true, "line": 1, "message": "Incorrect indentation for \`{{#if}}\` beginning at L1:C1. Expected \`{{#if}}\` to be at an indentation of 0, but was found at 1.", "rule": "block-indentation", @@ -796,6 +883,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 2, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`if\` beginning at L1:C1. Expected \`{{/if}}\` ending at L2:C7 to be at an indentation of 1 but was found at 0.", "rule": "block-indentation", @@ -809,6 +897,7 @@ generateRuleTests({ }, { template: ['{{#if foo}}foo{{else}}', ' bar', '{{/if}}'].join('\n'), + fixedTemplate: ['{{#if foo}}', ' foo', '{{else}}', ' bar', '{{/if}}'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -818,6 +907,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 1, "message": "Incorrect indentation for inverse block of \`{{#if}}\` beginning at L1:C0. Expected \`{{else}}\` starting at L1:C14 to be at an indentation of 0 but was found at 14.", "rule": "block-indentation", @@ -831,6 +921,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 1, "message": "Incorrect indentation for \`foo\` beginning at L1:C11. Expected \`foo\` to be at an indentation of 2 but was found at 11.", "rule": "block-indentation", @@ -845,6 +936,7 @@ generateRuleTests({ }, { template: ['{{#if foo}}', ' foo', '{{else}}', ' bar', '{{/if}}'].join('\n'), + fixedTemplate: ['{{#if foo}}', ' foo', '{{else}}', ' bar', '{{/if}}'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -854,6 +946,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 5, "filePath": "layout.hbs", + "isFixable": true, "line": 4, "message": "Incorrect indentation for \`bar \` beginning at L4:C4. Expected \`bar @@ -880,6 +973,15 @@ generateRuleTests({ ' {{/if}}', '{{/if}}', ].join('\n'), + fixedTemplate: [ + '{{#if foo}}', + ' foo', + '{{else}}', + ' {{#if bar}}', + ' bar', + ' {{/if}}', + '{{/if}}', + ].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -889,6 +991,7 @@ generateRuleTests({ "endColumn": 7, "endLine": 7, "filePath": "layout.hbs", + "isFixable": true, "line": 4, "message": "Incorrect indentation for \`{{#if}}\` beginning at L4:C4. Expected \`{{#if}}\` to be at an indentation of 2 but was found at 4.", "rule": "block-indentation", @@ -907,6 +1010,7 @@ generateRuleTests({ }, { template: [' {{#foo-bar}}', ' {{/foo-bar}}'].join('\n'), + fixedTemplate: ['{{#foo-bar}}', '{{/foo-bar}}'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -916,6 +1020,7 @@ generateRuleTests({ "endColumn": 17, "endLine": 2, "filePath": "layout.hbs", + "isFixable": true, "line": 1, "message": "Incorrect indentation for \`{{#foo-bar}}\` beginning at L1:C5. Expected \`{{#foo-bar}}\` to be at an indentation of 0, but was found at 5.", "rule": "block-indentation", @@ -929,6 +1034,7 @@ generateRuleTests({ }, { template: ['
', '
'].join('\n'), + fixedTemplate: ['
', '
'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -938,6 +1044,7 @@ generateRuleTests({ "endColumn": 8, "endLine": 2, "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", @@ -956,6 +1063,12 @@ generateRuleTests({ ' {{foobar.baz}}', '{{/foo}}', ].join('\n'), + fixedTemplate: [ + '{{#foo bar as |foobar|}}', + ' {{#foobar.baz}}{{/foobar.baz}}', + ' {{foobar.baz}}', + '{{/foo}}', + ].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` @@ -965,6 +1078,7 @@ generateRuleTests({ "endColumn": 8, "endLine": 4, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`{{#foobar.baz}}\` beginning at L2:C3. Expected \`{{#foobar.baz}}\` to be at an indentation of 2 but was found at 3.", "rule": "block-indentation", @@ -979,6 +1093,7 @@ generateRuleTests({ "endColumn": 8, "endLine": 4, "filePath": "layout.hbs", + "isFixable": true, "line": 3, "message": "Incorrect indentation for \`{{foobar.baz}}\` beginning at L3:C3. Expected \`{{foobar.baz}}\` to be at an indentation of 2 but was found at 3.", "rule": "block-indentation", @@ -994,6 +1109,7 @@ generateRuleTests({ }, { template: ['
', '', '
'].join('\n'), + fixedTemplate: ['
', ' ', '
'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` Array [ @@ -1002,6 +1118,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`\` beginning at L2:C0. Expected \`\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -1016,6 +1133,7 @@ generateRuleTests({ }, { template: ['
', '{{! Comment }}', '
'].join('\n'), + fixedTemplate: ['
', ' {{! Comment }}', '
'].join('\n'), verifyResults(results) { expect(results).toMatchInlineSnapshot(` Array [ @@ -1024,6 +1142,7 @@ generateRuleTests({ "endColumn": 6, "endLine": 3, "filePath": "layout.hbs", + "isFixable": true, "line": 2, "message": "Incorrect indentation for \`{{! Comment }}\` beginning at L2:C0. Expected \`{{! Comment }}\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", @@ -1038,6 +1157,7 @@ generateRuleTests({ }, { template: ['
', 'test{{! Comment }}', '
'].join('\n'), + fixedTemplate: ['
', ' test{{! Comment }}', '
'].join('\n'), config: { ignoreComments: true, }, @@ -1049,8 +1169,9 @@ generateRuleTests({ "endColumn": 6, "endLine": 3, "filePath": "layout.hbs", - "line": 1, - "message": "Incorrect indentation for \`test\` beginning at L1:C0. Expected \`test\` to be at an indentation of 2 but was found at 0.", + "isFixable": true, + "line": 2, + "message": "Incorrect indentation for \`test\` beginning at L2:C0. Expected \`test\` to be at an indentation of 2 but was found at 0.", "rule": "block-indentation", "severity": 2, "source": "
From 0537cae6cbce4d869d8eb3022a666577206ff7b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Sat, 18 Dec 2021 13:28:01 +0100 Subject: [PATCH 2/8] chore: update readme --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e7ee376af..6931375f9 100644 --- a/README.md +++ b/README.md @@ -87,8 +87,8 @@ Each rule has emojis denoting: | Name | ✅ | 💅 | ⌨️ | 🔧 | | :-------------------------------------------------------------------------------------------------------- | :-- | :-- | :-- | --- | -| [attribute-indentation](./docs/rule/attribute-indentation.md) | | | | 🔧 | -| [block-indentation](./docs/rule/block-indentation.md) | | 💅 | | | +| [attribute-indentation](./docs/rule/attribute-indentation.md) | | | | | +| [block-indentation](./docs/rule/block-indentation.md) | | 💅 | | 🔧 | | [builtin-component-arguments](./docs/rule/builtin-component-arguments.md) | ✅ | | | | | [deprecated-each-syntax](./docs/rule/deprecated-each-syntax.md) | | | | | | [deprecated-inline-view-helper](./docs/rule/deprecated-inline-view-helper.md) | ✅ | | | | From a327c23f1f002b15df2c44def75cb012353d05be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Sat, 18 Dec 2021 13:50:47 +0100 Subject: [PATCH 3/8] chore: get rid of useless nodeToSource --- lib/rules/block-indentation.js | 91 +++++++++++++++------------------- 1 file changed, 41 insertions(+), 50 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index e0a342b5c..35f6eea97 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -176,7 +176,6 @@ module.exports = class BlockIndentation extends Rule { visitor() { this._elementStack = []; this.seen = new Set(); - this.nodeToSource = new WeakMap(); return { Template(node) { @@ -215,7 +214,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)) { - return nestedOptions ? nestedOptions.sourceToEdit : node; + return nestedOptions ? this.sourceEdited : node; } let fixedNode = this.validateBlockStart(node, nestedOptions); @@ -224,10 +223,8 @@ module.exports = class BlockIndentation extends Rule { fixedNode = this.validateBlockChildren(fixedNode, nestedOptions); if (this.mode === 'fix') { - const { sourceToEdit, path } = nestedOptions || { path: [] }; - const sourceToEditBeforeFix = - sourceToEdit || this.nodeToSource.get(fixedNode) || this.source.join(''); - let sourceToEditFixed = sourceToEditBeforeFix; + const { path } = nestedOptions || { path: [] }; + const sourceBeforeFix = this.sourceEdited || this.source.join(''); const childrenWithPath = []; @@ -254,19 +251,15 @@ module.exports = class BlockIndentation extends Rule { for (const { path, child } of childrenWithPath) { if (['BlockStatement', 'ElementNode'].includes(child.type)) { - sourceToEditFixed = this.process(child, { - sourceToEdit: sourceToEditFixed, + this.process(child, { path, }); - if (nestedOptions) { - nestedOptions.sourceToEdit = sourceToEditFixed; - } } } - if (sourceToEditBeforeFix !== sourceToEditFixed) { + if (sourceBeforeFix !== this.sourceEdited) { fixedNode = this.fixLine( - sourceToEditFixed.split('\n'), + this.sourceEdited.split('\n'), 0, fixedNode.loc.start.column, fixedNode.loc.start.column, @@ -277,7 +270,7 @@ module.exports = class BlockIndentation extends Rule { this.seen.add(fixedNode); - return nestedOptions ? nestedOptions.sourceToEdit : fixedNode; + return nestedOptions ? this.sourceEdited : fixedNode; } validateBlockStart(node, nestedOptions = {}) { @@ -294,12 +287,12 @@ module.exports = class BlockIndentation extends Rule { if (startLine === 1 && startColumn !== 0) { if (this.mode === 'fix') { - const elementSource = - nestedOptions.sourceToEdit || - sourceForLoc(this.nodeToSource.get(node) || this.source, { - end: node.loc.end, - start: { line: node.loc.start.line, column: 0 }, - }); + const elementSource = nestedOptions.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, @@ -307,7 +300,6 @@ module.exports = class BlockIndentation extends Rule { 0, nestedOptions.path ); - nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); return fixedNode; } else { let isElementNode = node && node.type === 'ElementNode'; @@ -349,24 +341,20 @@ module.exports = class BlockIndentation extends Rule { if (correctedEndColumn !== startColumn) { if (this.mode === 'fix') { - const elementSource = - nestedOptions.sourceToEdit || - `${' '.repeat(node.loc.start.column)}${sourceForLoc( - this.nodeToSource.get(node) || this.source, - { + const elementSource = nestedOptions.path + ? this.sourceEdited || this.source.join('') + : `${' '.repeat(node.loc.start.column)}${sourceForLoc(this.sourceEdited || this.source, { end: node.loc.end, start: { line: node.loc.start.line, column: node.loc.start.column }, - } - )}`; + })}`; const lines = elementSource.split('\n'); const fixedNode = this.fixLine( lines, - node.loc.end.line - (nestedOptions.sourceToEdit ? 1 : node.loc.start.line), + node.loc.end.line - (nestedOptions.path ? 1 : node.loc.start.line), correctedEndColumn, startColumn, nestedOptions.path ); - nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); return fixedNode; } else { let startLocation = `L${node.loc.start.line}:C${node.loc.start.column}`; @@ -497,12 +485,12 @@ module.exports = class BlockIndentation extends Rule { if (expectedStartColumn !== childStartColumn) { if (this.mode === 'fix') { - const sourceToFix = - nestedOptions.sourceToEdit || - sourceForLoc(this.nodeToSource.get(fixedNode) || this.source, { - end: fixedNode.loc.end, - start: { line: fixedNode.loc.start.line, column: 0 }, - }); + const sourceToFix = nestedOptions.path + ? this.sourceEdited || this.source.join('') + : sourceForLoc(this.sourceEdited || this.source, { + end: fixedNode.loc.end, + start: { line: fixedNode.loc.start.line, column: 0 }, + }); const lines = sourceToFix.split('\n'); fixedNode = this.fixLine( @@ -512,7 +500,6 @@ module.exports = class BlockIndentation extends Rule { expectedStartColumn, nestedOptions.path ); - nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); children = AstNodeInfo.childrenFor(fixedNode).filter((x) => !x._isElseIfBlock); } else { @@ -555,12 +542,12 @@ module.exports = class BlockIndentation extends Rule { if (elseStartColumn !== startColumn) { if (this.mode === 'fix') { - const sourceToFix = - nestedOptions.sourceToEdit || - sourceForLoc(this.nodeToSource.get(node) || this.source, { - end: node.loc.end, - start: { line: node.loc.start.line, column: 0 }, - }); + const sourceToFix = nestedOptions.path + ? this.sourceEdited || this.source.join('') + : sourceForLoc(this.sourceEdited || this.source, { + end: node.loc.end, + start: { line: node.loc.start.line, column: 0 }, + }); const lines = sourceToFix.split('\n'); const elseLine = node.program.loc.end.line; const fixedNode = this.fixLine( @@ -570,7 +557,6 @@ module.exports = class BlockIndentation extends Rule { startColumn, nestedOptions.path ); - nestedOptions.sourceToEdit = this.nodeToSource.get(fixedNode); return fixedNode; } else { let displayName = node.path.original; @@ -622,26 +608,31 @@ module.exports = class BlockIndentation extends Rule { fixLine(lines, lineNumber, actualColumn, expectedColumn, path) { const line = lines[lineNumber]; - if (!line.startsWith(' '.repeat(actualColumn))) { - lines[lineNumber] = `${line.slice(0, Math.max(0, actualColumn)).trimEnd()}\n${' '.repeat( - expectedColumn - )}${line.slice(Math.max(0, actualColumn))}`; - } else { + + // If line starts with only white spaces... + if (line.startsWith(' '.repeat(actualColumn))) { + // ... then it means we just need to fix the number of whitespace lines[lineNumber] = actualColumn ? line.replace(/^ +/, ' '.repeat(expectedColumn)) : `${' '.repeat(expectedColumn)}${line}`; + } else { + // ... otherwise this mean the node is on the same line as another one so we need to add new line between the 2 nodes + lines[lineNumber] = `${line.slice(0, Math.max(0, actualColumn)).trimEnd()}\n${' '.repeat( + expectedColumn + )}${line.slice(Math.max(0, actualColumn))}`; } let node = parse(lines.join('\n')).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 if (this.detectNestedElseIfBlock(node)) { let elseBlockStatement = node.inverse.body[0]; elseBlockStatement._isElseIfBlock = true; } - this.nodeToSource.set(node, lines.join('\n')); + this.sourceEdited = lines.join('\n'); return node; } From 268688c53a55558db4a544cce4b54cc2dc2ca6dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Sat, 18 Dec 2021 13:54:13 +0100 Subject: [PATCH 4/8] chore: get rid of nestedOptions --- lib/rules/block-indentation.js | 68 ++++++++++++++-------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index 35f6eea97..ba8c7f084 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -209,27 +209,27 @@ module.exports = class BlockIndentation extends Rule { }; } - process(node, nestedOptions) { + process(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 nestedOptions ? this.sourceEdited : node; + return path ? this.sourceEdited : node; } - let fixedNode = this.validateBlockStart(node, nestedOptions); - fixedNode = this.validateBlockElse(fixedNode, nestedOptions); - fixedNode = this.validateBlockEnd(fixedNode, nestedOptions); - fixedNode = this.validateBlockChildren(fixedNode, nestedOptions); + let fixedNode = this.validateBlockStart(node, path); + fixedNode = this.validateBlockElse(fixedNode, path); + fixedNode = this.validateBlockEnd(fixedNode, path); + fixedNode = this.validateBlockChildren(fixedNode, path); if (this.mode === 'fix') { - const { path } = nestedOptions || { path: [] }; + const currentPath = path || []; const sourceBeforeFix = this.sourceEdited || this.source.join(''); const childrenWithPath = []; if (fixedNode.inverse) { - const inverseChildPath = [...path, 'inverse', 'body']; + const inverseChildPath = [...currentPath, 'inverse', 'body']; for (const [index, inverseNestedNode] of fixedNode.inverse.body.entries()) { childrenWithPath.push({ path: [...inverseChildPath, index], @@ -239,7 +239,9 @@ module.exports = class BlockIndentation extends Rule { } const childPath = - fixedNode.type === 'BlockStatement' ? [...path, 'program', 'body'] : [...path, 'children']; + fixedNode.type === 'BlockStatement' + ? [...currentPath, 'program', 'body'] + : [...currentPath, 'children']; const children = fixedNode.type === 'BlockStatement' ? fixedNode.program.body : fixedNode.children; for (const [index, nestedNode] of children.entries()) { @@ -249,11 +251,9 @@ module.exports = class BlockIndentation extends Rule { }); } - for (const { path, child } of childrenWithPath) { + for (const { path: childPath, child } of childrenWithPath) { if (['BlockStatement', 'ElementNode'].includes(child.type)) { - this.process(child, { - path, - }); + this.process(child, childPath); } } @@ -263,17 +263,17 @@ module.exports = class BlockIndentation extends Rule { 0, fixedNode.loc.start.column, fixedNode.loc.start.column, - nestedOptions && nestedOptions.path + path ); } } this.seen.add(fixedNode); - return nestedOptions ? this.sourceEdited : fixedNode; + return path ? this.sourceEdited : fixedNode; } - validateBlockStart(node, nestedOptions = {}) { + validateBlockStart(node, path) { if (!this.shouldValidateBlockEnd(node)) { return node; } @@ -287,19 +287,13 @@ module.exports = class BlockIndentation extends Rule { if (startLine === 1 && startColumn !== 0) { if (this.mode === 'fix') { - const elementSource = nestedOptions.path + 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, - nestedOptions.path - ); + const fixedNode = this.fixLine(elementSource.split('\n'), 0, startColumn, 0, path); return fixedNode; } else { let isElementNode = node && node.type === 'ElementNode'; @@ -321,7 +315,7 @@ module.exports = class BlockIndentation extends Rule { return node; } - validateBlockEnd(node, nestedOptions = {}) { + validateBlockEnd(node, path) { if (!this.shouldValidateBlockEnd(node)) { return node; } @@ -341,7 +335,7 @@ module.exports = class BlockIndentation extends Rule { if (correctedEndColumn !== startColumn) { if (this.mode === 'fix') { - const elementSource = nestedOptions.path + const elementSource = path ? this.sourceEdited || this.source.join('') : `${' '.repeat(node.loc.start.column)}${sourceForLoc(this.sourceEdited || this.source, { end: node.loc.end, @@ -350,10 +344,10 @@ module.exports = class BlockIndentation extends Rule { const lines = elementSource.split('\n'); const fixedNode = this.fixLine( lines, - node.loc.end.line - (nestedOptions.path ? 1 : node.loc.start.line), + node.loc.end.line - (path ? 1 : node.loc.start.line), correctedEndColumn, startColumn, - nestedOptions.path + path ); return fixedNode; } else { @@ -378,7 +372,7 @@ module.exports = class BlockIndentation extends Rule { return node; } - validateBlockChildren(node, nestedOptions = {}) { + validateBlockChildren(node, path) { if (this.isWithinIgnoredElement()) { return node; } @@ -485,7 +479,7 @@ module.exports = class BlockIndentation extends Rule { if (expectedStartColumn !== childStartColumn) { if (this.mode === 'fix') { - const sourceToFix = nestedOptions.path + const sourceToFix = path ? this.sourceEdited || this.source.join('') : sourceForLoc(this.sourceEdited || this.source, { end: fixedNode.loc.end, @@ -498,7 +492,7 @@ module.exports = class BlockIndentation extends Rule { childStartLine - 1, childStartColumn, expectedStartColumn, - nestedOptions.path + path ); children = AstNodeInfo.childrenFor(fixedNode).filter((x) => !x._isElseIfBlock); @@ -525,7 +519,7 @@ module.exports = class BlockIndentation extends Rule { return fixedNode; } - validateBlockElse(node, nestedOptions = {}) { + validateBlockElse(node, path) { if (node.type !== 'BlockStatement' || !node.inverse) { return node; } @@ -542,7 +536,7 @@ module.exports = class BlockIndentation extends Rule { if (elseStartColumn !== startColumn) { if (this.mode === 'fix') { - const sourceToFix = nestedOptions.path + const sourceToFix = path ? this.sourceEdited || this.source.join('') : sourceForLoc(this.sourceEdited || this.source, { end: node.loc.end, @@ -550,13 +544,7 @@ module.exports = class BlockIndentation extends Rule { }); const lines = sourceToFix.split('\n'); const elseLine = node.program.loc.end.line; - const fixedNode = this.fixLine( - lines, - elseLine - 1, - elseStartColumn, - startColumn, - nestedOptions.path - ); + const fixedNode = this.fixLine(lines, elseLine - 1, elseStartColumn, startColumn, path); return fixedNode; } else { let displayName = node.path.original; From ffda9d003e0b1f1d5e7bd1ba4062409ec06afcf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Sat, 18 Dec 2021 14:07:10 +0100 Subject: [PATCH 5/8] chore: add comments --- lib/rules/block-indentation.js | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index ba8c7f084..0f751ced3 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -83,6 +83,11 @@ function isControlChar(char) { return char === '~' || char === '{' || char === '}'; } +/** + * @param {Node} node + * @param {String[]} path - A list of property to search for in the node + * @returns {Node} + */ function get(node, path) { let value = node; @@ -95,6 +100,10 @@ function get(node, path) { return value; } +function removeWhitespaceEnd(str) { + return str.replace(/ +$/, ''); +} + module.exports = class BlockIndentation extends Rule { parseConfig(config) { let configType = typeof config; @@ -183,7 +192,7 @@ module.exports = class BlockIndentation extends Rule { if ( node.body[0] && node.body[0].type === 'TextNode' && - !node.body[0].chars.replace(/ +/, '') + !removeWhitespaceEnd(node.body[0].chars) ) { // We need to remove the text node in this case node.body.shift(); @@ -209,12 +218,18 @@ module.exports = class BlockIndentation extends Rule { }; } + /** + * + * @param {Node} node - Current node or parent node if path is set + * @param {String[]} path - path of the node being currently edited + * @returns {Node} + */ process(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 path ? this.sourceEdited : node; + return node; } let fixedNode = this.validateBlockStart(node, path); @@ -222,6 +237,8 @@ module.exports = class BlockIndentation extends Rule { fixedNode = this.validateBlockEnd(fixedNode, path); fixedNode = this.validateBlockChildren(fixedNode, path); + // If we are in fix mode we need to also fix all the children right now as fixing a child node + // might introduce new line and so change the position ending of its parent if (this.mode === 'fix') { const currentPath = path || []; const sourceBeforeFix = this.sourceEdited || this.source.join(''); @@ -251,12 +268,14 @@ module.exports = class BlockIndentation extends Rule { }); } + // Process all the child with the path to access the child from the current node for (const { path: childPath, child } of childrenWithPath) { if (['BlockStatement', 'ElementNode'].includes(child.type)) { this.process(child, childPath); } } + // Process all the child with the path to access the child from the current node if (sourceBeforeFix !== this.sourceEdited) { fixedNode = this.fixLine( this.sourceEdited.split('\n'), @@ -270,7 +289,7 @@ module.exports = class BlockIndentation extends Rule { this.seen.add(fixedNode); - return path ? this.sourceEdited : fixedNode; + return fixedNode; } validateBlockStart(node, path) { @@ -287,6 +306,7 @@ module.exports = class BlockIndentation extends Rule { if (startLine === 1 && 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, { @@ -335,6 +355,7 @@ module.exports = class BlockIndentation extends Rule { if (correctedEndColumn !== startColumn) { 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('') : `${' '.repeat(node.loc.start.column)}${sourceForLoc(this.sourceEdited || this.source, { @@ -479,6 +500,7 @@ module.exports = class BlockIndentation extends Rule { if (expectedStartColumn !== childStartColumn) { 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 sourceToFix = path ? this.sourceEdited || this.source.join('') : sourceForLoc(this.sourceEdited || this.source, { @@ -536,6 +558,7 @@ module.exports = class BlockIndentation extends Rule { if (elseStartColumn !== startColumn) { 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 sourceToFix = path ? this.sourceEdited || this.source.join('') : sourceForLoc(this.sourceEdited || this.source, { From af67a6d7690e67c465f538f64867b2f33c704c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Sat, 18 Dec 2021 14:09:25 +0100 Subject: [PATCH 6/8] chore: remove useless condition --- lib/rules/block-indentation.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index 0f751ced3..9fc8a045b 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -297,10 +297,6 @@ module.exports = class BlockIndentation extends Rule { return node; } - if (this.isWithinIgnoredElement()) { - return node; - } - let startColumn = node.loc.start.column; let startLine = node.loc.start.line; @@ -340,10 +336,6 @@ module.exports = class BlockIndentation extends Rule { return node; } - if (this.isWithinIgnoredElement()) { - return node; - } - let isElementNode = node && node.type === 'ElementNode'; let displayName = isElementNode ? node.tag : node.path.original; let display = isElementNode ? `` : `{{/${displayName}}}`; @@ -676,7 +668,7 @@ module.exports = class BlockIndentation extends Rule { } if (this.isWithinIgnoredElement()) { - return; + return false; } // do not validate nodes without children (whitespace will count as TextNodes) From 5ec705fe11dd5d9a8b03103a486bbe557e650308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Sun, 19 Dec 2021 09:36:27 +0100 Subject: [PATCH 7/8] chore: add example --- lib/rules/block-indentation.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index 9fc8a045b..da6def14c 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -84,9 +84,14 @@ function isControlChar(char) { } /** + * This function allows to get a node inside a node given a path + * * @param {Node} node * @param {String[]} path - A list of property to search for in the node * @returns {Node} + * + * @example + * get(node, ['program', 'body', 1]) === node.program.body[1] */ function get(node, path) { let value = node; From 179bc67ed3ce50034a2ba424c2455fffc815fa26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Molini=C3=A9?= Date: Fri, 24 Dec 2021 14:23:51 +0100 Subject: [PATCH 8/8] chore: review fixes --- lib/rules/block-indentation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/block-indentation.js b/lib/rules/block-indentation.js index da6def14c..59ac80e00 100644 --- a/lib/rules/block-indentation.js +++ b/lib/rules/block-indentation.js @@ -280,7 +280,7 @@ module.exports = class BlockIndentation extends Rule { } } - // Process all the child with the path to access the child from the current node + // Recreate the node only if the source has changed, otherwise this would mean useless computation and possible mistake if (sourceBeforeFix !== this.sourceEdited) { fixedNode = this.fixLine( this.sourceEdited.split('\n'),