diff --git a/Changelog.md b/Changelog.md index fff3bd8..d9a7e36 100644 --- a/Changelog.md +++ b/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 diff --git a/dune-project b/dune-project index ac1ebe9..e1c3091 100644 --- a/dune-project +++ b/dune-project @@ -2,7 +2,7 @@ (name rescript_linter) -(version 0.4.0) +(version 0.4.1) (generate_opam_files true) diff --git a/lib/rules/DisallowModuleRule.ml b/lib/rules/DisallowModuleRule.ml index afd2893..93206a6 100644 --- a/lib/rules/DisallowModuleRule.ml +++ b/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,19 +69,17 @@ 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 ) @@ -66,16 +87,32 @@ module Make (OPT : Rule.OPTIONS with type options = Options.options) (LinterOpti 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 diff --git a/lib/rules/DisallowedFunctionRule.ml b/lib/rules/DisallowedFunctionRule.ml index 9319f32..16d3448 100644 --- a/lib/rules/DisallowedFunctionRule.ml +++ b/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 diff --git a/test/rescript_linter_test.ml b/test/rescript_linter_test.ml index 2333cb2..3b01fa8 100644 --- a/test/rescript_linter_test.ml +++ b/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]) ] diff --git a/test/testData/disallow_module_test_4.res b/test/testData/disallow_module_test_4.res new file mode 100644 index 0000000..58564b9 --- /dev/null +++ b/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) diff --git a/test/testData/disallow_module_test_5.res b/test/testData/disallow_module_test_5.res new file mode 100644 index 0000000..cc98218 --- /dev/null +++ b/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) diff --git a/test/testData/disallowed_function_rule_test_1.res b/test/testData/disallowed_function_rule_test_1.res index e2175f7..c8a4503 100644 --- a/test/testData/disallowed_function_rule_test_1.res +++ b/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`