Fixes from self-review

Signed-off-by: Shon Feder <[email protected]>

Author: shonfeder

src/dune_lang/lib_kind.ml

@@ -99,13 +99,14 @@ module Dune_file = struct
     | x -> x
   ;;
 
-let to_dyn x =
-  let open Dyn in
-  match x with
-  | Normal -> variant "Normal" []
-  | Parameter -> variant "Parameter" []
-  | Ppx_deriver args -> variant "Ppx_deriver" [ Ppx_args.to_dyn args ]
-  | Ppx_rewriter args -> variant "Ppx_rewriter" [ Ppx_args.to_dyn args ]
+  let to_dyn x =
+    let open Dyn in
+    match x with
+    | Normal -> variant "Normal" []
+    | Parameter -> variant "Parameter" []
+    | Ppx_deriver args -> variant "Ppx_deriver" [ Ppx_args.to_dyn args ]
+    | Ppx_rewriter args -> variant "Ppx_rewriter" [ Ppx_args.to_dyn args ]
+  ;;
 end
 
 type t =
@@ -117,24 +118,14 @@ let equal = Poly.equal
 let to_dyn x =
   let open Dyn in
   match x with
-  | Dune_file t -> variant "Dune_file" [Dune_file.to_dyn t]
+  | Dune_file t -> variant "Dune_file" [ Dune_file.to_dyn t ]
   | Virtual -> variant "Virtual" []
 ;;
 
 let decode =
   let open Decoder in
   (* TODO: Less code reuse with either? *)
-  sum
-    [ "normal", return (Dune_file Normal)
-    ; "parameter", return (Dune_file Parameter)
-    ; ( "ppx_deriver"
-      , let+ args = Ppx_args.decode in
-        Dune_file (Ppx_deriver args) )
-    ; ( "ppx_rewriter"
-      , let+ args = Ppx_args.decode in
-        Dune_file (Ppx_rewriter args) )
-    ; "virtual", return Virtual
-    ]
+  map ~f:(fun k -> Dune_file k) Dune_file.decode <|> enum [ "virtual", Virtual ]
 ;;
 
 let encode t =

src/dune_lang/lib_kind.mli

@@ -34,6 +34,11 @@ end
 (** Internal representation the of the possible kinds of libraries *)
 type t =
   | Dune_file of Dune_file.t
+  (** A kind which is represented explicitly in the [kind] field of a dune
+      library stanza.
+
+      The remaining variants are derived from other fields in the library
+      stanza. *)
   | Virtual
 
 val to_dyn : t Dyn.builder

src/dune_rules/dune_package.ml

@@ -198,11 +198,11 @@ module Lib = struct
            (* Backward compatible support for dune-project files
               that include the [(virtual)] field. *)
            Lib_kind.Virtual
-         | _incompatible_kind when virtual_ ->
+         | incompatible_kind when virtual_ ->
            Code_error.raise
              "invalid combination of 'kind' and 'virtual' fields in library stanza of \
               dune-package file"
-             [ "kind", Lib_kind.to_dyn kind; "virtual", Dyn.Bool virtual_ ]
+             [ "kind", Lib_kind.to_dyn incompatible_kind; "virtual", Dyn.Bool virtual_ ]
          | otherwise -> otherwise
        and+ archives = mode_paths "archives"
        and+ plugins = mode_paths "plugins"

src/dune_rules/dune_package.mli

@@ -1,6 +1,6 @@
 (** Representation of dune-package files.
 
-    dune-package files represent package data for the purposes of installation.
+    dune-package files record package data for the purposes of installation.
     They are not intended to be written or read by human users. *)
 
 open Import

src/dune_rules/lib_info.ml

@@ -368,7 +368,7 @@ let kind t = t.kind
 let default_implementation t = t.default_implementation
 let obj_dir t = t.obj_dir
 let virtual_ t = t.kind = Virtual
-let is_parameter t = t.kind = (Dune_file Parameter)
+let is_parameter t = t.kind = Dune_file Parameter
 let implements t = t.implements
 let synopsis t = t.synopsis
 let wrapped t = t.wrapped

src/dune_rules/ml_sources.ml

@@ -365,7 +365,7 @@ let make_lib_modules
       in
       let kind : Modules_field_evaluator.kind =
         match lib.virtual_modules with
-        | None -> if lib.kind = (Dune_file Parameter) then Parameter else Exe_or_normal_lib
+        | None -> if Library.is_parameter lib then Parameter else Exe_or_normal_lib
         | Some virtual_modules -> Virtual { virtual_modules }
       in
       Memo.return (Resolve.return (kind, main_module_name, wrapped))

src/dune_rules/stanzas/library.ml

@@ -135,7 +135,7 @@ let decode =
            (match dune_file_kind with
             | Normal ->
               (* Libraries are virtual just in case [virtual_modules] are specified
-                 and they do not have an *non-normal* kind specified. *)
+                 and they do not have a *non-normal* kind specified. *)
               Virtual
             | (Parameter | Ppx_deriver _ | Ppx_rewriter _) as incompatable_kind ->
               User_error.raise