Opinionated OCaml linter with Merlin integration for code quality, naming conventions, and style checks
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

merlint: load tag vocabulary from categories.toml; expand E523 dune coverage

Adds a Categories module that reads slugs from categories.toml at the
project root, and switches E915 to use it (falling back to the topics:
list in .merlint when the file is absent). Expands E523 to handle more
dune stanzas: (libraries (select ...)), (generate_sites_module ...),
(copy_files ...), (include_subdirs unqualified|qualified). Tightens
classify_modules so sublists are treated as Standard. Refreshes docs
and the generated index.html.

+224 -52
+1 -1
docs/STYLE_GUIDE.md
··· 259 259 260 260 ### [E915] Opam tag metadata 261 261 262 - Every *.opam file must declare tags: ["org:blacksun" "<topic>" ...] where each topic is listed in the topics: field of .merlint. Edit the package's dune-project so dune regenerates the opam file. 262 + Every *.opam file must declare tags: ["org:blacksun" "<topic>" ...] where each topic is a slug declared in categories.toml at the project root (or listed in the topics: field of .merlint). Edit the package's dune-project so dune regenerates the opam file. 263 263 264 264 ## Command-Line Applications 265 265
+15 -1
docs/index.html
··· 1357 1357 </div> 1358 1358 <div class="error-hint"><p>Move cram tests (.t files or .t/ directories) under the package's test/cram/ umbrella. Shared driver exes go in test/cram/helpers/; shell setup goes in test/cram/helpers.sh (sourced via (setup_scripts helpers.sh)).</p></div> 1359 1359 </div> 1360 + <div class="error-card" id="E522"> 1361 + <div> 1362 + <span class="error-code">E522</span> 1363 + <span class="error-title">Package-prefixed module in main lib/ instead of wrapped submodule</span> 1364 + </div> 1365 + <div class="error-hint"><p>Rename &lt;pkg&gt;/lib/&lt;pkg&gt;_foo.ml to &lt;pkg&gt;/lib/foo.ml (and update the .mli similarly). Dune's default wrapped mode will expose it as &lt;Pkg&gt;.Foo. For something that really needs its own public name, create a sublib directory (&lt;pkg&gt;/lib_foo/ with its own dune) instead.</p></div> 1366 + </div> 1367 + <div class="error-card" id="E523"> 1368 + <div> 1369 + <span class="error-code">E523</span> 1370 + <span class="error-title">Redundant or incomplete (modules ...) in dune</span> 1371 + </div> 1372 + <div class="error-hint"><p>A dune file with a single library/executable/test stanza doesn't need (modules ...) — dune auto-discovers every .ml in the directory. When multiple stanzas share a directory the (modules ...) fields must together cover every .ml file, otherwise some module is silently dropped. Prefer splitting into sibling directories when the stanza split is a design choice rather than a build requirement.</p></div> 1373 + </div> 1360 1374 <h2 id="testing">Testing</h2> 1361 1375 <div class="category">E600-E699 • Test coverage and test quality issues</div> 1362 1376 <div class="error-card" id="E600"> ··· 1712 1726 <span class="error-code">E915</span> 1713 1727 <span class="error-title">Opam tag metadata</span> 1714 1728 </div> 1715 - <div class="error-hint"><p>Every *.opam file must declare tags: ["org:blacksun" "&lt;topic&gt;" ...] where each topic is listed in the topics: field of .merlint. Edit the package's dune-project so dune regenerates the opam file.</p></div> 1729 + <div class="error-hint"><p>Every *.opam file must declare tags: ["org:blacksun" "&lt;topic&gt;" ...] where each topic is a slug declared in categories.toml at the project root (or listed in the topics: field of .merlint). Edit the package's dune-project so dune regenerates the opam file.</p></div> 1716 1730 </div> 1717 1731 1718 1732 <a href="#top" class="back-to-top">↑ Top</a>
+31
lib/categories.ml
··· 1 + let filename = "categories.toml" 2 + 3 + let is_section_header line = 4 + let len = String.length line in 5 + len >= 3 && line.[0] = '[' && line.[1] <> '[' && line.[len - 1] = ']' 6 + 7 + let parse_header line = 8 + let len = String.length line in 9 + String.sub line 1 (len - 2) |> String.trim 10 + 11 + let read_lines path = 12 + let ic = open_in path in 13 + let rec loop acc = 14 + match input_line ic with 15 + | line -> loop (line :: acc) 16 + | exception End_of_file -> 17 + close_in ic; 18 + List.rev acc 19 + in 20 + try loop [] 21 + with exn -> 22 + close_in_noerr ic; 23 + raise exn 24 + 25 + let load project_root = 26 + let path = Filename.concat project_root filename in 27 + try 28 + read_lines path |> List.map String.trim 29 + |> List.filter is_section_header 30 + |> List.map parse_header 31 + with Sys_error _ -> []
+10
lib/categories.mli
··· 1 + (** Canonical tag vocabulary for E915. 2 + 3 + Loads slugs from [categories.toml] at the project root. A slug is any 4 + top-level or nested table header in the file (e.g., [codec], [codec.text]). 5 + *) 6 + 7 + val load : string -> string list 8 + (** [load project_root] returns the list of declared slugs from 9 + [categories.toml] at [project_root], or the empty list if the file does not 10 + exist or cannot be read. *)
+154 -42
lib/rules/e523.ml
··· 1 - (** E523: Avoid explicit [(modules ...)] stanzas in dune files. 1 + (** E523: [(modules ...)] fields must be meaningful and complete. 2 + 3 + Dune picks up every [.ml] / [.mli] in a directory automatically. A 4 + [(modules ...)] field is only justified when multiple stanzas share a 5 + directory (and so need to split files between them) or when a 6 + [:standard \ foo] exclusion keeps a scratch module out of the build. 2 7 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 + This rule flags two cases: 8 9 9 - {b How to fix:} drop the [(modules ...)] field. If two stanzas in one dune 10 - share a directory, split them into sibling directories. *) 10 + - {b Redundant:} a dune file has a single module-accepting stanza ([library] 11 + / [executable(s)] / [test(s)]) with an explicit [(modules foo bar baz)] 12 + list. Drop the field and let dune auto-discover. 13 + - {b Uncovered:} a dune file has multiple module-accepting stanzas whose 14 + [(modules ...)] fields together do not mention every [.ml] file in the 15 + directory. Some file is falling through and dune is silently dropping it 16 + from the build. *) 11 17 12 - type payload = { dune : string; stanza : string } 18 + type kind = Redundant | Uncovered of string list 19 + type payload = { dune : string; kind : kind } 13 20 14 21 let find_dune_files root = 15 22 let try_readdir d = ··· 41 48 Some s 42 49 with Sys_error _ -> None 43 50 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' ] ]) 51 + let is_module_stanza = function 52 + | "library" | "executable" | "executables" | "test" | "tests" -> true 53 + | _ -> false 49 54 50 - let has_modules_field contents = Re.execp modules_re contents 55 + (** Interpret a [(modules ...)] field. 51 56 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 - ]) 57 + [Standard]: uses [:standard] (with or without exclusions) or other patterns 58 + we cannot resolve statically. 61 59 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 ...)" 60 + [Explicit names]: plain list of module-name atoms. *) 61 + type modules_spec = Standard | Explicit of string list 62 + 63 + let classify_modules rest = 64 + let rec loop acc = function 65 + | [] -> Explicit (List.rev acc) 66 + | Sexp.Atom a :: _ when String.length a > 0 && a.[0] = ':' -> Standard 67 + | Sexp.Atom "\\" :: _ -> Standard 68 + | Sexp.Atom a :: tl -> loop (a :: acc) tl 69 + | _ :: tl -> loop acc tl 70 + in 71 + loop [] rest 72 + 73 + let extract_modules_field = function 74 + | Sexp.List (Sexp.Atom "modules" :: rest) -> Some (classify_modules rest) 75 + | _ -> None 76 + 77 + let stanza_modules = function 78 + | Sexp.List (Sexp.Atom kind :: fields) when is_module_stanza kind -> 79 + Some (List.filter_map extract_modules_field fields) 80 + | _ -> None 81 + 82 + (** Collect generator-produced module names so they are not falsely reported as 83 + uncovered. [(ocamllex x)] and [(menhir (modules x))] both produce [x.ml]; a 84 + [(rule (target foo.ml) ...)] or [(rule (targets a.ml b.ml) ...)] likewise 85 + produces one or more [.ml] files. *) 86 + let generator_modules stanzas = 87 + let from_rule fields = 88 + let target_name atom = 89 + let a = match atom with Sexp.Atom a -> a | _ -> "" in 90 + if Filename.check_suffix a ".ml" || Filename.check_suffix a ".mli" then 91 + [ Filename.remove_extension a ] 92 + else [] 93 + in 94 + List.concat_map 95 + (function 96 + | Sexp.List [ Sexp.Atom "target"; a ] -> target_name a 97 + | Sexp.List (Sexp.Atom "targets" :: atoms) -> 98 + List.concat_map target_name atoms 99 + | _ -> []) 100 + fields 101 + in 102 + List.concat_map 103 + (function 104 + | Sexp.List (Sexp.Atom "ocamllex" :: rest) -> 105 + List.filter_map (function Sexp.Atom a -> Some a | _ -> None) rest 106 + | Sexp.List (Sexp.Atom "menhir" :: fields) -> 107 + List.concat_map 108 + (function 109 + | Sexp.List (Sexp.Atom "modules" :: atoms) -> 110 + List.filter_map 111 + (function Sexp.Atom a -> Some a | _ -> None) 112 + atoms 113 + | _ -> []) 114 + fields 115 + | Sexp.List (Sexp.Atom "rule" :: fields) -> from_rule fields 116 + | _ -> []) 117 + stanzas 118 + 119 + let ml_modules_in_dir dir = 120 + let entries = try Sys.readdir dir |> Array.to_list with Sys_error _ -> [] in 121 + List.filter_map 122 + (fun name -> 123 + if Filename.check_suffix name ".ml" then 124 + Some (Filename.chop_suffix name ".ml") 125 + else None) 126 + entries 127 + 128 + let check_dune path contents = 129 + match Sexp.Value.parse_string_many contents with 130 + | Error _ -> None 131 + | Ok stanzas -> ( 132 + let module_stanzas = List.filter_map stanza_modules stanzas in 133 + match module_stanzas with 134 + | [] -> None 135 + | [ specs ] -> 136 + (* Single module-accepting stanza. An explicit list is redundant. *) 137 + if List.exists (function Explicit _ -> true | _ -> false) specs then 138 + Some (Issue.v { dune = path; kind = Redundant }) 139 + else None 140 + | _ :: _ :: _ -> 141 + (* Multiple module-accepting stanzas share a directory. Check that 142 + every .ml in the dir is covered by some stanza's (modules ...) 143 + field; if any stanza uses :standard we cannot evaluate it. *) 144 + let all_specs = List.concat module_stanzas in 145 + let any_standard = 146 + List.exists (function Standard -> true | _ -> false) all_specs 147 + in 148 + if any_standard then None 149 + else 150 + let covered = 151 + List.concat_map 152 + (function Explicit xs -> xs | Standard -> []) 153 + all_specs 154 + |> List.map String.lowercase_ascii 155 + in 156 + let dir = Filename.dirname path in 157 + let generated = 158 + generator_modules stanzas |> List.map String.lowercase_ascii 159 + in 160 + let files = ml_modules_in_dir dir in 161 + let missing = 162 + List.filter 163 + (fun m -> 164 + let ml = String.lowercase_ascii m in 165 + not (List.mem ml covered || List.mem ml generated)) 166 + files 167 + in 168 + if missing = [] then None 169 + else Some (Issue.v { dune = path; kind = Uncovered missing })) 66 170 67 171 let check (ctx : Context.project) = 68 172 let dunes = find_dune_files ctx.project_root in ··· 70 174 (fun path -> 71 175 match read_file path with 72 176 | None -> None 73 - | Some contents when has_modules_field contents -> 74 - Some (Issue.v { dune = path; stanza = stanza_excerpt contents }) 75 - | Some _ -> None) 177 + | Some contents -> check_dune path contents) 76 178 dunes 77 179 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 180 + let pp ppf { dune; kind } = 181 + match kind with 182 + | Redundant -> 183 + Fmt.pf ppf 184 + "%s has a single stanza with a redundant (modules ...) field; drop it \ 185 + and let dune auto-discover the .ml files" 186 + dune 187 + | Uncovered files -> 188 + Fmt.pf ppf 189 + "%s has multiple stanzas but the (modules ...) fields do not cover %a; \ 190 + those .ml files are silently excluded from the build" 191 + dune 192 + Fmt.(list ~sep:comma string) 193 + files 83 194 84 195 let rule = 85 - Rule.v ~code:"E523" ~title:"Explicit (modules ...) stanza in dune" 196 + Rule.v ~code:"E523" ~title:"Redundant or incomplete (modules ...) in dune" 86 197 ~category:Rule.Project_structure 87 198 ~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." 199 + "A dune file with a single library/executable/test stanza doesn't need \ 200 + (modules ...) — dune auto-discovers every .ml in the directory. When \ 201 + multiple stanzas share a directory the (modules ...) fields must \ 202 + together cover every .ml file, otherwise some module is silently \ 203 + dropped. Prefer splitting into sibling directories when the stanza \ 204 + split is a design choice rather than a build requirement." 93 205 ~examples:[] ~pp (Project check)
+13 -8
lib/rules/e915.ml
··· 1 1 (** E915: Opam tag metadata enforcement. 2 2 3 3 Every [*.opam] file must declare a [tags:] field that contains the 4 - [org:blacksun] marker plus one or more topics from the canonical vocabulary 5 - declared as [topics:] in [.merlint]. 4 + [org:blacksun] marker plus one or more topics from the canonical vocabulary. 6 5 7 - A topic-grouped view of the monorepo (generated from these tags) only works 8 - if the vocabulary is kept honest, so every finding is an error: 6 + The vocabulary is loaded from [categories.toml] at the project root (each 7 + table header is a slug, e.g. [codec], [codec.text]). If that file is absent, 8 + falls back to the [topics:] list in [.merlint]. 9 + 10 + Every finding is an error: 9 11 - No [tags:] field in the opam file. 10 12 - [tags:] present but [org:blacksun] missing. 11 - - A tag that is not [org:*] and not in the configured [topics:] list. *) 13 + - A tag that is not [org:*] and not in the vocabulary. *) 12 14 13 15 type finding = Missing_tags | Missing_org | Unknown_topic of string 14 16 type payload = { package : string; opam : string; findings : finding list } ··· 65 67 66 68 let check (ctx : Context.project) = 67 69 let root = ctx.project_root in 68 - let topics = ctx.config.topics in 70 + let topics = 71 + match Categories.load root with [] -> ctx.config.topics | slugs -> slugs 72 + in 69 73 let issues = ref [] in 70 74 let try_readdir d = 71 75 try Sys.readdir d |> Array.to_list with Sys_error _ -> [] ··· 98 102 Rule.v ~code:"E915" ~title:"Opam tag metadata" 99 103 ~hint: 100 104 "Every *.opam file must declare tags: [\"org:blacksun\" \"<topic>\" ...] \ 101 - where each topic is listed in the topics: field of .merlint. Edit the \ 102 - package's dune-project so dune regenerates the opam file." 105 + where each topic is a slug declared in categories.toml at the project \ 106 + root (or listed in the topics: field of .merlint). Edit the package's \ 107 + dune-project so dune regenerates the opam file." 103 108 ~category:Rule.Project_structure ~examples:[] ~pp (Project check)