Added dependency parsing with better errors and cleaner interface

Signed-off-by: Ambre Austen Suhamy <[email protected]>

Author: ElectreAAS

src/dune_lang/package_dependency.ml

@@ -5,6 +5,34 @@ type t =
   ; constraint_ : Package_constraint.t option
   }
 
+module Well_formed_name = struct
+  module T = struct
+    include Package_name.Opam_compatible
+
+    let module_ = "Package.Dependency.Name"
+    let description = "package dependency"
+    let description_of_valid_string = Some description_of_valid_string
+
+    let better_hint s =
+      match String.lsplit2 s ~on:'.' with
+      | None -> make_valid s
+      | Some (before, after) ->
+        if Option.is_some (of_string_opt before) && not (String.is_empty after)
+        then sprintf "(%s (= %s))" before after
+        else make_valid s
+    ;;
+
+    let hint_valid = Some better_hint
+  end
+
+  module TT = Dune_util.Stringlike.Make (T)
+
+  let decode =
+    let open Dune_sexp.Decoder in
+    TT.decode >>| T.to_package_name
+  ;;
+end
+
 let encode { name; constraint_ } =
   let open Dune_sexp.Encoder in
   match constraint_ with
@@ -20,7 +48,7 @@ let decode =
     { name; constraint_ = Some expr }
   in
   enter constrained
-  <|> let+ name = Package_name.decode in
+  <|> let+ name = Well_formed_name.decode in
       { name; constraint_ = None }
 ;;
 

src/dune_lang/package_name.ml

@@ -49,24 +49,11 @@ module Opam_compatible = struct
 
     let of_string_opt s = Option.some_if (is_valid_string s) s
 
-    let escape s =
+    let make_valid s =
       let replaced = String.map s ~f:(fun c -> if is_valid_char c then c else '_') in
       if is_valid_string replaced then replaced else "p" ^ replaced
     ;;
 
-    let make_valid s =
-      match index_opt s '.' with
-      | None -> escape s
-      | Some i ->
-        let before = sub s ~pos:0 ~len:i in
-        let s_len = length s in
-        if is_valid_string before && s_len > i + 1
-        then (
-          let after = sub s ~pos:(i + 1) ~len:(s_len - i - 1) in
-          Printf.sprintf "(%s (= %s))" before after)
-        else escape s
-    ;;
-
     let hint_valid = Some make_valid
   end
 

src/dune_rules/package.ml

@@ -421,19 +421,6 @@ let decode_name ~version =
   if version >= (3, 11) then Name.decode_opam_compatible else Name.decode
 ;;
 
-let decode_dep ~version =
-  let open Dune_lang.Decoder in
-  let open Dune_lang.Package_dependency in
-  let constrained =
-    let+ name = decode_name ~version
-    and+ expr = Dune_lang.Package_constraint.decode in
-    { name; constraint_ = Some expr }
-  in
-  enter constrained
-  <|> let+ name = decode_name ~version in
-      { name; constraint_ = None }
-;;
-
 let decode =
   let open Dune_lang.Decoder in
   let name_map syntax of_list_map to_string name decode print_value error_msg =
@@ -458,9 +445,9 @@ let decode =
          field_o
            "version"
            (Dune_lang.Syntax.since Stanza.syntax (2, 5) >>> Package_version.decode)
-       and+ depends = field ~default:[] "depends" (repeat (decode_dep ~version))
-       and+ conflicts = field ~default:[] "conflicts" (repeat (decode_dep ~version))
-       and+ depopts = field ~default:[] "depopts" (repeat (decode_dep ~version))
+       and+ depends = field ~default:[] "depends" (repeat Dependency.decode)
+       and+ conflicts = field ~default:[] "conflicts" (repeat Dependency.decode)
+       and+ depopts = field ~default:[] "depopts" (repeat Dependency.decode)
        and+ info = Info.decode ~since:(2, 0) ()
        and+ tags = field "tags" (enter (repeat string)) ~default:[]
        and+ deprecated_package_names =

test/blackbox-tests/test-cases/invalid-package-version.t

@@ -16,10 +16,10 @@ Building doesn't work as the dependency is invalid.
   File "dune-project", line 5, characters 10-19:
   5 |  (depends foo.1.2.3))
                 ^^^^^^^^^
-  Error: "foo.1.2.3" is an invalid opam package name.
+  Error: "foo.1.2.3" is an invalid package dependency.
   Package names can contain letters, numbers, '-', '_' and '+', and need to
   contain at least a letter.
-  Hint: (foo (= 1.2.3)) would be a correct opam package name
+  Hint: (foo (= 1.2.3)) would be a correct package dependency
   [1]
 
 We can take this further and add some really invalid characters into the name:
@@ -37,8 +37,8 @@ We can take this further and add some really invalid characters into the name:
   File "dune-project", line 5, characters 10-15:
   5 |  (depends f{oo."1.2.3"))
                 ^^^^^
-  Error: "f{oo." is an invalid opam package name.
+  Error: "f{oo." is an invalid package dependency.
   Package names can contain letters, numbers, '-', '_' and '+', and need to
   contain at least a letter.
-  Hint: f_oo_ would be a correct opam package name
+  Hint: f_oo_ would be a correct package dependency
   [1]

test/blackbox-tests/test-cases/pkg/invalid-version.t

@@ -25,8 +25,8 @@ In this case we could also hint at the correct syntax for dune-project files.
   File "dune-project", line 4, characters 10-19:
   4 |  (depends foo.1.2.3))
                 ^^^^^^^^^
-  Error: "foo.1.2.3" is an invalid opam package name.
+  Error: "foo.1.2.3" is an invalid package dependency.
   Package names can contain letters, numbers, '-', '_' and '+', and need to
   contain at least a letter.
-  Hint: (foo (= 1.2.3)) would be a correct opam package name
+  Hint: (foo (= 1.2.3)) would be a correct package dependency
   [1]