Skip to content

Commit

Permalink
chore: make undeclared modules check an error
Browse files Browse the repository at this point in the history
Signed-off-by: Ali Caglayan <alizter@gmail.com>
  • Loading branch information
Alizter committed Jul 14, 2023
1 parent 103c503 commit 761f73e
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 19 deletions.
6 changes: 5 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
@@ -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)

- Add `dune show rules` as alias of the `dune rules` command. (#8000, @Alizter)

- Add `dune show installed-libraries` as an alias of the `dune
Expand Down Expand Up @@ -168,7 +172,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
Expand Down
4 changes: 3 additions & 1 deletion src/dune_rules/melange/melange_stanzas.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/melange/melange_stanzas.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions src/dune_rules/ml_sources.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions src/dune_rules/modules_field_evaluator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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, 10))
[ Pp.textf "These modules appear in the %s field:" field ]
mods
[ Pp.text "They must also appear in the modules field." ]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
1 change: 1 addition & 0 deletions src/dune_rules/modules_field_evaluator.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
^
Expand All @@ -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

Expand Down

0 comments on commit 761f73e

Please sign in to comment.