This document lists all error codes that Merlint can detect, along with their descriptions and fix hints.
High cyclomatic complexity makes code harder to understand and test. Consider breaking complex functions into smaller, more focused functions. Each function should ideally do one thing well.
let check_input x y z =
if x > 0 then
if y > 0 then
if z > 0 then
if x + y > z then
if y + z > x then
if x + z > y then
"valid"
else "invalid"
else "invalid"
else "invalid"
else "invalid"
else "invalid"
else "invalid"
let check_positive x = x > 0
let check_triangle x y z =
x + y > z && y + z > x && x + z > y
let check_input x y z =
if not (check_positive x && check_positive y && check_positive z) then
"invalid"
else if not (check_triangle x y z) then
"invalid"
else
"valid"
This issue means your functions are too long and hard to read. Fix them by extracting logical sections into separate functions with descriptive names. Note: Functions with pattern matching get additional allowance (2 lines per case). Pure data structures (lists, records) are exempt from length checks. For better readability, consider using helper functions for complex logic. Aim for functions under 50 lines of actual logic.
let process_all_data x y z =
(* This function is intentionally long to demonstrate the rule *)
let a = x + 1 in
let b = y + 1 in
let c = z + 1 in
let d = a * 2 in
let e = b * 2 in
let f = c * 2 in
let g = d + e in
let h = e + f in
let i = f + d in
let j = g * 2 in
let k = h * 2 in
let l = i * 2 in
let m = j + k in
let n = k + l in
let o = l + j in
let p = m * 2 in
let q = n * 2 in
let r = o * 2 in
let s = p + q in
let t = q + r in
let u = r + p in
let v = s * 2 in
let w = t * 2 in
let x1 = u * 2 in
let y1 = v + w in
let z1 = w + x1 in
let a1 = x1 + v in
let b1 = y1 * 2 in
let c1 = z1 * 2 in
let d1 = a1 * 2 in
let e1 = b1 + c1 in
let f1 = c1 + d1 in
let g1 = d1 + b1 in
let h1 = e1 * 2 in
let i1 = f1 * 2 in
let j1 = g1 * 2 in
let k1 = h1 + i1 in
let l1 = i1 + j1 in
let m1 = j1 + h1 in
let n1 = k1 * 2 in
let o1 = l1 * 2 in
let p1 = m1 * 2 in
let q1 = n1 + o1 in
let r1 = o1 + p1 in
let s1 = p1 + n1 in
let t1 = q1 * 2 in
let u1 = r1 * 2 in
let v1 = s1 * 2 in
let w1 = t1 + u1 in
let x2 = u1 + v1 in
let y2 = v1 + t1 in
let result = w1 + x2 + y2 in
result
let step1 x y = (x + 1, y + 1)
let step2 (a, b) = (a * 2, b * 2)
let combine (c, d) = (c + d) * 2
let process_all x y =
let (a, b) = step1 x y in
let (c, d) = step2 (a, b) in
combine (c, d)
(* Functions with large pattern matches get 2 lines per case + 10% allowance *)
type property =
| Color | Background_color | Border_color | Outline_color | Border_top_color
| Border_right_color | Border_bottom_color | Border_left_color | Text_color
let with_context f = f ()
(* Test the pattern: fun xxx -> fun () -> match ... *)
let read_value prop =
with_context @@ fun () ->
match prop with
| Color ->
let x = 1 in
let y = 2 in
x + y
| Background_color ->
let x = 3 in
let y = 4 in
x + y
| Border_color ->
let x = 5 in
let y = 6 in
x + y
| Outline_color ->
let x = 7 in
let y = 8 in
x + y
| Border_top_color ->
let x = 9 in
let y = 10 in
x + y
| Border_right_color ->
let x = 11 in
let y = 12 in
x + y
| Border_bottom_color ->
let x = 13 in
let y = 14 in
x + y
| Border_left_color ->
let x = 15 in
let y = 16 in
x + y
| Text_color ->
let x = 17 in
let y = 18 in
x + y
This issue means your code has too many nested conditions making it hard to follow. Fix it by extracting nested logic into helper functions, using early returns to reduce nesting, or combining conditions when appropriate. Aim for maximum nesting depth of 4.
let process x y z =
if x > 0 then
if y > 0 then
if z > 0 then
if x < 100 then
if y < 100 then
x + y + z
else 0
else 0
else 0
else 0
else 0
(* The "heavy work in else" pattern: depth grows on the else side too,
so a chain of three guards followed by a real else-block counts as
nesting depth 4. *)
let classify_with_work x =
if x < 0 then `Negative
else
let abs_x = abs x in
if abs_x = 0 then `Zero
else if abs_x < 10 then `Small
else
let bucket =
if abs_x < 100 then `Medium
else if abs_x < 1000 then `Large
else `Huge
in
bucket
(* [else if] chains stay flat: each [else if] counts at the same level
as the surrounding [if], so a guard cascade isn't penalised for
reading as flat. *)
let classify x =
if x < 0 then `Negative
else if x = 0 then `Zero
else if x < 10 then `Small
else if x < 100 then `Medium
else `Large
let process x y z =
if x <= 0 || y <= 0 || z <= 0 then 0
else if x >= 100 then 0
else x + y + z
(* Record literals carrying inert data don't count as nesting. *)
type point = { x : int; y : int; z : int }
let translate p dx dy dz = { x = p.x + dx; y = p.y + dy; z = p.z + dz }
(* A nested function literal is a separate control-flow body. Its body should
not inflate the enclosing function's nesting depth. *)
type 'a visitor = { visit : 'a visitor -> 'a -> unit }
let collect xs =
let acc = ref [] in
let visitor =
{
visit =
(fun _this x ->
if x > 0 then
if x > 10 then
if x > 100 then acc := x :: !acc else ()
else ()
else ());
}
in
List.iter (visitor.visit visitor) xs;
!acc
The Obj module bypasses OCaml's type system and is not part of the language. Any use (Obj.magic, Obj.repr, Obj.obj, Obj.tag, ...) can cause segmentation faults, data corruption, and unpredictable behavior. Use proper type definitions, GADTs, or polymorphic variants instead. If an unsafe boundary is truly unavoidable, isolate it in one module and document why.
let coerce x = Obj.magic x
let erased x = Obj.repr x
let recovered o : int = Obj.obj o
let tag_of x = Obj.tag (Obj.repr x)
(* Use proper type conversions *)
let int_of_string_opt s =
try Some (int_of_string s) with _ -> None
(* Or use variant types *)
type value = Int of int | String of string
let to_int = function Int i -> Some i | _ -> None
The Marshal module (and output_value/input_value) serializes values without their type. A marshalled value carries no type information, so Marshal.from_* can be read back at any type - including an abstract type - forging values that violate the invariants their module guarantees. Deserializing attacker-controlled data this way is also a code-execution risk. Use a typed codec (a wire or cbor encoder and decoder, or a hand-written printer and parser) instead. If a trusted in-process boundary truly needs it, isolate it in one module and document why.
let dump x = Marshal.to_string x []
let load s : int = Marshal.from_string s 0
let save oc x = output_value oc x
let restore ic : int = input_value ic
(* Use a typed encoder and decoder instead of Marshal *)
let to_string i = string_of_int i
let of_string s = int_of_string_opt s
Catch-all exception handlers (with _ ->) can hide unexpected errors and make debugging difficult. Always handle specific exceptions explicitly. If you must catch all exceptions, log them or re-raise after cleanup.
let parse_int s =
try int_of_string s with _ -> 0
let read_config () =
try
let ic = open_in "config.txt" in
let data = input_line ic in
close_in ic;
data
with _ -> "default"
let parse_int s =
try int_of_string s with
| Failure _ -> 0
let read_config () =
try
let ic = open_in "config.txt" in
let data = input_line ic in
close_in ic;
data
with
| Sys_error msg ->
Fmt.epr "Config error: %s@." msg;
"default"
| End_of_file ->
Fmt.epr "Config file is empty@.";
"default"
module Log = struct
let err f = f Format.err_formatter
end
let dangerous_operation () =
failwith "Something went wrong"
let safe_wrapper () =
try dangerous_operation () with
| exn ->
Log.err (fun m ->
Format.fprintf m "Operation failed: %s@."
(Printexc.to_string exn));
raise exn
OCaml's structural (=), (<>), (<), (>), (<=), (>=), compare, min, max and Hashtbl.hash compare values by walking their runtime representation. On a type from the current module that is fine - you can see its representation, and you expose your own equal in the .mli - but on another module's type it walks past the abstraction (ordering two abstract handles leaks their hidden contents), and on a function it raises Invalid_argument at runtime. Across modules, call that type's own equal, compare or hash. Comparing scalars, transparent containers (list, array, option) and tuples of those is always fine, as is a tag check against a nullary constructor ([], None, an enum tag). Defining a type's own equal or compare with these operators inside its defining module - let equal a b = a = b - is fine and not flagged: there you see the representation and are the authority on whether it is sound.
(* Id.t is abstract here: another module's type, so it must be compared
through Id.equal, not by walking its hidden representation. *)
module Id : sig
type t
val v : int -> t
end = struct
type t = int
let v x = x
end
(* (=) on another module's type *)
let same_id (a : Id.t) b = a = b
(* compare on another module's type *)
let order_id (a : Id.t) b = compare a b
(* Hashtbl.hash on another module's type *)
let key (a : Id.t) = Hashtbl.hash a
(* (=) on an abstract type from another library (Re.t is a compiled regex) *)
let same_re (a : Re.t) b = a = b
(* (=) on a function value: this also raises Invalid_argument at runtime *)
let same_fn (a : int -> int) b = a = b
(* Stdlib (>) on another module's type: should use Id.compare *)
let gt (a : Id.t) b = a > b
(* Comparing scalars with the polymorphic operators is fine *)
let same_int (a : int) b = a = b
let same_string s = s = "hello"
let bigger (a : float) b = a > b
(* Transparent containers and tuples of safe types are fine too, however the
container is spelled - [sorted] infers its list through Stdlib.List *)
let same_pair (a : int * int) b = a = b
let same_path (a : string list) b = a = b
let has_afl o = o = Some "afl"
let sorted xs = List.sort_uniq String.compare xs = [ "x" ]
(* This module's own types: (=) is fine here because the representation is
visible; expose `equal` in the .mli for other modules to call. *)
type point = { x : int; y : int }
type color = Red | Green | Blue
let equal_point (a : point) b = a = b
let equal_color (a : color) b = a = b
(* Tag checks against a nullary constructor *)
let is_red c = c = Red
let nonempty l = l <> []
let absent o = o = None
(* An operator from another module - like Z.(p > zero) on zarith - is that
module's own comparison, not Stdlib's, so it is fine even though the
operand's type is not local. *)
module Money : sig
type t
val ( > ) : t -> t -> bool
val zero : t
end = struct
type t = int
let ( > ) = Stdlib.( > )
let zero = 0
end
let positive (p : Money.t) = Money.(p > zero)
Warnings should be addressed rather than silenced. Fix the underlying issue instead of using warning suppression attributes. If you must suppress a warning, document why it's necessary.
[@@@ocaml.warning "-32"]
let unused_function x = x + 1
[@@ocaml.warning "-27"]
let partial_match = function
| Some x -> x
[@ocaml.warning "-9"]
type t = { mutable field : int; another : string }
let used_function x = x + 1
let () = print_int (used_function 5)
let complete_match = function
| Some x -> x
| None -> 0
type t = { field : int; another : string }
The Str module is outdated and has a problematic API. Use the Re module instead for regular expressions. Re provides a better API, is more performant, and doesn't have global state issues.
let contains_at s =
Str.string_match (Str.regexp ".*@.*") s 0
(* Requires: opam install re *)
let at_re = Re.compile (Re.str "@")
let contains_at s = Re.execp at_re s
The Fmt module provides a more modern and composable approach to formatting. It offers better type safety and cleaner APIs compared to Printf/Format modules.
let make_error msg line =
Printf.sprintf "Error: %s at line %d" msg line
let print_count n =
Printf.printf "Processing %d items...\n" n
let format_count n =
Format.asprintf "Processing %d items..." n
(* Requires: opam install fmt *)
let make_error msg line =
Fmt.str "Error: %s at line %d" msg line
let print_count n =
Fmt.pr "Processing %d items...@." n
Avoid using double underscore module access like 'Module__Submodule'. Use dot notation 'Module.Submodule' instead. Double underscore notation is internal to the OCaml module system and should not be used in application code.
let result = Merlint__Location.v ~file:"test.ml"
~start_line:1 ~start_col:0 ~end_line:1 ~end_col:10
let result = Merlint.Location.v ~file:"test.ml"
~start_line:1 ~start_col:0 ~end_line:1 ~end_col:10
Use Fmt.failwith instead of failwith (Fmt.str ...). Fmt.failwith provides printf-style formatting directly, making the code more concise and readable.
let validate_input input =
if String.length input = 0 then
failwith (Fmt.str "Empty input provided")
else if String.length input > 100 then
failwith (Fmt.str "Input too long: %d characters" (String.length input))
else
input
let validate_input input =
if String.length input = 0 then
Fmt.failwith "Empty input provided"
else if String.length input > 100 then
Fmt.failwith "Input too long: %d characters" (String.length input)
else
input
Use Fmt.invalid_arg instead of invalid_arg (Fmt.str ...). Fmt.invalid_arg provides printf-style formatting directly, making the code more concise and readable.
let validate_port port =
if port < 0 || port > 65535 then
invalid_arg (Fmt.str "Invalid port: %d" port)
else port
let validate_port port =
if port < 0 || port > 65535 then
Fmt.invalid_arg "Invalid port: %d" port
else port
Most calls of the shape [<f> (Fmt.str ...)] have a direct [Fmt.X] equivalent that avoids the intermediate [Fmt.str] string allocation and reads better: - [Buffer.add_string buf (Fmt.str ...)] -> [Fmt.pf (Fmt.with_buffer buf) "..."] (hoist the formatter outside any hot loop to avoid re-allocating); - [print_endline (Fmt.str ...)] -> [Fmt.pr "...@."]; - [print_string (Fmt.str ...)] -> [Fmt.pr "..."]; - [prerr_endline (Fmt.str ...)] -> [Fmt.epr "...@."]; - [prerr_string (Fmt.str ...)] -> [Fmt.epr "..."]; - [Error (... formatted with Fmt.str ...)] -> [Fmt.kstr (fun e -> Error e) "..."], or a one-shot [error_msgf] helper in the package; - any other [<f> (Fmt.str ...)] -> [Fmt.kstr <f> "..."]. Specialised cases for [failwith], [invalid_arg], [Alcotest.fail], and bare [fail] are handled by E215, E216, and E616 respectively.
let parse s =
if s = "" then Fmt.kstr (fun e -> Error e) "empty input"
else Ok s
let log_event buf ev =
Buffer.add_string buf (Fmt.str "[%s] %s\n" ev.kind ev.message)
let trace n = print_endline (Fmt.str "n=%d" n)
let parse s =
if s = "" then Fmt.kstr (fun e -> Error e) "empty input"
else Ok s
let log_event buf ev =
Fmt.bprintf buf "[%s] %s\n" ev.kind ev.message
let trace n = Fmt.pr "n=%d@." n
When the same [Fmt.kstr (fun s -> Error (Constructor s)) ...] or [Fmt.kstr (fun s -> raise (Constructor s)) ...] lambda appears at multiple call sites, extract a small helper at the top of the file: let err_x fmt = Fmt.kstr (fun s -> Error (Constructor s)) fmt let fail_x fmt = Fmt.kstr (fun s -> raise (Constructor s)) fmt and replace each call site with [err_x "..." args] / [fail_x "..." args]. The lambda reads as noise at the call site; the helper reads as the domain operation ("emit a wire error", "raise a parse failure"). For a single one-off site the inline form is fine — the helper is a deduplication tool. The rule only flags inline call sites (kstr with a literal-string format) and skips helper definitions, which thread a [fmt] parameter.
exception Parse_error of string
let parse_int s =
match int_of_string_opt s with
| Some n -> n
| None -> Fmt.kstr (fun s -> raise (Parse_error s)) "not an int: %S" s
let parse s =
if s = "" then Fmt.kstr (fun s -> Error s) "empty input"
else if String.length s > 100 then
Fmt.kstr (fun s -> Error s) "input too long: %d" (String.length s)
else Ok s
exception Parse_error of string
let fail_parse fmt = Fmt.kstr (fun s -> raise (Parse_error s)) fmt
let err fmt = Fmt.kstr (fun s -> Error s) fmt
let parse_int s =
match int_of_string_opt s with
| Some n -> n
| None -> fail_parse "not an int: %S" s
let parse s =
if s = "" then err "empty input"
else if String.length s > 100 then err "input too long: %d" (String.length s)
else Ok s
A [let rec f and g and h] chain only needs the [and] when at least one binding calls another binding in the group. When [h] references neither [f] nor [g] (and isn't recursive itself), the [and] is just a coupling: future readers assume the bindings are co-dependent. Lift the standalone binding to its own [let] above or below the group. The same applies inside expressions: [let rec ... and ... in body] follows the same rule.
let rec is_even n = n = 0 || is_odd (n - 1)
and is_odd n = n <> 0 && is_even (n - 1)
and double x = x + x
let double x = x + x
let rec is_even n = n = 0 || is_odd (n - 1)
and is_odd n = n <> 0 && is_even (n - 1)
[module X : module type of Y] re-elaborates the target signature and hides type equalities behind a fresh abstraction; an alias [module X = Y] is cheaper to typecheck and preserves equalities. Keep [module type of] only when you immediately narrow with [with type t = ...] / [with module M = ...].
This file references a module banned by the [disallowed_modules] list in a merlint.toml that covers it. Use an allowed alternative, or relax the ban for this subtree. The ban is empty by default and scoped to the directory tree of the merlint.toml that declares it, so a subtree that must not depend on Printf/Format/Fmt can forbid them without affecting the rest of the project.
let make_error msg line = Printf.sprintf "Error: %s at line %d" msg line
let print_count n = Printf.printf "Processing %d items...\n" n
let format_count n = Format.asprintf "Processing %d items..." n
let greet name = Fmt.str "hello %s" name
let make_error msg line = "Error: " ^ msg ^ " at line " ^ string_of_int line
let print_count n = print_string ("Processing " ^ string_of_int n ^ " items...")
let greet name = "hello " ^ name
Variant constructors should use Snake_case (e.g., Waiting_for_input, Processing_data), not CamelCase. This matches the project's naming conventions.
type status =
| WaitingForInput
| ProcessingData
| ErrorOccurred
type status =
| Waiting_for_input (* Snake_case *)
| Processing_data
| Error_occurred
type os =
| Linux
| MacOS (* short trailing acronym: compound term *)
| Windows
Module names should use Snake_case (e.g., My_module, User_profile) or all-uppercase for acronyms (e.g., HTML, JSON, ONF). File names use lowercase_with_underscores which OCaml automatically converts to module names.
module UserProfile = struct end
module User_profile = struct end
Values and function names should use snake_case (e.g., find_user, create_channel). Short, descriptive, and lowercase with underscores. This is the standard convention in OCaml for values and functions.
type user = { name : string }
let myValue = 42
let getUserName user = user.name
type user = { name : string }
let my_value = 42
let get_user_name user = user.name
Type names should use snake_case. The primary type in a module should be named t, and identifiers should be id. This convention helps maintain consistency across the codebase.
type userProfile = { name : string }
type http_response = HttpOk | HttpError
type user_profile = { name: string }
type http_response = Ok | Error
Avoid using too many underscores in identifier names as they make code harder to read. Consider using more descriptive names or restructuring the code to avoid deeply nested concepts.
let get_user_profile_data_from_database_by_id () = 42
let get_user_by_id () = 42
Functions that return option types should be prefixed with 'find_', while functions that return non-option types should be prefixed with 'get_'. This convention helps communicate the function's behavior to callers.
let get_user () = None (* returns option but named get_* *)
let find_name () = "John" (* returns string but named find_* *)
let find_user () = None (* returns option, correctly named *)
let get_name () = "John" (* returns string, correctly named *)
Avoid prefixing type or function names with the module name. The module already provides the namespace, so Message.message_type should just be Message.t. Exception: Pp.pp is idiomatic for pretty-printing modules.
(* Module process.ml *)
let process_start () = ()
let process_stop () = ()
type process_config = {timeout: int}
(* In process.ml *)
let start () = ()
let stop () = ()
type config = {timeout: int}
(* Usage: Process.start (), Process.config *)
Functions prefixed with 'create_', 'make_', 'get_', or 'find_' can often omit the prefix when the remaining name is descriptive enough. For example, 'create_user' can be just 'user', 'make_widget' can be 'widget', 'get_name' can be 'name', and 'find_user' can be 'user' (returning option). Keep the prefix only when it adds meaningful distinction or when the bare name would be ambiguous. In modules, 'Module.create_module' should be 'Module.v'.
(* Bad examples - functions with redundant prefixes *)
(* Define necessary types *)
type user = { name : string; email : string; id : int }
type project = { name : string; title : string; description : string; status : string }
module Widget = struct
let make ~visible ~bordered = (visible, bordered)
let construct config = config
end
(* create_ prefix examples *)
let create_user name email = { name; email; id = 0 }
let create_project title = { name = ""; title; description = ""; status = "" }
let create_widget ~visible ~bordered = Widget.make ~visible ~bordered
(* get_ prefix examples *)
let get_name user = user.name
let get_email user = user.email
let get_status project = project.status
(* find_ prefix examples *)
let find_user_by_id users id = List.find_opt (fun u -> u.id = id) users
let find_project_by_name projects name = List.find_opt (fun p -> p.name = name) projects
(* Module.create_module pattern *)
module Progress = struct
type t = { current : int; total : int }
let create_progress current total = { current; total }
end
module User = struct
type t = { name : string; email : string }
let create_user name email = { name; email }
end
(* More aggressive cases that should be flagged *)
let create_temp_file prefix = Filename.temp_file prefix ".tmp"
let make_widget config = Widget.construct config
let get_current_time () = Unix.gettimeofday ()
let find_free_port start_port = start_port + 1 (* dummy implementation *)
let find_next_available_port start_port = find_free_port start_port
(* Good examples - functions without redundant prefixes *)
(* Define necessary types *)
type user = { name : string; email : string; id : int }
type project = { name : string; title : string; description : string; status : string }
module Widget = struct
let make ~visible ~bordered = (visible, bordered)
let construct config = config
end
module Filesystem = struct
let find path = path
end
module Http = struct
let get remote = remote
end
module Db = struct
let query db query = (db, query)
end
(* Direct constructor-style functions *)
let user name email = { name; email; id = 0 }
let project title = { name = ""; title; description = ""; status = "" }
let widget ~visible ~bordered = Widget.make ~visible ~bordered
(* Property accessors *)
let name user = user.name
let email user = user.email
let status project = project.status
(* Finders returning options *)
let user_by_id users id = List.find_opt (fun u -> u.id = id) users
let project_by_name projects name = List.find_opt (fun p -> p.name = name) projects
(* Good module patterns *)
module Progress = struct
type t = { current : int; total : int }
let v current total = { current; total }
let create current total = { current; total }
end
module User = struct
type t = { name : string; email : string }
let v name email = { name; email }
let create name email = { name; email }
end
(* Helper function *)
let construct_user data = data
(* These are fine - different semantic meanings *)
let build_user data = construct_user data (* build vs create *)
let build_widget config = Widget.construct config (* build vs make *)
let fetch_name remote = Http.get remote (* fetch vs get *)
let retrieve_email user = user.email (* retrieve vs get *)
let search_user db query = Db.query db query (* search vs find *)
let locate_file path = Filesystem.find path (* locate vs find *)
In OCaml modules, the idiomatic name for the primary constructor is 'v' rather than 'create' or 'make'. This follows the convention used by many standard libraries. For example, 'Module.create' should be 'Module.v'. This makes the API more consistent and idiomatic.
(* Bad examples - using create/make instead of v *)
module User = struct
type t = { name : string; email : string }
(* Should be 'v' *)
let create name email = { name; email }
end
module Widget = struct
type t = { id : int; label : string }
(* Should be 'v' *)
let make id label = { id; label }
end
module Config = struct
type t = { host : string; port : int }
(* Should be 'v' *)
let create ~host ~port = { host; port }
end
(* At top-level, the rule flags create/make *)
module Unix = struct
type file_descr = int
end
type connection = { socket : Unix.file_descr }
let create socket = { socket }
let make socket = { socket }
(* Good examples - using idiomatic 'v' constructor *)
module User = struct
type t = { name : string; email : string }
(* Idiomatic constructor *)
let v name email = { name; email }
end
module Widget = struct
type t = { id : int; label : string }
(* Idiomatic constructor *)
let v id label = { id; label }
(* Other constructors can have descriptive names *)
let parse_widget json = { id = 0; label = "" } (* dummy *)
let from_json json = parse_widget json
let empty = { id = 0; label = "" }
end
module Config = struct
type t = { host : string; port : int }
(* Idiomatic constructor *)
let v ~host ~port = { host; port }
(* Specialized constructors are fine *)
let default = { host = "localhost"; port = 8080 }
let getenv _ = "dummy"
let from_env () = { host = getenv "HOST"; port = int_of_string (getenv "PORT") }
end
(* At top-level, descriptive names are often better than 'v' *)
module Unix = struct
type file_descr = int
let socket = 0
end
type connection = { socket : Unix.file_descr }
let connection socket = { socket }
let connect ~host ~port = { socket = Unix.socket }
Standalone conversion functions in OCaml use the [<dst>_of_<src>] form ([int_of_string], [string_of_float], ...) — never [<src>_to_<dst>], [<dst>_from_<src>], [<X>_of_t], or [t_of_<X>]. The convention reads as 'an X out of a Y' and matches the stdlib precedent. The [to_<X>] prefix is reserved for conversions whose source is the module's own [t] ([List.to_seq], [Bytes.to_string]); use [<X>_of_<src>] when the source is anything else. The rule checks top-level declarations in [.ml] and [.mli] files. It only flags single-arrow functions ([T1 -> T2] with non-unit [T2]), so multi-argument actions ([add_to_set : 'a -> 'a list -> 'a list]) and writes-into-sink functions ([print_to_buffer : Buffer.t -> string -> unit]) are skipped by their type. Add domain-specific exceptions to [allowed_words].
(* Bad examples: single-arrow [T1 -> T2] with non-unit [T2] — these are
real conversions and should use the [<dst>_of_<src>] form. *)
let int_to_string : int -> string = fun n -> string_of_int n
let bytes_to_hex : bytes -> string = fun b ->
let buf = Buffer.create (Bytes.length b * 2) in
Bytes.iter (fun c -> Buffer.add_string buf (Printf.sprintf "%02x" (Char.code c))) b;
Buffer.contents buf
let path_to_uri : string -> string = fun p -> "file://" ^ p
let value_from_string : string -> int = fun s -> int_of_string s
let bytes_from_hex : string -> bytes = fun h -> Bytes.of_string h
(* Inside a module whose abstract type is [t], the [t] in conversion
names is implicit; [<X>_of_t] and [t_of_<X>] should collapse to
[to_<X>] and [of_<X>]. *)
type t = int
let int_of_t : t -> int = fun x -> x
let t_of_int : int -> t = fun n -> n
(* [to_<X>] is only canonical when the source is the module's [t] (or
one of [t]'s transparent aliases when the [.ml] omits an annotation).
A function whose source is unrelated to [t] is a disguised
[<src>_to_<X>] and must use [<X>_of_<src>] instead. *)
let to_pair : string -> string * string = fun s -> (s, s)
(* Good examples: [<dst>_of_<src>] form, matching stdlib's [int_of_string]. *)
let string_of_int' n = string_of_int n
let hex_of_bytes (b : bytes) =
let buf = Buffer.create (Bytes.length b * 2) in
Bytes.iter (fun c -> Buffer.add_string buf (Printf.sprintf "%02x" (Char.code c))) b;
Buffer.contents buf
let uri_of_path p = "file://" ^ p
let value_of_string s = int_of_string s
let bytes_of_hex h = Bytes.of_string h
(* These have [_to_]/[_from_] in the name but are NOT conversions by type:
the type-shape check excludes multi-arrow and unit-return functions. *)
let add_to_set s x = x :: s
(* val add_to_set : 'a list -> 'a -> 'a list — two arrows, not flagged. *)
let print_to_buffer buf x = Buffer.add_string buf x
(* val print_to_buffer : Buffer.t -> string -> unit — unit return, not flagged. *)
let recover_from_error e = ignore e
(* val recover_from_error : 'a -> unit — unit return, not flagged. *)
(* Inside a module whose type is [t]: the canonical conversion names are
[to_<X>] (source implicit [t]) and [of_<X>] (destination implicit [t]). *)
type t = int
let to_int (x : t) = x
let of_int n : t = n
(* When [type t = X] is a transparent alias and the [.ml] omits a type
annotation, Merlin infers the body as [X -> Y]. The rule still recognises
the canonical [to_<Y>] / [of_<X>] forms by tracking the alias declared at
the top of the file: [Tid.to_string] below is [string -> string] after
alias resolution, but [type t = string] makes that legitimate. *)
module Tid = struct
type t = string
let to_string s = s
let of_string s : t = s
end
(* Multiple sibling modules, each with their own [type t = X]. The alias
tracking is scoped per module: [Inner.to_int : t -> int] resolves to
[int -> int] (where [t = int] in Inner), and [Outer.to_string : t ->
string] resolves to [string -> string] (where [t = string] in Outer).
Neither cross-contaminates the other. *)
module Inner = struct
type t = int
let to_int t = t
let of_int n : t = n
end
module Outer = struct
type t = string
let to_string t = t
let of_string s : t = s
end
(* Aliases on parameterised types: [type t = entry list] should accept
both [list] and [entry] as legitimate source-type names. *)
type entry = { name : string }
type tree = entry list
(* A function on the parameterised alias is accepted as [t -> ...]
even when Merlin's inferred type uses the unfolded [entry list]. *)
module Tree = struct
type t = tree
let to_entries (t : t) = t
end
When every constructor of a variant, or every field of a record, starts with the same prefix, that prefix is redundant: the type already names the concept and OCaml's type-directed disambiguation resolves the bare name at call sites. Drop the shared prefix (type foo = Bar | Baz, not Foo_bar | Foo_baz; { bar; baz }, not { foo_bar; foo_baz }). When a bare field would be an OCaml keyword the convention is a trailing underscore (entity_type -> type_). Keep a prefix only when a bare case would be a spec-mandated name listed in allowed_words.
(* Bad examples - variants and records whose cases share a redundant prefix. *)
(* Every constructor repeats the type name as a prefix. *)
type status =
| Status_pending
| Status_running
| Status_done
(* Every field repeats the type name as a prefix. *)
type point = { point_x : int; point_y : int }
(* allowed_words exempts Token_eof; Token_word is still flagged. *)
type token =
| Token_word
| Token_eof
(* 'entity_type' would strip to the reserved keyword 'type'; the convention is a
trailing underscore, so the suggestion is 'type_'. *)
type entity = { entity_id : int; entity_type : string }
(* Good examples - cases carry no redundant shared prefix. *)
(* Bare constructors; the type name supplies the context. *)
type status =
| Pending
| Running
| Done
(* Bare fields. *)
type point = { x : int; y : int }
(* Shared leading letters but no underscore-delimited word in common. *)
type fruit =
| Apple
| Apricot
(* "start" and "end" are not shared by every field, so nothing is redundant. *)
type span = { start_line : int; start_col : int; end_line : int; end_col : int }
(* A single constructor cannot share a prefix with anything. *)
type wrapper = Wrapped of int
Bindings prefixed with underscore (like '_x') indicate they are meant to be unused. If you need to use the binding, remove the underscore prefix. If the binding is truly unused, consider using a wildcard pattern '_' instead. Note: PPX-generated code with double underscore prefix (like '__ppx_generated') is excluded from this check.
let _debug_mode = true in
if _debug_mode then
print_endline "Debug mode enabled"
let debug_mode = true in
if debug_mode then
print_endline "Debug mode enabled"
Pretty-printers — values of type [_ Fmt.t] or [Format.formatter -> _ -> unit] — follow a fixed naming convention: [pp] when the value type is the module's main type [t], and [pp_<type>] otherwise. The convention matches [Fmt.pp_print_*] and [Format.pp_print_*] in the stdlib.
type t = int
(* Bad: [print] is a printer-typed value but doesn't start with [pp]/[pp_]
or [dump]/[dump_]. *)
let print : int Fmt.t = fun fmt v -> Format.fprintf fmt "%d" v
(* Good: [dump_widget] is accepted as an in-tree convention for human-readable
diagnostic dumps. *)
type widget = { x : int }
let dump_widget fmt w = Format.fprintf fmt "%d" w.x
type t = int
let pp fmt v = Format.fprintf fmt "%d" v
type widget = { x : int }
let pp_widget fmt w = Format.fprintf fmt "%d" w.x
Using raw Error constructors with Fmt.str (including polymorphic variants like `Msg) can lead to inconsistent error messages. Consider creating error helper functions (prefixed with 'err_') that encapsulate common error patterns and provide consistent formatting. Place these error helpers at the top of the file to make it easier to see all the different error cases in one place.
let process_data x =
match x with
| 0 -> Error (Fmt.str "Invalid data: %d" x)
| n ->
if n > 100 then
Error (Fmt.str "Data too large: %d" n)
else Ok n
let process_string s =
if s = "" then
Error (`Msg (Fmt.str "Invalid string: '%s' is empty" s))
else if String.length s > 100 then
Error (`Msg (Fmt.str "String too long: %d characters" (String.length s)))
else
Ok s
(* Define error helpers at the top of the file *)
let err_invalid x = Error (Fmt.str "Invalid data: %d" x)
let err_too_large n = Error (Fmt.str "Data too large: %d" n)
let err_invalid_string s = Error (`Msg (Fmt.str "Invalid string: '%s' is empty" s))
let err_string_too_long len = Error (`Msg (Fmt.str "String too long: %d characters" len))
let process_data x =
match x with
| 0 -> err_invalid x
| n ->
if n > 100 then
err_too_large n
else Ok n
let process_string s =
if s = "" then
err_invalid_string s
else if String.length s > 100 then
err_string_too_long (String.length s)
else
Ok s
Functions with multiple boolean parameters are hard to use correctly. It's easy to mix up the order of arguments at call sites. Consider using variant types, labeled arguments, or a configuration record instead.
(* BAD - Boolean blindness *)
let create_widget visible bordered = ...
let w = create_widget true false (* What does this mean? *)
(* GOOD - Explicit variants *)
type visibility = Visible | Hidden
type border = With_border | Without_border
let create_widget ~visibility ~border = ...
let w = create_widget ~visibility:Visible ~border:Without_border
Exposing global mutable state in interfaces (.mli files) breaks encapsulation and makes programs harder to reason about. Instead of exposing refs or mutable arrays directly, provide functions that encapsulate state manipulation. This preserves module abstraction and makes the API clearer. Internal mutable state in .ml files is fine as long as it's not exposed in the interface.
(* Global mutable state - avoid this *)
let counter = ref 0
let incr_counter () = counter := !counter + 1
let global_cache = Array.make 100 None
let cached_results = Hashtbl.create 100
(* Good example - encapsulated state *)
(* Internal state - not exposed in interface *)
let counter = ref 0
let cache = Hashtbl.create 100
(* Functions that encapsulate state access *)
let init () =
counter := 0;
Hashtbl.clear cache
let increment () =
incr counter;
!counter
let get_count () = !counter
let cache_get key =
try Some (Hashtbl.find cache key)
with Not_found -> None
let cache_set key value =
Hashtbl.replace cache key value
let cache_clear () =
Hashtbl.clear cache
let process _ _ = 0
let build _ = [||]
let update_counter _ = ()
MLI files should start with a documentation comment (** ... *) that describes the module's purpose and API. This helps users understand how to use the module.
(* user.mli - no module doc *)
type t
val create : string -> t
(** User management module
Handles user creation and authentication. *)
type t
val create : string -> t
All public values should have documentation explaining their purpose and usage. Add doc comments (** ... *) before or after value declarations in .mli files.
type t
val parse : string -> t
(** Documentation after is also acceptable *)
val format : t -> string
val missing_documentation : int -> int
type t
(** [parse str] converts a string to type [t].
@raise Invalid_argument if [str] is malformed. *)
val parse : string -> t
val format : t -> string
(** [format t] converts a value of type [t] to a string representation. *)
(** [process n] processes an integer value. *)
val process : int -> int
Follow OCaml documentation conventions: when documentation starts with [name ...], [name] should be the function or value being documented. Operators should use infix notation like '[x op y] description.' All documentation should end with a period. Avoid redundant phrases like 'This function...'.
val default : t
(** The default configuration. *)
val is_bot : t -> bool
(** [is_bot u] is [true] if [u] is a bot user. *)
type id = string
(** A user identifier. *)
val default : t
(** [default] is the default configuration. *)
The main type 't' should implement a pretty-printer function (pp) for better debugging and logging. Unlike equality and comparison which can use polymorphic functions (= and compare), pretty-printing requires a custom implementation to provide meaningful output.
type t = { id: int; name: string }
type t = { id: int; name: string }
val pp : Format.formatter -> t -> unit
When documentation mentions another exported API item, use an odoc cross-reference link such as {!type-t}, {!val-v}, {!module-M}, {!module-type-S}, {!exception-E}, {!extension-X}, {!constructor-C}, {!field-f}, {!class-c}, {!class-type-c}, {!method-m}, or {!instance-variable-v}. Keep [x] for code literals and documented arguments.
Every package needs its own .ocamlformat so each standalone subtree formats consistently, not just the umbrella root. Create one in each directory the linter flags.
Library modules should have corresponding .mli files for proper encapsulation and API documentation. Create interface files to hide implementation details and provide a clean API.
type t = { name: string; id: int }
let create name id = { name; id }
let name t = t.name
(* user.mli *)
type t
val create : string -> int -> t
val name : t -> string
Modules that use logging should declare a log source for better debugging and log filtering. Add 'let src = Logs.Src.create "module.name" ~doc:"..."'
let log_src = Logs.Src.create "project_name.module_name"
module Log = (val Logs.src_log log_src : Logs.LOG)
Log.info (fun m ->
m "Received event: %s" event_type
~tags:(Logs.Tag.add "channel_id" channel_id Logs.Tag.empty))
Libraries and tests should be in separate directories for clear project structure. Put library code in lib/ and tests in test/. Using explicit (modules ...) to co-locate them in the same directory is discouraged.
The monorepo convention is lib/ for library code. Rename src/ to lib/ with `git mv`; no dune changes are needed because dune auto-discovers modules in either directory.
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)).
Rename <pkg>/lib/<pkg>_foo.ml to <pkg>/lib/foo.ml (and update the .mli similarly). Dune's default wrapped mode will expose it as <Pkg>.Foo. For something that really needs its own public name, create a sublib directory (<pkg>/lib_foo/ with its own dune) instead.
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.
Each Cmd.v subcommand should live in its own file. Move each Cmd.v into a sibling file (e.g. cmd_<name>.ml exposing a single val cmd) and reference it from main.ml's Cmd.group. Sub-subcommands of a grouped subcommand follow the same rule — use cmd_<parent>/<leaf>.ml or cmd_<parent>_<leaf>.ml siblings.
Create <package>/dune containing (env (dev (flags :standard %{dune-warnings}))), and bump <package>/dune-project to (lang dune 3.21) or newer. This mirrors the workspace-root dune so that a standalone opam build of the package still enforces strict warnings under the dev profile. Reference: alcobar/dune.
Add (implicit_transitive_deps false) to <package>/dune-project (or (implicit_transitive_deps false-if-hidden-includes-supported) if you need to keep compatibility with OCaml < 5.2). Then audit each (libraries ...) clause to list any transitive deps the package actually uses directly. This makes (re_export ...) meaningful again and prevents deps from leaking into downstream opam depends via META requires.
Enforces proper test organization: (1) Test executables (test.ml) should use test suites from test modules (e.g., Test_user.suite) rather than defining their own test lists directly. (2) Test module interfaces (test_*.mli) should only export a 'suite' value with type 'string * unit Alcotest.test_case list' and no other values. (3) Alcotest.run should only appear in test.ml, not in individual test_*.ml modules.
This rule is not a checkbox exercise. The goal is real test coverage and real code quality, not a [test_<module>.ml] file that satisfies the linter. Treat each untested module as an opportunity: write the tests you'd want a reviewer to write before you trusted the code in production. Anchor on the spec when one exists -- cite section numbers, copy test vectors verbatim. Probe the edge cases the implementation hopes you won't try: empty / single-element / maximum-length inputs, NaN, malformed UTF-8, negative sizes, off-by-one boundaries, integer overflow. Drive at least one hostile case (random bytes, truncated payloads, malicious lengths); the public API should fail with a clear error, not crash or corrupt state. Use exact expected values, not loose ranges. A trivial module still deserves the exercise -- writing the tests usually surfaces the corner the author didn't think through.
Test files for different libraries should not be mixed in the same test directory. Organize test files so that each test directory contains tests for only one library to maintain clear test organization.
let test_utils () = assert true
let () = test_utils ()
(** Good: Test is in generic 'test' stanza which is allowed *)
let parse s = int_of_string s
Test stanzas without declared library dependencies should not contain test files for multiple different libraries. Split tests into separate test stanzas, one per library.
Every test module should have a corresponding library module. This ensures that tests are testing actual library functionality rather than testing code that doesn't exist in the library.
All test modules should be included in the main test runner (test.ml). Add the missing test suite to ensure all tests are run.
In test files, use Alcotest.failf or failf instead of Alcotest.fail (Fmt.str ...) or fail (Fmt.str ...). The failf function provides printf-style formatting directly, making the code more concise and readable.
let test_parse () =
match parse input with
| Error e -> Alcotest.fail (Fmt.str "Parse error: %s" (Json.Error.to_string e))
| Ok _ -> ()
let test_invalid () =
if not (is_valid data) then
fail (Fmt.str "Invalid data: %a" pp_data data)
let test_parse () =
match parse input with
| Error e -> Alcotest.failf "Parse error: %s" (Json.Error.to_string e)
| Ok _ -> ()
let test_invalid () =
if not (is_valid data) then
failf "Invalid data: %a" pp_data data
Test suite names should follow these conventions: (1) Use lowercase snake_case for the suite name. (2) The suite name should match the test file name - for example, test_foo.ml should have suite name 'foo'. This makes it easier to identify which test file contains which suite.
(* In test_config.ml *)
let suite = ("Config", tests) (* Wrong: uppercase *)
(* In test_user_auth.ml *)
let suite = ("auth", tests) (* Wrong: doesn't match filename *)
(* In test_parser.ml *)
let suite = ("parser-tests", tests) (* Wrong: not snake_case *)
(* In test_config.ml *)
let suite = ("config", tests)
(* In test_user_auth.ml *)
let suite = ("user_auth", tests)
(* In test_parser.ml *)
let suite = ("parser", tests)
All .ml files in a test/ directory should follow the test_ naming convention (e.g., test_parser.ml) or be the test runner (test.ml). Non-test modules should either be extracted into a private (library ...) stanza or renamed to test_<module>.ml.
Each test directory should have exactly one test stanza with a single test runner (test.ml). Multiple test stanzas in the same directory cause module ownership conflicts and break @check builds.
An empty test suite provides no value. Tests should: (1) cover the module's public API with representative inputs, (2) exercise boundary conditions and edge cases, (3) verify error handling and invalid inputs, (4) document expected behaviour through concrete examples. A suite with no test cases will never catch regressions.
let suite = ("parser", []) (* no tests: regressions go undetected *)
let suite =
( "parser",
[
Alcotest.test_case "parses valid input" `Quick test_parse_valid;
Alcotest.test_case "rejects empty input" `Quick test_parse_empty;
Alcotest.test_case "handles malformed data" `Quick test_parse_malformed;
] )
In a multi-package dune project, test stanzas must say which package owns them with [(package PKG)]. Without that field, project-index cannot safely attribute test modules or test dependency scopes. Add [(package <package-name>)] rather than relying on default-package inference.
The fuzz runner (fuzz.ml) should collect Fuzz_*.suite from each fuzz module rather than defining test_case directly. This keeps fuzz tests organized per-module.
Fuzz modules (fuzz_*.ml) should have corresponding .mli files that export only 'suite : string * Alcobar.test_case list'. This enforces proper encapsulation of fuzz test internals.
Every fuzz module (fuzz_<module>.ml) should have a corresponding library module (<module>.ml). This ensures fuzz tests are testing actual library functionality.
All fuzz modules should be included in the fuzz runner (fuzz.ml) via Fuzz_*.suite references. This ensures all fuzz tests are actually executed.
All .ml files in a fuzz/ directory should follow the fuzz_ naming convention (e.g., fuzz_parser.ml) or be the fuzz runner (fuzz.ml). Each fuzz directory must use fuzz.exe --gen-corpus in its dune rule.
Each fuzz directory should have exactly one executable stanza with a single fuzz runner (fuzz.ml). Use (modules ...) to list all fuzz modules in a single stanza.
Fuzz directories should be at the same level as test directories (siblings), not nested inside them. Move fuzz/ to be a sibling of test/.
Fuzz targets should use (executable ...) stanzas with explicit (rule (alias runtest) ...) and (rule (alias fuzz) ...) rules, not (test ...) stanzas. This enables property-based testing during dune test and separate AFL campaign workflows.
Each fuzz directory should have (rule (alias runtest) ...) for property-based testing during dune test, and (rule (alias fuzz) ...) using fuzz.exe --gen-corpus for AFL fuzzing campaigns.
Fuzz tests must declare let suite = ("<module>", [...]) where <module> matches the filename: fuzz_<module>.ml should use suite:"<module>".
An empty fuzz suite provides no coverage. Fuzz tests should: (1) test crash safety of all parsers and decoders on arbitrary input, (2) verify roundtrip invariants (decode(encode(x)) = x), (3) exercise state machine transitions including invalid ones, (4) cover boundary conditions and edge cases that unit tests miss. A suite with no test cases will never find bugs.
let suite = ("parser", Alcobar.[]) (* no fuzz tests: bugs go undetected *)
let suite =
( "parser",
Alcobar.
[
test_case "parse crash safety" [ bytes ] test_parse;
test_case "roundtrip" [ bytes ] test_roundtrip;
test_case "boundary: empty input" [ const () ] test_empty;
] )
In a multi-package dune project, the rules that attach a private fuzz runner to the runtest and fuzz aliases must say which package owns them with [(package PKG)]. The executable stays private; put the package field on the [(rule (alias runtest) ...)] and [(rule (alias fuzz) ...)] stanzas.
Every interop test must have scripts/generate.sh as the single entry point for trace regeneration via `dune build @traces`.
Interop test directories should be named after the oracle tool (e.g. spacepackets, dariol83, crcmod), not the language (e.g. python, go). This makes it clear which external implementation is the reference.
Traces must be committed to git. The test stanza must depend on (source_tree traces) so tests run from traces alone — the external tool is NOT required at test time. This is the 'generate once, replay always' principle.
Interop tests must run from committed traces without needing the external tool at test time. The test.ml should only read trace files, never shell out to run the oracle. If you need the oracle, put it in the generator script.
Python oracles must pin dependencies in requirements.txt with exact versions (e.g. crcmod==1.7). This ensures reproducible trace generation without depending on local installs.
Go oracles must pin the upstream module in go.mod with a tagged version or pseudo-version. This ensures reproducible trace generation without depending on $GOPATH or local clones.
Rust oracles must pin the upstream crate in Cargo.toml with a tagged version or git rev. This ensures reproducible trace generation without depending on local checkouts.
traces/dune must define the trace-regeneration rule as `(rule (alias regen) (mode promote) (enabled_if (= %{env:REGEN=0} 1)) ...)`, the single trigger for refreshing traces: `REGEN=1 dune build @regen`.
The rule that regenerates traces must use (mode promote) guarded by (enabled_if (= %{env:REGEN=0} 1)), living in traces/dune under (alias regen), so a normal `dune test` leaves the traces as committed source and never runs the external oracle. Regenerate with `REGEN=1 dune build @regen`.
scripts/generate.sh must take the output directory as its first argument, e.g. `TRACE_DIR="$(cd "${1:-$SCRIPT_DIR/../traces}" && pwd)"`. The traces/dune regen rule runs it with the build trace dir as $1; a script that hardcodes ../traces writes to the wrong place.
Use csv (Csv.decode_file with a Csv.Row codec) for CSV trace parsing. Never hand-roll CSV readers with open_in/input_line/split_on_char.
Interop tests with CSV traces should use csv for parsing. Add csv to the (libraries ...) in the dune file and use Csv.decode_file with a Row codec.
The generator MUST call the upstream tool's public API. Never reimplement the algorithm being verified — this defeats the purpose of interop testing. If the public API doesn't expose what you need, drop the test rather than inlining.
Python deps must live in a venv. Never use pip install --break-system-packages. The generate.sh wrapper should create/reuse a venv automatically.
Add a c/ directory whose gen.ml calls Wire_3d.main ~mode:`Standalone to project the Wire codecs into a single <Name>.3d EverParse spec and validator, and wire it into the build with a dune that compiles gen and includes the generated dune.inc. See ocaml-clcw/c/ for the pattern.
Move struct_, module_, c_stubs, ml_stubs out of the .mli. These belong in c/gen.ml where they are used to generate EverParse 3D files and C stubs. The codec is the public API; the 3D projection is a build artifact.
Add x-quality field to *.opam.template to declare required quality features. Merlint checks that declared features actually exist.
Organise each opam package into the standard top-level component directories: lib/ for libraries, bin/ for executables, test/ for tests, fuzz/ for fuzzers, bench/ for benchmarks, c/ for C codegen and examples/ for example programs. A scripts/ directory may hold private helper executables (codegen, dune-configurator probes); anything with a public_name belongs in bin/. Sub-components nest under those roots (lib/<name>/ with test/<name>/, IO adapters in lib/eio/), never as new top-level directories (foo/lib/, foo/test/).
Add the human-facing metadata files every opam package ships at its source root: a README.md describing the package and a license file (LICENSE.md). These travel with the package when it is split out to its own repository.
Runs only when the project root has a sources.toml (monopam monorepo marker) or a categories.toml (the tag vocabulary). When it does run, every *.opam must declare tags: ["org:<your-org>" "<topic>" ...] where each topic is a slug from categories.toml / merlint.toml's topics list. Edit the package's dune-project so dune regenerates the opam file.
When a README.md, .mli or .mld contains OCaml code examples, add an (mdx (files <file>)) stanza to the owning dune file so the snippets are type-checked and run during dune test.
A README.md, .mli, or .mld file contains an mdx-error block produced by [dune promote] after a failing mdx run. The error output belongs in a [.corrected] file for review, not in the committed source. Fix the underlying example so it type-checks and remove the block.
In a multi-package dune project, MDX stanzas must say which package owns the documentation check with [(package PKG)]. Dune attaches MDX to runtest automatically; the package field is for ownership and package-filtering, not alias plumbing.
Do not use [$MDX skip] on OCaml or ML examples in README.md, .mld, or .mli files. These snippets are user-facing copy/paste material. Fix the setup, split hidden support code into earlier MDX blocks, or make the example smaller, but keep it type-checked.
Any package tagged codec.* or protocol must follow the I/O-free contract. The org standardises on Eio: no package may depend on lwt, miou, or mirage runtimes. Pure I/O-free packages (codec.* / protocol without an eio tag) must additionally not depend on eio*, unix, or ambient clocks.
I/O-free packages (tagged [codec.*] or [protocol]) must not read the wall clock or monotonic clock from library code. Time is an input, not a side effect: take [~now] as a parameter, let the caller's adapter or CLI [bin/] call [Mtime_clock.now] / [Ptime_clock.now] / [Unix.gettimeofday] / [Sys.time] and pass the value in. Library attribution follows dune's [(public_name P.X)] via [Project_index] so a sibling adapter package is scanned against its own tags, not its I/O-free sister's.
Every package tagged [protocol] must depend on [probe], and each public protocol library in that package must link [probe] in its [(libraries ...)] stanza. Protocol libraries should declare a closed [Event.t] vocabulary and expose [Event.emit_probe] so adapters can publish typed Runtime_events probes without owning an in-process subscription API.
Every project (or subtree) should enable [%{dune-warnings}] on the dev profile so warnings are uniform across builds. Add the stanza [(env (dev (flags :standard %{dune-warnings})))] to the top-level [dune] file. Note [%{dune-warnings}] requires [(lang dune 3.21)] or newer in the corresponding [dune-project].
When a library in your package's [(libraries L)] resolves to opam package P, P must appear in your package's [depends:]. This includes libraries linked by public executables, since [opam install] builds those through [@install]. Otherwise [opam install] from a fresh switch fails for downstream users -- your local build only works because P happens to be in the active switch. The fix depends on how you author opam metadata: if you hand-write [<pkg>.opam], add the package to its [depends:]; if you let dune generate [<pkg>.opam] from [dune-project], add it to the [(package (depends ...))] stanza (use [<pkg>.opam.template] only for fields dune can't generate). Builtin libraries (unix, str, threads, ...), build-tool packages dune resolves separately (ocaml, dune, js_of_ocaml), [conf-*] system-library wrappers, and libraries owned by the package itself are exempt.
This package links or declares a dependency banned by the [disallowed_libraries] list in merlint.toml. Drop the dependency, or relax the ban. The list is empty by default; set it (e.g. [disallowed_libraries = ["fmt"]]) to enforce a build-level boundary, the companion to E221's source-level module ban.
A dep used only from [(test ...)] / [(tests ...)] or a runtest-attached private executable belongs in [:with-test]. A dep used only from a private executable that's not attached to [runtest] (generator, benchmark, dev tool) belongs in [:with-dev-setup]. Add the appropriate filter: in a hand-written [<pkg>.opam], wrap the entry as ["alcotest" {with-test}] or ["bench" {with-dev-setup}]; in a [dune-project] [(package (depends ...))] stanza, write [(alcotest :with-test)] or [(bench :with-dev-setup)]. A dep not reached by any stanza in this package should be removed from this package; a sibling executable must declare its own dependency. Build-tool packages dune resolves separately (dune, dune-configurator, js_of_ocaml, ocaml) and [conf-*] system wrappers are exempt.
When you use [X.suffix] from package X, X's installation might have skipped that sub-library because the system dep it gates wasn't available at X's build time. The gating dep is one of X's opam [depopts:]. Declare it in your own [depends:] so opam-install picks it up in a fresh switch. The (parent, sub) → depopt mapping lives in [merlint/lib/rules/e944.ml]'s [gating_table]; extend it when you find a new case. Each entry is verified against X's actual depopts at run time; stale entries surface as findings too.
One layering for codecs and protocols: the AST (Value for data, Message for a protocol) is the base, and the typed Codec depends on it -- never the reverse. An AST module that references its sibling Codec inverts the order. A protocol is a codec plus a state machine, so message.ml follows the same rule. Dependency order only -- which parser the codec uses is unconstrained.
A protocol package (tagged protocol) is a codec plus an I/O-free state machine. Expose that state machine as a State module, or a role pair (Client/Server, Sender/Receiver, Initiator/Responder, Requester/Responder) for asymmetric protocols. A package whose machines use other names lists them in allowed_states in merlint.toml, which replaces the default vocabulary. Purity of the core is E930; AST/codec layering is E945.
A protocol's state machine is a pure value, so its state type carries no mutable field and no bytes / ref / array. Mutable scratch (decrypt ring, reused buffer, dynamic table) is passed in as a borrowed ~rx argument or lives in the I/O adapter, never in the state. See E946 for the state-machine module, E948 for verb names, E949 for one machine per module.
A state-machine module uses the canonical verb vocabulary (v, client, server, incoming, outgoing, close, timer, next_timeout). The anti-pattern synonyms parse_* / process_* / eat_* and bare send / recv / receive / read / write / step / make / create / init / shutdown are rejected -- each maps to a canonical verb. See E946 for the module, E947 for immutable state, E949 for one machine per module.
A state-machine module holds exactly one state-machine type t. Two roles are two top-level modules (Client/Server, Sender/Receiver, ...), not nested module Sender / module Receiver in one state.ml. See E946 for the module, E947 for immutable state, E948 for verb names.
A protocol's state machine is total: it returns errors as values, never raises. In a state-machine module, reject bad input with an Error rather than raise / failwith / invalid_arg. See E946-E949 for the rest of the protocol shape.
A catch-all arm over the protocol message type must reject the message (| _ -> Error _, or | s -> Error (`Unexpected s)), not silently accept it by returning a normal value (Ok / a state / unit). A silent accept is the FREAK/SKIP-class bug: the machine keeps going on a message it should have rejected. See E946-E950 and E952 for the rest of the protocol shape.
A protocol transition (handle / incoming / outgoing / close / timer) returns a result carrying the new state, output, events, and an Error on bad input -- never unit, which forces in-place mutation and raising. See E947 (immutable state) and E950 (total transitions) for the related invariants.
A data-codec package (tagged codec) exposes its top-level entry points from a fixed vocabulary: of_string / to_string / of_reader / to_writer for I/O, and decode / encode over the AST, each of_/decode with an _exn twin (never a ' variant). The bare synonyms parse / from_string / unmarshal / print / unparse / marshal / read / input / write / output are rejected -- each maps to a canonical verb. The prefixed parse_* / print_* helpers are internal and left alone. See E945 for the codec/AST layering, E948 for the protocol verb vocabulary.
A protocol's state machine exposes both an inbound verb (incoming) and a constructor (v, or a role constructor: client / server / sender / receiver / initiator / responder / requester), so a reviewer can always find where the machine is built and where it steps. See E946 for the module being present, E948 for the verb names, E950 for total transitions.
A raising variant of a codec entry point is named with the _exn suffix (of_string_exn), never a ' suffix (of_string'). The ' suffix is reserved for a format-native keyword escape (object', the JSON object sort) or a pp / pp' configuration-variant pair; there are no built-in exceptions, so any ' name a codec package keeps must be documented in allowed-names in its merlint.toml. See E953 for the encoding verb vocabulary.