Make inline record fields that overlap with a variant's tag a compile error

[mediremi]

Currently, the following:

@tag("name")
type animal = Cat({name: string})

let cat: animal = Cat({name: "my cat"})

compiles to:

let cat = {
  name: "Cat",
  name: "my cat"
};

export {
  cat,
}

Since we don't treat the overlap in tag name and inline record field as a compiler error, this can lead to issues at runtime:

@tag("name")
type animal = Cat({name: string}) | Dog({age: int})

let cat: animal = Cat({name: "my cat"})

let classifyAnimal = (v: animal) => {
  switch v {
  | Cat(_) => "cat"
  | Dog(_) => "dog"
  }
}

classifyAnimal(cat)->Console.log // "dog"

---

In this PR, I've fixed this issue by making inline record fields that overlap with a variant's tag a compile error.

I've also added checks for duplicate @as tags on inline record fields.

[mediremi]

This function is based on process_tag_name, located above:

rescript/compiler/ml/ast_untagged_variants.ml

Lines 312 to 324 in c293649

	let process_tag_name (attrs : Parsetree.attributes) =
	let st = ref None in
	Ext_list.iter attrs (fun ({txt; loc}, payload) ->
	match txt with
	| "tag" ->
	if !st = None then (
	(match Ast_payload.is_single_string payload with
	| None -> ()
	| Some (s, _dec) -> st := Some s);
	if !st = None then raise (Error (loc, InvalidVariantTagAnnotation)))
	else raise (Error (loc, Duplicated_bs_as))
	| _ -> ());
	!st

[zth]

Nice! Looks good to me. But good if @cristianoc takes a look as well.

[pkg-pr-new]

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7875

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7875

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7875

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7875

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7875

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7875

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7875

commit: 08afd62

[tsnobip]

nice improvement! Thanks @mediremi!

[mediremi]

@cristianoc would you be able to have a quick check of this PR when you have time please? Just in case I've broken something without realising

[cristianoc]

Looks great!
Left a comment on reuse.
@zth any suggestions for the phrasing of the error to make it super clear?

[zth]

In terms of the message, the only thing I don't think is clear now and that could perhaps be tweaked is that the @tag names an actual expected field on the runtime object, which is why you can't have a conflicting inline record field with the same name.

[mediremi]

I replaced process_as_name with the existing helper process_tag_type in 5c5f2bd

I clarified the error message to mention both the field name in the type and its runtime value in 40d5e14