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

Improve UndefinedFunctionError for unqualified module #12859

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

davydog187
Copy link
Contributor

@davydog187 davydog187 commented Aug 14, 2023

This PR builds off of #12839, handling the case where a developer forgets to alias a module, resulting in UndefinedFunctionError.

Imagine we have the following modules in our Elixir program.

def MyAppWeb.Context.Event do
  def foo, do: :bar
end
def MyAppWeb.OtherContext.Event do
  def foo, do: :bar
end

If the developer attempts to reference this module, but forgets to type the alias, they will now get module suggestions like.

** (UndefinedFunctionError) function Event.foo/0 is undefined (module Event is not available). Did you mean:

      * MyAppWeb.Context.Event.foo/0
      * MyAppWeb.OtherContext.Event.foo/0

    Event.foo()
    iex:4: (file)

TODO

  • Clean up implementation. I wanted to get it working first to demonstrate the idea
  • Test on some real codebases with many modules to check the number of false positives

This PR builds off of elixir-lang#12839, handling the case where a developer
forgets to alias a module, resulting in UndefinedFunctionError.

Imagine we have the following modules in our Elixir program.

```elixir
def MyAppWeb.Context.Event do
  def foo, do: :bar
end
```

If the developer attempts to reference this module, but forgets to type
the alias, they will now get module suggestions like.

```elixir
** (UndefinedFunctionError) function Event.foo/0 is undefined (module Event is not available). Did you mean:

      * MyAppWeb.Context.Event.foo/0

    Event.foo()
    iex:2: (file)
```
@davydog187
Copy link
Contributor Author

@josevalim updated

@davydog187
Copy link
Contributor Author

Screenshot 2023-08-14 at 8 11 59 PM

I tested this on a production app, looking goooooood

Copy link
Contributor

@sabiwara sabiwara left a comment

Choose a reason for hiding this comment

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

Nice! 💜

lib/elixir/lib/exception.ex Outdated Show resolved Hide resolved
Copy link
Member

@whatyouhide whatyouhide left a comment

Choose a reason for hiding this comment

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

Fantastique 🥁

@josevalim josevalim merged commit 44cce79 into elixir-lang:main Aug 15, 2023
11 checks passed
@josevalim
Copy link
Member

Thank you @davydog187. I think these heuristics will likely do a much better job than jaro_distance for modules.

@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

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

Successfully merging this pull request may close these issues.

None yet

4 participants