From 14b764dfa5f70665dd2ae14359cdca3c8999a4f0 Mon Sep 17 00:00:00 2001 From: Jeff Williams Date: Sun, 11 Sep 2022 16:54:00 -0700 Subject: [PATCH 1/3] fix: for ESTree, attach comments when a directive is followed by a comment, then EOF If a source file contains 1) a directive, followed by 2) a comment, and nothing else, then `@babel/parser` attaches the comment to the `Directive` node as a trailing comment. The ESTree plugin handles `Directive` nodes by creating an `ExpressionStatement` node, then transferring information from the `Directive` node to the `ExpressionStatement` node. Previously, at the time it transfers information, comments hadn't yet been attached to the `Directive` node. And even if they had been, the ESTree plugin didn't do anything with comments attached to the `Directive` node. As a result, the source file's comment wasn't attached to any node in the resulting AST. With this change, the ESTree plugin generates an AST in which the comment is attached to the `ExpressionStatement` node as a trailing comment. --- packages/babel-parser/src/plugins/estree.ts | 6 +++ .../directives/trailing-comment-eof/input.js | 3 ++ .../trailing-comment-eof/output.json | 37 +++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/input.js create mode 100644 packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/output.json diff --git a/packages/babel-parser/src/plugins/estree.ts b/packages/babel-parser/src/plugins/estree.ts index 2a23979dfa33..e97af9aabbca 100644 --- a/packages/babel-parser/src/plugins/estree.ts +++ b/packages/babel-parser/src/plugins/estree.ts @@ -111,6 +111,9 @@ export default (superClass: typeof Parser) => // @ts-expect-error TS2339: Property 'raw' does not exist on type 'Undone '. expression.raw = directiveLiteral.extra.raw; + stmt.leadingComments = directive.leadingComments?.slice(); + stmt.trailingComments = directive.trailingComments?.slice(); + stmt.expression = this.finishNodeAt( expression, "Literal", @@ -175,6 +178,9 @@ export default (superClass: typeof Parser) => end, afterBlockParse, ); + if (node.directives.length) { + this.processComment(node); + } const directiveStatements = node.directives.map(d => this.directiveToStmt(d), diff --git a/packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/input.js b/packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/input.js new file mode 100644 index 000000000000..ec942871d7de --- /dev/null +++ b/packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/input.js @@ -0,0 +1,3 @@ +"use strict"; + +/** @external foo */ diff --git a/packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/output.json b/packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/output.json new file mode 100644 index 000000000000..4c9909c37f59 --- /dev/null +++ b/packages/babel-parser/test/fixtures/estree/directives/trailing-comment-eof/output.json @@ -0,0 +1,37 @@ +{ + "type": "File", + "start":0,"end":35,"loc":{"start":{"line":1,"column":0},"end":{"line":3,"column":20}}, + "program": { + "type": "Program", + "start":0,"end":35,"loc":{"start":{"line":1,"column":0},"end":{"line":3,"column":20}}, + "sourceType": "script", + "interpreter": null, + "body": [ + { + "type": "ExpressionStatement", + "start":0,"end":13,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":13}}, + "trailingComments": [ + { + "type": "CommentBlock", + "value": "* @external foo ", + "start":15,"end":35,"loc":{"start":{"line":3,"column":0,"index":15},"end":{"line":3,"column":20,"index":35}} + } + ], + "expression": { + "type": "Literal", + "start":0,"end":12,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":12}}, + "value": "use strict", + "raw": "\"use strict\"" + }, + "directive": "use strict" + } + ] + }, + "comments": [ + { + "type": "CommentBlock", + "value": "* @external foo ", + "start":15,"end":35,"loc":{"start":{"line":3,"column":0,"index":15},"end":{"line":3,"column":20,"index":35}} + } + ] +} From cac12526d8a5e133adb871f33234adea5f8c33cd Mon Sep 17 00:00:00 2001 From: Jeff Williams Date: Mon, 12 Sep 2022 23:26:55 -0700 Subject: [PATCH 2/3] use safer and more reliable approach to cast Directive to ExpressionStatement --- packages/babel-parser/src/plugins/estree.ts | 43 +++--------- .../directives/interleaved-comments/input.js | 4 ++ .../interleaved-comments/output.json | 67 +++++++++++++++++++ 3 files changed, 82 insertions(+), 32 deletions(-) create mode 100644 packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/input.js create mode 100644 packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/output.json diff --git a/packages/babel-parser/src/plugins/estree.ts b/packages/babel-parser/src/plugins/estree.ts index e97af9aabbca..60e7b4871552 100644 --- a/packages/babel-parser/src/plugins/estree.ts +++ b/packages/babel-parser/src/plugins/estree.ts @@ -95,38 +95,20 @@ export default (superClass: typeof Parser) => return this.estreeParseLiteral(value); } + // Cast a Directive to an ExpressionStatement. Mutates the input Directive. directiveToStmt(directive: N.Directive): N.ExpressionStatement { - const directiveLiteral = directive.value; + const stmt = directive as any; + stmt.type = "ExpressionStatement"; + stmt.directive = stmt.value.extra.rawValue; + stmt.expression = stmt.value; + delete stmt.value; - const stmt = this.startNodeAt( - directive.start, - directive.loc.start, - ); - const expression = this.startNodeAt( - directiveLiteral.start, - directiveLiteral.loc.start, - ); - - expression.value = directiveLiteral.extra.expressionValue; - // @ts-expect-error TS2339: Property 'raw' does not exist on type 'Undone '. - expression.raw = directiveLiteral.extra.raw; - - stmt.leadingComments = directive.leadingComments?.slice(); - stmt.trailingComments = directive.trailingComments?.slice(); + stmt.expression.type = "Literal"; + stmt.expression.raw = stmt.expression.extra.raw; + stmt.expression.value = stmt.expression.extra.expressionValue; + delete stmt.expression.extra; - stmt.expression = this.finishNodeAt( - expression, - "Literal", - directiveLiteral.loc.end, - ); - // @ts-expect-error N.Directive.value is not defined - stmt.directive = directiveLiteral.extra.raw.slice(1, -1); - - return this.finishNodeAt( - stmt, - "ExpressionStatement", - directive.loc.end, - ) as N.ExpressionStatement; + return stmt as N.ExpressionStatement; } // ================================== @@ -178,9 +160,6 @@ export default (superClass: typeof Parser) => end, afterBlockParse, ); - if (node.directives.length) { - this.processComment(node); - } const directiveStatements = node.directives.map(d => this.directiveToStmt(d), diff --git a/packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/input.js b/packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/input.js new file mode 100644 index 000000000000..24c853e6bc11 --- /dev/null +++ b/packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/input.js @@ -0,0 +1,4 @@ +// 1; +"use strict"; +// 2; +"use strict"; diff --git a/packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/output.json b/packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/output.json new file mode 100644 index 000000000000..b71ae9db581d --- /dev/null +++ b/packages/babel-parser/test/fixtures/estree/directives/interleaved-comments/output.json @@ -0,0 +1,67 @@ +{ + "type": "File", + "start":0,"end":39,"loc":{"start":{"line":1,"column":0},"end":{"line":4,"column":13}}, + "program": { + "type": "Program", + "start":0,"end":39,"loc":{"start":{"line":1,"column":0},"end":{"line":4,"column":13}}, + "sourceType": "script", + "interpreter": null, + "body": [ + { + "type": "ExpressionStatement", + "start":6,"end":19,"loc":{"start":{"line":2,"column":0},"end":{"line":2,"column":13}}, + "trailingComments": [ + { + "type": "CommentLine", + "value": " 2;", + "start":20,"end":25,"loc":{"start":{"line":3,"column":0,"index":20},"end":{"line":3,"column":5,"index":25}} + } + ], + "leadingComments": [ + { + "type": "CommentLine", + "value": " 1;", + "start":0,"end":5,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":5,"index":5}} + } + ], + "directive": "use strict", + "expression": { + "type": "Literal", + "start":6,"end":18,"loc":{"start":{"line":2,"column":0},"end":{"line":2,"column":12}}, + "value": "use strict", + "raw": "\"use strict\"" + } + }, + { + "type": "ExpressionStatement", + "start":26,"end":39,"loc":{"start":{"line":4,"column":0},"end":{"line":4,"column":13}}, + "leadingComments": [ + { + "type": "CommentLine", + "value": " 2;", + "start":20,"end":25,"loc":{"start":{"line":3,"column":0,"index":20},"end":{"line":3,"column":5,"index":25}} + } + ], + "directive": "use strict", + "expression": { + "type": "Literal", + "start":26,"end":38,"loc":{"start":{"line":4,"column":0},"end":{"line":4,"column":12}}, + "value": "use strict", + "raw": "\"use strict\"" + } + } + ] + }, + "comments": [ + { + "type": "CommentLine", + "value": " 1;", + "start":0,"end":5,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":5,"index":5}} + }, + { + "type": "CommentLine", + "value": " 2;", + "start":20,"end":25,"loc":{"start":{"line":3,"column":0,"index":20},"end":{"line":3,"column":5,"index":25}} + } + ] +} From 79bfb2bf5e41836d9ac81b98e2099aded0acbcd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Wed, 14 Sep 2022 00:34:15 +0900 Subject: [PATCH 3/3] Better typechecking --- packages/babel-parser/src/plugins/estree.ts | 23 +++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/babel-parser/src/plugins/estree.ts b/packages/babel-parser/src/plugins/estree.ts index 60e7b4871552..89531c705b1b 100644 --- a/packages/babel-parser/src/plugins/estree.ts +++ b/packages/babel-parser/src/plugins/estree.ts @@ -97,18 +97,23 @@ export default (superClass: typeof Parser) => // Cast a Directive to an ExpressionStatement. Mutates the input Directive. directiveToStmt(directive: N.Directive): N.ExpressionStatement { - const stmt = directive as any; + const expression = directive.value as any as N.EstreeLiteral; + delete directive.value; + + expression.type = "Literal"; + // @ts-expect-error N.EstreeLiteral.raw is not defined. + expression.raw = expression.extra.raw; + expression.value = expression.extra.expressionValue; + + const stmt = directive as any as N.ExpressionStatement; stmt.type = "ExpressionStatement"; - stmt.directive = stmt.value.extra.rawValue; - stmt.expression = stmt.value; - delete stmt.value; + stmt.expression = expression; + // @ts-expect-error N.ExpressionStatement.directive is not defined + stmt.directive = expression.extra.rawValue; - stmt.expression.type = "Literal"; - stmt.expression.raw = stmt.expression.extra.raw; - stmt.expression.value = stmt.expression.extra.expressionValue; - delete stmt.expression.extra; + delete expression.extra; - return stmt as N.ExpressionStatement; + return stmt; } // ==================================