Skip to content
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

Better error message for undefined variables within remote calls to macro's missing a corresponding require #13465

Open
zachdaniel opened this issue Apr 2, 2024 · 4 comments

Comments

@zachdaniel
Copy link
Contributor

zachdaniel commented Apr 2, 2024

Current behavior

When calling macros like Ecto.Query, you often end up with an error message that can be cryptic, especially with beginners.

For example, the following code without adding require Ecto.Query.

Ecto.Query.from row in Table, where: row.name == "fred"

will produce error: undefined variable "row"

This should be an error, but I think that a hint here would go a long way towards helping users understand what needs to happen.

Desired behavior

In this situation, the compiler should check to see if it is within a remote call to a macro that has not been required, and if so provide a helpful message to the user, something along the lines of:

perhaps you are missing require Ecto.Query?

@zachdaniel
Copy link
Contributor Author

zachdaniel commented Apr 3, 2024

I will put this on my list for ~5-6 weeks from now unless someone else gets to it :)

Here is some feedback from the mailing list from @josevalim about how this could be implemented.


We have a record that controls compilation:

https://github.com/elixir-lang/elixir/blob/main/lib/elixir/src/elixir.hrl#L8

You are going to add a new field called "debug". This field can have three values:

  • enabled
  • disabled
  • pending

Whenever we raise because of an undefined variable, we will set the debug mode to pending (if it is disabled).

Now, when compiling a function, we will see if it returns debug as "pending":

https://github.com/elixir-lang/elixir/blob/main/lib/elixir/src/elixir_clauses.erl#L38

If pending, you will set "debug" to enabled and expand it again. The enabled debug mode will disable the variables from raising again and change elixir_dispatch to code:ensure_loaded modules when looking for remote functions, in an attempt to find pending macros.

@viniciusmuller
Copy link
Contributor

Hey there! I'm taking a look at this issue and I'm currently a bit confused at this part:

change elixir_dispatch to code:ensure_loaded modules when looking for remote functions, in an attempt to find pending macros.

I see that for the Ecto example, there's no mention of Ecto at all on the %Env{} structure for example, how should I get available macros using this helper function?

Another thing that I was thinking about, would it be possible to use the AST from EBody to extract the module + macro name from the call and try to check if it exists, and if so throwing a different error message?

@josevalim
Copy link
Member

Ecto's case has already been fixed, because the undefined variable warning doesn't stop compilation and we warn about unknown function. So that's no longer a concern. :)

@josevalim
Copy link
Member

To be clear, the forgotten import case is addressed. Now we only need to deal with forgotten require.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants