Skip to content

Commit d6388f3

Browse files
cknittclaude
andcommitted
Fix fixed-argument comments, integer spelling and validation
Keep comments around and inside fixed arguments, spell integer payloads as plain JavaScript integers, require %raw payloads to be exactly one literal, reject optional fixed arguments clearly, and read legacy @as payloads the same way in the parser, the frontend and the v0 bridge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP47qMfjMbjUTKZDsLthDo Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent bdedeec commit d6388f3

17 files changed

Lines changed: 239 additions & 185 deletions

‎compiler/frontend/ast_attributes.ml‎

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -169,41 +169,12 @@ type as_const_payload = Fixed of string
169169

170170
let as_const (attrs : t) =
171171
single_as attrs ~decode:(fun ~loc payload ->
172-
let is_fixed_value_payload =
173-
Ast_payload.is_single_int payload <> None
174-
|| Ast_payload.semantic_string_of_payload payload <> None
175-
||
176-
match Ast_payload.unwrap_payload_expression payload with
177-
| PStr
178-
[
179-
{
180-
pstr_desc =
181-
Pstr_eval
182-
( {
183-
pexp_desc =
184-
Pexp_tagged_template
185-
{
186-
tag =
187-
{pexp_desc = Pexp_ident {txt = Lident "json"}};
188-
raw_sources = [_];
189-
values = [];
190-
};
191-
},
192-
_ );
193-
};
194-
] ->
195-
true
196-
| _ -> false
197-
in
198-
if not is_fixed_value_payload then
199-
Bs_syntaxerr.err loc Expect_int_or_string_or_json_literal;
200172
match Ast_payload.fixed_source_of_payload payload with
201-
| Some source -> (
202-
match Classify_function.classify source with
203-
| Js_literal _ -> Fixed source
204-
| _ ->
205-
Location.raise_errorf ~loc
206-
"The fixed-value @as payload must be a JavaScript literal")
173+
| Some source when Classify_function.is_fixed_literal source ->
174+
Fixed source
175+
| Some _ ->
176+
Location.raise_errorf ~loc
177+
"The fixed-value @as payload must be a JavaScript literal"
207178
| None -> Bs_syntaxerr.err loc Expect_int_or_string_or_json_literal)
208179

209180
let locg = Location.none

‎compiler/frontend/ast_external_process.ml‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -392,12 +392,15 @@ type response = {
392392
}
393393

394394
let fixed_arg_type ({txt = source; loc} : Parsetree.fixed_value) =
395-
match Classify_function.classify source with
396-
| Js_literal _ ->
395+
if Classify_function.is_fixed_literal source then
397396
External_arg_spec.Arg_cst (External_arg_spec.cst_fixed source)
398-
| _ ->
397+
else
399398
Location.raise_errorf ~loc "The %%raw payload must be a JavaScript literal"
400399

400+
(* A fixed argument is always passed, so it has nothing to make optional. *)
401+
let reject_optional_fixed ({loc} : Parsetree.fixed_value) =
402+
Location.raise_errorf ~loc "A %%raw argument cannot be optional"
403+
401404
let process_obj (loc : Location.t) (st : external_desc) (prim_name : string)
402405
(arg_types_ty : Parsetree.arg list) (result_type : Ast_core_type.t) :
403406
int * Parsetree.core_type * External_ffi_types.t =
@@ -515,7 +518,7 @@ let process_obj (loc : Location.t) (st : external_desc) (prim_name : string)
515518
in
516519
let obj_arg_type =
517520
match fixed with
518-
| Some value -> fixed_arg_type value
521+
| Some value -> reject_optional_fixed value
519522
| None -> get_opt_arg_type ~nolabel:false ty
520523
in
521524
match obj_arg_type with
@@ -954,7 +957,7 @@ let handle_attributes (loc : Location.t) (type_annotation : Parsetree.core_type)
954957
| Optional {txt = s} -> (
955958
let arg_type =
956959
match fixed with
957-
| Some value -> fixed_arg_type value
960+
| Some value -> reject_optional_fixed value
958961
| None -> get_opt_arg_type ~nolabel:false ty
959962
in
960963
match arg_type with

‎compiler/ml/ast_payload.ml‎

Lines changed: 30 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -78,67 +78,51 @@ let semantic_string_of_payload (x : t) =
7878
let quote_js_string semantic =
7979
"\"" ^ String_literal.encode_js_string semantic ^ "\""
8080

81-
let rec fixed_source_of_expression (expression : Parsetree.expression) =
82-
let map_all f values =
83-
let rec loop acc = function
84-
| [] -> Some (List.rev acc)
85-
| value :: rest -> (
86-
match f value with
87-
| Some mapped -> loop (mapped :: acc) rest
88-
| None -> None)
89-
in
90-
loop [] values
81+
(* Spell a ReScript integer literal as a JavaScript one. Underscore separators
82+
need ES2021, and a decimal literal with a leading zero is a legacy octal
83+
literal, which strict-mode JavaScript rejects. *)
84+
let js_integer_source source =
85+
let source = String.concat "" (String.split_on_char '_' source) in
86+
let sign, digits =
87+
if source <> "" && source.[0] = '-' then
88+
("-", String.sub source 1 (String.length source - 1))
89+
else ("", source)
90+
in
91+
let length = String.length digits in
92+
let has_radix_prefix =
93+
length > 1
94+
&& digits.[0] = '0'
95+
&&
96+
match digits.[1] with
97+
| 'x' | 'X' | 'o' | 'O' | 'b' | 'B' -> true
98+
| _ -> false
9199
in
100+
if has_radix_prefix then sign ^ digits
101+
else
102+
let rec first_significant i =
103+
if i < length - 1 && digits.[i] = '0' then first_significant (i + 1)
104+
else i
105+
in
106+
let start = first_significant 0 in
107+
sign ^ String.sub digits start (length - start)
108+
109+
let fixed_source_of_expression (expression : Parsetree.expression) =
92110
match (unwrap_braces expression).pexp_desc with
93111
| Pexp_constant (Pconst_string payload) ->
94112
Some (quote_js_string (String_literal.string_semantic payload))
95113
| Pexp_template {source_segments = [{txt = source}]; values = []} -> (
96114
match String_literal.decode_js_template_escapes source with
97115
| Some semantic -> Some (quote_js_string semantic)
98116
| None -> None)
99-
| Pexp_constant (Pconst_integer (source, None)) -> Some source
100-
| Pexp_constant (Pconst_integer (source, Some 'n')) -> Some (source ^ "n")
101-
| Pexp_constant (Pconst_float (source, None)) -> Some source
117+
| Pexp_constant (Pconst_integer (source, None)) ->
118+
Some (js_integer_source source)
102119
| Pexp_tagged_template
103120
{
104121
tag = {pexp_desc = Pexp_ident {txt = Lident "json"}};
105122
raw_sources = [{txt = source}];
106123
values = [];
107124
} ->
108125
Some source
109-
| Pexp_construct ({txt = Lident "true"}, {txt = []}) -> Some "true"
110-
| Pexp_construct ({txt = Lident "false"}, {txt = []}) -> Some "false"
111-
| Pexp_ident {txt = Lident "null"} -> Some "null"
112-
| Pexp_ident {txt = Lident "undefined"} -> Some "undefined"
113-
| Pexp_array values -> (
114-
match map_all fixed_source_of_expression values with
115-
| Some values -> Some ("[" ^ String.concat "," values ^ "]")
116-
| None -> None)
117-
| Pexp_object_literal fields ->
118-
let field_source
119-
(({txt = name}, value) : string Location.loc * Parsetree.expression) =
120-
match fixed_source_of_expression value with
121-
| Some value -> Some (quote_js_string name ^ ":" ^ value)
122-
| None -> None
123-
in
124-
begin match map_all field_source fields with
125-
| Some fields -> Some ("{" ^ String.concat "," fields ^ "}")
126-
| None -> None
127-
end
128-
| Pexp_record (fields, None) ->
129-
let field_source
130-
({lid; x = value; opt} : Parsetree.expression Parsetree.record_element)
131-
=
132-
if opt then None
133-
else
134-
match (lid.txt, fixed_source_of_expression value) with
135-
| Lident name, Some value -> Some (quote_js_string name ^ ":" ^ value)
136-
| Ldot _, _ | _, None -> None
137-
in
138-
begin match map_all field_source fields with
139-
| Some fields -> Some ("{" ^ String.concat "," fields ^ "}")
140-
| None -> None
141-
end
142126
| _ -> None
143127

144128
let fixed_source_of_payload (payload : t) =

‎compiler/ml/ast_payload.mli‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,10 @@ val semantic_string_of_payload : t -> string option
5151
Other prefixed literals, such as [json], are not semantic strings. *)
5252

5353
val fixed_source_of_payload : t -> string option
54-
(** Convert a recursively static literal payload to validated JavaScript source.
55-
This does not accept calls, spreads, interpolation, or other executable
56-
expressions. *)
54+
(** The JavaScript source of a legacy fixed-value [@as] payload: an integer, a
55+
string, a non-interpolated backquoted string, or a non-interpolated [json]
56+
template. Callers still validate the [json] source with
57+
[Classify_function.is_fixed_literal]. *)
5758

5859
val is_single_int : t -> int option
5960

‎compiler/ml/classify_function.ml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,24 @@ let classify ?(check : (Location.t * int) option) (prog : string) :
104104
| Some _, [] | None, [] -> classify_exp prog
105105
| None, _ :: _ -> Js_exp_unknown
106106

107+
let is_fixed_literal (source : string) : bool =
108+
let open Parser_flow in
109+
let env = Parser_env.init_env None source in
110+
match
111+
do_parse env
112+
(fun env ->
113+
let expression = Parse.expression env in
114+
let token = Parser_env.Peek.token env in
115+
(* Eating the last token records the comments lexed before it,
116+
including those already attached to [expression]. *)
117+
Parser_env.Eat.token env;
118+
(expression, token, Parser_env.comments env))
119+
false
120+
with
121+
| (expression, Token.T_EOF, []), [] -> is_obj_literal expression
122+
| _ -> false
123+
| exception _ -> false
124+
107125
let classify_stmt (prog : string) : Js_raw_info.stmt =
108126
let result = Parser_flow.parse_program false None prog in
109127
match fst result with

‎compiler/ml/classify_function.mli‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,8 @@ val classify : ?check:Location.t * int -> string -> Js_raw_info.exp
2727
val classify_exp : (Loc.t, Loc.t) Flow_ast.Expression.t -> Js_raw_info.exp
2828

2929
val classify_stmt : string -> Js_raw_info.stmt
30+
31+
val is_fixed_literal : string -> bool
32+
(** Whether [source] is exactly one JavaScript literal. The generated code
33+
embeds the source verbatim, so trailing tokens, which [classify] ignores,
34+
and comments, which could swallow the code that follows, are rejected. *)

0 commit comments

Comments
 (0)