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

Provide next best definition when no exact match is present. #807

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Moosieus
Copy link
Collaborator

Pursuant to #780

Copy link
Collaborator

@zachallaun zachallaun left a comment

Choose a reason for hiding this comment

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

We should also add some tests to ensure that, when multiple arities are available, it goes to the correct one. I.e. |some_call(1, 2) should go to some_call/1 if that's all that's there, but we should test that it still goes to some_call/2 if there's both an arity-1 and arity-2 definition.

apps/common/lib/lexical/ast/analysis.ex Outdated Show resolved Hide resolved
apps/common/lib/lexical/ast/analysis/scope.ex Outdated Show resolved Hide resolved
@Moosieus
Copy link
Collaborator Author

I'll check on the test failure 🔎

Go-to-definition supports multiple results, so we could just show all arities and let users select one of them, rather than resolving to any specific one. If that's not preferable though, we can resolve to the lowest arity definition.

LMK what you think and I'll amend things accordingly.

* Prevent exact_lookup from returning private definitions for public calls.
* Jump to lowest-arity in next best definition function.
@Moosieus
Copy link
Collaborator Author

Moosieus commented Aug 4, 2024

Found this behavior which applies to main branch as well. Public calls to non-existent arities can jump to privately defined arities.
Screencast from 2024-08-03 23-24-56.webm

This comes from both Lexical's existing ("exact") lookup, and elixir_sense. At the moment, I've filtered out private definitions for public calls on Lexical's lookup ("exact" + "nearest arity"), and left elixir_sense as-is.

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

Successfully merging this pull request may close these issues.

3 participants