Skip to content

Commit

Permalink
Fix idempotence bug
Browse files Browse the repository at this point in the history 
Summary:
Hack formatter is supposed to be idempotent, i.e., there is no difference between invoking it once vs many times.

There was a bug because we preserve line breaks for collection literals even if we can in principle squash it in a single line.

This works as intended except for the empty literal case where we do (and want to) squash it onto one line, but the code explicitly looks for a newline character at the end of the left brace.

So on the first run we squash it into one line and the absence of the newline character leads to causes it to format further on the second run.

Reviewed By: mheiber

Differential Revision: D62971173

fbshipit-source-id: e469329ad1a2d7c68cb9fe12d5c6500f0036095a
  • Loading branch information
@facebook-github-bot
Mistral Contrastin authored and facebook-github-bot committed Sep 19, 2024
1 parent 4cf5c38 commit 495af87
Show file tree
Hide file tree
Showing 9 changed files with 933 additions and 1 deletion.
8 changes: 7 additions & 1 deletion hphp/hack/src/hackfmt/hack_format.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3439,7 +3439,13 @@ and transform_container_literal
left_p
members
right_p =
let force_newlines = node_has_trailing_newline left_p in
let force_newlines =
node_has_trailing_newline left_p
&&
match members.Syntax.syntax with
| Syntax.Missing -> false
| _ -> true
in
let ty =
match explicit_type with
| Some ex_ty -> t env ex_ty
Expand Down
8 changes: 8 additions & 0 deletions hphp/hack/test/full_fidelity/cases/idempotence_regression.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?hh

function f() {
g(
vec[
]
);
}
272 changes: 272 additions & 0 deletions hphp/hack/test/full_fidelity/cases/idempotence_regression.php.json.exp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,272 @@
{
"kind":"script",
"script_declarations":{
"kind":"list",
"elements":[
{
"kind":"markup_section",
"markup_suffix":{
"kind":"markup_suffix",
"markup_suffix_less_than_question":{
"kind":"token",
"token":{
"kind":"<?",
"text":"<?",
"offset":0,
"leading_width":0,
"width":2,
"trailing_width":0,
"leading":[],
"trailing":[],
"line_number":1
}
},
"markup_suffix_name":{
"kind":"token",
"token":{
"kind":"name",
"text":"hh",
"offset":2,
"leading_width":0,
"width":2,
"trailing_width":1,
"leading":[],
"trailing":[{"kind":"end_of_line","text":"\n","offset":4,"width":1}],
"line_number":1
}
}
}
},
{
"kind":"function_declaration",
"function_declaration_header":{
"kind":"function_declaration_header",
"function_keyword":{
"kind":"token",
"token":{
"kind":"function",
"text":"function",
"offset":5,
"leading_width":1,
"width":8,
"trailing_width":1,
"leading":[{"kind":"end_of_line","text":"\n","offset":5,"width":1}],
"trailing":[{"kind":"whitespace","text":" ","offset":14,"width":1}],
"line_number":3
}
},
"function_name":{
"kind":"token",
"token":{
"kind":"name",
"text":"f",
"offset":15,
"leading_width":0,
"width":1,
"trailing_width":0,
"leading":[],
"trailing":[],
"line_number":3
}
},
"function_left_paren":{
"kind":"token",
"token":{
"kind":"(",
"text":"(",
"offset":16,
"leading_width":0,
"width":1,
"trailing_width":0,
"leading":[],
"trailing":[],
"line_number":3
}
},
"function_right_paren":{
"kind":"token",
"token":{
"kind":")",
"text":")",
"offset":17,
"leading_width":0,
"width":1,
"trailing_width":1,
"leading":[],
"trailing":[{"kind":"whitespace","text":" ","offset":18,"width":1}],
"line_number":3
}
}
},
"function_body":{
"kind":"compound_statement",
"compound_left_brace":{
"kind":"token",
"token":{
"kind":"{",
"text":"{",
"offset":19,
"leading_width":0,
"width":1,
"trailing_width":1,
"leading":[],
"trailing":[{"kind":"end_of_line","text":"\n","offset":20,"width":1}],
"line_number":3
}
},
"compound_statements":{
"kind":"list",
"elements":[
{
"kind":"expression_statement",
"expression_statement_expression":{
"kind":"function_call_expression",
"function_call_receiver":{
"kind":"token",
"token":{
"kind":"name",
"text":"g",
"offset":21,
"leading_width":2,
"width":1,
"trailing_width":0,
"leading":[{"kind":"whitespace","text":" ","offset":21,"width":2}],
"trailing":[],
"line_number":4
}
},
"function_call_left_paren":{
"kind":"token",
"token":{
"kind":"(",
"text":"(",
"offset":24,
"leading_width":0,
"width":1,
"trailing_width":1,
"leading":[],
"trailing":[{"kind":"end_of_line","text":"\n","offset":25,"width":1}],
"line_number":4
}
},
"function_call_argument_list":{
"kind":"list",
"elements":[
{
"kind":"list_item",
"list_item":{
"kind":"vector_intrinsic_expression",
"vector_intrinsic_keyword":{
"kind":"token",
"token":{
"kind":"vec",
"text":"vec",
"offset":26,
"leading_width":2,
"width":3,
"trailing_width":0,
"leading":[{"kind":"whitespace","text":" ","offset":26,"width":2}],
"trailing":[],
"line_number":5
}
},
"vector_intrinsic_left_bracket":{
"kind":"token",
"token":{
"kind":"[",
"text":"[",
"offset":31,
"leading_width":0,
"width":1,
"trailing_width":1,
"leading":[],
"trailing":[{"kind":"end_of_line","text":"\n","offset":32,"width":1}],
"line_number":5
}
},
"vector_intrinsic_right_bracket":{
"kind":"token",
"token":{
"kind":"]",
"text":"]",
"offset":33,
"leading_width":2,
"width":1,
"trailing_width":1,
"leading":[{"kind":"whitespace","text":" ","offset":33,"width":2}],
"trailing":[{"kind":"end_of_line","text":"\n","offset":36,"width":1}],
"line_number":6
}
}
}
}
]
},
"function_call_right_paren":{
"kind":"token",
"token":{
"kind":")",
"text":")",
"offset":37,
"leading_width":2,
"width":1,
"trailing_width":0,
"leading":[{"kind":"whitespace","text":" ","offset":37,"width":2}],
"trailing":[],
"line_number":7
}
}
},
"expression_statement_semicolon":{
"kind":"token",
"token":{
"kind":";",
"text":";",
"offset":40,
"leading_width":0,
"width":1,
"trailing_width":1,
"leading":[],
"trailing":[{"kind":"end_of_line","text":"\n","offset":41,"width":1}],
"line_number":7
}
}
}
]
},
"compound_right_brace":{
"kind":"token",
"token":{
"kind":"}",
"text":"}",
"offset":42,
"leading_width":0,
"width":1,
"trailing_width":1,
"leading":[],
"trailing":[{"kind":"end_of_line","text":"\n","offset":43,"width":1}],
"line_number":8
}
}
}
},
{
"kind":"end_of_file",
"end_of_file_token":{
"kind":"token",
"token":{
"kind":"end_of_file",
"text":"",
"offset":44,
"leading_width":0,
"width":0,
"trailing_width":0,
"leading":[],
"trailing":[],
"line_number":9
}
}
}
]
}
}
14 changes: 14 additions & 0 deletions hphp/hack/test/full_fidelity/cases/idempotence_regression2.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?hh

function aaa() {
return vec[
bbb()
->ccc(ddd(
eee(vec[
])
,
)->fff()
)
,
];
}
Loading

0 comments on commit 495af87

Please sign in to comment.