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

feat: completion list suggests methods in the order we most likely want #16044

Closed
wants to merge 63 commits into from

Conversation

mustakimali
Copy link
Contributor

@mustakimali mustakimali commented Dec 7, 2023

When typing MyType:: the completion items' order could be re-ordered based on how likely we want to select those:

  • Constructors: new like functions to be able to create the type,
  • Builder Methods: any builder methods available,
  • Constructors that take args: Any other function that creates Self,
  • Regular methods & associated functions (no change there)

image

In this photo, the order is:

  • new constructor is first
  • new_builder second is a builder method
  • aaaanew is a constructor that takes arguments, is third and is irrespective of its alphabetical order among names.

I've dropped my previous idea of highlighting these functions in the rustdoc (rust-lang/rust#107926)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2023
@mustakimali mustakimali changed the title chore: dot completion suggests new like functions first feat: completion list suggests methods in the order we most likely want Dec 13, 2023
@mustakimali mustakimali marked this pull request as ready for review December 13, 2023 23:17
@rustbot rustbot added has-merge-commits S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 13, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2023

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

cardoso and others added 25 commits December 13, 2023 23:22
Before
Private functions have RawVisibility module, but were
missed because take_types returned None early. After resolve_visibility
returned None, Visibility::Public was set instead and private functions
ended up being offered in autocompletion.

Choosing such a function results in an immediate error diagnostic
about using a private function.

After
Pattern match of take_types that returns None and
query for Module-level visibility from the original_module

Fix rust-lang#15134 - tested with a unit test and a manual end-to-end
test of building rust-analyzer from my branch and opening
the reproduction repository

REVIEW
Refactor to move scope_def_applicable and check function visibility
from a module

Please let me know what's the best way to add a unit tests to
nameres, which is where the root cause was
colon_complete_preferred_order_relevances
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2023

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits (since this message was last posted):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-merge-commits S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.