Skip to content

Error message: Suggest module in place of value where it makes sense #7384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@

- Better representation of JSX in AST. https://github.com/rescript-lang/rescript/pull/7286

#### :nail_care: Polish

- Improve error message for missing value when the identifier is also the name of a module in scope. https://github.com/rescript-lang/rescript/pull/7384

# 12.0.0-alpha.11

#### :bug: Bug fix
Expand Down
26 changes: 24 additions & 2 deletions compiler/ml/typetexp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ let report_error env ppf = function
Printtyp.reset_and_mark_loops_list [ty; ty'];
fprintf ppf "@[<hov>Method '%s' has type %a,@ which should be %a@]" l
Printtyp.type_expr ty Printtyp.type_expr ty')
| Unbound_value lid ->
| Unbound_value lid -> (
(* modified *)
(match lid with
| Ldot (outer, inner) ->
Expand All @@ -850,7 +850,29 @@ let report_error env ppf = function
| other_ident ->
Format.fprintf ppf "The value %a can't be found" Printtyp.longident
other_ident);
super_spellcheck ppf Env.fold_values env lid |> ignore
let did_spellcheck = super_spellcheck ppf Env.fold_values env lid in
(* For cases such as when the user refers to something that's a value with
a lowercase identifier in JS but a module in ReScript.

'Console' is a typical example, where JS is `console.log` and ReScript is `Console.log`. *)
(* TODO(codemods) Add codemod for refering to the module instead. *)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the todo for a code action here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is one of those places we can add a code action for when we get that setup.

let as_module =
match lid with
| Lident name -> (
try
Some
(env
|> Env.lookup_module ~load:false
(Lident (String.capitalize_ascii name)))
with _ -> None)
| _ -> None
in
match as_module with
| None -> ()
| Some module_path ->
Format.fprintf ppf "@,@[<v 2>@,@[%s to use the module @{<info>%a@}?@]@]"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is "@,@[<v 2>@,@[%s here exactly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OCaml format printing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I meant more specially the @,@[< incantation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, just asked an LLM, this is some gourmet camel you are cooking.

(if did_spellcheck then "Or did you mean" else "Maybe you meant")
Printtyp.path module_path)
| Unbound_module lid ->
(* modified *)
(match lid with
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

We've found a bug for you!
/.../fixtures/suggest_module_for_missing_identifier.res:1:1-7

1 │ console.log("Hello")
2 │

The value console can't be found

Maybe you meant to use the module Console?
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

We've found a bug for you!
/.../fixtures/suggest_module_for_missing_identifier_with_spellcheck.res:2:1-7

1 │ let consol = 1
2 │ console.log("Hello")
3 │

The value console can't be found

Hint: Did you mean consol?

Or did you mean to use the module Console?
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("Hello")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
let consol = 1
console.log("Hello")
4 changes: 2 additions & 2 deletions tests/build_tests/super_errors/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ const { bsc } = setup(import.meta.dirname);

const expectedDir = path.join(import.meta.dirname, "expected");

const fixtures = readdirSync("fixtures").filter(
fileName => path.extname(fileName) === ".res",
const fixtures = readdirSync(path.join(import.meta.dirname, "fixtures")).filter(
(fileName) => path.extname(fileName) === ".res"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did the formatting change here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the parenthesis on (fileName)?

);

const prefix = ["-w", "+A", "-bs-jsx", "4"];
Expand Down