Better error when trying to use a keyword as a record field (#7784)

* better error when trying to use a keyword as a record field

* error recovery

* changelog

* cleanup

* format

* cleanup

Author: zth

CHANGELOG.md

@@ -33,6 +33,7 @@
 #### :nail_care: Polish
 
 - Allow skipping the leading pipe in variant definition with a leading constructor with an attribute. https://github.com/rescript-lang/rescript/pull/7782
+- Better error message (and recovery) when using a keyword as a record field name. https://github.com/rescript-lang/rescript/pull/7784
 
 #### :house: Internal
 

compiler/syntax/src/res_core.ml

@@ -155,6 +155,22 @@ module ErrorMessages = struct
   let multiple_inline_record_definitions_at_same_path =
     "Only one inline record definition is allowed per record field. This \
      defines more than one inline record."
+
+  let keyword_field_in_expr keyword_txt =
+    "Cannot use keyword `" ^ keyword_txt
+    ^ "` as a record field name. Suggestion: rename it (e.g. `" ^ keyword_txt
+    ^ "_`)"
+
+  let keyword_field_in_pattern keyword_txt =
+    "Cannot use keyword `" ^ keyword_txt
+    ^ "` here. Keywords are not allowed as record field names."
+
+  let keyword_field_in_type keyword_txt =
+    "Cannot use keyword `" ^ keyword_txt
+    ^ "` as a record field name. Suggestion: rename it (e.g. `" ^ keyword_txt
+    ^ "_`)\n  If you need the field to be \"" ^ keyword_txt
+    ^ "\" at runtime, annotate the field: `@as(\"" ^ keyword_txt ^ "\") "
+    ^ keyword_txt ^ "_ : ...`"
 end
 
 module InExternal = struct
@@ -403,6 +419,30 @@ let build_longident words =
   | [] -> assert false
   | hd :: tl -> List.fold_left (fun p s -> Longident.Ldot (p, s)) (Lident hd) tl
 
+let emit_keyword_field_error (p : Parser.t) ~mk_message =
+  let keyword_txt = Token.to_string p.token in
+  let keyword_start = p.Parser.start_pos in
+  let keyword_end = p.Parser.end_pos in
+  Parser.err ~start_pos:keyword_start ~end_pos:keyword_end p
+    (Diagnostics.message (mk_message keyword_txt))
+
+(* Recovers a keyword used as field name if it's probable that it's a full
+   field name (not punning etc), by checking if there's a colon after it. *)
+let recover_keyword_field_name_if_probably_field p ~mk_message :
+    (string * Location.t) option =
+  if
+    Token.is_keyword p.Parser.token
+    && Parser.lookahead p (fun st ->
+           Parser.next st;
+           st.Parser.token = Colon)
+  then (
+    emit_keyword_field_error p ~mk_message;
+    let loc = mk_loc p.Parser.start_pos p.Parser.end_pos in
+    let recovered_field_name = Token.to_string p.token ^ "_" in
+    Parser.next p;
+    Some (recovered_field_name, loc))
+  else None
+
 let make_infix_operator (p : Parser.t) token start_pos end_pos =
   let stringified_token =
     if token = Token.Equal then (
@@ -1382,7 +1422,25 @@ and parse_record_pattern_row p =
   | Underscore ->
     Parser.next p;
     Some (false, PatUnderscore)
-  | _ -> None
+  | _ ->
+    if Token.is_keyword p.token then (
+      match
+        recover_keyword_field_name_if_probably_field p
+          ~mk_message:ErrorMessages.keyword_field_in_pattern
+      with
+      | Some (recovered_field_name, loc) ->
+        Parser.expect Colon p;
+        let optional = parse_optional_label p in
+        let pat = parse_pattern p in
+        let field =
+          Location.mkloc (Longident.Lident recovered_field_name) loc
+        in
+        Some (false, PatField {lid = field; x = pat; opt = optional})
+      | None ->
+        emit_keyword_field_error p
+          ~mk_message:ErrorMessages.keyword_field_in_pattern;
+        None)
+    else None
 
 and parse_record_pattern ~attrs p =
   let start_pos = p.start_pos in
@@ -2928,6 +2986,26 @@ and parse_braced_or_record_expr p =
   let start_pos = p.Parser.start_pos in
   Parser.expect Lbrace p;
   match p.Parser.token with
+  | token when Token.is_keyword token -> (
+    match
+      recover_keyword_field_name_if_probably_field p
+        ~mk_message:ErrorMessages.keyword_field_in_expr
+    with
+    | Some (recovered_field_name, loc) ->
+      Parser.expect Colon p;
+      let optional = parse_optional_label p in
+      let field_expr = parse_expr p in
+      let field = Location.mkloc (Longident.Lident recovered_field_name) loc in
+      let first_row = {Parsetree.lid = field; x = field_expr; opt = optional} in
+      let expr = parse_record_expr ~start_pos [first_row] p in
+      Parser.expect Rbrace p;
+      expr
+    | None ->
+      let expr = parse_expr_block p in
+      Parser.expect Rbrace p;
+      let loc = mk_loc start_pos p.prev_end_pos in
+      let braces = make_braces_attr loc in
+      {expr with pexp_attributes = braces :: expr.pexp_attributes})
   | Rbrace ->
     Parser.next p;
     let loc = mk_loc start_pos p.prev_end_pos in
@@ -3244,7 +3322,25 @@ and parse_record_expr_row p :
       in
       Some {lid = field; x = value; opt = true}
     | _ -> None)
-  | _ -> None
+  | _ ->
+    if Token.is_keyword p.token then (
+      match
+        recover_keyword_field_name_if_probably_field p
+          ~mk_message:ErrorMessages.keyword_field_in_expr
+      with
+      | Some (recovered_field_name, loc) ->
+        Parser.expect Colon p;
+        let optional = parse_optional_label p in
+        let field_expr = parse_expr p in
+        let field =
+          Location.mkloc (Longident.Lident recovered_field_name) loc
+        in
+        Some {lid = field; x = field_expr; opt = optional}
+      | None ->
+        emit_keyword_field_error p
+          ~mk_message:ErrorMessages.keyword_field_in_expr;
+        None)
+    else None
 
 and parse_dict_expr_row p =
   match p.Parser.token with
@@ -4742,17 +4838,36 @@ and parse_field_declaration_region ?current_type_name_path ?inline_types_context
     let loc = mk_loc start_pos typ.ptyp_loc.loc_end in
     Some (Ast_helper.Type.field ~attrs ~loc ~mut ~optional name typ)
   | _ ->
-    if attrs <> [] then
-      Parser.err ~start_pos p
-        (Diagnostics.message
-           "Attributes and doc comments can only be used at the beginning of a \
-            field declaration");
-    if mut = Mutable then
-      Parser.err ~start_pos p
-        (Diagnostics.message
-           "The `mutable` qualifier can only be used at the beginning of a \
-            field declaration");
-    None
+    if Token.is_keyword p.token then (
+      match
+        recover_keyword_field_name_if_probably_field p
+          ~mk_message:ErrorMessages.keyword_field_in_type
+      with
+      | Some (recovered_field_name, name_loc) ->
+        let optional = parse_optional_label p in
+        Parser.expect Colon p;
+        let typ =
+          parse_poly_type_expr ?current_type_name_path ?inline_types_context p
+        in
+        let loc = mk_loc start_pos typ.ptyp_loc.loc_end in
+        let name = Location.mkloc recovered_field_name name_loc in
+        Some (Ast_helper.Type.field ~attrs ~loc ~mut ~optional name typ)
+      | None ->
+        emit_keyword_field_error p
+          ~mk_message:ErrorMessages.keyword_field_in_type;
+        None)
+    else (
+      if attrs <> [] then
+        Parser.err ~start_pos p
+          (Diagnostics.message
+             "Attributes and doc comments can only be used at the beginning of \
+              a field declaration");
+      if mut = Mutable then
+        Parser.err ~start_pos p
+          (Diagnostics.message
+             "The `mutable` qualifier can only be used at the beginning of a \
+              field declaration");
+      None)
 
 (* record-decl ::=
  *  | { field-decl }

tests/syntax_tests/data/parsing/errors/structure/expected/recordFieldKeywordInExpr.res.txt

@@ -0,0 +1,10 @@
+
+  Syntax error!
+  syntax_tests/data/parsing/errors/structure/recordFieldKeywordInExpr.res:1:10-13
+
+  1 │ let r = {type: 1}
+  2 │ 
+
+  Cannot use keyword `type` as a record field name. Suggestion: rename it (e.g. `type_`)
+
+let r = { type_ = 1 }

tests/syntax_tests/data/parsing/errors/structure/expected/recordFieldKeywordInExpr2.res.txt

@@ -0,0 +1,10 @@
+
+  Syntax error!
+  syntax_tests/data/parsing/errors/structure/recordFieldKeywordInExpr2.res:1:16-19
+
+  1 │ let r = {a: 1, type: 2}
+  2 │ 
+
+  Cannot use keyword `type` as a record field name. Suggestion: rename it (e.g. `type_`)
+
+let r = { a = 1; type_ = 2 }

tests/syntax_tests/data/parsing/errors/structure/expected/recordFieldKeywordInPattern.res.txt

@@ -0,0 +1,22 @@
+
+  Syntax error!
+  syntax_tests/data/parsing/errors/structure/recordFieldKeywordInPattern.res:1:6-9
+
+  1 │ let {type} = r
+  2 │ 
+  3 │ 
+
+  Cannot use keyword `type` here. Keywords are not allowed as record field names.
+
+
+  Syntax error!
+  syntax_tests/data/parsing/errors/structure/recordFieldKeywordInPattern.res:1:10
+
+  1 │ let {type} = r
+  2 │ 
+  3 │ 
+
+  I'm not sure what to parse here when looking at "}".
+
+let {  } = [%rescript.exprhole ]
+;;r

tests/syntax_tests/data/parsing/errors/structure/expected/recordFieldKeywordInPattern2.res.txt

@@ -0,0 +1,10 @@
+
+  Syntax error!
+  syntax_tests/data/parsing/errors/structure/recordFieldKeywordInPattern2.res:1:12-15
+
+  1 │ let {a: _, type: x} = r
+  2 │ 
+
+  Cannot use keyword `type` here. Keywords are not allowed as record field names.
+
+let { a = _; type_ = x } = r

tests/syntax_tests/data/parsing/errors/structure/expected/recordFieldKeywordInType.res.txt

@@ -0,0 +1,16 @@
+
+  Syntax error!
+  syntax_tests/data/parsing/errors/structure/recordFieldKeywordInType.res:3:3-6
+
+  1 │ type r = {
+  2 │   id: string,
+  3 │   type: int,
+  4 │ }
+  5 │ 
+
+  Cannot use keyword `type` as a record field name. Suggestion: rename it (e.g. `type_`)
+  If you need the field to be "type" at runtime, annotate the field: `@as("type") type_ : ...`
+
+type nonrec r = {
+  id: string ;
+  type_: int }

tests/syntax_tests/data/parsing/errors/structure/expected/recordFieldKeywordInType2.res.txt

@@ -0,0 +1,17 @@
+
+  Syntax error!
+  syntax_tests/data/parsing/errors/structure/recordFieldKeywordInType2.res:3:3-6
+
+  1 │ type r = {
+  2 │   id: string,
+  3 │   type: int,
+  4 │   x: bool,
+  5 │ }
+
+  Cannot use keyword `type` as a record field name. Suggestion: rename it (e.g. `type_`)
+  If you need the field to be "type" at runtime, annotate the field: `@as("type") type_ : ...`
+
+type nonrec r = {
+  id: string ;
+  type_: int ;
+  x: bool }

tests/syntax_tests/data/parsing/errors/structure/recordFieldKeywordInExpr.res

@@ -0,0 +1 @@
+let r = {type: 1}

tests/syntax_tests/data/parsing/errors/structure/recordFieldKeywordInExpr2.res

@@ -0,0 +1 @@
+let r = {a: 1, type: 2}