Skip to content

Commit d52051c

Browse files
authored
Fix DisallowModule rule missing Pexp_ident — pipe calls with no extra args (#15) (#16)
* Fix DisallowModule rule missing Pexp_ident — pipe calls with no extra args now caught (#15) Replace Pexp_apply-only matching with Pexp_ident matching in lintExpression so bare module references (e.g. x->Belt.List.toArray) are detected. Adds tests for the bug case and verifies all existing Pexp_apply scenarios still pass. Bump version to 0.4.3. * Apply dune fmt formatting to test file
1 parent 017c844 commit d52051c

7 files changed

Lines changed: 110 additions & 6 deletions

File tree

Changelog.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# ReScript Linter Changelog
22

3+
### 2026-02-26 - v0.4.3
4+
* Fixed DisallowModule rule missing `Pexp_ident` — pipe calls with no extra arguments now correctly trigger lint errors (#15)
5+
* `x->Belt.List.toArray` and similar single-argument pipe expressions were silently bypassing the rule
6+
* Replaced `Pexp_apply`-only matching with `Pexp_ident` matching to catch all disallowed module references
7+
38
### 2026-02-09 - v0.4.2
49
* Improved error handling for config file parsing
510
* Added `ConfigParseError` exception with detailed error messages

dune-project

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
(name rescript_linter)
44

5-
(version 0.4.2)
5+
(version 0.4.3)
66

77
(generate_opam_files true)
88

lib/rules/DisallowModuleRule.ml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,8 @@ module Make (OPT : Rule.OPTIONS with type options = Options.options) (LinterOpti
8787
Rule.LintExpression
8888
(fun expr ->
8989
match expr with
90-
(* match M.function or M.N.function in function calls *)
91-
| { Parsetree.pexp_desc=
92-
Pexp_apply {funct= {pexp_desc= Pexp_ident {txt= ident}; Parsetree.pexp_loc= loc}; args= _} }
90+
(* match M.function or M.N.function — covers direct calls, pipe targets, and bare references *)
91+
| {Parsetree.pexp_desc= Pexp_ident {txt= ident; _}; Parsetree.pexp_loc= loc}
9392
when match extract_module_path ident with
9493
| Some module_path -> matches_disallowed_module module_path
9594
| None -> false ->

rescript_linter.opam

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# This file is generated by dune, edit dune-project instead
22
opam-version: "2.0"
3-
version: "0.4.2"
3+
version: "0.4.3"
44
synopsis: "AST-based linter for ReScript"
55
description: "AST-based linter for ReScript"
66
maintainer: ["Shulhi Sapli"]

test/rescript_linter_test.ml

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,28 @@ module DisallowBeltRule =
107107
end)
108108
(TestingLinterOptions)
109109

110+
module DisallowBeltListRule =
111+
DisallowModuleRule.Make
112+
(struct
113+
type options = DisallowModuleRule.Options.options
114+
115+
let options =
116+
{ DisallowModuleRule.Options.disallowed_module= "Belt.List"
117+
; DisallowModuleRule.Options.suggested_module= Some "List" }
118+
end)
119+
(TestingLinterOptions)
120+
121+
module DisallowBeltArrayRule =
122+
DisallowModuleRule.Make
123+
(struct
124+
type options = DisallowModuleRule.Options.options
125+
126+
let options =
127+
{ DisallowModuleRule.Options.disallowed_module= "Belt.Array"
128+
; DisallowModuleRule.Options.suggested_module= Some "Array" }
129+
end)
130+
(TestingLinterOptions)
131+
110132
module DisallowedEmbeddedRegexLiteralRule =
111133
DisallowedEmbeddedRegexLiteralRule.Make
112134
(struct
@@ -317,6 +339,48 @@ module Tests = struct
317339
^ string_of_int (List.length errors)
318340
^ " errors" )
319341

342+
(* Issue #15: pipe with no extra arguments bypasses DisallowModule lint *)
343+
let disallow_module_pipe_no_args_test () =
344+
let parseResult = parseAst "testData/disallow_module_test_6.res" in
345+
let errors, _warnings =
346+
Linter.lint [(module DisallowBeltListRule : Rule.HASRULE)] parseResult.ast parseResult.comments
347+
in
348+
match errors with
349+
| [_; _; _; _] -> Alcotest.(check pass) "Same error message" [] []
350+
| errors ->
351+
Alcotest.fail
352+
( "Should have four lint errors (direct call, pipe+args, pipe no-args toArray, pipe no-args \
353+
length), but got "
354+
^ string_of_int (List.length errors)
355+
^ " errors" )
356+
357+
(* Issue #15: verify Pexp_apply scenarios still caught after Pexp_ident fix *)
358+
let disallow_module_pexp_apply_test () =
359+
let parseResult = parseAst "testData/disallow_module_test_7.res" in
360+
let errors, _warnings =
361+
Linter.lint
362+
[(module DisallowBeltListRule : Rule.HASRULE); (module DisallowBeltArrayRule : Rule.HASRULE)]
363+
parseResult.ast parseResult.comments
364+
in
365+
(* Expected errors:
366+
1. Belt.List.toArray — direct call, single arg
367+
2. Belt.List.has — direct call, multiple args
368+
3. Belt.List.map — pipe with extra args
369+
4. Belt.List.toArray — pipe with no extra args (was the bug)
370+
5. Belt.List.toArray — chained pipe, no-args (was the bug)
371+
6. Belt.Array.map — chained pipe, with args
372+
7. Belt.List.toArray — nested outer call
373+
8. Belt.List.map — nested inner call
374+
*)
375+
match errors with
376+
| [_; _; _; _; _; _; _; _] -> Alcotest.(check pass) "Same error message" [] []
377+
| errors ->
378+
Alcotest.fail
379+
( "Should have eight lint errors (6 Belt.List + 2 Belt.Array... wait, 7 Belt.List + 1 Belt.Array), \
380+
but got "
381+
^ string_of_int (List.length errors)
382+
^ " errors" )
383+
320384
let disallowed_embedded_regex_literal_test () =
321385
let parseResult = parseAst "testData/disallowed_embedded_regex_literal_test.res" in
322386
let errors, _warnings =
@@ -441,7 +505,9 @@ let () =
441505
; test_case "alias module" `Quick Tests.disallow_module_test_2
442506
; test_case "direct access module" `Quick Tests.disallow_module_test_3
443507
; test_case "qualified module with constructors (Belt.Result)" `Quick Tests.disallow_module_test_4
444-
; test_case "module prefix matching (Belt.*)" `Quick Tests.disallow_module_test_5 ] )
508+
; test_case "module prefix matching (Belt.*)" `Quick Tests.disallow_module_test_5
509+
; test_case "pipe with no extra args (Issue #15)" `Quick Tests.disallow_module_pipe_no_args_test
510+
; test_case "Pexp_apply still caught (Issue #15)" `Quick Tests.disallow_module_pexp_apply_test ] )
445511
; ( "Disallowed embedded regex literal"
446512
, [test_case "Disallowed embedded regex literal" `Quick Tests.disallowed_embedded_regex_literal_test] )
447513
; ("Disallowed dead code", [test_case "Disallowed dead code" `Quick Tests.disallowed_dead_code_test])
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Test for pipe expressions with no extra arguments (Issue #15)
2+
// Rule: DisallowModule[Belt.List]
3+
4+
// Caught: direct call with arguments
5+
let a = Belt.List.toArray(list{1, 2, 3})
6+
7+
// Caught: pipe with extra arguments
8+
let b = list{1, 2, 3}->Belt.List.map(x => x + 1)
9+
10+
// BUG: pipe with no extra arguments (should be caught)
11+
let c = list{1, 2, 3}->Belt.List.toArray
12+
13+
// BUG: pipe with no extra arguments (should be caught)
14+
let d = list{1, 2, 3}->Belt.List.length
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Test that Pexp_apply scenarios (function calls) are still caught after Issue #15 fix
2+
// Rule: DisallowModule[Belt.List] + DisallowModule[Belt.Array]
3+
4+
// 1. Direct call with single argument (Pexp_apply, Belt.List)
5+
let a = Belt.List.toArray(list{1, 2, 3})
6+
7+
// 2. Direct call with multiple arguments (Pexp_apply, Belt.List)
8+
let b = Belt.List.has(list{1, 2, 3}, 1, (a, b) => a == b)
9+
10+
// 3. Pipe with extra arguments (Pexp_apply, Belt.List)
11+
let c = list{1, 2, 3}->Belt.List.map(x => x + 1)
12+
13+
// 4. Pipe with no extra arguments (Pexp_ident, Belt.List — was the bug)
14+
let d = list{1, 2, 3}->Belt.List.toArray
15+
16+
// 5. Chained pipe: no-args into with-args (Belt.List missed + Belt.Array caught)
17+
let e = list{1, 2, 3}->Belt.List.toArray->Belt.Array.map(x => x)
18+
19+
// 6. Nested calls (both inner and outer are Pexp_apply, Belt.List)
20+
let f = Belt.List.toArray(Belt.List.map(list{1, 2, 3}, x => x + 1))

0 commit comments

Comments
 (0)