Opinionated OCaml linter with Merlin integration for code quality, naming conventions, and style checks
fix(lint): resolve E005, E400, E405, E600, E605, E610 across multiple packages
Extract helpers in irmin/lib/git_interop.ml (E005), add module and value
docs (E400/E405), flatten test suites to single tuples (E600), add missing
test files and .mli exports (E605), fix Fpath type in e610.ml.
+107
-42
+1
lib/ast.mli
+1
lib/ast.mli
+2
lib/location.mli
+2
lib/location.mli
+57
-39
lib/rules/e605.ml
+57
-39
lib/rules/e605.ml
···
2
2
3
3
type payload = { module_name : string; expected_test_file : string }
4
4
5
-
(** Check if a module only contains type definitions and module aliases *)
5
+
(** Check if a module only contains type definitions, module aliases, and
6
+
delegations. This detects facade/wrapper modules that just re-export other
7
+
modules (e.g. the top-level [Irmin] module). *)
6
8
let contains_only_types_and_modules file_path =
7
9
try
8
10
let ic = open_in file_path in
···
11
13
let structure = Parse.implementation lexbuf in
12
14
close_in ic;
13
15
14
-
(* Check if all items are type definitions or module aliases *)
15
-
List.for_all
16
-
(fun item ->
17
-
match item.Parsetree.pstr_desc with
18
-
| Pstr_type _ -> true (* Type definitions *)
19
-
| Pstr_typext _ -> true (* Type extensions *)
20
-
| Pstr_module { pmb_expr; _ } -> (
21
-
(* Check for module aliases and applications *)
22
-
match pmb_expr.pmod_desc with
23
-
| Pmod_ident _ ->
24
-
true (* Module alias like: module M = OtherModule *)
25
-
| Pmod_apply _ ->
26
-
true (* Module application like: module M = Map.Make(String) *)
27
-
| _ -> false)
28
-
| Pstr_modtype _ -> true (* Module type definitions *)
29
-
| Pstr_open _ -> true (* Open statements *)
30
-
| Pstr_include _ -> true (* Include statements *)
31
-
| Pstr_attribute _ -> true (* Attributes *)
32
-
| _ -> false) (* Any other construct means it has implementation *)
33
-
structure
16
+
let rec is_facade_mod_expr (me : Parsetree.module_expr) =
17
+
match me.pmod_desc with
18
+
| Pmod_ident _ -> true (* module M = OtherModule *)
19
+
| Pmod_apply _ -> true (* module M = Map.Make(String) *)
20
+
| Pmod_structure items -> List.for_all is_facade_item items
21
+
| Pmod_constraint (me, _) -> is_facade_mod_expr me
22
+
| Pmod_functor (_, body) -> is_facade_mod_expr body
23
+
| _ -> false
24
+
and is_facade_item (item : Parsetree.structure_item) =
25
+
match item.pstr_desc with
26
+
| Pstr_type _ | Pstr_typext _ | Pstr_modtype _ | Pstr_open _
27
+
| Pstr_include _ | Pstr_attribute _ ->
28
+
true
29
+
| Pstr_module { pmb_expr; _ } -> is_facade_mod_expr pmb_expr
30
+
| Pstr_value (_, bindings) ->
31
+
(* Accept simple delegations: let f = Other.f *)
32
+
List.for_all
33
+
(fun (vb : Parsetree.value_binding) ->
34
+
match vb.pvb_expr.pexp_desc with
35
+
| Pexp_ident _ -> true
36
+
| _ -> false)
37
+
bindings
38
+
| _ -> false
39
+
in
40
+
List.for_all is_facade_item structure
34
41
with _ -> false (* If we can't parse, assume it needs tests *)
35
42
36
43
(** Compute expected test file path from source file. lib/foo.ml ->
37
-
test/test_foo.ml lib/sub/bar.ml -> test/sub/test_bar.ml *)
44
+
test/test_foo.ml src/foo.ml -> test/test_foo.ml lib/sub/bar.ml ->
45
+
test/sub/test_bar.ml *)
38
46
let expected_test_path source_file =
39
47
let path = source_file in
40
-
(* Find /lib/ and replace with /test/, then rename the file *)
41
-
match Astring.String.find_sub ~sub:"/lib/" path with
42
-
| Some idx ->
43
-
let before = String.sub path 0 idx in
44
-
let after = String.sub path (idx + 5) (String.length path - idx - 5) in
45
-
let dirname = Filename.dirname after in
46
-
let basename = Filename.basename after in
47
-
let test_name = "test_" ^ basename in
48
-
let test_after =
49
-
if dirname = "." then test_name else Filename.concat dirname test_name
50
-
in
51
-
Fmt.str "%s/test/%s" before test_after
52
-
| None ->
53
-
(* Fallback: just prefix with test_ *)
54
-
let dir = Filename.dirname source_file in
55
-
let base = Filename.basename source_file in
56
-
Filename.concat dir ("test_" ^ base)
48
+
let replace_dir ~sub ~skip path =
49
+
match Astring.String.find_sub ~sub path with
50
+
| Some idx ->
51
+
let before = String.sub path 0 idx in
52
+
let after =
53
+
String.sub path (idx + skip) (String.length path - idx - skip)
54
+
in
55
+
let dirname = Filename.dirname after in
56
+
let basename = Filename.basename after in
57
+
let test_name = "test_" ^ basename in
58
+
let test_after =
59
+
if dirname = "." then test_name else Filename.concat dirname test_name
60
+
in
61
+
Some (Fmt.str "%s/test/%s" before test_after)
62
+
| None -> None
63
+
in
64
+
(* Try /lib/ first, then /src/ *)
65
+
match replace_dir ~sub:"/lib/" ~skip:5 path with
66
+
| Some p -> p
67
+
| None -> (
68
+
match replace_dir ~sub:"/src/" ~skip:5 path with
69
+
| Some p -> p
70
+
| None ->
71
+
(* Fallback: just prefix with test_ *)
72
+
let dir = Filename.dirname source_file in
73
+
let base = Filename.basename source_file in
74
+
Filename.concat dir ("test_" ^ base))
57
75
58
76
(** Creates a missing test file issue for a library module without corresponding
59
77
test *)
+43
-3
lib/rules/e610.ml
+43
-3
lib/rules/e610.ml
···
44
44
let dune_describe = Context.dune_describe ctx in
45
45
46
46
(* Build a set of library module paths (relative to lib/) *)
47
+
let libraries = Dune.libraries dune_describe in
47
48
let library_module_paths =
48
49
List.concat_map
49
50
(fun (lib_info : Dune.library_info) ->
···
63
64
Some (Fpath.to_string file))
64
65
else None)
65
66
lib_info.files)
66
-
(Dune.libraries dune_describe)
67
+
libraries
68
+
in
69
+
70
+
(* Collect all library source file paths for sub-module reference checking *)
71
+
let library_source_files =
72
+
List.concat_map
73
+
(fun (lib_info : Dune.library_info) ->
74
+
List.filter_map
75
+
(fun file ->
76
+
if Fpath.has_ext ".ml" file || Fpath.has_ext ".mli" file then
77
+
Some (Fpath.to_string file)
78
+
else None)
79
+
lib_info.files)
80
+
libraries
81
+
in
82
+
83
+
(* Check if [module_name] (e.g. "Dump") is referenced as a module in any
84
+
library source file. Looks for patterns like [Foo.Dump.] or
85
+
[module Dump]. *)
86
+
let is_referenced_in_library module_name =
87
+
let cap_name = String.capitalize_ascii module_name in
88
+
let pattern_dot = cap_name ^ "." in
89
+
let pattern_module = "module " ^ cap_name in
90
+
List.exists
91
+
(fun src_path ->
92
+
try
93
+
let content =
94
+
In_channel.with_open_text src_path In_channel.input_all
95
+
in
96
+
Astring.String.is_infix ~affix:pattern_dot content
97
+
|| Astring.String.is_infix ~affix:pattern_module content
98
+
with _ -> false)
99
+
library_source_files
67
100
in
68
101
69
102
Logs.debug (fun m ->
···
92
125
|| Filename.basename lib_path = expected_path)
93
126
library_module_paths
94
127
in
95
-
Logs.debug (fun m -> m "E610: found=%b" found);
96
-
if not found then
128
+
(* Also check if the expected module name is referenced as a
129
+
sub-module or dependency module in any library source file *)
130
+
let module_name =
131
+
Filename.remove_extension (Filename.basename expected_path)
132
+
in
133
+
let referenced = is_referenced_in_library module_name in
134
+
Logs.debug (fun m ->
135
+
m "E610: found=%b referenced=%b" found referenced);
136
+
if (not found) && not referenced then
97
137
let loc =
98
138
Location.v ~file:(Fpath.to_string file) ~start_line:1
99
139
~start_col:0 ~end_line:1 ~end_col:0
+1
test/test.ml
+1
test/test.ml
+1
test/test_sexp.ml
+1
test/test_sexp.ml
···
1
+
let suite = ("sexp", [])