Skip to content
This repository has been archived by the owner on Aug 18, 2021. It is now read-only.

Remove comment attachment #736

Merged
merged 2 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 0 additions & 59 deletions lib/babylon-to-espree/attachComments.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use strict";

const t = require("@babel/core").types;
const convertComments = require("./convertComments");
const convertProgramNode = require("./convertProgramNode");

module.exports = function(ast, traverse, code) {
const state = { source: code };
Expand All @@ -20,6 +20,8 @@ module.exports = function(ast, traverse, code) {

delete t.VISITOR_KEYS.Property;
delete t.VISITOR_KEYS.MethodDefinition;

convertProgramNode(ast);
};

const astTransformVisitor = {
Expand All @@ -31,16 +33,15 @@ const astTransformVisitor = {
node._babelType = node.type;

if (node.innerComments) {
node.trailingComments = node.innerComments;
delete node.innerComments;
}

if (node.trailingComments) {
convertComments(node.trailingComments);
delete node.trailingComments;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my own understanding, does ESLint just not used these properties?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right! We dropped the need for comment attachment to happen at the parser level in v4 (see docs here). We decided to move away from thinking about comments in the context of AST nodes (which had to be reimplemented by every parser and was buggy at best) and instead to just think of them in the context of tokens. ESLint now just takes the tokens and comments arrays provided by the parser and gives rules access to comments through the use of utility methods like sourceCode#getTokenBefore(nodeOrToken, { includeComments: true }).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking to Kai about being able to turn off even adding the comments in @babel/parser but if the only use case is for babel-eslint we probably don't need to make that option then? Unless there's a case for it being much faster (would need to test)

Copy link
Member Author

@kaicataldo kaicataldo Jan 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could explore that - I haven't looked at how comment attachment works in @babel/parser in a long time now, but if it's not too crazy of a thing to add I'm happy to do it! That said, I don't think it needs to be tied to this release, since it won't result in any user-facing changes.

}

if (node.leadingComments) {
convertComments(node.leadingComments);
delete node.leadingComments;
}
},
exit(path) {
Expand Down
40 changes: 40 additions & 0 deletions lib/babylon-to-espree/convertProgramNode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
"use strict";

module.exports = function(ast) {
ast.type = "Program";
ast.sourceType = ast.program.sourceType;
ast.directives = ast.program.directives;
ast.body = ast.program.body;
delete ast.program;

if (ast.comments.length) {
const lastComment = ast.comments[ast.comments.length - 1];

if (!ast.tokens.length) {
// if no tokens, the program starts at the end of the last comment
ast.start = lastComment.end;
ast.loc.start.line = lastComment.loc.end.line;
ast.loc.start.column = lastComment.loc.end.column;
} else {
const lastToken = ast.tokens[ast.tokens.length - 1];

if (lastComment.end > lastToken.end) {
// If there is a comment after the last token, the program ends at the
// last token and not the comment
ast.range[1] = lastToken.end;
ast.loc.end.line = lastToken.loc.end.line;
ast.loc.end.column = lastToken.loc.end.column;
}
}
} else {
if (!ast.tokens.length) {
ast.loc.start.line = 1;
ast.loc.end.line = 1;
}
}

if (ast.body && ast.body.length > 0) {
ast.loc.start.line = ast.body[0].loc.start.line;
ast.range[0] = ast.body[0].start;
}
};
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
"use strict";

const convertTemplateType = require("./convertTemplateType");
const toToken = require("./toToken");
const convertToken = require("./convertToken");

module.exports = function(tokens, tt, code) {
return convertTemplateType(tokens, tt)
.filter(t => t.type !== "CommentLine" && t.type !== "CommentBlock")
.map(t => toToken(t, tt, code));
.map(t => convertToken(t, tt, code));
};
27 changes: 4 additions & 23 deletions lib/babylon-to-espree/index.js
Original file line number Diff line number Diff line change
@@ -1,30 +1,11 @@
"use strict";

const attachComments = require("./attachComments");
const convertTokens = require("./convertTokens");
const convertComments = require("./convertComments");
const toTokens = require("./toTokens");
const toAST = require("./toAST");
const convertAST = require("./convertAST");

module.exports = function(ast, traverse, tt, code) {
// convert tokens
ast.tokens = toTokens(ast.tokens, tt, code);

// add comments
ast.tokens = convertTokens(ast.tokens, tt, code);
convertComments(ast.comments);

// transform esprima and acorn divergent nodes
toAST(ast, traverse, code);

// ast.program.tokens = ast.tokens;
// ast.program.comments = ast.comments;
// ast = ast.program;

// remove File
ast.type = "Program";
ast.sourceType = ast.program.sourceType;
ast.directives = ast.program.directives;
ast.body = ast.program.body;
delete ast.program;

attachComments(ast, ast.comments, ast.tokens);
convertAST(ast, traverse, code);
};
35 changes: 1 addition & 34 deletions test/specs/babel-eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,9 @@ const assert = require("assert");
const babelEslint = require("../..");
const espree = require("espree");
const escope = require("eslint-scope");
const util = require("util");
const unpad = require("dedent");
const assertImplementsAST = require("../helpers/assert-implements-ast");

function lookup(obj, keypath, backwardsDepth) {
if (!keypath) {
return obj;
}

return keypath
.split(".")
.slice(0, -1 * backwardsDepth)
.reduce((base, segment) => {
return base && base[segment], obj;
});
}

function parseAndAssertSame(code) {
code = unpad(code);
const esAST = espree.parse(code, {
Expand All @@ -38,33 +24,14 @@ function parseAndAssertSame(code) {
loc: true,
range: true,
comment: true,
attachComment: true,
ecmaVersion: 2018,
sourceType: "module",
});
const babylonAST = babelEslint.parseForESLint(code, {
eslintVisitorKeys: true,
eslintScopeManager: true,
}).ast;
try {
assertImplementsAST(esAST, babylonAST);
} catch (err) {
const traversal = err.message.slice(3, err.message.indexOf(":"));
err.message += unpad(`
espree:
${util.inspect(lookup(esAST, traversal, 2), {
depth: err.depth,
colors: true,
})}
babel-eslint:
${util.inspect(lookup(babylonAST, traversal, 2), {
depth: err.depth,
colors: true,
})}
`);
throw err;
}
//assert.equal(esAST, babylonAST);
assertImplementsAST(esAST, babylonAST);
}

describe("babylon-to-espree", () => {
Expand Down