From bea1cbcda3a4eadf4bfada34a07c94f71099c05a Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 28 Oct 2026 15:34:01 +0000 Subject: [PATCH] Parse attributes after `= private` and a doc comment before a manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: `private @attr {x: int}` took the type expression path and rejected the record, and a doc comment right after `=` always started a constructor list, so `/** d */ int` failed. Both now use the lookahead that attributes after `=` already had: past attributes and doc comments, a record, `..`, `|` or constructor is a representation, anything else a manifest. AST modified (doc comment before a manifest): Example: type z = /** d */ int Before (diagnostic): Did you mean `Int` instead of `int`? After (parse): type_declaration "z" ptype_kind = Ptype_abstract ptype_private = Public ptype_manifest = Some core_type attribute "res.doc" … Ptyp_constr "int" Format modified (attribute after `private`): Example: type t = private @attr {x: int} type u = private /** d */ int Before (diagnostic): An inline record type declaration is only allowed in a variant constructor's declaration or nested inside of a record type declaration After (rescript): @attr type t = private {x: int} type u = private /** d */ int Review: Does a leading doc comment change any form that parsed before? No. After a doc comment the lookahead keeps the representation for every start the constructor and record parsers accept (`|`, a constructor, `...base`, `{...`, a record, `..`); only starts they rejected, such as `int`, `M.t`, `'a`, `(a, b)`, `[#A]`, `{.}` or `{"a": int}`, become a manifest. A differential run over 672 declarations (prefixes x bodies, with and without `private`, .res and .resi) gives the same tree as before for every one that parsed. Fixes #9106 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel --- changes/private-attr-doc-comment-type.fix.md | 1 + compiler/syntax/src/res_core.ml | 95 +++++++++++-------- .../expected/typeRepresentation.res.txt | 71 +++++++++++++- .../typedefinition/typeRepresentation.res | 42 ++++++++ .../data/printer/typeDef/attributes.res | 17 ++++ .../typeDef/expected/attributes.res.txt | 31 ++++++ 6 files changed, 218 insertions(+), 39 deletions(-) create mode 100644 changes/private-attr-doc-comment-type.fix.md diff --git a/changes/private-attr-doc-comment-type.fix.md b/changes/private-attr-doc-comment-type.fix.md new file mode 100644 index 0000000000..1aa75e8524 --- /dev/null +++ b/changes/private-attr-doc-comment-type.fix.md @@ -0,0 +1 @@ +- Accept attributes and doc comments after `= private`, and a doc comment after `=` in front of a type expression such as `int`. diff --git a/compiler/syntax/src/res_core.ml b/compiler/syntax/src/res_core.ml index 83fc4c9bbe..d260fa90ef 100644 --- a/compiler/syntax/src/res_core.ml +++ b/compiler/syntax/src/res_core.ml @@ -5799,6 +5799,11 @@ and parse_private_eq_or_repr p = | Bar | DotDot -> let _, kind, attrs = parse_type_representation p in (None, Asttypes.Private, kind, attrs) + | (At | DocComment _) when is_representation_after_attrs p -> + let _, kind, attrs = parse_type_representation p in + (None, Asttypes.Private, kind, attrs) + | DocComment _ -> + (Some (parse_typ_expr p), Asttypes.Private, Parsetree.Ptype_abstract, []) | t when Grammar.is_typ_expr_start t -> (Some (parse_typ_expr p), Asttypes.Private, Parsetree.Ptype_abstract, []) | _ -> @@ -6019,6 +6024,55 @@ and parse_polymorphic_variant_type_args p = Parser.expect Rparen p; Location.mkloc args (mk_loc start_pos (Parser.position p)) +(* Attributes and doc comments after `=` (or `= private`) can start a + representation (variant/record/open variant) or a manifest. Look ahead past + them: if a representation-like token follows, it is a representation; + otherwise a manifest type expression. *) +and is_representation_after_attrs p = + let starts_with_doc_comment = + match Parser.peek p with + | DocComment _ -> true + | _ -> false + in + Parser.lookahead p (fun state -> + ignore (parse_attributes state); + (* optionally skip a run of doc comments before deciding *) + skip_doc_comments state; + match Parser.peek state with + | Lbrace -> ( + (* Disambiguate record declaration vs object type. + Peek inside the braces; if it looks like an object (String/Dot/DotDot/DotDotDot), + then this is a manifest type expression, not a representation. If it looks like + a record field (e.g. Lident or attributes before one), treat as representation. *) + Parser.next state; + (* consume Lbrace *) + ignore (parse_attributes state); + skip_doc_comments state; + match Parser.peek state with + | String _ | Dot | DotDot -> false (* object type => manifest *) + | DotDotDot -> + (* A spread (`{...base, y: int}`) after a leading doc comment has + always been parsed as a record declaration, and after a leading + attribute as an object type. *) + starts_with_doc_comment + | _ -> true + (* record decl => representation *)) + | Bar -> true (* variant constructor list *) + | DotDot -> true (* extensible/open variant ".." *) + | DotDotDot -> + (* A variant spread (`...base | C`) after a leading doc comment has + always been parsed as a constructor list. *) + starts_with_doc_comment + | Uident _ -> ( + (* constructor vs module-qualified manifest *) + Parser.next state; + match Parser.peek state with + | Dot -> false (* M.t => manifest *) + | _ -> true + (* Uident starting a constructor *)) + | DocComment _ -> true (* doc before constructor list *) + | _ -> false) + and parse_type_equation_and_representation ?current_type_name_path ?inline_types_context p = match Parser.peek p with @@ -6034,49 +6088,14 @@ and parse_type_equation_and_representation ?current_type_name_path in (manifest, priv, kind, []) | Private -> parse_private_eq_or_repr p - | Bar | DotDot | DocComment _ -> + | Bar | DotDot -> let priv, kind, attrs = parse_type_representation ?current_type_name_path ?inline_types_context p in (None, priv, kind, attrs) - | At -> ( - (* Attributes can start a representation (variant/record/open variant) or a manifest. - Look ahead past attributes (and doc comments). If a representation-like token follows, - parse it as a representation; otherwise treat as a manifest. *) - let is_representation_after_attrs = - Parser.lookahead p (fun state -> - ignore (parse_attributes state); - (* optionally skip a run of doc comments before deciding *) - skip_doc_comments state; - match Parser.peek state with - | Lbrace -> ( - (* Disambiguate record declaration vs object type. - Peek inside the braces; if it looks like an object (String/Dot/DotDot/DotDotDot), - then this is a manifest type expression, not a representation. If it looks like - a record field (e.g. Lident or attributes before one), treat as representation. *) - Parser.next state; - (* consume Lbrace *) - ignore (parse_attributes state); - skip_doc_comments state; - match Parser.peek state with - | String _ | Dot | DotDot | DotDotDot -> - false (* object type => manifest *) - | _ -> true - (* record decl => representation *)) - | Bar -> true (* variant constructor list *) - | DotDot -> true (* extensible/open variant ".." *) - | Uident _ -> ( - (* constructor vs module-qualified manifest *) - Parser.next state; - match Parser.peek state with - | Dot -> false (* M.t => manifest *) - | _ -> true - (* Uident starting a constructor *)) - | DocComment _ -> true (* doc before constructor list *) - | _ -> false) - in - if is_representation_after_attrs then + | At | DocComment _ -> ( + if is_representation_after_attrs p then let priv, kind, attrs = parse_type_representation ?current_type_name_path ?inline_types_context p diff --git a/tests/syntax_tests/data/parsing/grammar/typedefinition/expected/typeRepresentation.res.txt b/tests/syntax_tests/data/parsing/grammar/typedefinition/expected/typeRepresentation.res.txt index 20333b4fa0..3a14489885 100644 --- a/tests/syntax_tests/data/parsing/grammar/typedefinition/expected/typeRepresentation.res.txt +++ b/tests/syntax_tests/data/parsing/grammar/typedefinition/expected/typeRepresentation.res.txt @@ -79,4 +79,73 @@ type nonrec t = | B [@res.doc {js|doc2|js}] type nonrec t = | A [@res.doc {js|doc|js}][@attr ] - | B \ No newline at end of file + | B +type nonrec t = private { + x: int }[@@attr ] +type nonrec t = private ..[@@attr ] +type nonrec t = private + | A [@attr ] + | B +type nonrec t = private ((int)[@attr ]) +type nonrec t = private ((< a: int > )[@attr ]) +type nonrec t = private ((int)[@res.doc {js|doc|js}]) +type nonrec t = private ((M.t)[@res.doc {js|doc|js}]) +type nonrec t = private + | A [@res.doc {js|doc|js}] + | B +type nonrec t = private { + x: int }[@@res.doc {js|doc|js}][@@attr ] +type nonrec t = ((int)[@res.doc {js|doc|js}]) +type nonrec t = ((< a: int > )[@res.doc {js|doc|js}]) +type nonrec t = (((int * int))[@res.doc {js|doc|js}]) +type nonrec t = (([ `A ])[@res.doc {js|doc|js}]) +type nonrec t = ((M.t)[@res.doc {js|doc|js}]) +type nonrec t = ((M.t)[@res.doc {js|doc|js}]) = + | A + | B +type nonrec t = ((int)[@res.doc {js|doc|js}][@attr ]) +type nonrec t = { + ...: base ; + y: int }[@@res.doc {js|doc|js}] +type nonrec t = { + ...: base }[@@res.doc {js|doc|js}] +type nonrec t = { + ...: base ; + y: int }[@@res.doc {js|doc|js}][@@attr ] +type nonrec t = ((< .. > )[@res.doc {js|doc|js}]) +type nonrec t = ((< > )[@res.doc {js|doc|js}]) +type nonrec t = ((< a: int ;.. > )[@res.doc {js|doc|js}]) +type nonrec t = ((< base ;y: int > )[@attr ]) +type nonrec t = ((< base > )[@attr ]) +type nonrec t = private { + ...: base ; + y: int }[@@res.doc {js|doc|js}] +type nonrec t = private { + ...: base }[@@res.doc {js|doc|js}] +type nonrec t = private { + ...: base ; + y: int }[@@res.doc {js|doc|js}][@@attr ] +type nonrec t = private ((< .. > )[@res.doc {js|doc|js}]) +type nonrec t = private ((< a: int ;.. > )[@res.doc {js|doc|js}]) +type nonrec t = private ((< base ;y: int > )[@attr ]) +type nonrec t = private ((< base > )[@attr ]) +type nonrec t = + | ... of base [@res.doc {js|doc|js}] + | C +type nonrec t = + | ... of base [@res.doc {js|doc|js}][@res.doc {js|doc2|js}] + | C +type nonrec t = + | ... of base [@res.doc {js|doc|js}][@attr ] + | C +type nonrec t = private + | ... of base [@res.doc {js|doc|js}] + | C +type nonrec t = private + | ... of base [@res.doc {js|doc|js}][@attr ] + | C +type nonrec t = (('a)[@res.doc {js|doc|js}]) +type nonrec t = ((int)[@res.doc {js|doc|js}]) -> int (a:1) +type nonrec t = ((M.t)[@res.doc {js|doc|js}]) = { + x: int } +type nonrec t = private (('a Dict.t)[@res.doc {js|doc|js}]) \ No newline at end of file diff --git a/tests/syntax_tests/data/parsing/grammar/typedefinition/typeRepresentation.res b/tests/syntax_tests/data/parsing/grammar/typedefinition/typeRepresentation.res index f633bbb2d5..3b5af1576e 100644 --- a/tests/syntax_tests/data/parsing/grammar/typedefinition/typeRepresentation.res +++ b/tests/syntax_tests/data/parsing/grammar/typedefinition/typeRepresentation.res @@ -45,3 +45,45 @@ type t = private /**doc*/ {x: int} type t = /**doc*/ A | B type t = /**doc*/ | A | /**doc2*/ B type t = /**doc*/ @attr A | B + +// attributes and doc comments after `private`, and doc comments before a manifest +type t = private @attr {x: int} +type t = private @attr .. +type t = private @attr A | B +type t = private @attr int +type t = private @attr {"a": int} +type t = private /**doc*/ int +type t = private /**doc*/ M.t +type t = private /**doc*/ A | B +type t = private /**doc*/ @attr {x: int} +type t = /**doc*/ int +type t = /**doc*/ {"a": int} +type t = /**doc*/ (int, int) +type t = /**doc*/ [#A] +type t = /**doc*/ M.t +type t = /**doc*/ M.t = A | B +type t = /**doc*/ @attr int +type t = /**doc*/ {...base, y: int} +type t = /**doc*/ {...base} +type t = /**doc*/ @attr {...base, y: int} +type t = /**doc*/ {..} +type t = /**doc*/ {.} +type t = /**doc*/ {.. "a": int} +type t = @attr {...base, "y": int} +type t = @attr {...base} +type t = private /**doc*/ {...base, y: int} +type t = private /**doc*/ {...base} +type t = private /**doc*/ @attr {...base, y: int} +type t = private /**doc*/ {..} +type t = private /**doc*/ {.. "a": int} +type t = private @attr {...base, "y": int} +type t = private @attr {...base} +type t = /**doc*/ ...base | C +type t = /**doc*/ /**doc2*/ ...base | C +type t = /**doc*/ @attr ...base | C +type t = private /**doc*/ ...base | C +type t = private /**doc*/ @attr ...base | C +type t = /**doc*/ 'a +type t = /**doc*/ int => int +type t = /**doc*/ M.t = {x: int} +type t = private /**doc*/ Dict.t<'a> diff --git a/tests/syntax_tests/data/printer/typeDef/attributes.res b/tests/syntax_tests/data/printer/typeDef/attributes.res index 7c2662e0f3..0518e45f2a 100644 --- a/tests/syntax_tests/data/printer/typeDef/attributes.res +++ b/tests/syntax_tests/data/printer/typeDef/attributes.res @@ -26,3 +26,20 @@ type t = @attr /** doc */ @attr2 .. type t = /* c1 */ /** doc */ /* c2 */ {x: int} // trail type t = /** doc */ A | B type t = | /** doc */ A | /** doc2 */ B +type t = private @attr {x: int} +type t = private /** doc */ @attr .. +type t = private @attr int +type t = private /** doc */ int +type t = private /** doc */ A | B +type t = /** doc */ int +type t = /** doc */ {"a": int} +type t = /** doc */ (int, int) +type t = /** doc */ [#A] +type t = /** doc */ M.t = A | B +type t = /** doc */ {...base, y: int} +type t = /** doc */ {.. "a": int} +type t = private /** doc */ {...base, y: int} +type t = private /** doc */ {.} +type t = /** doc */ ...base | C +type t = private /** doc */ ...base | C +type t = /** doc */ 'a diff --git a/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt b/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt index 600f0dd242..d39dfe746a 100644 --- a/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt +++ b/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt @@ -46,3 +46,34 @@ type t = | A /** doc2 */ | B +@attr type t = private {x: int} +/** doc */ +@attr type t = private .. +type t = private @attr int +type t = private /** doc */ int +type t = + private + /** doc */ + | A + | B +type t = /** doc */ int +type t = /** doc */ {"a": int} +type t = /** doc */ (int, int) +type t = /** doc */ [#A] +type t = /** doc */ M.t = A | B +/** doc */ +type t = {...base, y: int} +type t = /** doc */ {.."a": int} +/** doc */ +type t = private {...base, y: int} +type t = private /** doc */ {.} +type t = + /** doc */ + | ...base + | C +type t = + private + /** doc */ + | ...base + | C +type t = /** doc */ 'a