fix: decoding dune-package crashes with (include_subdirs qualified) (#…

…10269)

* test: add repro for #10264

This is only one part of the issue; the other part is more difficult to
test because it is related to `Modules.t` round-tripping correctly.

Signed-off-by: Etienne Millon <[email protected]>

* fix: make `Module.Kind.t` round-trip correctly

Signed-off-by: Etienne Millon <[email protected]>

* fix: make Modules.t round-trip

Fixes #10264

This part is harder to test because we can't easily load `dune-package`
files in tests, nor build `Modules.t` values.

Signed-off-by: Etienne Millon <[email protected]>

* changelog

Signed-off-by: Etienne Millon <[email protected]>

---------

Signed-off-by: Etienne Millon <[email protected]>

doc/changes/10269.md

@@ -0,0 +1,2 @@
+- fix crash when decoding dune-package for libraries with `(include_subdirs
+  qualified)` (#10269, fixes #10264, @emillon)

src/dune_rules/module.ml

@@ -92,7 +92,7 @@ module Kind = struct
           match next with
           | None -> return (Alias [])
           | Some _ ->
-            let+ path = Module_name.Path.decode in
+            let+ path = enter Module_name.Path.decode in
             Alias path )
       ]
   ;;

src/dune_rules/module.mli

@@ -21,6 +21,8 @@ module Kind : sig
     | Root
 
   include Dune_lang.Conv.S with type t := t
+
+  val to_dyn : t -> Dyn.t
 end
 
 module Source : sig

src/dune_rules/modules.ml

@@ -398,7 +398,8 @@ module Group = struct
     Module_name.Map.to_list_map modules ~f:(fun _ t ->
       Dune_lang.List
         (match t with
-         | Group g -> Dune_lang.atom "group" :: encode ~src_dir g
+         | Group g ->
+           Dune_lang.atom "group" :: Module_name.encode g.name :: encode ~src_dir g
          | Module m -> Dune_lang.atom "module" :: Module.encode ~src_dir m))
   ;;
 

test/expect-tests/module_tests.ml

@@ -0,0 +1,31 @@
+open Stdune
+module Kind = Dune_rules.Module.Kind
+
+(* See #10264 *)
+let%expect_test "Module.Kind encoding round trip" =
+  let module_name s = Dune_rules.Module_name.of_string s in
+  let test k =
+    let ast = Kind.encode k in
+    let sexp = Dune_sexp.Ast.add_loc ~loc:Loc.none ast in
+    let decoded =
+      match Dune_lang.Decoder.parse Kind.decode Univ_map.empty sexp with
+      | r -> Ok r
+      | exception e -> Error e
+    in
+    let dyn =
+      Dyn.record
+        [ "ast", Dyn.string (Dune_sexp.to_string ast)
+        ; "decoded", Or_exn.to_dyn Kind.to_dyn decoded
+        ]
+    in
+    Dune_tests_common.print_dyn dyn
+  in
+  test Impl;
+  [%expect {| { ast = "impl"; decoded = Ok Impl } |}];
+  test (Alias []);
+  [%expect {| { ast = "alias"; decoded = Ok Alias [] } |}];
+  test (Alias [ module_name "A" ]);
+  [%expect {| { ast = "(alias (A))"; decoded = Ok Alias [ "A" ] } |}];
+  test (Alias [ module_name "A"; module_name "B" ]);
+  [%expect {| { ast = "(alias (A B))"; decoded = Ok Alias [ "A"; "B" ] } |}]
+;;