From 98d61f0ae9dabcdda7e5e311721d46766a87b1e5 Mon Sep 17 00:00:00 2001 From: Jake Bailey Date: Thu, 21 Sep 2017 19:55:12 -0700 Subject: [PATCH] Use the line splitting solution from the original source for splits outside of the formatting range Summary: Currently, range formatting simply prints a substring of the full formatted source. This causes problems when the original source represents a line-breaking solution different from the one hackfmt would produce. For example, consider as-you-type formatting on this source: ``` Vec\map($foo, $x ==> { bar($x); baz($x); }); ``` If you try to format at the semicolon after the call to `bar`, you'll get incorrect indentation: ``` Vec\map($foo, $x ==> { bar($x); baz($x); }); ``` The reason is that hackfmt actually wants to format the whole code block like this: ``` Vec\map( $foo, $x ==> { bar($x); baz($x); }, ); ``` But the result of hackfmt is only the changed range, which includes this extra indent. This diff binds all splits outside of the formatting range to the solution in the original source, constraining hackfmt to change line breaks only within the formatting range. This fixes the indentation problem in this example. Reviewed By: arxanas Differential Revision: D5853970 fbshipit-source-id: 575ecd9b5286f4294ecc8e6328ad5bef6321f5ee --- hphp/hack/src/hackfmt/chunk_group.ml | 10 +++++++-- hphp/hack/src/hackfmt/line_splitter.ml | 25 +++++++++++++++++++--- hphp/hack/src/hackfmt/solve_state.ml | 8 +++++-- .../test/hackfmt/tests/at_char_inside_lambda.flags | 1 + .../test/hackfmt/tests/at_char_inside_lambda.php | 6 ++++++ .../hackfmt/tests/at_char_inside_lambda.php.exp | 3 +++ .../at_char_last_arg_removes_trailing_comma.flags | 2 +- .../at_char_last_arg_removes_trailing_comma.php | 4 +--- ...at_char_last_arg_removes_trailing_comma.php.exp | 4 ++-- 9 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 hphp/hack/test/hackfmt/tests/at_char_inside_lambda.flags create mode 100644 hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php create mode 100644 hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php.exp diff --git a/hphp/hack/src/hackfmt/chunk_group.ml b/hphp/hack/src/hackfmt/chunk_group.ml index 87e17e9f2de..eb307cd3b15 100644 --- a/hphp/hack/src/hackfmt/chunk_group.ml +++ b/hphp/hack/src/hackfmt/chunk_group.ml @@ -31,8 +31,9 @@ let get_rule_kind t id = let get_char_range t = t.chunks |> List.fold ~init:(max_int,0) ~f:(fun (start_char, end_char) chunk -> - min start_char chunk.Chunk.start_char, - max end_char chunk.Chunk.end_char + let chunk_start, chunk_end = Chunk.get_range chunk in + min start_char chunk_start, + max end_char chunk_end ) let constrain_rules t rbm rule_list = @@ -40,6 +41,11 @@ let constrain_rules t rbm rule_list = let rules_that_care = List.filter rule_list ~f:aux in List.fold rules_that_care ~init:rbm ~f:(fun acc k -> IMap.add k true acc) +let get_always_rule_bindings t = + let is_always_rule _k v = v.Rule.kind = Rule.Always in + let always_rules = IMap.filter is_always_rule t.rule_map in + IMap.map (fun _ -> true) always_rules + let get_initial_rule_bindings t = let is_always_rule _k v = v.Rule.kind = Rule.Always in let always_rules = IMap.filter is_always_rule t.rule_map in diff --git a/hphp/hack/src/hackfmt/line_splitter.ml b/hphp/hack/src/hackfmt/line_splitter.ml index 83340e7dc8e..70e4d328c68 100644 --- a/hphp/hack/src/hackfmt/line_splitter.ml +++ b/hphp/hack/src/hackfmt/line_splitter.ml @@ -51,8 +51,27 @@ let find_best_state env init_state = in aux 0 init_state -let solve_chunk_group env source_text chunk_group = - let rbm = Chunk_group.get_initial_rule_bindings chunk_group in +let solve_chunk_group env ?range source_text chunk_group = + let rbm = + match range with + | Some range + when Interval.intervals_overlap range + (Chunk_group.get_char_range chunk_group) + -> + let source_rbm = Solve_state.rbm_from_source source_text chunk_group in + List.fold chunk_group.Chunk_group.chunks + ~init:(Chunk_group.get_always_rule_bindings chunk_group) + ~f:begin fun rbm chunk -> + let rule = chunk.Chunk.rule in + if Interval.intervals_overlap range (Chunk.get_range chunk) + then rbm + else + match IMap.get rule source_rbm with + | Some true -> IMap.add rule true rbm + | _ -> rbm + end + | _ -> Chunk_group.get_initial_rule_bindings chunk_group + in let init_state = Solve_state.make env chunk_group rbm in match find_best_state env init_state with | Some state -> state @@ -72,7 +91,7 @@ let find_solve_states Interval.intervals_overlap range group_range ) in - chunk_groups |> List.map ~f:(solve_chunk_group env source_text) + chunk_groups |> List.map ~f:(solve_chunk_group ?range env source_text) let print (env: Env.t) diff --git a/hphp/hack/src/hackfmt/solve_state.ml b/hphp/hack/src/hackfmt/solve_state.ml index 1e59febfbe0..ef234818206 100644 --- a/hphp/hack/src/hackfmt/solve_state.ml +++ b/hphp/hack/src/hackfmt/solve_state.ml @@ -212,8 +212,8 @@ let make env chunk_group rbm = { chunk_group; lines; rbm; cost; overflow; nesting_set; candidate_rules; unprocessed_overflow; rules_on_partially_bound_lines; } -let from_source env source_text chunk_group = - let rbm = Chunk_group.get_initial_rule_bindings chunk_group in +let rbm_from_source source_text chunk_group = + let rbm = Chunk_group.get_always_rule_bindings chunk_group in let rbm, _ = List.fold chunk_group.Chunk_group.chunks ~init:(rbm, 0) ~f:begin fun (rbm, prev_chunk_end) chunk -> let chunk_start, chunk_end = Chunk.get_range chunk in @@ -228,6 +228,10 @@ let from_source env source_text chunk_group = rbm, chunk_end end in + rbm + +let from_source env source_text chunk_group = + let rbm = rbm_from_source source_text chunk_group in make env chunk_group rbm let is_rule_bound t rule_id = diff --git a/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.flags b/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.flags new file mode 100644 index 00000000000..001a3008550 --- /dev/null +++ b/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.flags @@ -0,0 +1 @@ +--at-char 38 diff --git a/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php b/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php new file mode 100644 index 00000000000..b20cdb43279 --- /dev/null +++ b/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php @@ -0,0 +1,6 @@ + { + bar($x); + baz($x); +}); diff --git a/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php.exp b/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php.exp new file mode 100644 index 00000000000..d9d8d8d33e0 --- /dev/null +++ b/hphp/hack/test/hackfmt/tests/at_char_inside_lambda.php.exp @@ -0,0 +1,3 @@ +27 39 +{ + bar($x); \ No newline at end of file diff --git a/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.flags b/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.flags index 11121a117b8..9275be3ff03 100644 --- a/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.flags +++ b/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.flags @@ -1 +1 @@ ---at-char 24 +--at-char 19 diff --git a/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.php b/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.php index 845cffdd19a..53fbed78d8c 100644 --- a/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.php +++ b/hphp/hack/test/hackfmt/tests/at_char_last_arg_removes_trailing_comma.php @@ -1,4 +1,2 @@