Skip to content

Commit

Permalink
Intern comments on expression statements
Browse files Browse the repository at this point in the history
Summary:
Attaches comments that trail expression statements. Expression statements cannot have leading comments attached, as those leading comments would actually be attached to the inner expression node.

```
/* L expr */ expr; /* T expr statement */
```

Note that trailing comments are still often attached as leading comments of the next statment, but this is a problem across all statements at the moment.

Reviewed By: mroch

Differential Revision: D20517647

fbshipit-source-id: afcf9b589dacddafb074f8e2bbdb238bffb8fe9c
  • Loading branch information
Hans Halverson authored and facebook-github-bot committed Mar 24, 2020
1 parent 4938e6f commit d046a1b
Show file tree
Hide file tree
Showing 26 changed files with 385 additions and 20 deletions.
3 changes: 2 additions & 1 deletion src/parser/estree_translator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ with type t = Impl.t = struct
function
| (loc, Empty) -> node "EmptyStatement" loc []
| (loc, Block b) -> block (loc, b)
| (loc, Expression { Expression.expression = expr; directive }) ->
| (loc, Expression { Expression.expression = expr; directive; comments }) ->
node
?comments
"ExpressionStatement"
loc
[("expression", expression expr); ("directive", option string directive)]
Expand Down
1 change: 1 addition & 0 deletions src/parser/flow_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,7 @@ and Statement : sig
type ('M, 'T) t = {
expression: ('M, 'T) Expression.t;
directive: string option;
comments: ('M, unit) Syntax.t option;
}
[@@deriving show]
end
Expand Down
16 changes: 14 additions & 2 deletions src/parser/statement_parser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -672,13 +672,20 @@ module Statement
{ Statement.Labeled.label; body; comments = Flow_ast_utils.mk_comments_opt ~leading () }
| (expression, _) ->
Eat.semicolon ~expected:"the end of an expression statement (`;`)" env;
let trailing = Peek.comments env in
let open Statement in
Expression { Expression.expression; directive = None })
Expression
{
Expression.expression;
directive = None;
comments = Flow_ast_utils.mk_comments_opt ~trailing ();
})

and expression =
with_loc (fun env ->
let expression = Parse.expression env in
Eat.semicolon ~expected:"the end of an expression statement (`;`)" env;
let trailing = Peek.comments env in
let directive =
if allow_directive env then
match expression with
Expand All @@ -688,7 +695,12 @@ module Statement
else
None
in
Statement.Expression { Statement.Expression.expression; directive })
Statement.Expression
{
Statement.Expression.expression;
directive;
comments = Flow_ast_utils.mk_comments_opt ~trailing ();
})

and type_alias_helper env =
if not (should_parse_types env) then error env Parse_error.UnexpectedTypeAlias;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":3,"column":0},"end":{"line":3,"column":17}},
"range":[71,88],
"value":" 2.1 L arrow "
}
],
"loc":{"source":null,"start":{"line":1,"column":18},"end":{"line":1,"column":69}},
"range":[18,69],
"expression":{
Expand Down
8 changes: 8 additions & 0 deletions src/parser/test/flow/comment_interning/assignment.tree.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@
},
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":5,"column":0},"end":{"line":5,"column":27}},
"range":[128,155],
"value":" 2.1 Leading on assign "
}
],
"loc":{"source":null,"start":{"line":3,"column":26},"end":{"line":3,"column":114}},
"range":[38,126],
"expression":{
Expand Down
8 changes: 8 additions & 0 deletions src/parser/test/flow/comment_interning/binary.tree.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":3,"column":0},"end":{"line":3,"column":27}},
"range":[116,143],
"value":" 2.1 Leading on binary "
}
],
"loc":{"source":null,"start":{"line":1,"column":26},"end":{"line":1,"column":114}},
"range":[26,114],
"expression":{
Expand Down
16 changes: 16 additions & 0 deletions src/parser/test/flow/comment_interning/class_expression.tree.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":3,"column":65},"end":{"line":3,"column":74}},
"range":[78,87],
"value":" c05 "
}
],
"loc":{"source":null,"start":{"line":3,"column":0},"end":{"line":3,"column":64}},
"range":[13,77],
"expression":{
Expand Down Expand Up @@ -89,6 +97,14 @@
},
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":5,"column":46},"end":{"line":5,"column":55}},
"range":[135,144],
"value":" c15 "
}
],
"loc":{"source":null,"start":{"line":5,"column":0},"end":{"line":5,"column":45}},
"range":[89,134],
"expression":{
Expand Down
8 changes: 8 additions & 0 deletions src/parser/test/flow/comment_interning/conditional.tree.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":3,"column":0},"end":{"line":3,"column":23}},
"range":[113,136],
"value":" 2.1 L conditional "
}
],
"loc":{"source":null,"start":{"line":1,"column":17},"end":{"line":1,"column":111}},
"range":[17,111],
"expression":{
Expand Down
16 changes: 16 additions & 0 deletions src/parser/test/flow/comment_interning/import.tree.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":3,"column":0},"end":{"line":3,"column":18}},
"range":[54,72],
"value":" 2.1 L import "
}
],
"loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":52}},
"range":[19,52],
"expression":{
Expand Down Expand Up @@ -46,6 +54,14 @@
},
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":5,"column":0},"end":{"line":5,"column":18}},
"range":[167,185],
"value":" 3.1 L import "
}
],
"loc":{"source":null,"start":{"line":3,"column":19},"end":{"line":3,"column":111}},
"range":[73,165],
"expression":{
Expand Down
16 changes: 16 additions & 0 deletions src/parser/test/flow/comment_interning/member.tree.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":6,"column":2},"end":{"line":6,"column":20}},
"range":[127,145],
"value":" 2.1 L member "
}
],
"loc":{"source":null,"start":{"line":4,"column":17},"end":{"line":4,"column":68}},
"range":[72,123],
"expression":{
Expand Down Expand Up @@ -222,6 +230,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":12,"column":2},"end":{"line":12,"column":20}},
"range":[333,351],
"value":" 4.1 L member "
}
],
"loc":{"source":null,"start":{"line":10,"column":17},"end":{"line":10,"column":87}},
"range":[259,329],
"expression":{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@
"body":[
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":4,"column":2},"end":{"line":4,"column":18}},
"range":[97,113],
"value":" 2.1 L meta "
}
],
"loc":{"source":null,"start":{"line":2,"column":17},"end":{"line":2,"column":75}},
"range":[35,93],
"expression":{
Expand Down
16 changes: 16 additions & 0 deletions src/parser/test/flow/comment_interning/new.tree.json
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,14 @@
},
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":12,"column":2},"end":{"line":12,"column":32}},
"range":[208,238],
"value":" 2.1 unreachable trailing "
}
],
"loc":{"source":null,"start":{"line":11,"column":2},"end":{"line":11,"column":29}},
"range":[178,205],
"expression":{
Expand Down Expand Up @@ -305,6 +313,14 @@
},
{
"type":"ExpressionStatement",
"trailingComments":[
{
"type":"Block",
"loc":{"source":null,"start":{"line":20,"column":2},"end":{"line":20,"column":32}},
"range":[436,466],
"value":" 3.5 unreachable trailing "
}
],
"loc":{"source":null,"start":{"line":19,"column":2},"end":{"line":19,"column":63}},
"range":[372,433],
"expression":{
Expand Down

0 comments on commit d046a1b

Please sign in to comment.