Skip to content

Fix Assist "replace named generic type with impl trait" #14945

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

Merged
merged 1 commit into from
Jun 2, 2023
Merged

Fix Assist "replace named generic type with impl trait" #14945

merged 1 commit into from
Jun 2, 2023

Conversation

justahero
Copy link
Contributor

@justahero justahero commented Jun 2, 2023

This is a follow-up PR to fix the assist "replace named generic type with impl trait" described in #14626 to filter invalid param types. It integrates the feedback given in PR #14816 .

The change updates the logic to determine when a function parameter is safe to replace a type param with its trait implementation. Some parameter definitions are invalid & should not be replaced by their traits, therefore skipping the assist completely.

First, all usages of the generic type under the cursor are determined. These usage references are checked to see if they occur outside the function parameter list. If an outside reference is found, e.g. in body, return type or where clause, the assist is skipped. All remaining usages need to appear only in the function param list. For each usage the param type is further inspected to see if it's valid. The logic to determine if a function parameter is valid, follows a heuristic and may not cover all possible parameter definitions.

With this change the following param types (as given in this comment) are not replaced & therefore skip the assist.

fn foo<P: Trait>(
    _: <P as Trait>::Assoc,          // within path type qualifier
    _: <() as OtherTrait<P>>::Assoc, // same as above
    _: P::Assoc,                     // associated type shorthand
    _: impl OtherTrait<P>            // generic arg in impl trait (note that associated type bindings are fine)
    _: &dyn Fn(P)                    // param type and/or return type for Fn* traits
) {}

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 2, 2023
The change updates the logic to determine if a function parameter is
valid for replacing the type param with the trait implementation.

First all usages are determined, to check if they are used outside the function
parameter list. If an outside reference is found, e.g. in body, return type or
where clause, the assist is skipped. All remaining usages only appear in the
function param list. For each usage the param type is checked to see if
it's valid.

**Please note** the logic currently follows a heuristic and may not cover
all existing parameter declarations.

* determine valid usage references by checking ancestors (on AST level)
* split test into separate ones
@Veykril
Copy link
Member

Veykril commented Jun 2, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jun 2, 2023

📌 Commit c0e9b57 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 2, 2023

⌛ Testing commit c0e9b57 with merge 7738ff4...

@bors
Copy link
Contributor

bors commented Jun 2, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 7738ff4 to master...

@bors bors merged commit 7738ff4 into rust-lang:master Jun 2, 2023
@justahero justahero deleted the gh-14626 branch June 2, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants