Improve DisallowedModuleRule and DisallowedFunctionRule (#13)

* Support for qualified identifiers

* Better handle of module rule

* Update tests

* Bump version

* Run ocamlformat

Author: shulhi

Changelog.md

@@ -1,5 +1,8 @@
 # ReScript Linter Changelog
 
+### 2026-02-02 - v0.4.1
+* Update DisallowModuleRule and DisallowedFunctionRule
+
 ### 2026-01-27 - v0.4.0
 * Updated the AST to ReScript v12.1.0
 

dune-project

@@ -2,7 +2,7 @@
 
 (name rescript_linter)
 
-(version 0.4.0)
+(version 0.4.1)
 
 (generate_opam_files true)
 

lib/rules/DisallowModuleRule.ml

@@ -20,6 +20,29 @@ module Make (OPT : Rule.OPTIONS with type options = Options.options) (LinterOpti
     ; Rule.ruleIdentifier= "DisallowModule" ^ "[" ^ op ^ "]"
     ; Rule.ruleDescription= description }
 
+  (* Helper function to convert Longident to string *)
+  let rec longident_to_string = function
+    | Longident.Lident s -> s
+    | Longident.Ldot (t, s) -> longident_to_string t ^ "." ^ s
+    | Longident.Lapply (a, b) -> longident_to_string a ^ "(" ^ longident_to_string b ^ ")"
+
+  (* Extract the module path from a Longident (everything except the last component) *)
+  let extract_module_path = function
+    | Longident.Lident _ -> None
+    | Longident.Ldot (t, _) -> Some (longident_to_string t)
+    | Longident.Lapply (_, _) -> None
+
+  (* Check if a module path matches the disallowed module
+     - Exact match: "Belt" matches "Belt"
+     - Prefix match: "Belt" matches "Belt.List", "Belt.Array", etc.
+     - But "Belt" should NOT match "BeltExtra"
+  *)
+  let matches_disallowed_module module_path =
+    module_path = op
+    || String.length module_path > String.length op
+       && String.sub module_path 0 (String.length op) = op
+       && String.get module_path (String.length op) = '.'
+
   (* There are three cases that we need to handle when linting for module usage (assume M is the module name)
 
      1. open M
@@ -46,36 +69,50 @@ module Make (OPT : Rule.OPTIONS with type options = Options.options) (LinterOpti
     Rule.LintStructureItem
       (fun expr ->
         match expr with
-        (* match open M *)
+        (* match open M or open M.N *)
         | { Parsetree.pstr_desc=
-              Parsetree.Pstr_open
-                {Parsetree.popen_lid= {txt= Longident.Lident ident}; Parsetree.popen_loc= loc} }
-          when ident = op ->
+              Parsetree.Pstr_open {Parsetree.popen_lid= {txt= ident}; Parsetree.popen_loc= loc} }
+          when matches_disallowed_module (longident_to_string ident) ->
             Rule.LintError (meta.ruleDescription, loc)
-        (* match J = M *)
+        (* match J = M or J = M.N *)
         | { Parsetree.pstr_desc=
               Parsetree.Pstr_module
                 { Parsetree.pmb_expr=
-                    { Parsetree.pmod_desc= Parsetree.Pmod_ident {txt= Longident.Lident ident}
-                    ; Parsetree.pmod_loc= loc } } }
-          when ident = op ->
+                    {Parsetree.pmod_desc= Parsetree.Pmod_ident {txt= ident}; Parsetree.pmod_loc= loc} } }
+          when matches_disallowed_module (longident_to_string ident) ->
             Rule.LintError (meta.ruleDescription, loc)
         | _ -> Rule.LintOk )
 
   let lintExpression =
     Rule.LintExpression
       (fun expr ->
         match expr with
-        (* match M.function or M.attribute *)
+        (* match M.function or M.N.function in function calls *)
         | { Parsetree.pexp_desc=
-              Pexp_apply
-                { funct=
-                    { pexp_desc= Pexp_ident {txt= Longident.Ldot (Longident.Lident ident, _)}
-                    ; Parsetree.pexp_loc= loc }
-                ; args= _ } }
-          when ident = op ->
+              Pexp_apply {funct= {pexp_desc= Pexp_ident {txt= ident}; Parsetree.pexp_loc= loc}; args= _} }
+          when match extract_module_path ident with
+               | Some module_path -> matches_disallowed_module module_path
+               | None -> false ->
+            Rule.LintError (meta.ruleDescription, loc)
+        (* match M.Constructor or M.N.Constructor like Belt.Result.Ok *)
+        | {Parsetree.pexp_desc= Pexp_construct ({txt= ident; _}, _); Parsetree.pexp_loc= loc}
+          when match extract_module_path ident with
+               | Some module_path -> matches_disallowed_module module_path
+               | None -> false ->
+            Rule.LintError (meta.ruleDescription, loc)
+        | _ -> Rule.LintOk )
+
+  let lintPattern =
+    Rule.LintPattern
+      (fun pat ->
+        match pat with
+        (* match M.Constructor or M.N.Constructor in patterns like Belt.Result.Ok(x) *)
+        | {Parsetree.ppat_desc= Ppat_construct ({txt= ident; _}, _); Parsetree.ppat_loc= loc}
+          when match extract_module_path ident with
+               | Some module_path -> matches_disallowed_module module_path
+               | None -> false ->
             Rule.LintError (meta.ruleDescription, loc)
         | _ -> Rule.LintOk )
 
-  let linters = [lintStructureItem; lintExpression]
+  let linters = [lintStructureItem; lintExpression; lintPattern]
 end

lib/rules/DisallowedFunctionRule.ml

@@ -19,23 +19,27 @@ module Make (OPT : Rule.OPTIONS with type options = Options.options) (LinterOpti
     ; Rule.ruleIdentifier= "DisallowFunction" ^ "[" ^ function_name ^ "]"
     ; Rule.ruleDescription= description }
 
+  (* Helper function to convert Longident to string *)
+  let rec longident_to_string = function
+    | Longident.Lident s -> s
+    | Longident.Ldot (t, s) -> longident_to_string t ^ "." ^ s
+    | Longident.Lapply (a, b) -> longident_to_string a ^ "(" ^ longident_to_string b ^ ")"
+
   let lintExpression =
     Rule.LintExpression
       (fun expr ->
         match expr with
-        (* matches string_of_int(x) *)
+        (* matches string_of_int(x) or Js.log(x) *)
         | { Parsetree.pexp_desc=
-              Pexp_apply
-                { funct= {pexp_desc= Pexp_ident {txt= Longident.Lident ident}; Parsetree.pexp_loc= loc}
-                ; args= _ } }
-          when ident = function_name ->
+              Pexp_apply {funct= {pexp_desc= Pexp_ident {txt= ident}; Parsetree.pexp_loc= loc}; args= _} }
+          when longident_to_string ident = function_name ->
             Rule.LintError (meta.ruleDescription, loc)
-        (* matches x->string_of_int *)
+        (* matches x->string_of_int or x->Js.log *)
         | {Parsetree.pexp_desc= Pexp_apply {args= xs; _}; Parsetree.pexp_loc= loc} -> (
             let f expr =
               match expr with
-              | Asttypes.Nolabel, {Parsetree.pexp_desc= Pexp_ident {txt= Longident.Lident ident}}
-                when ident = function_name ->
+              | Asttypes.Nolabel, {Parsetree.pexp_desc= Pexp_ident {txt= ident}}
+                when longident_to_string ident = function_name ->
                   true
               | _ -> false
             in

test/rescript_linter_test.ml

@@ -30,6 +30,17 @@ module DisallowStringOfIntRuleWarning =
     end)
     (TestingLinterOptionsWarning)
 
+module DisallowJsLogRule =
+  DisallowedFunctionRule.Make
+    (struct
+      type options = DisallowedFunctionRule.Options.options
+
+      let options =
+        { DisallowedFunctionRule.Options.disallowed_function= "Js.log"
+        ; DisallowedFunctionRule.Options.suggested_function= Some "Console.log" }
+    end)
+    (TestingLinterOptions)
+
 module DisallowInOfStringOptRule =
   DisallowedFunctionRule.Make
     (struct
@@ -74,6 +85,28 @@ module NoCSSModuleRule =
     end)
     (TestingLinterOptions)
 
+module DisallowBeltResultRule =
+  DisallowModuleRule.Make
+    (struct
+      type options = DisallowModuleRule.Options.options
+
+      let options =
+        { DisallowModuleRule.Options.disallowed_module= "Belt.Result"
+        ; DisallowModuleRule.Options.suggested_module= Some "Result" }
+    end)
+    (TestingLinterOptions)
+
+module DisallowBeltRule =
+  DisallowModuleRule.Make
+    (struct
+      type options = DisallowModuleRule.Options.options
+
+      let options =
+        { DisallowModuleRule.Options.disallowed_module= "Belt"
+        ; DisallowModuleRule.Options.suggested_module= Some "Core" }
+    end)
+    (TestingLinterOptions)
+
 module DisallowedEmbeddedRegexLiteralRule =
   DisallowedEmbeddedRegexLiteralRule.Make
     (struct
@@ -139,10 +172,10 @@ module Tests = struct
     in
     match (errors, warnings) with
     | [], [(msg, _); _] ->
-        Alcotest.(check string) "Same error message" DisallowStringOfIntRule.meta.ruleDescription msg
+        Alcotest.(check string) "Same error message" DisallowStringOfIntRuleWarning.meta.ruleDescription msg
     | errors, warnings ->
         Alcotest.fail
-          ( "Should only have two lint warnings, there were "
+          ( "Should only have two lint warnings for string_of_int, there were "
           ^ string_of_int (List.length errors)
           ^ " errors found and "
           ^ string_of_int (List.length warnings)
@@ -246,6 +279,44 @@ module Tests = struct
     | [_; _] -> Alcotest.(check pass) "Same error message" [] []
     | _ -> Alcotest.fail "Should only have two lint error"
 
+  let disallow_qualified_function_test () =
+    let parseResult = parseAst "testData/disallowed_function_rule_test_1.res" in
+    let errors, _warnings =
+      Linter.lint [(module DisallowJsLogRule : Rule.HASRULE)] parseResult.ast parseResult.comments
+    in
+    match errors with
+    | [_] -> Alcotest.(check pass) "Same error message" [] []
+    | errors ->
+        Alcotest.fail
+          ("Should only have one lint error, but got " ^ string_of_int (List.length errors) ^ " errors")
+
+  let disallow_module_test_4 () =
+    let parseResult = parseAst "testData/disallow_module_test_4.res" in
+    let errors, _warnings =
+      Linter.lint [(module DisallowBeltResultRule : Rule.HASRULE)] parseResult.ast parseResult.comments
+    in
+    match errors with
+    | [_; _; _; _; _] -> Alcotest.(check pass) "Same error message" [] []
+    | errors ->
+        Alcotest.fail
+          ( "Should have five lint errors (2 Ok + 2 Error in expressions/patterns, plus map function), but \
+             got "
+          ^ string_of_int (List.length errors)
+          ^ " errors" )
+
+  let disallow_module_test_5 () =
+    let parseResult = parseAst "testData/disallow_module_test_5.res" in
+    let errors, _warnings =
+      Linter.lint [(module DisallowBeltRule : Rule.HASRULE)] parseResult.ast parseResult.comments
+    in
+    match errors with
+    | [_; _; _] -> Alcotest.(check pass) "Same error message" [] []
+    | errors ->
+        Alcotest.fail
+          ( "Should have three lint errors (Belt.List, Belt.Array, Belt.Option), but got "
+          ^ string_of_int (List.length errors)
+          ^ " errors" )
+
   let disallowed_embedded_regex_literal_test () =
     let parseResult = parseAst "testData/disallowed_embedded_regex_literal_test.res" in
     let errors, _warnings =
@@ -316,7 +387,8 @@ let () =
   run "ReScript Linter"
     [ ( "Disallow Function Rule"
       , [ test_case "Lint only functions" `Quick Tests.disallow_test_1
-        ; test_case "Does not lint variable with the same function name" `Quick Tests.disallow_test_2 ] )
+        ; test_case "Does not lint variable with the same function name" `Quick Tests.disallow_test_2
+        ; test_case "Lint qualified functions (Js.log)" `Quick Tests.disallow_qualified_function_test ] )
     ; ( "Warning Lint Rule"
       , [test_case "Lint only functions (as warning)" `Quick Tests.disallow_test_1_warning] )
     ; ( "Disable lint test"
@@ -330,7 +402,9 @@ let () =
     ; ( "Disallow module"
       , [ test_case "open module" `Quick Tests.disallow_module_test_1
         ; test_case "alias module" `Quick Tests.disallow_module_test_2
-        ; test_case "direct access module" `Quick Tests.disallow_module_test_3 ] )
+        ; test_case "direct access module" `Quick Tests.disallow_module_test_3
+        ; test_case "qualified module with constructors (Belt.Result)" `Quick Tests.disallow_module_test_4
+        ; test_case "module prefix matching (Belt.*)" `Quick Tests.disallow_module_test_5 ] )
     ; ( "Disallowed embedded regex literal"
       , [test_case "Disallowed embedded regex literal" `Quick Tests.disallowed_embedded_regex_literal_test] )
     ; ("Disallowed dead code", [test_case "Disallowed dead code" `Quick Tests.disallowed_dead_code_test]) ]

test/testData/disallow_module_test_4.res

@@ -0,0 +1,10 @@
+// Test qualified module paths with Belt.Result
+let x = Belt.Result.Ok(42)
+let y = Belt.Result.Error("failed")
+
+let z = switch x {
+| Belt.Result.Ok(value) => value
+| Belt.Result.Error(_) => 0
+}
+
+let a = Belt.Result.map(x, v => v + 1)

test/testData/disallow_module_test_5.res

@@ -0,0 +1,4 @@
+// Test for disallowing Belt module (should catch Belt.List and Belt.Array)
+let x = Belt.List.toArray([1, 2, 3])
+let y = Belt.Array.map([1, 2, 3], x => x + 1)
+let z = Belt.Option.getWithDefault(None, 0)

test/testData/disallowed_function_rule_test_1.res

@@ -8,4 +8,6 @@ let z2 = Some(`hello ${string_of_int(x)}`)
 
 let string_of_int = ignore
 
+Js.log("hello")
+
 let word = j`hello world`