-
Notifications
You must be signed in to change notification settings - Fork 468
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) -> | ||
|
@@ -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. *) | ||
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@}?@]@]" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OCaml format printing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I meant more specially the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
|
||
[1;31mWe've found a bug for you![0m | ||
[36m/.../fixtures/suggest_module_for_missing_identifier.res[0m:[2m1:1-7[0m | ||
|
||
[1;31m1[0m [2m│[0m [1;31mconsole[0m.log("Hello") | ||
2 [2m│[0m | ||
|
||
The value console can't be found | ||
|
||
Maybe you meant to use the module [1;33mConsole[0m? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
|
||
[1;31mWe've found a bug for you![0m | ||
[36m/.../fixtures/suggest_module_for_missing_identifier_with_spellcheck.res[0m:[2m2:1-7[0m | ||
|
||
1 [2m│[0m let consol = 1 | ||
[1;31m2[0m [2m│[0m [1;31mconsole[0m.log("Hello") | ||
3 [2m│[0m | ||
|
||
The value console can't be found | ||
|
||
[1;33mHint: Did you mean consol?[0m | ||
|
||
Or did you mean to use the module [1;33mConsole[0m? |
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") |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did the formatting change here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean the parenthesis on |
||
); | ||
|
||
const prefix = ["-w", "+A", "-bs-jsx", "4"]; | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.