Reject coercions and variant spreads that treat bool and unit as variants

changes/bool-unit-coercion-errors.polish.md

@@ -0,0 +1 @@
+- Coercion errors no longer describe `bool` and `unit` as variants that could be given `@as` or `@unboxed`.

changes/reject-bool-unit-variant-coercion.fix.md

@@ -0,0 +1,2 @@
+- Reject coercing `unit` to `string`, as in `(() :> string)`, and a closed polymorphic variant to `bool` or `unit`, as from `[#"true" | #"false"]` to `bool`. These compiled, but left `undefined` typed as a `string`, or a string typed as a `bool` or `unit`, and type errors suggested them as fixes.
+- Reject spreading `bool` or `unit` into a variant, as in `type t = | ...bool | Other`. The variant held `true`, `false` and `undefined`, but its switches and coercions took them for the strings `"true"`, `"false"` and `"()"`.

compiler/ml/ctype.ml

@@ -3698,6 +3698,12 @@ let rec subtype_rec env trace t1 t2 cstrs =
         match
           (extract_concrete_typedecl env t1, extract_concrete_typedecl env t2)
         with
+        | (_, p1, _), (_, p2, _)
+          when Variant_coercion.is_bool_or_unit p1
+               || Variant_coercion.is_bool_or_unit p2 ->
+          (* No other type coerces to or from [bool] or [unit], and the
+             variant errors below would describe them as variants *)
+          (trace, t1, t2, !(current_univar_pairs ()), None) :: cstrs
         | ( ( p1,
               _,
               {
@@ -3918,7 +3924,7 @@ let rec subtype_rec env trace t1 t2 cstrs =
           (trace, t1, t2, !(current_univar_pairs ()), None) :: cstrs)
       | Tvariant {row_closed = true; row_fields}, Tconstr (_, [], _)
         when extract_concrete_typedecl_opt env t2
-             |> Variant_coercion.type_is_variant -> (
+             |> Variant_coercion.can_try_coerce_polyvariant_to_variant -> (
         (* TODO(subtype-errors) Polyvariant to variant *)
         match extract_concrete_typedecl env t2 with
         | ( _,

compiler/ml/variant_coercion.ml

@@ -19,6 +19,12 @@ type variant_runtime_representation_issue =
       expected_typename: Path.t;
     }
 
+(* [bool] and [unit] are declared as variants, but their values are [true],
+   [false] and [undefined] at runtime, not what the constructors of a declared
+   variant compile to. No variant coercion or spread applies to them. *)
+let is_bool_or_unit (path : Path.t) =
+  Path.same path Predef.path_bool || Path.same path Predef.path_unit
+
 (* Right now we only allow coercing to primitives string/int/float *)
 let can_coerce_primitive (path : Path.t) =
   Path.same path Predef.path_string
@@ -122,9 +128,7 @@ let can_try_coerce_variant_to_primitive
    type_params = [];
    type_representation;
   }
-    when not (Path.same p Predef.path_bool) ->
-    (* bool is represented as a variant internally, so we need to account for that *)
-    (* TODO(subtype-errors) Report about bool? *)
+    when not (is_bool_or_unit p) ->
     let layout = Variant_runtime.get_layout layout_ref in
     Some
       ( p,
@@ -268,7 +272,8 @@ let can_coerce_polyvariant_to_variant ~row_fields ~variant_constructors ~unboxed
   then Ok ()
   else Error `Unknown
 
-let type_is_variant (typ : (Path.t * Path.t * Types.type_declaration) option) =
+let can_try_coerce_polyvariant_to_variant
+    (typ : (Path.t * Path.t * Types.type_declaration) option) =
   match typ with
-  | Some (_, _, {type_kind = Type_variant _; _}) -> true
+  | Some (_, p, {type_kind = Type_variant _}) -> not (is_bool_or_unit p)
   | _ -> false

compiler/ml/variant_type_spread.ml

@@ -36,14 +36,18 @@ let map_constructors ~(sdecl : Parsetree.type_declaration) ~all_constructors env
    pcd_args = Pcstr_tuple [{ptyp_loc; ptyp_desc = Ptyp_constr (loc, _)}];
   } -> (
     (* This is a variant type spread constructor. Look up its type *)
-    let _, type_decl =
+    let path, type_decl =
       try Typetexp.find_type env ptyp_loc loc.txt
       with _ -> raise (VariantTypeSpreadError (loc.loc, CouldNotFindType))
     in
 
     match type_decl with
     | {type_kind = Type_variant ([], _)} ->
       raise (VariantTypeSpreadError (loc.loc, InvalidType))
+    | {type_kind = Type_variant _} when Variant_coercion.is_bool_or_unit path ->
+      (* Their constructors would build [true], [false] and [undefined], but
+         would be matched and coerced as the strings of their names *)
+      raise (VariantTypeSpreadError (loc.loc, InvalidType))
     | {type_kind = Type_variant (cstrs, _); type_attributes; type_params} ->
       if List.length type_params > 0 then
         raise (VariantTypeSpreadError (loc.loc, HasTypeParams));

tests/build_tests/super_errors/expected/coercion_hint_unit_to_string.res.expected

@@ -0,0 +1,10 @@
+
+  We've found a bug for you!
+  /.../fixtures/coercion_hint_unit_to_string.res:2:17-19
+
+  1 │ let u = (): unit => ()
+  2 │ let s: string = u()
+  3 │ 
+
+  This has type: unit
+  But it's expected to have type: string

tests/build_tests/super_errors/expected/coercion_polyvariant_to_bool.res.expected

@@ -0,0 +1,9 @@
+
+  We've found a bug for you!
+  /.../fixtures/coercion_polyvariant_to_bool.res:2:16-24
+
+  1 │ let p: [#"true"] = #"true"
+  2 │ let b: bool = (p :> bool)
+  3 │ 
+
+  Type [#"true"] is not a subtype of bool

tests/build_tests/super_errors/expected/coercion_polyvariant_to_unit.res.expected

@@ -0,0 +1,9 @@
+
+  We've found a bug for you!
+  /.../fixtures/coercion_polyvariant_to_unit.res:2:16-24
+
+  1 │ let p: [#"()"] = #"()"
+  2 │ let u: unit = (p :> unit)
+  3 │ 
+
+  Type [#"()"] is not a subtype of unit

tests/build_tests/super_errors/expected/coercion_string_to_unit.res.expected

@@ -0,0 +1,8 @@
+
+  We've found a bug for you!
+  /.../fixtures/coercion_string_to_unit.res:1:16-26
+
+  1 │ let u: unit = ("a" :> unit)
+  2 │ 
+
+  Type string is not a subtype of unit

tests/build_tests/super_errors/expected/coercion_unit_to_int.res.expected

@@ -0,0 +1,8 @@
+
+  We've found a bug for you!
+  /.../fixtures/coercion_unit_to_int.res:1:15-23
+
+  1 │ let i: int = (() :> int)
+  2 │ 
+
+  Type unit is not a subtype of int

tests/build_tests/super_errors/expected/coercion_unit_to_string.res.expected

@@ -0,0 +1,8 @@
+
+  We've found a bug for you!
+  /.../fixtures/coercion_unit_to_string.res:1:18-29
+
+  1 │ let s: string = (() :> string)
+  2 │ 
+
+  Type unit is not a subtype of string

tests/build_tests/super_errors/expected/coercion_unit_to_unknown.res.expected

@@ -0,0 +1,8 @@
+
+  We've found a bug for you!
+  /.../fixtures/coercion_unit_to_unknown.res:1:19-31
+
+  1 │ let u: unknown = (() :> unknown)
+  2 │ 
+
+  Type unit is not a subtype of unknown

tests/build_tests/super_errors/expected/variant_spread_bool.res.expected

@@ -0,0 +1,10 @@
+
+  We've found a bug for you!
+  /.../fixtures/variant_spread_bool.res:2:8-11
+
+  1 │ type t =
+  2 │   | ...bool
+  3 │   | Other
+  4 │ 
+
+  This type is not a valid type to spread. It's only possible to spread other variants.

tests/build_tests/super_errors/expected/variant_spread_unit.res.expected

@@ -0,0 +1,10 @@
+
+  We've found a bug for you!
+  /.../fixtures/variant_spread_unit.res:2:8-11
+
+  1 │ type t =
+  2 │   | ...unit
+  3 │   | Other
+  4 │ 
+
+  This type is not a valid type to spread. It's only possible to spread other variants.

tests/build_tests/super_errors/fixtures/coercion_hint_unit_to_string.res

@@ -0,0 +1,2 @@
+let u = (): unit => ()
+let s: string = u()

tests/build_tests/super_errors/fixtures/coercion_polyvariant_to_bool.res

@@ -0,0 +1,2 @@
+let p: [#"true"] = #"true"
+let b: bool = (p :> bool)

tests/build_tests/super_errors/fixtures/coercion_polyvariant_to_unit.res

@@ -0,0 +1,2 @@
+let p: [#"()"] = #"()"
+let u: unit = (p :> unit)

tests/build_tests/super_errors/fixtures/coercion_string_to_unit.res

@@ -0,0 +1 @@
+let u: unit = ("a" :> unit)

tests/build_tests/super_errors/fixtures/coercion_unit_to_int.res

@@ -0,0 +1 @@
+let i: int = (() :> int)

tests/build_tests/super_errors/fixtures/coercion_unit_to_string.res

@@ -0,0 +1 @@
+let s: string = (() :> string)

tests/build_tests/super_errors/fixtures/coercion_unit_to_unknown.res

@@ -0,0 +1 @@
+let u: unknown = (() :> unknown)

tests/build_tests/super_errors/fixtures/variant_spread_bool.res

@@ -0,0 +1,3 @@
+type t =
+  | ...bool
+  | Other

tests/build_tests/super_errors/fixtures/variant_spread_unit.res

@@ -0,0 +1,3 @@
+type t =
+  | ...unit
+  | Other

tests/tests/src/VariantCoercion.mjs

@@ -104,6 +104,12 @@ let CoerceVariantBinaryOp = {
   z: z
 };
 
+let CoerceBoolLikeVariant = {
+  t: "True",
+  s: "True",
+  fromPoly: "True"
+};
+
 let PrivateColor = {
   red: "Red"
 };
@@ -145,6 +151,7 @@ export {
   CoerceFromBigintToVariant,
   CoerceFromPolyvariantToVariant,
   CoerceVariantBinaryOp,
+  CoerceBoolLikeVariant,
   PrivateColor,
   colorToPlain,
   colorToString,

tests/tests/src/VariantCoercion.res

@@ -153,6 +153,18 @@ module CoerceVariantBinaryOp = {
   let z = (X :> float) +. (Y :> float) +. 1.5
 }
 
+// [bool] and [unit] coerce to no primitive, since they are not variants at
+// runtime, but a declared variant whose constructors are named like them does
+module CoerceBoolLikeVariant = {
+  type t = False | True
+
+  let t: t = True
+  let s = (t :> string)
+
+  type polyTrue = [#True]
+  let fromPoly = ((#True: polyTrue) :> t)
+}
+
 // A private variant coerces out of itself, but nothing coerces into it from
 // another type
 module PrivateColor: {