diff --git a/formatTest/.gitignore b/formatTest/.gitignore new file mode 100644 index 000000000..374959514 --- /dev/null +++ b/formatTest/.gitignore @@ -0,0 +1,3 @@ +typeCheckedTests/*.out +typeCheckedTests/*.cmo +typeCheckedTests/*.cmi diff --git a/formatTest/formatOutput.re b/formatTest/formatOutput.re index bc6c9a22a..9b759ec42 100644 --- a/formatTest/formatOutput.re +++ b/formatTest/formatOutput.re @@ -4763,7 +4763,8 @@ type hasA = {a: int}; let a = 10; -let thisReturnsARecord () => {a}; +let returnsASequenceExpressionWithASingleIdentifier + () => a; let thisReturnsA () => a; @@ -6123,3 +6124,69 @@ let module M = Something.Create { type resource1 = MyModule.MySubmodule.t; type resource2 = MyModule.MySubmodule.t; }; +let result = { + let twenty = 20; + let result = twenty; + result; +}; + +/* Final semicolon is not required */ +let result = { + let twenty = result; + twenty; +}; + +let anInt = result + 20; + +let twenty = 20; + +/** + * Each of these are a sequence with a single item - they will be + * printed in reduced form because sequences are a *parse* time construct. + * To ensure these are parsed correctly, adding to an integer. + */ +let result = 0 + twenty; + +let result = 0 + twenty; + +let result = 0 + twenty; + +let unitValue = (); + +/* While loops/for loops merely accept a "simple expression" (which means + * it is either a simple token or balanced with parens/braces). However, + * the formatter ensures that the bodies are printed in "sequence" form even if + * it's not required. + */ +while false { + unitValue; +}; + +while false { + print_string "test"; +}; + +while false { + print_string "test"; +}; + +type myRecord = {number: int}; + +let x = {number: 20}; + +let number = 20; + +/* + * The (mild) consequence of not requiring a final semi in a sequence, + * is that we can no longer "pun" a single field record (which would) + * be very rare anyways. + */ +let cannotPunASingleFieldRecord = { + number: number +}; + +let fourty = 20 + cannotPunASingleFieldRecord.number; + +let thisIsASequenceNotPunedRecord = number; + +let fourty = 20 + thisIsASequenceNotPunedRecord; diff --git a/formatTest/syntax.re b/formatTest/syntax.re index 97c766223..dd508289e 100644 --- a/formatTest/syntax.re +++ b/formatTest/syntax.re @@ -350,7 +350,7 @@ let onlyDoingThisTopLevelLetToBypassTopLevelSequence = { type hasA = {a:int}; let a = 10; -let thisReturnsARecord () => {a}; +let returnsASequenceExpressionWithASingleIdentifier () => {a}; let thisReturnsA () => {a;}; let thisReturnsAAsWell () => a; diff --git a/formatTest/test.sh b/formatTest/test.sh index 3f48df58a..9b20f042e 100755 --- a/formatTest/test.sh +++ b/formatTest/test.sh @@ -22,3 +22,12 @@ ../reasonfmt_impl.native -print-width 50 -print re ./trailingSpaces.re 2>&1 >>./formatOutput.re ../reasonfmt_impl.native -print-width 50 -print re ./wrappingTest.rei 2>&1 >./formatOutput.rei + + + +# Let's start creating formatting test cases that must type check. +# Errors in parsing/printing often are caught via the type system. +ocamlc -c -pp ../reasonfmt_impl.native -intf-suffix rei -impl ./typeCheckedTests/sequences.re +rm ./typeCheckedTests/sequences.cmi +rm ./typeCheckedTests/sequences.cmo +../reasonfmt_impl.native -print-width 50 -print re ./typeCheckedTests/sequences.re 2>&1 >>./formatOutput.re diff --git a/formatTest/typeCheckedTests/sequences.re b/formatTest/typeCheckedTests/sequences.re new file mode 100644 index 000000000..11cb4290e --- /dev/null +++ b/formatTest/typeCheckedTests/sequences.re @@ -0,0 +1,52 @@ +let result = { + let twenty = 20; + let result = twenty; + result; +}; + +/* Final semicolon is not required */ +let result = { + let twenty = result; + twenty +}; +let anInt = result + 20; + +let twenty = 20; + +/** + * Each of these are a sequence with a single item - they will be + * printed in reduced form because sequences are a *parse* time construct. + * To ensure these are parsed correctly, adding to an integer. + */ +let result = 0 + {twenty}; +let result = 0 + {twenty;}; +let result = 0 + twenty; + +let unitValue = (); +/* While loops/for loops merely accept a "simple expression" (which means + * it is either a simple token or balanced with parens/braces). However, + * the formatter ensures that the bodies are printed in "sequence" form even if + * it's not required. + */ +while false unitValue; +while (false) { + print_string "test" +}; +while (false) { + print_string "test"; +}; + +type myRecord = { + number: int +}; +let x = {number:20}; +let number = 20; +/* + * The (mild) consequence of not requiring a final semi in a sequence, + * is that we can no longer "pun" a single field record (which would) + * be very rare anyways. + */ +let cannotPunASingleFieldRecord = {number: number}; +let fourty = 20 + cannotPunASingleFieldRecord.number; +let thisIsASequenceNotPunedRecord = {number}; +let fourty = 20 + thisIsASequenceNotPunedRecord; \ No newline at end of file diff --git a/src/reason_parser.mly b/src/reason_parser.mly index 269a8b1ff..1f3329317 100644 --- a/src/reason_parser.mly +++ b/src/reason_parser.mly @@ -1444,7 +1444,7 @@ semi_terminated_seq_expr: * a + b; * }; */ - | expr SEMI { reloc_exp $1 } + | expr opt_semi { reloc_exp $1 } | LET ext_attributes rec_flag let_bindings_no_attrs SEMI semi_terminated_seq_expr { mkexp_attrs (Pexp_let($3, List.rev $4, $6)) $2 } | LET MODULE ext_attributes UIDENT module_binding_body SEMI semi_terminated_seq_expr @@ -2129,7 +2129,7 @@ expr_optional_constraint: record_expr: DOTDOTDOT expr_optional_constraint COMMA lbl_expr_list { (Some $2, $4) } - | lbl_expr_list { (None, $1) } + | lbl_expr_list_with_at_least_one_non_punned_field { (None, $1) } ; lbl_expr_list: lbl_expr { [$1] } @@ -2143,14 +2143,37 @@ lbl_expr: { (mkrhs $1 1, exp_of_label $1 1) } ; -field_expr: - LIDENT COLON expr - { (mkrhs $1 1, $3) } - | LIDENT - { (mkrhs $1 1, mkexp (Pexp_ident(mkrhs (Lident $1) 1))) } +non_punned_lbl_expression: + label_longident COLON expr + { (mkrhs $1 1, $3) } ; -field_expr_list: +/** + * To allow sequence expressions to not *require* a final semicolon, we make a small tradeoff: + * {label} would normally be an ambiguity: Is it a punned record, or a sequence expression + * with only one item? It makes more sense to break the tie by interpreting it as a sequence + * expression with a single item, as opposed to a punned record with one field. Justification: + * + * - Constructing single field records is very rare. + * - Block sequences should not require a semicolon after their final item: + * - let something = {print "hi"; foo}; + * - We often want to be able to print *single* values in block form, without a trailing semicolon. + * - You should be able to delete `print "hi"` in the above statement and still parse intuitively. + * - An equivalent scenario when you delete all but a single field from a punned record is more rare: + * - let myRecord = {deleteThisField, butLeaveThisOne}; + * - For whatever tiny remaining confusion would occur, virtually all of them would be caught by the type system. + */ +lbl_expr_list_with_at_least_one_non_punned_field: + | non_punned_lbl_expression + { [$1] } + | non_punned_lbl_expression COMMA lbl_expr_list + { $1::$3 } +; + +/** + * field_expr is distinct from record_expr because labels cannot/shouldn't be scoped. + */ +field_expr: /* Using LIDENT instead of label here, because a reduce/reduce conflict occurs on: * {blah:x} * @@ -2159,12 +2182,20 @@ field_expr_list: * Another approach would have been to place the `label` rule at at a precedence * of below_COLON or something. */ + LIDENT COLON expr + { (mkrhs $1 1, $3) } + | LIDENT + { (mkrhs $1 1, mkexp (Pexp_ident(mkrhs (Lident $1) 1))) } +; + +field_expr_list: field_expr { [$1] } | field_expr_list COMMA field_expr { $3::$1 } ; + type_constraint_right_of_colon: core_type { (Some $1, None) } | core_type COLONGREATER core_type { (Some $1, Some $3) } diff --git a/src/reason_pprint_ast.ml b/src/reason_pprint_ast.ml index fa3756ee1..d90fb1f42 100644 --- a/src/reason_pprint_ast.ml +++ b/src/reason_pprint_ast.ml @@ -1235,6 +1235,7 @@ let layoutEasy x = x let semiTerminated term = makeList [term; atom ";"] +(* TODO: Do not print the final semicolon *) let makeLetSequence letItems = makeList ~wrap:("{", "}") ~break:Always ~inline:(true, false) letItems @@ -3289,7 +3290,7 @@ class printer ()= object(self:'self) | Pexp_variant (l, None) -> ensureSingleTokenSticksToLabel (atom ("`" ^ l)) | Pexp_record (l, eo) -> - let makeRow (li, e) appendComma = + let makeRow (li, e) appendComma shouldPun = let comma = atom "," in let totalRowLoc = { loc_start = li.Asttypes.loc.loc_start; @@ -3299,7 +3300,7 @@ class printer ()= object(self:'self) let theRow = match e.pexp_desc with (* Punning *) - | Pexp_ident {txt} when li.txt = txt -> + | Pexp_ident {txt} when li.txt = txt && shouldPun -> makeList ((self#longident_loc li)::(if appendComma then [comma] else [])) | _ -> let (argsList, return) = self#curriedPatternsAndReturnVal e in ( @@ -3328,12 +3329,18 @@ class printer ()= object(self:'self) let rec getRows l = match l with | [] -> [] - | hd::[] -> [makeRow hd false] - | hd::hd2::tl -> (makeRow hd true)::(getRows (hd2::tl)) + | hd::[] -> [makeRow hd false true] + | hd::hd2::tl -> (makeRow hd true true)::(getRows (hd2::tl)) in let allRows = match eo with - | None -> getRows l + | None -> ( + match l with + (* No punning (or comma) for records with only a single field. *) + (* See comment in parser.mly for lbl_expr_list_with_at_least_one_non_punned_field *) + | [hd] -> [makeRow hd false false] + | _ -> getRows l + ) | Some withRecord -> let firstRow = ( match self#expressionToFormattedApplicationItems withRecord with