fix: use warning mechanism for duplicate deps warning

Signed-off-by: Sachin Beniwal <s474996633@gmail.com>

Author: benodiwal

src/dune_lang/package.ml

@@ -13,6 +13,14 @@ type original_opam_file =
   ; contents : string
   }
 
+module Duplicate_dep_warning = struct
+  type t =
+    { loc : Loc.t
+    ; dep_string : string
+    ; field_name : string
+    }
+end
+
 type t =
   { id : Id.t
   ; opam_file : Path.Source.t
@@ -31,6 +39,7 @@ type t =
   ; allow_empty : bool
   ; original_opam_file : original_opam_file option
   ; exclusive_dir : (Loc.t * Path.Source.t) option
+  ; duplicate_dep_warnings : Duplicate_dep_warning.t list
   }
 
 (* Package name are globally unique, so we can reasonably expect that there will
@@ -55,6 +64,7 @@ let original_opam_file t = t.original_opam_file
 let set_inside_opam_dir t ~dir = { t with opam_file = Name.file t.id.name ~dir }
 let set_version_and_info t ~version ~info = { t with version; info }
 let exclusive_dir t = t.exclusive_dir
+let duplicate_dep_warnings t = t.duplicate_dep_warnings
 
 let encode
       (name : Name.t)
@@ -75,6 +85,7 @@ let encode
       ; opam_file = _
       ; original_opam_file = _
       ; exclusive_dir
+      ; duplicate_dep_warnings = _
       }
   =
   let open Encoder in
@@ -121,6 +132,25 @@ let decode =
         ; Pp.textf "- %s" (print_value loc2)
         ]
   in
+  let collect_duplicate_deps deps field_name =
+    let warnings, _ =
+      List.fold_left deps ~init:([], []) ~f:(fun (warnings, seen) (loc, dep) ->
+        let is_duplicate =
+          List.exists seen ~f:(fun (_, seen_dep) ->
+            Package_name.equal
+              dep.Package_dependency.name
+              seen_dep.Package_dependency.name)
+        in
+        if is_duplicate
+        then (
+          let dep_sexp = Package_dependency.encode dep in
+          let dep_string = Dune_sexp.to_string dep_sexp in
+          let warning = { Duplicate_dep_warning.loc; dep_string; field_name } in
+          warning :: warnings, (loc, dep) :: seen)
+        else warnings, (loc, dep) :: seen)
+    in
+    warnings
+  in
   fun ~dir ->
     fields
     @@ let* version = Syntax.get_exn Stanza.syntax in
@@ -130,9 +160,12 @@ let decode =
        and+ description = field_o "description" string
        and+ version =
          field_o "version" (Syntax.since Stanza.syntax (2, 5) >>> Package_version.decode)
-       and+ depends = field ~default:[] "depends" (repeat Package_dependency.decode)
-       and+ conflicts = field ~default:[] "conflicts" (repeat Package_dependency.decode)
-       and+ depopts = field ~default:[] "depopts" (repeat Package_dependency.decode)
+       and+ depends_with_locs =
+         field ~default:[] "depends" (repeat (located Package_dependency.decode))
+       and+ conflicts_with_locs =
+         field ~default:[] "conflicts" (repeat (located Package_dependency.decode))
+       and+ depopts_with_locs =
+         field ~default:[] "depopts" (repeat (located Package_dependency.decode))
        and+ info = Package_info.decode ~since:(2, 0) ()
        and+ tags = field "tags" (enter (repeat string)) ~default:[]
        and+ exclusive_dir =
@@ -161,6 +194,16 @@ let decode =
        and+ allow_empty = field_b "allow_empty" ~check:(Syntax.since Stanza.syntax (3, 0))
        and+ lang_version = Syntax.get_exn Stanza.syntax in
        let allow_empty = lang_version < (3, 0) || allow_empty in
+       let duplicate_dep_warnings =
+         List.concat
+           [ collect_duplicate_deps depends_with_locs "depends"
+           ; collect_duplicate_deps conflicts_with_locs "conflicts"
+           ; collect_duplicate_deps depopts_with_locs "depopts"
+           ]
+       in
+       let depends = List.map ~f:snd depends_with_locs in
+       let conflicts = List.map ~f:snd conflicts_with_locs in
+       let depopts = List.map ~f:snd depopts_with_locs in
        let id = Id.create ~name ~dir in
        let opam_file = Name.file id.name ~dir:id.dir in
        { id
@@ -180,6 +223,7 @@ let decode =
        ; opam_file
        ; original_opam_file = None
        ; exclusive_dir
+       ; duplicate_dep_warnings
        }
 ;;
 
@@ -208,6 +252,7 @@ let to_dyn
       ; opam_file = _
       ; original_opam_file = _
       ; exclusive_dir = _
+      ; duplicate_dep_warnings = _
       }
   =
   let open Dyn in
@@ -272,5 +317,6 @@ let create
   ; original_opam_file
   ; exclusive_dir =
       Option.map contents_basename ~f:(fun (loc, s) -> loc, Path.Source.relative dir s)
+  ; duplicate_dep_warnings = []
   }
 ;;

src/dune_lang/package.mli

@@ -74,3 +74,13 @@ val create
   -> t
 
 val original_opam_file : t -> original_opam_file option
+
+module Duplicate_dep_warning : sig
+  type t =
+    { loc : Loc.t
+    ; dep_string : string
+    ; field_name : string
+    }
+end
+
+val duplicate_dep_warnings : t -> Duplicate_dep_warning.t list

src/dune_rules/gen_rules.ml

@@ -335,6 +335,11 @@ let missing_project_name =
     ~since:(3, 11)
 ;;
 
+(* Warn about duplicate dependencies in package definitions *)
+let duplicate_deps =
+  Warning.make ~default:(fun _ -> `Enabled) ~name:"duplicate_deps" ~since:(3, 18)
+;;
+
 (* To be called once per project, when we are generating the rules for the root
    directory of the project *)
 let gen_project_rules =
@@ -386,6 +391,28 @@ let gen_project_rules =
                        to your dune-project file to make sure that $ dune subst works in \
                        release or pinned builds"
                   ]))
+    and+ () =
+      (* Emit warnings for duplicate dependencies in packages *)
+      let packages = Dune_project.packages project in
+      Dune_lang.Package.Name.Map.values packages
+      |> Memo.parallel_iter ~f:(fun pkg ->
+        Dune_lang.Package.duplicate_dep_warnings pkg
+        |> Memo.parallel_iter
+             ~f:(fun (warning : Dune_lang.Package.Duplicate_dep_warning.t) ->
+               Warning_emit.emit
+                 duplicate_deps
+                 (Warning_emit.Context.project project)
+                 (fun () ->
+                    Memo.return
+                      (User_message.make
+                         ~loc:warning.loc
+                         [ Pp.textf
+                             "Duplicate dependency on package %s in '%s' field. If you \
+                              want to specify multiple constraints, combine them using \
+                              (and ...)."
+                             warning.dep_string
+                             warning.field_name
+                         ]))))
     in
     ()
   in