chore: make undeclared modules check an error (#7674)

Signed-off-by: Ali Caglayan <alizter@gmail.com>
ocaml · Aug 7, 2023 · af1531e · af1531e
1 parent 23c6282
commit af1531e
Show file tree

Hide file tree

Showing 10 changed files with 67 additions and 10 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -208,7 +208,7 @@ changelog properly.
 
 - 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.11. (#7608,
   fixes #7026, @Alizter)
 
 - Preliminary support for Coq compiled intefaces (`.vos` files) enabled via

diff --git a/doc/changes/7674.md b/doc/changes/7674.md
@@ -0,0 +1,3 @@
+- Modules that were declared in `(modules_without_implementation)`,
+  `(private_modules)` or `(virtual_modules)` but not declared in `(modules)`
+  will raise an error. (#7674, @Alizter)
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
@@ -113,7 +114,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
@@ -142,6 +143,7 @@ module Emit = struct
        ; compile_flags
        ; allow_overlapping_dependencies
        ; enabled_if
+       ; dune_version
        })
   ;;
 

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
@@ -297,6 +297,7 @@ let make_lib_modules
   ~(lib : Library.t)
   ~modules
   ~include_subdirs:(loc_include_subdirs, (include_subdirs : Dune_file.Include_subdirs.t))
+  ~version
   =
   let open Resolve.Memo.O in
   let* kind, main_module_name, wrapped =
@@ -354,6 +355,7 @@ let make_lib_modules
         (Option.value ~default:Ordered_set_lang.standard lib.private_modules)
       ~src_dir:dir
       modules_settings
+      ~version
   in
   let () =
     match lib.stdlib, include_subdirs with
@@ -424,6 +426,7 @@ let modules_of_stanzas dune_file ~dir ~scope ~lookup_vlib ~modules ~include_subd
           ~modules
           ~lib
           ~include_subdirs
+          ~version:lib.dune_version
         >>= Resolve.read_memo
       in
       let obj_dir = Library.obj_dir lib ~dir in
@@ -440,6 +443,7 @@ let modules_of_stanzas dune_file ~dir ~scope ~lookup_vlib ~modules ~include_subd
           ~src_dir:dir
           ~kind:Modules_field_evaluator.Exe_or_normal_lib
           ~private_modules:Ordered_set_lang.standard
+          ~version:exes.dune_version
           modules_settings
       in
       let modules =
@@ -457,6 +461,7 @@ let modules_of_stanzas dune_file ~dir ~scope ~lookup_vlib ~modules ~include_subd
           ~modules
           ~stanza_loc:mel.loc
           ~kind:Modules_field_evaluator.Exe_or_normal_lib
+          ~version:mel.dune_version
           ~private_modules:Ordered_set_lang.standard
           ~src_dir:dir
           mel.modules

diff --git a/src/dune_rules/modules_field_evaluator.ml b/src/dune_rules/modules_field_evaluator.ml
@@ -153,6 +153,7 @@ let check_invalid_module_listing
   ~existing_virtual_modules
   ~allow_new_public_modules
   ~is_vendored
+  ~version
   =
   let { errors; unimplemented_virt_modules } =
     find_errors
@@ -240,12 +241,11 @@ let check_invalid_module_listing
       [ 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
+          ~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." ]
@@ -320,6 +320,7 @@ let eval
   ~kind
   ~src_dir
   ~is_vendored
+  ~version
   { Stanza_common.Modules_settings.modules = _
   ; root_module
   ; modules_without_implementation
@@ -360,7 +361,8 @@ let eval
     ~private_modules
     ~existing_virtual_modules
     ~allow_new_public_modules
-    ~is_vendored;
+    ~is_vendored
+    ~version;
   let all_modules =
     Module_trie.mapi modules ~f:(fun _path (_, m) ->
       let name = [ Module.Source.name m ] in
@@ -394,6 +396,7 @@ let eval
   ~private_modules
   ~kind
   ~src_dir
+  ~version
   (settings : Stanza_common.Modules_settings.t)
   =
   let eval0 =
@@ -418,6 +421,7 @@ let eval
       ~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.11 onwards this warning becomes an error
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.11)
+  > 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.11 onwards this warning becomes an error
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.11)
+  > 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.11 onwards this warning becomes an error
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.11)
+  > 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