Skip to content

Commit

Permalink
Fix false negatives with block-indentation rule (#2318)
Browse files Browse the repository at this point in the history 
Co-authored-by: Vincent Molinié <vincent.molinie@epita.fr>
  • Loading branch information
@VincentMolinie
VincentMolinie and Vincent Molinié committed Jan 5, 2022
1 parent b39d9d1 commit 1f3c1e7
Show file tree
Hide file tree
Showing 2 changed files with 253 additions and 78 deletions.
177 changes: 103 additions & 74 deletions lib/rules/block-indentation.js
View file
Expand Up @@ -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) {
Expand All @@ -230,14 +226,16 @@ 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;
this.seen.add(fixedNode);
return fixedNode;
}

let fixedNode = this.validateBlockStart(node, path);
fixedNode = this.validateBlockElse(fixedNode, path);
fixedNode = this.validateBlockEnd(fixedNode, path);
fixedNode = this.validateBlockChildren(fixedNode, path);
Expand Down Expand Up @@ -281,7 +279,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,
Expand All @@ -297,25 +295,24 @@ module.exports = class BlockIndentation extends Rule {
return fixedNode;
}

validateBlockStart(node, path) {
if (!this.shouldValidateBlockEnd(node)) {
validateBlockStart(node) {
if (!this.shouldValidateBlockStart(node)) {
return node;
}

let startColumn = node.loc.start.column;
let startLine = node.loc.start.line;
const indexOfNodeInTemplate = this.template.body.indexOf(node);
const isDirectChildOfTemplate = indexOfNodeInTemplate !== -1;
const hasLeadingContent =
!isDirectChildOfTemplate || this.hasLeadingContent(node, this.template.body);
const startColumn = node.loc.start.column;

if (startLine === 1 && 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 && !hasLeadingContent) {
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;
const previousNode = this.template.body[indexOfNodeInTemplate - 1];
if (previousNode.type === 'TextNode') {
previousNode.chars = removeWhitespaceEnd(previousNode.chars);
}
} else {
let isElementNode = node && node.type === 'ElementNode';
let displayName = isElementNode ? node.tag : node.path.original;
Expand Down Expand Up @@ -426,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
Expand Down Expand Up @@ -589,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':
Expand Down Expand Up @@ -618,7 +621,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))
Expand All @@ -630,7 +633,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
Expand All @@ -640,8 +645,6 @@ module.exports = class BlockIndentation extends Rule {
elseBlockStatement._isElseIfBlock = true;
}

this.sourceEdited = lines.join('\n');

return node;
}

Expand All @@ -662,14 +665,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()) {
Expand All @@ -678,20 +695,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 = this.sourceForNode(node);
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;
}

return indexOfEnding !== -1;
if (this.isWithinIgnoredElement()) {
return false;
}

// 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) {
Expand All @@ -700,7 +729,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);

Expand Down

0 comments on commit 1f3c1e7

Please sign in to comment.