-
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
Conversation
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 comment
The reason will be displayed to describe this comment to others. Learn more.
What is "@,@[<v 2>@,@[%s
here exactly?
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.
OCaml format printing.
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, I meant more specially the @,@[<
incantation.
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.
Damn, just asked an LLM, this is some gourmet camel you are cooking.
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. *) |
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.
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
You mean the parenthesis on (fileName)
?
There are some subtle differences between how certain common things are called in ReScript vs in JS. One example is logging to the console:
This difference (value vs module) can be subtle and not easily discoverable. So, this PR does a small addition to the "missing value" error message, and suggests the module version of the identifier if it exists in scope. This is intended to give a small hint for what you might be looking for, instead of just telling you to "go fish".