Merge pull request #2660 from Julow/warn_unused_prepro_deps_later

Warn about unused preprocessor_deps

src/dune/cinaps.ml

@@ -7,7 +7,7 @@ type t =
   ; files : Predicate_lang.t
   ; libraries : Lib_dep.t list
   ; preprocess : Dune_file.Preprocess_map.t
-  ; preprocessor_deps : Dune_file.Dep_conf.t list
+  ; preprocessor_deps : Loc.t * Dune_file.Dep_conf.t list
   ; flags : Ocaml_flags.Spec.t
   }
 
@@ -26,7 +26,10 @@ let decode =
        field "preprocess" Dune_file.Preprocess_map.decode
          ~default:Dune_file.Preprocess_map.default
      and+ preprocessor_deps =
-       field "preprocessor_deps" (repeat Dune_file.Dep_conf.decode) ~default:[]
+       located
+         (field "preprocessor_deps"
+            (repeat Dune_file.Dep_conf.decode)
+            ~default:[])
      and+ libraries =
        field "libraries"
          (Dune_file.Lib_deps.decode ~allow_re_export:false)
@@ -79,9 +82,7 @@ let gen_rules sctx t ~dir ~scope =
   let preprocess =
     Preprocessing.make sctx ~dir ~expander ~dep_kind:Required
       ~lint:Dune_file.Preprocess_map.no_preprocessing ~preprocess:t.preprocess
-      ~preprocessor_deps:
-        (Super_context.Deps.interpret sctx ~expander t.preprocessor_deps)
-      ~lib_name:None ~scope
+      ~preprocessor_deps:t.preprocessor_deps ~lib_name:None ~scope
   in
   let modules =
     Modules.exe_unwrapped modules

src/dune/dune_file.ml

@@ -537,7 +537,7 @@ module Buildable = struct
     ; c_names : Ordered_set_lang.t option
     ; cxx_names : Ordered_set_lang.t option
     ; preprocess : Preprocess_map.t
-    ; preprocessor_deps : Dep_conf.t list
+    ; preprocessor_deps : Loc.t * Dep_conf.t list
     ; lint : Preprocess_map.t
     ; flags : Ocaml_flags.Spec.t
     ; js_of_ocaml : Js_of_ocaml.t
@@ -554,7 +554,7 @@ module Buildable = struct
     and+ preprocess =
       field "preprocess" Preprocess_map.decode ~default:Preprocess_map.default
     and+ preprocessor_deps =
-      field "preprocessor_deps" (repeat Dep_conf.decode) ~default:[]
+      located (field "preprocessor_deps" (repeat Dep_conf.decode) ~default:[])
     and+ lint = field "lint" Lint.decode ~default:Lint.default
     and+ c_flags = Dune_env.Stanza.c_flags ~since:since_c
     and+ c_names = field_o "c_names" (check_c Ordered_set_lang.decode)

src/dune/dune_file.mli

@@ -116,7 +116,7 @@ module Buildable : sig
     ; c_names : Ordered_set_lang.t option
     ; cxx_names : Ordered_set_lang.t option
     ; preprocess : Preprocess_map.t
-    ; preprocessor_deps : Dep_conf.t list
+    ; preprocessor_deps : Loc.t * Dep_conf.t list
     ; lint : Lint.t
     ; flags : Ocaml_flags.Spec.t
     ; js_of_ocaml : Js_of_ocaml.t

src/dune/exe_rules.ml

@@ -14,12 +14,10 @@ let executables_rules ~sctx ~dir ~expander ~dir_contents ~scope ~compile_info
   let modules =
     Dir_contents.modules_of_executables dir_contents ~first_exe ~obj_dir
   in
-  let preprocessor_deps =
-    SC.Deps.interpret sctx exes.buildable.preprocessor_deps ~expander
-  in
   let pp =
     Preprocessing.make sctx ~dir ~dep_kind:Required ~scope ~expander
-      ~preprocess:exes.buildable.preprocess ~preprocessor_deps
+      ~preprocess:exes.buildable.preprocess
+      ~preprocessor_deps:exes.buildable.preprocessor_deps
       ~lint:exes.buildable.lint ~lib_name:None
   in
   let modules =

src/dune/lib_rules.ml

@@ -278,9 +278,7 @@ let cctx (lib : Library.t) ~sctx ~source_modules ~dir ~expander ~scope
   let pp =
     Preprocessing.make sctx ~dir ~dep_kind ~scope
       ~preprocess:lib.buildable.preprocess ~expander
-      ~preprocessor_deps:
-        (Super_context.Deps.interpret sctx ~expander
-           lib.buildable.preprocessor_deps)
+      ~preprocessor_deps:lib.buildable.preprocessor_deps
       ~lint:lib.buildable.lint
       ~lib_name:(Some (snd lib.name))
   in

src/dune/preprocessing.ml

@@ -566,20 +566,36 @@ type t = (Module.t -> lint:bool -> Module.t) Per_module.t
 
 let dummy = Per_module.for_all (fun m ~lint:_ -> m)
 
+let is_preprocessing t =
+  Per_module.fold t ~init:true ~f:(fun p acc ->
+      match p with
+      | Preprocess.Without_future_syntax.No_preprocessing -> false
+      | _ -> acc)
+
 let make sctx ~dir ~expander ~dep_kind ~lint ~preprocess ~preprocessor_deps
     ~lib_name ~scope =
+  let preprocess =
+    Per_module.map preprocess ~f:(fun pp ->
+        Dune_file.Preprocess.remove_future_syntax ~for_:Compiler pp
+          (Super_context.context sctx).version)
+  in
   let preprocessor_deps =
-    Build.memoize "preprocessor deps" preprocessor_deps
+    let loc, deps = preprocessor_deps in
+    ( if not (is_preprocessing preprocess) && not (List.is_empty deps) then
+      let is_error = Dune_project.dune_version (Scope.project scope) >= (2, 0) in
+      User_warning.emit ~loc ~is_error
+        [ Pp.text
+            "This preprocessor_deps field will be ignored because no \
+             preprocessor is configured."
+        ] );
+    SC.Deps.interpret sctx deps ~expander |> Build.memoize "preprocessor deps"
   in
   let lint_module =
     Staged.unstage
       (lint_module sctx ~dir ~expander ~dep_kind ~lint ~lib_name ~scope)
   in
   Per_module.map preprocess ~f:(fun pp ->
-      match
-        Dune_file.Preprocess.remove_future_syntax ~for_:Compiler pp
-          (Super_context.context sctx).version
-      with
+      match pp with
       | No_preprocessing ->
         fun m ~lint ->
           let ast = setup_dialect_rules sctx ~dir ~dep_kind ~expander m in

src/dune/preprocessing.mli

@@ -15,7 +15,7 @@ val make :
   -> dep_kind:Lib_deps_info.Kind.t
   -> lint:Dune_file.Preprocess_map.t
   -> preprocess:Dune_file.Preprocess_map.t
-  -> preprocessor_deps:unit Build.t
+  -> preprocessor_deps:Loc.t * Dune_file.Dep_conf.t list
   -> lib_name:Lib_name.Local.t option
   -> scope:Scope.t
   -> t

test/blackbox-tests/dune.inc

@@ -1606,6 +1606,14 @@
    test-cases/unreadable-src
    (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))
 
+(alias
+ (name unused-preprocessor-deps)
+ (deps (package dune) (source_tree test-cases/unused-preprocessor-deps))
+ (action
+  (chdir
+   test-cases/unused-preprocessor-deps
+   (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))
+
 (alias
  (name upgrader)
  (deps (package dune) (source_tree test-cases/upgrader))
@@ -2003,6 +2011,7 @@
   (alias trace-file)
   (alias transitive-deps-mode)
   (alias unreadable-src)
+  (alias unused-preprocessor-deps)
   (alias upgrader)
   (alias use-meta)
   (alias variables-for-artifacts)
@@ -2199,6 +2208,7 @@
   (alias trace-file)
   (alias transitive-deps-mode)
   (alias unreadable-src)
+  (alias unused-preprocessor-deps)
   (alias upgrader)
   (alias variables-for-artifacts)
   (alias variant-dep-stack)

test/blackbox-tests/test-cases/unused-preprocessor-deps/dune

@@ -0,0 +1,11 @@
+(executable
+ (name a)
+ (modules a)
+ ;(preprocess (pps ppx_deriving.std))
+ (preprocessor_deps does-not-exist.txt))
+
+(library
+ (name b)
+ (modules b)
+ (preprocess future_syntax)
+ (preprocessor_deps does-not-exist.txt))

test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t

@@ -0,0 +1,27 @@
+`preprocessor_deps` is provided without `preprocess` and is ignored.
+Should warn.
+
+  $ touch a.ml b.ml
+
+  $ echo "(lang dune 1.11)" > dune-project
+  $ echo "(allow_approximate_merlin)" >> dune-project
+  $ dune build
+  File "dune", line 5, characters 1-39:
+  5 |  (preprocessor_deps does-not-exist.txt))
+       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Warning: This preprocessor_deps field will be ignored because no preprocessor
+  is configured.
+  File "dune", line 11, characters 1-39:
+  11 |  (preprocessor_deps does-not-exist.txt))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Warning: This preprocessor_deps field will be ignored because no preprocessor
+  is configured.
+
+  $ echo "(lang dune 2.0)" > dune-project
+  $ dune build
+  File "dune", line 5, characters 1-39:
+  5 |  (preprocessor_deps does-not-exist.txt))
+       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Error: This preprocessor_deps field will be ignored because no preprocessor
+  is configured.
+  [1]