Skip to content

Commit

Permalink
Fix block-indentation rule for embedded templates
Browse files Browse the repository at this point in the history
  • Loading branch information
robinborst95 committed Mar 8, 2023
1 parent 8dee696 commit 7f13c58
Show file tree
Hide file tree
Showing 6 changed files with 416 additions and 17 deletions.
14 changes: 11 additions & 3 deletions lib/extract-templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { parseTemplates } from 'ember-template-imports/lib/parse-templates.js';
import * as util from 'ember-template-imports/src/util.js';

export const SUPPORTED_EXTENSIONS = ['.js', '.ts', '.gjs', '.gts'];
const LOCATION_START = Object.freeze({ line: 0, column: 0, start: 0, end: 0 });
const LOCATION_START = Object.freeze({ line: 0, column: 0, start: 0, end: 0, columnOffset: 0 });
/**
* Processes results and corrects for template location offsets.
*
Expand Down Expand Up @@ -86,12 +86,18 @@ export function extractTemplates(moduleSource, relativePath) {
*
* @returns {TemplateInfo}
*/
function makeTemplateInfo({ line, column, start, end }, template, templateInfo, isEmbedded) {
function makeTemplateInfo(
{ line, column, start, end, columnOffset },
template,
templateInfo,
isEmbedded
) {
return {
line,
start,
end,
column,
columnOffset,
template,
isEmbedded,
isStrictMode: !isEmbedded || isStrictMode(templateInfo),
Expand All @@ -118,11 +124,13 @@ export function isSupportedScriptFileExtension(filePath = '') {

export function coordinatesOf(text, offset, start, end) {
const contentBeforeTemplateStart = text.slice(0, offset).split('\n');
const lineBeforeTemplateStart = contentBeforeTemplateStart[contentBeforeTemplateStart.length - 1];
return {
line: contentBeforeTemplateStart.length,
column: contentBeforeTemplateStart[contentBeforeTemplateStart.length - 1].length,
column: lineBeforeTemplateStart.length,
start,
end,
columnOffset: lineBeforeTemplateStart.length - lineBeforeTemplateStart.trimStart().length,
};
}

Expand Down
2 changes: 2 additions & 0 deletions lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ export default class Linter {
let rule = this._buildRule(ruleName, {
shouldFix: true,
filePath: options.filePath,
columnOffset: templateInfo.columnOffset,
rawSource: templateInfo.template,
fileConfig,
});
Expand Down Expand Up @@ -367,6 +368,7 @@ export default class Linter {
fileConfig,
filePath: options.filePath,
configResolver: options.configResolver,
columnOffset: templateInfo.columnOffset,
rawSource: templateInfo.template,
isStrictMode: templateInfo.isStrictMode,
});
Expand Down
2 changes: 2 additions & 0 deletions lib/rules/_base.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ export default class {
this[MODULE_NAME] = options.moduleName;
this._rawSource = options.rawSource;

this.columnOffset = options.columnOffset;

// split into a source array (allow windows and posix line endings)
this.source = options.rawSource.match(reLines);
}
Expand Down
55 changes: 41 additions & 14 deletions lib/rules/block-indentation.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,12 @@ function get(node, path) {
return value;
}

function removeWhitespaceEnd(str) {
return str.replace(/ +$/, '');
function addWhitespaceEnd(str, count) {
return `${str}${' '.repeat(count)}`;
}

function removeWhitespaceEnd(str, count) {
return str.replace(new RegExp(` {${count}}$`), '');
}

export default class BlockIndentation extends Rule {
Expand Down Expand Up @@ -306,13 +310,20 @@ export default class BlockIndentation extends Rule {
const hasLeadingContent =
!isDirectChildOfTemplate || this.hasLeadingContent(node, this.template.body);
const startColumn = node.loc.start.column;
const expectedStartColumn =
this.columnOffset === 0 || node.loc.start.line === 1
? 0
: this.columnOffset + this.config.indentation;

// If it's not a direct child of the template the block start is already fixed by the validateBlockChildren function
if (isDirectChildOfTemplate && startColumn !== 0 && !hasLeadingContent) {
if (isDirectChildOfTemplate && startColumn !== expectedStartColumn && !hasLeadingContent) {
if (this.mode === 'fix') {
const previousNode = this.template.body[indexOfNodeInTemplate - 1];
if (previousNode.type === 'TextNode') {
previousNode.chars = removeWhitespaceEnd(previousNode.chars);
previousNode.chars =
startColumn > expectedStartColumn
? removeWhitespaceEnd(previousNode.chars, startColumn - expectedStartColumn)
: addWhitespaceEnd(previousNode.chars, expectedStartColumn - startColumn);
}
} else {
let isElementNode = node && node.type === 'ElementNode';
Expand All @@ -322,7 +333,7 @@ export default class BlockIndentation extends Rule {

let warning =
`Incorrect indentation for \`${display}\` beginning at ${startLocation}. ` +
`Expected \`${display}\` to be at an indentation of 0, but was found at ${startColumn}.`;
`Expected \`${display}\` to be at an indentation of ${expectedStartColumn}, but was found at ${startColumn}.`;

this.log({
message: warning,
Expand All @@ -347,8 +358,14 @@ export default class BlockIndentation extends Rule {

let controlCharCount = this.endingControlCharCount(node);
let correctedEndColumn = endColumn - displayName.length - controlCharCount;

if (correctedEndColumn !== startColumn) {
let expectedEndColumn =
this.columnOffset > 0
? node.loc.end.line === this.template.loc.end.line
? this.columnOffset
: startColumn
: startColumn;

if (correctedEndColumn !== expectedEndColumn) {
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
Expand All @@ -362,7 +379,7 @@ export default class BlockIndentation extends Rule {
lines,
node.loc.end.line - (path ? 1 : node.loc.start.line),
correctedEndColumn,
startColumn,
expectedEndColumn,
path
);
return fixedNode;
Expand All @@ -372,7 +389,7 @@ export default class BlockIndentation extends Rule {

let warning =
`Incorrect indentation for \`${displayName}\` beginning at ${startLocation}` +
`. Expected \`${display}\` ending at ${endLocation} to be at an indentation of ${startColumn} but ` +
`. Expected \`${display}\` ending at ${endLocation} to be at an indentation of ${expectedEndColumn} but ` +
`was found at ${correctedEndColumn}.`;

this.log({
Expand Down Expand Up @@ -405,7 +422,9 @@ export default class BlockIndentation extends Rule {
}

let startColumn = node.loc.start.column;
let expectedStartColumn = startColumn + this.config.indentation;
let correctedStartColumn =
this.columnOffset > 0 && node.loc.start.line === 1 ? this.columnOffset : startColumn;
let expectedStartColumn = correctedStartColumn + this.config.indentation;
let fixedNode = node;

let i = 0;
Expand Down Expand Up @@ -477,7 +496,7 @@ export default class BlockIndentation extends Rule {

fixedNode = this.fixLine(
lines,
childStartLine - 1,
childStartLine - (path ? 1 : fixedNode.loc.start.line),
childStartColumn,
expectedStartColumn,
path
Expand Down Expand Up @@ -520,9 +539,11 @@ export default class BlockIndentation extends Rule {

let inverse = node.inverse;
let startColumn = node.loc.start.column;
let expectedStartColumn =
this.columnOffset > 0 && node.loc.start.line === 1 ? this.columnOffset : startColumn;
let elseStartColumn = node.program.loc.end.column;

if (elseStartColumn !== startColumn) {
if (elseStartColumn !== expectedStartColumn) {
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
Expand All @@ -533,7 +554,13 @@ export default 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, path);
const fixedNode = this.fixLine(
lines,
elseLine - 1,
elseStartColumn,
expectedStartColumn,
path
);
return fixedNode;
} else {
let displayName = node.path.original;
Expand All @@ -542,7 +569,7 @@ export default class BlockIndentation extends Rule {

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 ` +
`. Expected \`{{else}}\` starting at ${elseLocation} to be at an indentation of ${expectedStartColumn} but ` +
`was found at ${elseStartColumn}.`;

this.log({
Expand Down
8 changes: 8 additions & 0 deletions test/unit/extract-templates-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('extractTemplates', function () {
[
{
"column": 0,
"columnOffset": 0,
"end": 0,
"isEmbedded": undefined,
"isStrictMode": true,
Expand All @@ -41,6 +42,7 @@ describe('extractTemplates', function () {
[
{
"column": 60,
"columnOffset": 6,
"end": 238,
"isEmbedded": true,
"isStrictMode": false,
Expand Down Expand Up @@ -71,6 +73,7 @@ describe('extractTemplates', function () {
[
{
"column": 60,
"columnOffset": 6,
"end": 242,
"isEmbedded": true,
"isStrictMode": true,
Expand Down Expand Up @@ -109,6 +112,7 @@ describe('extractTemplates', function () {
[
{
"column": 63,
"columnOffset": 6,
"end": 255,
"isEmbedded": true,
"isStrictMode": true,
Expand Down Expand Up @@ -139,6 +143,7 @@ describe('extractTemplates', function () {
[
{
"column": 39,
"columnOffset": 0,
"end": 58,
"isEmbedded": true,
"isStrictMode": true,
Expand Down Expand Up @@ -174,6 +179,7 @@ describe('extractTemplates', function () {
[
{
"column": 0,
"columnOffset": 0,
"end": 0,
"isEmbedded": undefined,
"isStrictMode": true,
Expand All @@ -190,6 +196,7 @@ describe('extractTemplates', function () {
[
{
"column": 39,
"columnOffset": 0,
"end": 58,
"isEmbedded": true,
"isStrictMode": true,
Expand Down Expand Up @@ -237,6 +244,7 @@ describe('calculate template coordinates', function () {
expect(coordinatesOf(typescript, 228)).toEqual({
line: 8,
column: 12,
columnOffset: 2,
});
});
});
Expand Down

0 comments on commit 7f13c58

Please sign in to comment.