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: E351 uses the typed signature from .cmti (not the outline string)

Replaces the regex-on-type_sig heuristic with a typed walk of the
.cmti signature. The rule now flags a value iff the head of its
[Types.type_expr] resolves to [Stdlib.array] or [Stdlib.ref] -- so

type 'a ref = 'a list
val x : int ref

is correctly ignored (the local [ref] shadows the stdlib one), while
the previous string check would have fired on the literal " ref"
suffix.

Implementation:
- [Context.cmt : file -> Cmt_format.cmt_infos option] is a new lazy
field that reads the .cmti via [Merlin.Project.cmt]. First rule
consuming it is E351; others (E100 etc.) still use the text dump
from [Context.dump]. The field is cached so multiple rules share
a single cmt load.
- [rules/e351.ml] walks [Tsig_value] items and matches [Types.get_desc
vd.val_val.val_type] against [Predef.path_array] and
(since ref isn't a predef) against [Path.name] equal to ["ref"] /
["Stdlib.ref"].
- [lib/dune] adds [merlin-lib.ocaml_typing] so the rule can reference
[Ocaml_typing.Cmt_format]/[Types]/[Typedtree]/[Path]/[Predef].

+71 -35
+13
lib/context.ml
··· 15 15 outline : Outline.t Lazy.t; 16 16 content : string Lazy.t; 17 17 functions : (string * Ast.expr) list Lazy.t; 18 + cmt : Ocaml_typing.Cmt_format.cmt_infos option Lazy.t; 18 19 } 19 20 20 21 type project = { ··· 55 56 Log.debug (fun m -> 56 57 m "Context: extracted %d functions" (List.length ast)); 57 58 ast); 59 + cmt = 60 + lazy 61 + (match Merlin.Project.cmt filename with 62 + | None -> None 63 + | Some cmt_path -> ( 64 + try Some (Ocaml_typing.Cmt_format.read_cmt cmt_path) 65 + with exn -> 66 + Log.debug (fun m -> 67 + m "Context: failed to read cmt for %s: %s" filename 68 + (Printexc.to_string exn)); 69 + None)); 58 70 } 59 71 60 72 let test_module_of_file f = ··· 132 144 let outline ctx = Lazy.force ctx.outline 133 145 let content ctx = Lazy.force ctx.content 134 146 let functions ctx = Lazy.force ctx.functions 147 + let cmt ctx = Lazy.force ctx.cmt 135 148 136 149 (* Project context accessors *) 137 150 let all_files ctx = Lazy.force ctx.all_files
+9
lib/context.mli
··· 16 16 content : string Lazy.t; (** File content (lazy). *) 17 17 functions : (string * Ast.expr) list Lazy.t; 18 18 (** Functions extracted from parsetree (lazy). *) 19 + cmt : Ocaml_typing.Cmt_format.cmt_infos option Lazy.t; 20 + (** Typed signature/structure from [_build]'s [.cmt]/[.cmti] (lazy). 21 + [None] when the project hasn't been built yet or when the cmt 22 + file cannot be read. *) 19 23 } 20 24 21 25 type project = { ··· 64 68 65 69 val functions : file -> (string * Ast.expr) list 66 70 (** [functions file] returns functions field. *) 71 + 72 + val cmt : file -> Ocaml_typing.Cmt_format.cmt_infos option 73 + (** [cmt file] returns the parsed [.cmt]/[.cmti] for [file] when 74 + available, [None] otherwise. The result is cached on the context so 75 + repeated rule queries share a single read. *) 67 76 68 77 (** {2 Project context accessors} *) 69 78
+1
lib/dune
··· 6 6 (libraries 7 7 ocaml-merlin 8 8 compiler-libs.common 9 + merlin-lib.ocaml_typing 9 10 eio 10 11 re 11 12 logs
+48 -35
lib/rules/e351.ml
··· 1 - (** E351: Detection of global mutable state patterns *) 1 + (** E351: Detection of global mutable state patterns. 2 + 3 + Walks the {!Ocaml_typing.Typedtree.signature} produced from the [.cmti] 4 + and checks every [Sig_value] whose {!Ocaml_typing.Types.type_expr} head 5 + resolves to [Predef.path_array] or [Predef.path_ref]. Because the 6 + check is on the fully-resolved {!Ocaml_typing.Path.t}, local type 7 + definitions that happen to be called [ref] or [array] do {e not} 8 + shadow the rule: only actual [Stdlib.array]/[Stdlib.ref] values are 9 + flagged. *) 2 10 3 11 type payload = { kind : string; name : string } 4 12 (** Payload for mutable state issues *) 5 13 6 - (* Precompiled regexes for efficiency *) 7 - let ref_type_re = Re.compile (Re.alt [ Re.str " ref"; Re.str "= ref" ]) 8 - let array_type_re = Re.compile (Re.str " array") 14 + module Types = Ocaml_typing.Types 15 + module Typedtree = Ocaml_typing.Typedtree 16 + module Path = Ocaml_typing.Path 17 + module Predef = Ocaml_typing.Predef 9 18 10 - (** Check if a type signature indicates mutable state *) 11 - let is_mutable_type type_sig = 12 - (* Skip function types that return refs/arrays *) 13 - if Re.execp (Re.compile (Re.str "->")) type_sig then false 14 - else 15 - (* Check for ref types - look for patterns like "int ref" or "= ref" *) 16 - Re.execp ref_type_re type_sig 17 - (* Check for array types *) 18 - || Re.execp array_type_re type_sig 19 - || 20 - (* Check for mutable record fields - this is harder to detect from type sig *) 21 - false 19 + let is_stdlib_ref p = 20 + (* [ref] is not a predef -- it's a regular record type in [Stdlib], so 21 + we compare against the qualified name directly. *) 22 + let n = Path.name p in 23 + n = "ref" || n = "Stdlib.ref" 24 + 25 + let mutable_kind_of_type_expr te = 26 + (* [Types.get_desc] unwraps [Tlink]s and other indirections without 27 + forcing an environment; the head constructor is what we need. *) 28 + match Types.get_desc te with 29 + | Tconstr (p, _, _) when Path.same p Predef.path_array -> Some "array" 30 + | Tconstr (p, _, _) when is_stdlib_ref p -> Some "ref" 31 + | _ -> None 32 + 33 + let location_of_cmt (loc : Ocaml_utils.Warnings.loc) = 34 + let loc_start = loc.loc_start and loc_end = loc.loc_end in 35 + Merlin.Location.v ~file:loc_start.Lexing.pos_fname 36 + ~start_line:loc_start.Lexing.pos_lnum 37 + ~start_col:(loc_start.Lexing.pos_cnum - loc_start.Lexing.pos_bol) 38 + ~end_line:loc_end.Lexing.pos_lnum 39 + ~end_col:(loc_end.Lexing.pos_cnum - loc_end.Lexing.pos_bol) 22 40 23 - (** Check outline for global mutable state *) 24 - let check_global_mutable_state ~filename outline = 41 + let check_signature (signature : Typedtree.signature) = 25 42 List.filter_map 26 - (fun (item : Outline.item) -> 27 - match item.kind with 28 - | Outline.Value -> ( 29 - match (item.type_sig, Outline.location filename item) with 30 - | Some type_sig, Some location when is_mutable_type type_sig -> 31 - let kind = 32 - if Re.execp ref_type_re type_sig then "ref" 33 - else if Re.execp array_type_re type_sig then "array" 34 - else "mutable" 35 - in 36 - Some (Issue.v ~loc:location { kind; name = item.name }) 37 - | _ -> None) 43 + (fun (item : Typedtree.signature_item) -> 44 + match item.sig_desc with 45 + | Tsig_value vd -> ( 46 + match mutable_kind_of_type_expr vd.val_val.val_type with 47 + | None -> None 48 + | Some kind -> 49 + let loc = location_of_cmt vd.val_loc in 50 + Some (Issue.v ~loc { kind; name = vd.val_name.txt })) 38 51 | _ -> None) 39 - outline 52 + signature.sig_items 40 53 41 54 let check (ctx : Context.file) = 42 - (* Only check .mli files - mutable state in .ml files is fine if not exposed *) 43 55 if not (String.ends_with ~suffix:".mli" ctx.filename) then [] 44 56 else 45 - let outline_data = Context.outline ctx in 46 - let filename = ctx.filename in 47 - check_global_mutable_state ~filename outline_data 57 + match Context.cmt ctx with 58 + | Some { cmt_annots = Interface signature; _ } -> 59 + check_signature signature 60 + | _ -> [] 48 61 49 62 let pp ppf { kind; name } = 50 63 Fmt.pf ppf