Opinionated OCaml linter with Merlin integration for code quality, naming conventions, and style checks
merlint: add E522/E523 for package-prefixed modules and explicit (modules ...)
E522 flags <pkg>/lib/<pkg>_foo.ml files that belong in wrapped submodules
or a sublib directory. E523 flags explicit (modules ...) stanzas in dune
files where auto-discovery would work — listing modules by hand drifts as
files are added.
+169
+2
lib/data.ml
+2
lib/data.ml
+74
lib/rules/e522.ml
+74
lib/rules/e522.ml
···
1
+
(** E522: Package-prefixed module names belong in sublibs, not the main lib.
2
+
3
+
If a package [foo] keeps modules named [foo_bar.ml], [foo_baz.ml] etc.
4
+
directly under [lib/] it is usually a leftover from a period where the
5
+
package wasn't wrapped. Dune's default [wrapped = true] makes the modules
6
+
accessible as [Foo.Bar] / [Foo.Baz] automatically, so the prefix adds
7
+
nothing and pollutes the source tree.
8
+
9
+
The legitimate use for a [<pkg>_xxx.ml] name is a {e sublibrary}: a sibling
10
+
directory ([lib_xxx/] or [lib/xxx/]) with its own [dune] and
11
+
[(public_name foo.xxx)]. In that case the prefix encodes the qualified
12
+
public name and is not flagged.
13
+
14
+
{b How to fix:} rename the prefixed modules to drop the package prefix and
15
+
rely on dune's wrapping; or move them into their own sublib directory if
16
+
they really need to be independently installable. *)
17
+
18
+
type payload = { package : string; file : string }
19
+
20
+
let check (ctx : Context.project) =
21
+
let root = ctx.project_root in
22
+
let try_readdir d =
23
+
try Sys.readdir d |> Array.to_list with Sys_error _ -> []
24
+
in
25
+
let issues = ref [] in
26
+
let packages = try_readdir root in
27
+
List.iter
28
+
(fun pkg ->
29
+
let pkg_dir = Filename.concat root pkg in
30
+
let lib_dir = Filename.concat pkg_dir "lib" in
31
+
let is_dir p = try Sys.is_directory p with Sys_error _ -> false in
32
+
if pkg <> "_build" && pkg <> "_opam" && pkg <> ".git" && is_dir lib_dir
33
+
then
34
+
let prefix =
35
+
(* Accept both [foo] and [ocaml-foo] package dirs; the module
36
+
prefix is [foo_] in both cases. *)
37
+
let p =
38
+
if String.starts_with ~prefix:"ocaml-" pkg then
39
+
String.sub pkg 6 (String.length pkg - 6)
40
+
else pkg
41
+
in
42
+
(* Dune mangles [-] to [_] in module names. *)
43
+
String.map (fun c -> if c = '-' then '_' else c) p ^ "_"
44
+
in
45
+
let has_ml name = Filename.check_suffix name ".ml" in
46
+
List.iter
47
+
(fun name ->
48
+
if
49
+
has_ml name
50
+
&& String.length name > String.length prefix + 3
51
+
&& String.sub name 0 (String.length prefix) = prefix
52
+
then
53
+
let path = Filename.concat (Filename.concat pkg "lib") name in
54
+
issues := Issue.v { package = pkg; file = path } :: !issues)
55
+
(try_readdir lib_dir))
56
+
packages;
57
+
!issues
58
+
59
+
let pp ppf { package = _; file } =
60
+
Fmt.pf ppf
61
+
"%s uses package-prefixed module name; drop the prefix and let dune's \
62
+
wrapping expose it as a submodule, or move it into a sublib directory"
63
+
file
64
+
65
+
let rule =
66
+
Rule.v ~code:"E522"
67
+
~title:"Package-prefixed module in main lib/ instead of wrapped submodule"
68
+
~category:Rule.Project_structure
69
+
~hint:
70
+
"Rename <pkg>/lib/<pkg>_foo.ml to <pkg>/lib/foo.ml (and update the .mli \
71
+
similarly). Dune's default wrapped mode will expose it as <Pkg>.Foo. \
72
+
For something that really needs its own public name, create a sublib \
73
+
directory (<pkg>/lib_foo/ with its own dune) instead."
74
+
~examples:[] ~pp (Project check)
+93
lib/rules/e523.ml
+93
lib/rules/e523.ml
···
1
+
(** E523: Avoid explicit [(modules ...)] stanzas in dune files.
2
+
3
+
Dune picks up every [.ml] / [.mli] in a directory automatically; listing
4
+
them by hand with [(modules foo bar baz)] is redundant and drifts as files
5
+
are added or renamed. The rare legitimate use (splitting a dir into two
6
+
libraries, or excluding a single scratch module) should be replaced with a
7
+
separate directory whose dune has no [(modules ...)].
8
+
9
+
{b How to fix:} drop the [(modules ...)] field. If two stanzas in one dune
10
+
share a directory, split them into sibling directories. *)
11
+
12
+
type payload = { dune : string; stanza : string }
13
+
14
+
let find_dune_files root =
15
+
let try_readdir d =
16
+
try Sys.readdir d |> Array.to_list with Sys_error _ -> []
17
+
in
18
+
let is_dir p = try Sys.is_directory p with Sys_error _ -> false in
19
+
let rec walk dir acc =
20
+
List.fold_left
21
+
(fun acc name ->
22
+
if
23
+
name = "_build" || name = "_opam" || name = ".git"
24
+
|| String.starts_with ~prefix:"." name
25
+
then acc
26
+
else
27
+
let p = Filename.concat dir name in
28
+
if is_dir p then walk p acc
29
+
else if name = "dune" then p :: acc
30
+
else acc)
31
+
acc (try_readdir dir)
32
+
in
33
+
walk root []
34
+
35
+
let read_file path =
36
+
try
37
+
let ic = open_in path in
38
+
let n = in_channel_length ic in
39
+
let s = really_input_string ic n in
40
+
close_in ic;
41
+
Some s
42
+
with Sys_error _ -> None
43
+
44
+
(* Detect a [(modules ...)] field inside a stanza. Simple textual scan for
45
+
"(modules" followed by whitespace — good enough for dune files, which are
46
+
s-expressions with straightforward syntax. *)
47
+
let modules_re =
48
+
Re.compile (Re.seq [ Re.str "(modules"; Re.alt [ Re.space; Re.char '\n' ] ])
49
+
50
+
let has_modules_field contents = Re.execp modules_re contents
51
+
52
+
let excerpt_re =
53
+
Re.compile
54
+
(Re.seq
55
+
[
56
+
Re.str "(modules";
57
+
Re.alt [ Re.space; Re.char '\n' ];
58
+
Re.rep (Re.compl [ Re.char ')' ]);
59
+
Re.char ')';
60
+
])
61
+
62
+
let stanza_excerpt contents =
63
+
match Re.exec_opt excerpt_re contents with
64
+
| Some g -> Re.Group.get g 0
65
+
| None -> "(modules ...)"
66
+
67
+
let check (ctx : Context.project) =
68
+
let dunes = find_dune_files ctx.project_root in
69
+
List.filter_map
70
+
(fun path ->
71
+
match read_file path with
72
+
| None -> None
73
+
| Some contents when has_modules_field contents ->
74
+
Some (Issue.v { dune = path; stanza = stanza_excerpt contents })
75
+
| Some _ -> None)
76
+
dunes
77
+
78
+
let pp ppf { dune; stanza = _ } =
79
+
Fmt.pf ppf
80
+
"%s lists modules explicitly; drop the (modules ...) field and let dune \
81
+
pick up every .ml in the directory"
82
+
dune
83
+
84
+
let rule =
85
+
Rule.v ~code:"E523" ~title:"Explicit (modules ...) stanza in dune"
86
+
~category:Rule.Project_structure
87
+
~hint:
88
+
"Dune auto-discovers every .ml/.mli in a stanza's directory. If two \
89
+
stanzas share a directory so you had to list modules explicitly, split \
90
+
them into sibling directories instead. The rare exception is a stanza \
91
+
that genuinely needs to exclude a scratch module — keep that aside and \
92
+
let the rest auto-discover."
93
+
~examples:[] ~pp (Project check)