chore(review): apply @art-w review

Co-authored-by: art-w <[email protected]>
Signed-off-by: Etienne Marais <[email protected]>

Author: maiste

doc/reference/dune/library_parameter.rst

@@ -6,8 +6,8 @@ library_parameter
    This feature is experimental and requires the compiler you are using to
    support parameterized libraries.
 
-The ``library_parameter`` describes a parameter. To be able to use the feature,
-you need to enable the stanza using the ``(using oxcaml 0.1)`` :doc:`extension
+The ``library_parameter`` stanza describes a parameter interface defined in a single ``.mli`` file. To enable this feature,
+you need to add ``(using oxcaml 0.1)`` :doc:`extension
 </reference/dune-project/using>` in your ``dune-project`` file.
 
 .. describe:: (library_parameter ...)
@@ -18,7 +18,7 @@ you need to enable the stanza using the ``(using oxcaml 0.1)`` :doc:`extension
 
   .. describe:: (name <parameter-name>)
 
-    ``parameter-name`` is the real of the library parameter. It must be a valid
+    ``parameter-name`` is the name of the library parameter. It must be a valid
     OCaml module name as for :doc:`/reference/dune/library`. 
 
     This must be specified if no `public_name` is specified.
@@ -44,10 +44,7 @@ you need to enable the stanza using the ``(using oxcaml 0.1)`` :doc:`extension
 
   .. describe:: (modules <modules>)
 
-    Specifies what modules are part of the library parameter. By default, Dune will use
-    all the ``.mli`` files in the same directory as the ``dune`` file. This
-    includes ones present in the file system as well as ones generated by user
-    rules. You can restrict this list by using a ``(modules <modules>)`` field.
+    Specifies a specific module to select as a `library_parameter`.
 
     ``<modules>`` uses the :doc:`/reference/ordered-set-language`, where
     elements are module names and don't need to start with an uppercase letter.
@@ -97,9 +94,9 @@ you need to enable the stanza using the ``(using oxcaml 0.1)`` :doc:`extension
 
   .. describe:: (enabled_if <blang expression>)
 
-    Conditionally disables a library.
+    Conditionally disables a library parameter.
 
-    A disabled library cannot be built and will not be installed.
+    A disabled library parameter cannot be built and will not be installed.
 
     The condition is specified using the :doc:`/reference/boolean-language`, and
     the field allows for the ``%{os_type}`` variable, which is expanded to the

src/dune_rules/gen_meta.ml

@@ -76,8 +76,7 @@ let gen_lib pub_name lib ~version =
   in
   let preds =
     match kind with
-    | Normal -> []
-    | Parameter -> [] (* failwith "TODO arthur" *)
+    | Normal | Parameter -> []
     | Ppx_rewriter _ | Ppx_deriver _ -> [ Pos "ppx_driver" ]
   in
   let name lib =
@@ -128,8 +127,7 @@ let gen_lib pub_name lib ~version =
          ; ppx_runtime_deps ppx_rt_deps
          ])
     ; (match kind with
-       | Normal -> []
-       | Parameter -> [] (* failwith "TODO arthur" *)
+       | Normal | Parameter -> []
        | Ppx_rewriter _ | Ppx_deriver _ ->
          (* Deprecated ppx method support *)
          let no_ppx_driver = Neg "ppx_driver"
@@ -141,8 +139,7 @@ let gen_lib pub_name lib ~version =
              ; requires ~preds:[ no_ppx_driver ] ppx_runtime_deps_for_deprecated_method
              ]
            ; (match kind with
-              | Normal -> assert false
-              | Parameter -> assert false (* failwith "TODO arthur" *)
+              | Normal | Parameter -> assert false
               | Ppx_rewriter _ ->
                 [ rule "ppx" [ no_ppx_driver; no_custom_ppx ] Set "./ppx.exe --as-ppx"
                 ; rule "library_kind" [] Set "ppx_rewriter"

src/dune_rules/modules_field_evaluator.ml

@@ -389,7 +389,7 @@ let eval
         if Module_trie.mem private_modules name then Visibility.Private else Public
       in
       let kind =
-        if kind = Parameter
+        if is_parameter
         then Module.Kind.Parameter
         else if Module_trie.mem virtual_modules name
         then Virtual

src/dune_rules/stanzas/parameter.ml

@@ -77,11 +77,10 @@ let decode =
        ; allow_overlapping_dependencies
        ; ctypes = None
        }
-     (* let+ buildable = Buildable.decode Parameter  *)
      and+ name = field_o "name" Lib_name.Local.decode_loc
      and+ public = field_o "public_name" (Public_lib.decode ~allow_deprecated_names:false)
      and+ package = field_o "package" (located Stanza_common.Pkg.decode)
-     and+ enabled_if = Enabled_if.decode ~allowed_vars:Any ~since:(Some (3, 20)) ()
+     and+ enabled_if = Enabled_if.decode ~allowed_vars:Any ~since:None ()
      and+ synopsis = field_o "synopsis" string
      and+ stdlib =
        field_o
@@ -97,11 +96,11 @@ let decode =
           | Error user_message ->
             User_error.raise
               ~loc
-              [ Pp.textf "Invalid library name."
+              [ Pp.textf "Invalid library_parameter name."
               ; Pp.text
-                  "Public library names don't have this restriction. You can either \
-                   change this public name to be a valid library name or add a \"name\" \
-                   field with a valid library name."
+                  "Public library_parameter names don't have this restriction. You can \
+                   either change this public name to be a valid library_parameter name \
+                   or add a \"name\" field with a valid library_parameter name."
               ]
               ~hints:(Lib_name.Local.valid_format_doc :: user_message.hints))
        | None, None ->

src/dune_rules/utop.ml

@@ -57,7 +57,10 @@ let add_stanza db ~dir (acc, pps) stanza =
        then (
          match Lib_info.kind info with
          | Normal -> Appendable_list.cons lib acc, pps
-         | Parameter -> failwith "TODO arthur"
+         | Parameter -> Appendable_list.cons lib acc, pps
+         (* CR @maiste or @art-w: the parametrized libraries in utop follows
+             the same schema as Normal library but it needs to be verified once
+             parametrized libraries are fully supported. *)
          | Lib_kind.Ppx_rewriter _ | Ppx_deriver _ ->
            ( Appendable_list.cons lib acc
            , Appendable_list.cons (Lib_info.loc info, Lib_info.name info) pps ))
@@ -96,7 +99,10 @@ let add_stanza db ~dir (acc, pps) stanza =
          let info = Lib.info lib in
          match Lib_info.kind info with
          | Normal -> Appendable_list.cons lib acc, pps
-         | Parameter -> failwith "TODO arthur"
+         | Parameter -> Appendable_list.cons lib acc, pps
+         (* CR @maiste or @art-w: the parametrized libraries in utop follows
+             the same schema as Normal library but it needs to be verified once
+             parametrized libraries are fully supported. *)
          | Ppx_rewriter _ | Ppx_deriver _ ->
            ( Appendable_list.cons lib acc
            , Appendable_list.cons (Lib_info.loc info, Lib_info.name info) pps )))

test/blackbox-tests/test-cases/oxcaml/library_parameter.t

@@ -1,4 +1,4 @@
-This test verifies `library_parameter` stanza works as expected.
+This test verifies that the `library_parameter` stanza works as expected.
 
   $ . ./helpers.sh
   $ cat > "dune-project" <<EOF
@@ -18,7 +18,7 @@ being lazy.
   >  (name param_intf))
   > EOF
 
-The test build should fails because of the oxcaml extension is not available.
+The test build should fails because the oxcaml extension is not available.
 
   $ dune build
   File "param/dune", lines 1-3, characters 0-64: