From b29377299dd2cd4c238822137f56452746544b54 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Mon, 9 Feb 2026 08:51:20 +0800 Subject: [PATCH 1/4] Handle parse error better when config is wrong --- lib/ConfigReader.ml | 110 ++++++++++++++++++++++++++++--------------- lib/Linter.ml | 7 ++- rescript_linter.opam | 2 +- 3 files changed, 80 insertions(+), 39 deletions(-) diff --git a/lib/ConfigReader.ml b/lib/ConfigReader.ml index 46aabf5..20b94d3 100644 --- a/lib/ConfigReader.ml +++ b/lib/ConfigReader.ml @@ -1,4 +1,5 @@ exception RuleDoesNotExist +exception ConfigParseError of string type t = {rules: (module Rule.HASRULE) list} @@ -111,42 +112,77 @@ let parseConfig path = let json = Yojson.Basic.from_file path in let open Yojson.Basic.Util in let filter_rule json = - (* Parse in the linter options that are common to every rule *) - let warning = - match json |> member "warning" with `Null -> false (* default to errors *) | json -> to_bool json - in - let linterOptions = - ( module struct - let warning = warning - end : Rule.LinterOptions ) - in - (* Parse the rule name, this is used to identify the rule being configured - * as each rule can have different parsing options - *) - let rule = json |> member "rule" |> to_string in - (* Not all rules are explicitly required to have options - * but perhaps in the future we can clean up the code duplication - * of parsing options for the rules that do have options - *) - match rule with - | "DisallowOperator" -> - let options = json |> member "options" in - createDisallowOperatorRule options linterOptions - | "DisallowFunction" -> - let options = json |> member "options" in - createDisallowFunctionRule options linterOptions - | "NoReactComponent" -> - let options = json |> member "options" in - createNoReactComponentRule options linterOptions - | "DisallowModule" -> - let options = json |> member "options" in - createDisallowModuleRule options linterOptions - | "DisallowEmbeddedRegexLiteral" -> - let options = json |> member "options" in - createDisallowEmbeddedRegexLiteralRule options linterOptions - | "DisallowAttribute" -> - let options = json |> member "options" in - createDisallowAttributeRule options linterOptions - | _ -> raise RuleDoesNotExist + try + (* Parse in the linter options that are common to every rule *) + let warning = + match json |> member "warning" with `Null -> false (* default to errors *) | json -> to_bool json + in + let linterOptions = + ( module struct + let warning = warning + end : Rule.LinterOptions ) + in + (* Parse the rule name, this is used to identify the rule being configured + * as each rule can have different parsing options + *) + let rule = json |> member "rule" |> to_string in + (* Not all rules are explicitly required to have options + * but perhaps in the future we can clean up the code duplication + * of parsing options for the rules that do have options + *) + match rule with + | "DisallowOperator" -> + let options = json |> member "options" in + createDisallowOperatorRule options linterOptions + | "DisallowFunction" -> + let options = json |> member "options" in + createDisallowFunctionRule options linterOptions + | "NoReactComponent" -> + let options = json |> member "options" in + createNoReactComponentRule options linterOptions + | "DisallowModule" -> + let options = json |> member "options" in + createDisallowModuleRule options linterOptions + | "DisallowEmbeddedRegexLiteral" -> + let options = json |> member "options" in + createDisallowEmbeddedRegexLiteralRule options linterOptions + | "DisallowAttribute" -> + let options = json |> member "options" in + createDisallowAttributeRule options linterOptions + | _ -> raise RuleDoesNotExist + with + | RuleDoesNotExist -> + let rule_name = + try json |> member "rule" |> to_string + with _ -> "" + in + raise (ConfigParseError ( + Printf.sprintf "Unknown rule '%s' in config file.\nValid rules are: DisallowOperator, DisallowFunction, DisallowModule, NoReactComponent, DisallowEmbeddedRegexLiteral, DisallowAttribute" + rule_name)) + | Type_error (msg, _) -> + let rule_name = + try json |> member "rule" |> to_string + with _ -> "" + in + let options_hint = match rule_name with + | "DisallowOperator" -> "Expected fields: disallowed_operator, suggested_operator" + | "DisallowFunction" -> "Expected fields: disallowed_function, suggested_function" + | "DisallowModule" -> "Expected fields: disallowed_module, suggested_module" + | "NoReactComponent" -> "Expected fields: component, suggested_component" + | "DisallowEmbeddedRegexLiteral" -> "Expected fields: test_directory" + | "DisallowAttribute" -> "Expected fields: attribute, suggestion (optional)" + | _ -> "Please check the rule configuration" + in + raise (ConfigParseError ( + Printf.sprintf "Error parsing rule '%s': %s\n%s\nRule config:\n%s" + rule_name msg options_hint (Yojson.Basic.pretty_to_string json))) + | e -> + let rule_name = + try json |> member "rule" |> to_string + with _ -> "" + in + raise (ConfigParseError ( + Printf.sprintf "Unexpected error parsing rule '%s': %s\nRule config:\n%s" + rule_name (Printexc.to_string e) (Yojson.Basic.pretty_to_string json))) in json |> member "rules" |> to_list |> List.map filter_rule diff --git a/lib/Linter.ml b/lib/Linter.ml index d2634b6..d5d66b3 100644 --- a/lib/Linter.ml +++ b/lib/Linter.ml @@ -48,7 +48,12 @@ let lintInfo_of_tuple_with_message_kind (message_kind : Printer.PrettyPrint.mess type jsonOutput = {errors: lintInfo list; warnings: lintInfo list} [@@deriving yojson] let run configPath path (outputJson : bool) = - let rules = ConfigReader.parseConfig configPath in + let rules = + try ConfigReader.parseConfig configPath + with ConfigReader.ConfigParseError msg -> + Printf.eprintf "Config Error: %s\n" msg ; + exit 1 + in if not outputJson then Format.fprintf Format.std_formatter "Linting rules:\n%s" (String.concat "\n" diff --git a/rescript_linter.opam b/rescript_linter.opam index a0a7644..6be7362 100644 --- a/rescript_linter.opam +++ b/rescript_linter.opam @@ -1,6 +1,6 @@ # This file is generated by dune, edit dune-project instead opam-version: "2.0" -version: "0.4.0" +version: "0.4.1" synopsis: "AST-based linter for ReScript" description: "AST-based linter for ReScript" maintainer: ["Shulhi Sapli"] From f62efd4b2effc84453cad374d192405455cc91e5 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Mon, 9 Feb 2026 09:25:37 +0800 Subject: [PATCH 2/4] Add tests --- test/rescript_linter_test.ml | 50 ++++++++++++++++++- .../config_invalid_disallow_module.json | 11 ++++ test/testData/config_missing_field.json | 10 ++++ test/testData/config_unknown_rule.json | 10 ++++ test/testData/config_valid.json | 18 +++++++ 5 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 test/testData/config_invalid_disallow_module.json create mode 100644 test/testData/config_missing_field.json create mode 100644 test/testData/config_unknown_rule.json create mode 100644 test/testData/config_valid.json diff --git a/test/rescript_linter_test.ml b/test/rescript_linter_test.ml index 3b01fa8..925280a 100644 --- a/test/rescript_linter_test.ml +++ b/test/rescript_linter_test.ml @@ -379,6 +379,49 @@ module Tests = struct (List.map (fun (str, loc) -> Printf.sprintf "\t* %s\n\t\t -> %s" str (loc_to_string loc)) errors ) ) + + (* Config parsing error tests *) + let config_invalid_field_names_test () = + try + let _rules = ConfigReader.parseConfig "testData/config_invalid_disallow_module.json" in + Alcotest.fail "Should have raised ConfigParseError for invalid field names" + with + | ConfigReader.ConfigParseError msg -> + if contains_substring msg "DisallowModule" && contains_substring msg "disallowed_module" then + Alcotest.(check pass) "Correct error message" [] [] + else + Alcotest.fail + ("Error message should mention DisallowModule and disallowed_module, but got: " ^ msg) + + let config_unknown_rule_test () = + try + let _rules = ConfigReader.parseConfig "testData/config_unknown_rule.json" in + Alcotest.fail "Should have raised ConfigParseError for unknown rule" + with + | ConfigReader.ConfigParseError msg -> + if contains_substring msg "Unknown rule" && contains_substring msg "NonExistentRule" then + Alcotest.(check pass) "Correct error message" [] [] + else Alcotest.fail ("Error message should mention unknown rule, but got: " ^ msg) + + let config_missing_field_test () = + try + let _rules = ConfigReader.parseConfig "testData/config_missing_field.json" in + Alcotest.fail "Should have raised ConfigParseError for missing field" + with + | ConfigReader.ConfigParseError msg -> + if contains_substring msg "DisallowOperator" && contains_substring msg "suggested_operator" then + Alcotest.(check pass) "Correct error message" [] [] + else + Alcotest.fail + ("Error message should mention DisallowOperator and suggested_operator, but got: " ^ msg) + + let config_valid_test () = + try + let rules = ConfigReader.parseConfig "testData/config_valid.json" in + match rules with + | [_; _] -> Alcotest.(check pass) "Valid config parsed successfully" [] [] + | _ -> Alcotest.fail "Should have parsed 2 rules" + with ConfigReader.ConfigParseError msg -> Alcotest.fail ("Should not have raised error: " ^ msg) end (* Run it *) @@ -407,4 +450,9 @@ let () = ; 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]) ] + ; ("Disallowed dead code", [test_case "Disallowed dead code" `Quick Tests.disallowed_dead_code_test]) + ; ( "Config parsing errors" + , [ test_case "Invalid field names" `Quick Tests.config_invalid_field_names_test + ; test_case "Unknown rule" `Quick Tests.config_unknown_rule_test + ; test_case "Missing required field" `Quick Tests.config_missing_field_test + ; test_case "Valid config" `Quick Tests.config_valid_test ] ) ] diff --git a/test/testData/config_invalid_disallow_module.json b/test/testData/config_invalid_disallow_module.json new file mode 100644 index 0000000..61dad1c --- /dev/null +++ b/test/testData/config_invalid_disallow_module.json @@ -0,0 +1,11 @@ +{ + "rules": [ + { + "rule": "DisallowModule", + "options": { + "disallowed_function": "Js.Json", + "suggested_function": "JSON" + } + } + ] +} diff --git a/test/testData/config_missing_field.json b/test/testData/config_missing_field.json new file mode 100644 index 0000000..5d98a1a --- /dev/null +++ b/test/testData/config_missing_field.json @@ -0,0 +1,10 @@ +{ + "rules": [ + { + "rule": "DisallowOperator", + "options": { + "disallowed_operator": "|>" + } + } + ] +} diff --git a/test/testData/config_unknown_rule.json b/test/testData/config_unknown_rule.json new file mode 100644 index 0000000..fb2ea49 --- /dev/null +++ b/test/testData/config_unknown_rule.json @@ -0,0 +1,10 @@ +{ + "rules": [ + { + "rule": "NonExistentRule", + "options": { + "some_field": "value" + } + } + ] +} diff --git a/test/testData/config_valid.json b/test/testData/config_valid.json new file mode 100644 index 0000000..4860796 --- /dev/null +++ b/test/testData/config_valid.json @@ -0,0 +1,18 @@ +{ + "rules": [ + { + "rule": "DisallowModule", + "options": { + "disallowed_module": "Belt.Result", + "suggested_module": "Result" + } + }, + { + "rule": "DisallowFunction", + "options": { + "disallowed_function": "string_of_int", + "suggested_function": "Int.toString" + } + } + ] +} From 4814e362215459e16f91d41dac6df05a33dfcf18 Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Mon, 9 Feb 2026 09:29:25 +0800 Subject: [PATCH 3/4] Bump version --- Changelog.md | 27 +++++++++++++++++++++++++++ dune-project | 2 +- rescript_linter.opam | 2 +- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index d9a7e36..a5b9685 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,32 @@ # ReScript Linter Changelog +### 2026-02-09 - v0.4.2 +* Improved error handling for config file parsing + * Added `ConfigParseError` exception with detailed error messages + * Config parsing errors now show: + * The problematic rule name + * Expected field names for each rule type + * The full rule configuration that failed to parse + * Example error message: + ``` + Config Error: Error parsing rule 'DisallowModule': Expected string, got null + Expected fields: disallowed_module, suggested_module + Rule config: + { + "rule": "DisallowModule", + "options": { + "disallowed_function": "Js.Json", + "suggested_function": "JSON" + } + } + ``` + * Unknown rules now list all valid rule names +* Added comprehensive tests for config parsing errors + * Tests for invalid field names + * Tests for unknown rules + * Tests for missing required fields + * Tests for valid configs + ### 2026-02-02 - v0.4.1 * Update DisallowModuleRule and DisallowedFunctionRule diff --git a/dune-project b/dune-project index e1c3091..6cb7718 100644 --- a/dune-project +++ b/dune-project @@ -2,7 +2,7 @@ (name rescript_linter) -(version 0.4.1) +(version 0.4.2) (generate_opam_files true) diff --git a/rescript_linter.opam b/rescript_linter.opam index 6be7362..fb6f1c5 100644 --- a/rescript_linter.opam +++ b/rescript_linter.opam @@ -1,6 +1,6 @@ # This file is generated by dune, edit dune-project instead opam-version: "2.0" -version: "0.4.1" +version: "0.4.2" synopsis: "AST-based linter for ReScript" description: "AST-based linter for ReScript" maintainer: ["Shulhi Sapli"] From 5271684e8b1f685b877eb08a833f1165bf88fe4a Mon Sep 17 00:00:00 2001 From: Shulhi Sapli <913103+shulhi@users.noreply.github.com> Date: Mon, 9 Feb 2026 09:33:56 +0800 Subject: [PATCH 4/4] Format code --- lib/ConfigReader.ml | 57 ++++++++++++++++++------------------ test/rescript_linter_test.ml | 32 ++++++++------------ 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/lib/ConfigReader.ml b/lib/ConfigReader.ml index 20b94d3..86d2423 100644 --- a/lib/ConfigReader.ml +++ b/lib/ConfigReader.ml @@ -1,4 +1,5 @@ exception RuleDoesNotExist + exception ConfigParseError of string type t = {rules: (module Rule.HASRULE) list} @@ -152,37 +153,35 @@ let parseConfig path = | _ -> raise RuleDoesNotExist with | RuleDoesNotExist -> - let rule_name = - try json |> member "rule" |> to_string - with _ -> "" - in - raise (ConfigParseError ( - Printf.sprintf "Unknown rule '%s' in config file.\nValid rules are: DisallowOperator, DisallowFunction, DisallowModule, NoReactComponent, DisallowEmbeddedRegexLiteral, DisallowAttribute" - rule_name)) + let rule_name = try json |> member "rule" |> to_string with _ -> "" in + raise + (ConfigParseError + (Printf.sprintf + "Unknown rule '%s' in config file.\n\ + Valid rules are: DisallowOperator, DisallowFunction, DisallowModule, NoReactComponent, \ + DisallowEmbeddedRegexLiteral, DisallowAttribute" + rule_name ) ) | Type_error (msg, _) -> - let rule_name = - try json |> member "rule" |> to_string - with _ -> "" + let rule_name = try json |> member "rule" |> to_string with _ -> "" in + let options_hint = + match rule_name with + | "DisallowOperator" -> "Expected fields: disallowed_operator, suggested_operator" + | "DisallowFunction" -> "Expected fields: disallowed_function, suggested_function" + | "DisallowModule" -> "Expected fields: disallowed_module, suggested_module" + | "NoReactComponent" -> "Expected fields: component, suggested_component" + | "DisallowEmbeddedRegexLiteral" -> "Expected fields: test_directory" + | "DisallowAttribute" -> "Expected fields: attribute, suggestion (optional)" + | _ -> "Please check the rule configuration" in - let options_hint = match rule_name with - | "DisallowOperator" -> "Expected fields: disallowed_operator, suggested_operator" - | "DisallowFunction" -> "Expected fields: disallowed_function, suggested_function" - | "DisallowModule" -> "Expected fields: disallowed_module, suggested_module" - | "NoReactComponent" -> "Expected fields: component, suggested_component" - | "DisallowEmbeddedRegexLiteral" -> "Expected fields: test_directory" - | "DisallowAttribute" -> "Expected fields: attribute, suggestion (optional)" - | _ -> "Please check the rule configuration" - in - raise (ConfigParseError ( - Printf.sprintf "Error parsing rule '%s': %s\n%s\nRule config:\n%s" - rule_name msg options_hint (Yojson.Basic.pretty_to_string json))) + raise + (ConfigParseError + (Printf.sprintf "Error parsing rule '%s': %s\n%s\nRule config:\n%s" rule_name msg options_hint + (Yojson.Basic.pretty_to_string json) ) ) | e -> - let rule_name = - try json |> member "rule" |> to_string - with _ -> "" - in - raise (ConfigParseError ( - Printf.sprintf "Unexpected error parsing rule '%s': %s\nRule config:\n%s" - rule_name (Printexc.to_string e) (Yojson.Basic.pretty_to_string json))) + let rule_name = try json |> member "rule" |> to_string with _ -> "" in + raise + (ConfigParseError + (Printf.sprintf "Unexpected error parsing rule '%s': %s\nRule config:\n%s" rule_name + (Printexc.to_string e) (Yojson.Basic.pretty_to_string json) ) ) in json |> member "rules" |> to_list |> List.map filter_rule diff --git a/test/rescript_linter_test.ml b/test/rescript_linter_test.ml index 925280a..dafcbee 100644 --- a/test/rescript_linter_test.ml +++ b/test/rescript_linter_test.ml @@ -385,35 +385,29 @@ module Tests = struct try let _rules = ConfigReader.parseConfig "testData/config_invalid_disallow_module.json" in Alcotest.fail "Should have raised ConfigParseError for invalid field names" - with - | ConfigReader.ConfigParseError msg -> - if contains_substring msg "DisallowModule" && contains_substring msg "disallowed_module" then - Alcotest.(check pass) "Correct error message" [] [] - else - Alcotest.fail - ("Error message should mention DisallowModule and disallowed_module, but got: " ^ msg) + with ConfigReader.ConfigParseError msg -> + if contains_substring msg "DisallowModule" && contains_substring msg "disallowed_module" then + Alcotest.(check pass) "Correct error message" [] [] + else Alcotest.fail ("Error message should mention DisallowModule and disallowed_module, but got: " ^ msg) let config_unknown_rule_test () = try let _rules = ConfigReader.parseConfig "testData/config_unknown_rule.json" in Alcotest.fail "Should have raised ConfigParseError for unknown rule" - with - | ConfigReader.ConfigParseError msg -> - if contains_substring msg "Unknown rule" && contains_substring msg "NonExistentRule" then - Alcotest.(check pass) "Correct error message" [] [] - else Alcotest.fail ("Error message should mention unknown rule, but got: " ^ msg) + with ConfigReader.ConfigParseError msg -> + if contains_substring msg "Unknown rule" && contains_substring msg "NonExistentRule" then + Alcotest.(check pass) "Correct error message" [] [] + else Alcotest.fail ("Error message should mention unknown rule, but got: " ^ msg) let config_missing_field_test () = try let _rules = ConfigReader.parseConfig "testData/config_missing_field.json" in Alcotest.fail "Should have raised ConfigParseError for missing field" - with - | ConfigReader.ConfigParseError msg -> - if contains_substring msg "DisallowOperator" && contains_substring msg "suggested_operator" then - Alcotest.(check pass) "Correct error message" [] [] - else - Alcotest.fail - ("Error message should mention DisallowOperator and suggested_operator, but got: " ^ msg) + with ConfigReader.ConfigParseError msg -> + if contains_substring msg "DisallowOperator" && contains_substring msg "suggested_operator" then + Alcotest.(check pass) "Correct error message" [] [] + else + Alcotest.fail ("Error message should mention DisallowOperator and suggested_operator, but got: " ^ msg) let config_valid_test () = try