chore: make undeclared modules check an error

Signed-off-by: Ali Caglayan <alizter@gmail.com>
ocaml · Aug 2, 2023 · ff0af78 · ff0af78
1 parent 2de23a6
commit ff0af78
Show file tree

Hide file tree

Showing 9 changed files with 71 additions and 19 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,6 +1,10 @@
 Unreleased
 ----------
 
+- Modules that were declared in `(modules_without_implementation)`,
+  `(private_modules)` or `(virtual_modules)` but not declared in `(modules)`
+  will raise an error. (#7674, @Alizter)
+
 - No longer emit linkopts(javascript) in META files (#8168, @hhugo)
 
 3.10.0 (2023-07-31)
@@ -206,7 +210,7 @@ Unreleased
 
 - Modules that were declared in `(modules_without_implementation)`,
   `(private_modules)` or `(virtual_modules)` but not declared in `(modules)`
-  will cause Dune to emit a warning which will become an error in 3.9. (#7608,
+  will cause Dune to emit a warning which will become an error in 3.10. (#7608,
   fixes #7026, @Alizter)
 
 - Preliminary support for Coq compiled intefaces (`.vos` files) enabled via

diff --git a/src/dune_rules/melange/melange_stanzas.ml b/src/dune_rules/melange/melange_stanzas.ml
@@ -18,6 +18,7 @@ module Emit = struct
     ; compile_flags : Ordered_set_lang.Unexpanded.t
     ; allow_overlapping_dependencies : bool
     ; enabled_if : Blang.t
+    ; dune_version : Dune_lang.Syntax.Version.t
     }
 
   type Stanza.t += T of t
@@ -114,7 +115,7 @@ module Emit = struct
          let open Enabled_if in
          let allowed_vars = Any in
          decode ~allowed_vars ~since:None ()
-       in
+       and+ dune_version = Dune_lang.Syntax.get_exn Stanza.syntax in
        let preprocess =
          let init =
            let f libname = Preprocess.With_instrumentation.Ordinary libname in
@@ -140,6 +141,7 @@ module Emit = struct
        ; compile_flags
        ; allow_overlapping_dependencies
        ; enabled_if
+       ; dune_version
        })
 
   let target_dir (emit : t) ~dir = Path.Build.relative dir emit.target

diff --git a/src/dune_rules/melange/melange_stanzas.mli b/src/dune_rules/melange/melange_stanzas.mli
@@ -18,6 +18,7 @@ module Emit : sig
     ; compile_flags : Ordered_set_lang.Unexpanded.t
     ; allow_overlapping_dependencies : bool
     ; enabled_if : Blang.t
+    ; dune_version : Dune_lang.Syntax.Version.t
     }
 
   type Stanza.t += T of t

diff --git a/src/dune_rules/ml_sources.ml b/src/dune_rules/ml_sources.ml
@@ -288,7 +288,8 @@ let virtual_modules ~lookup_vlib vlib =
 
 let make_lib_modules ~dir ~libs ~lookup_vlib ~(lib : Library.t) ~modules
     ~include_subdirs:
-      (loc_include_subdirs, (include_subdirs : Dune_file.Include_subdirs.t)) =
+      (loc_include_subdirs, (include_subdirs : Dune_file.Include_subdirs.t))
+    ~version =
   let open Resolve.Memo.O in
   let* kind, main_module_name, wrapped =
     match lib.implements with
@@ -342,7 +343,7 @@ let make_lib_modules ~dir ~libs ~lookup_vlib ~(lib : Library.t) ~modules
     Modules_field_evaluator.eval ~modules ~stanza_loc ~kind
       ~private_modules:
         (Option.value ~default:Ordered_set_lang.standard lib.private_modules)
-      ~src_dir:dir modules_settings
+      ~src_dir:dir modules_settings ~version
   in
   let () =
     match (lib.stdlib, include_subdirs) with
@@ -408,7 +409,7 @@ let modules_of_stanzas dune_file ~dir ~scope ~lookup_vlib ~modules
         let+ sources, modules =
           let lookup_vlib = lookup_vlib ~loc:lib.buildable.loc in
           make_lib_modules ~dir ~libs:(Scope.libs scope) ~lookup_vlib ~modules
-            ~lib ~include_subdirs
+            ~lib ~include_subdirs ~version:lib.dune_version
           >>= Resolve.read_memo
         in
         let obj_dir = Library.obj_dir lib ~dir in
@@ -421,7 +422,8 @@ let modules_of_stanzas dune_file ~dir ~scope ~lookup_vlib ~modules
           in
           Modules_field_evaluator.eval ~modules ~stanza_loc ~src_dir:dir
             ~kind:Modules_field_evaluator.Exe_or_normal_lib
-            ~private_modules:Ordered_set_lang.standard modules_settings
+            ~private_modules:Ordered_set_lang.standard
+            ~version:exes.dune_version modules_settings
         in
         let modules =
           let project = Scope.project scope in
@@ -436,7 +438,8 @@ let modules_of_stanzas dune_file ~dir ~scope ~lookup_vlib ~modules
         let+ sources, modules =
           Modules_field_evaluator.eval ~modules ~stanza_loc:mel.loc
             ~kind:Modules_field_evaluator.Exe_or_normal_lib
-            ~private_modules:Ordered_set_lang.standard ~src_dir:dir mel.modules
+            ~version:mel.dune_version ~private_modules:Ordered_set_lang.standard
+            ~src_dir:dir mel.modules
         in
         let modules =
           Modules_group.make_wrapped ~obj_dir:(Obj_dir.obj_dir obj_dir) ~modules

diff --git a/src/dune_rules/modules_field_evaluator.ml b/src/dune_rules/modules_field_evaluator.ml
@@ -136,7 +136,7 @@ let find_errors ~modules ~intf_only ~virtual_modules ~private_modules
 
 let check_invalid_module_listing ~stanza_loc ~modules_without_implementation
     ~intf_only ~modules ~virtual_modules ~private_modules
-    ~existing_virtual_modules ~allow_new_public_modules ~is_vendored =
+    ~existing_virtual_modules ~allow_new_public_modules ~is_vendored ~version =
   let { errors; unimplemented_virt_modules } =
     find_errors ~modules ~intf_only ~virtual_modules ~private_modules
       ~existing_virtual_modules ~allow_new_public_modules
@@ -229,10 +229,10 @@ let check_invalid_module_listing ~stanza_loc ~modules_without_implementation
       [ Pp.text "You must provide an implementation for all of these modules." ];
     (* Checking that (modules) includes all declared modules *)
     let print_undelared_modules field mods =
-      (* TODO: this is a warning for now, change to an error in 3.9. *)
       (* If we are in a vendored stanza we do nothing. *)
       if not is_vendored then
-        print ~is_error:false
+        print
+          ~is_error:(version >= (3, 11))
           [ Pp.textf "These modules appear in the %s field:" field ]
           mods
           [ Pp.text "They must also appear in the modules field." ]
@@ -291,7 +291,7 @@ let eval0 ~modules:(all_modules : Module.Source.t Module_trie.t) ~stanza_loc
   { modules; fake_modules = !fake_modules }
 
 let eval ~modules:(all_modules : Module.Source.t Module_trie.t) ~stanza_loc
-    ~private_modules ~kind ~src_dir ~is_vendored
+    ~private_modules ~kind ~src_dir ~is_vendored ~version
     { Stanza_common.Modules_settings.modules = _
     ; root_module
     ; modules_without_implementation
@@ -327,7 +327,7 @@ let eval ~modules:(all_modules : Module.Source.t Module_trie.t) ~stanza_loc
         ]);
   check_invalid_module_listing ~stanza_loc ~modules_without_implementation
     ~intf_only ~modules ~virtual_modules ~private_modules
-    ~existing_virtual_modules ~allow_new_public_modules ~is_vendored;
+    ~existing_virtual_modules ~allow_new_public_modules ~is_vendored ~version;
   let all_modules =
     Module_trie.mapi modules ~f:(fun _path (_, m) ->
         let name = [ Module.Source.name m ] in
@@ -354,7 +354,7 @@ let eval ~modules:(all_modules : Module.Source.t Module_trie.t) ~stanza_loc
     Module_trie.set all_modules path module_
 
 let eval ~modules:(all_modules : Module.Source.t Module_trie.t) ~stanza_loc
-    ~private_modules ~kind ~src_dir
+    ~private_modules ~kind ~src_dir ~version
     (settings : Stanza_common.Modules_settings.t) =
   let eval0 =
     eval0
@@ -369,6 +369,6 @@ let eval ~modules:(all_modules : Module.Source.t Module_trie.t) ~stanza_loc
   in
   let modules =
     eval ~modules:all_modules ~stanza_loc ~private_modules ~kind ~src_dir
-      ~is_vendored settings eval0
+      ~is_vendored settings eval0 ~version
   in
   (eval0.modules, modules)
diff --git a/src/dune_rules/modules_field_evaluator.mli b/src/dune_rules/modules_field_evaluator.mli
@@ -25,5 +25,6 @@ val eval :
   -> private_modules:Ordered_set_lang.t
   -> kind:kind
   -> src_dir:Path.Build.t
+  -> version:Dune_lang.Syntax.Version.t
   -> Stanza_common.Modules_settings.t
   -> ((Loc.t * Module.Source.t) Module_trie.t * Module.t Module_trie.t) Memo.t
diff --git a/test/blackbox-tests/test-cases/intf-only/excluded-by-modules-field.t b/test/blackbox-tests/test-cases/intf-only/excluded-by-modules-field.t
@@ -22,20 +22,34 @@ field
 
 X is warned about:
 
-  $ dune build --display short
+  $ dune build
   File "src/dune", line 4, characters 33-34:
   4 |  (modules_without_implementation x)
                                        ^
   Warning: These modules appear in the modules_without_implementation field:
   - X
   They must also appear in the modules field.
-        ocamlc src/.foo.objs/byte/y.{cmi,cmo,cmt} (exit 2)
   File "src/y.ml", line 1, characters 16-17:
   1 | module type F = X
                       ^
   Error: Unbound module type X
   [1]
 
+In 3.10 onwards this warning becomes an error
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.10)
+  > EOF
+
+  $ dune build
+  File "src/dune", line 4, characters 33-34:
+  4 |  (modules_without_implementation x)
+                                       ^
+  Error: These modules appear in the modules_without_implementation field:
+  - X
+  They must also appear in the modules field.
+  [1]
+
 This should be ignored if we are in vendored_dirs
 
   $ cat > dune << EOF

diff --git a/test/blackbox-tests/test-cases/private-modules/private-module-excluded-by-modules-field.t b/test/blackbox-tests/test-cases/private-modules/private-module-excluded-by-modules-field.t
@@ -37,6 +37,21 @@ X is warned about:
   Error: Unbound module X
   [1]
 
+In 3.10 onwards this warning becomes an error
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.10)
+  > EOF
+
+  $ dune build
+  File "src/dune", line 5, characters 18-19:
+  5 |  (private_modules x))
+                        ^
+  Error: These modules appear in the private_modules field:
+  - X
+  They must also appear in the modules field.
+  [1]
+
 This warning should be ignored if we are in vendored_dirs
 
   $ cat > dune << EOF

diff --git a/test/blackbox-tests/test-cases/virtual-libraries/virtual-modules-excluded-by-modules-field.t b/test/blackbox-tests/test-cases/virtual-libraries/virtual-modules-excluded-by-modules-field.t
@@ -29,15 +29,13 @@ Specifying a virtual module that isn't inside the (modules ..) field:
 
 X is warned about:
 
-  $ dune build --display short
+  $ dune build
   File "src/dune", line 4, characters 18-19:
   4 |  (virtual_modules x)
                         ^
   Warning: These modules appear in the virtual_modules field:
   - X
   They must also appear in the modules field.
-      ocamldep src/impl/.impl.objs/x.impl.d
-        ocamlc src/.foo.objs/byte/y.{cmi,cmo,cmt} (exit 2)
   File "src/y.ml", line 1, characters 16-17:
   1 | module type F = X
                       ^
@@ -49,6 +47,20 @@ X is warned about:
   Error: No rule found for src/.foo.objs/y.impl.all-deps
   [1]
 
+In 3.10 onwards this warning becomes an error
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.10)
+  > EOF
+
+  $ dune build ./bar.exe
+  File "src/dune", line 4, characters 18-19:
+  4 |  (virtual_modules x)
+                        ^
+  Error: These modules appear in the virtual_modules field:
+  - X
+  They must also appear in the modules field.
+  [1]
 
 This should be ignored if we are in vendored_dirs