-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: Add an assist for inlining type aliases #11690
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
Conversation
This intends to lead to a more useful assist to replace all users of an alias with its definition.
We'll go with similarity to inline call which has |
Thanks for the review :), splitting up the function lead to some more improvements as well. |
let replacement = if let Some(alias_generics) = alias.generic_param_list() { | ||
if alias_generics.generic_params().next().is_none() { | ||
cov_mark::hit!(no_generics_params); | ||
return None; | ||
} | ||
|
||
let instance_args = | ||
alias_instance.syntax().descendants().find_map(ast::GenericArgList::cast); | ||
|
||
create_replacement( | ||
&LifetimeMap::new(&instance_args, &alias_generics)?, | ||
&ConstAndTypeMap::new(&instance_args, &alias_generics)?, | ||
&concrete_type, | ||
) | ||
} else { | ||
concrete_type.to_string() | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this into the builder closure except for the early return.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two more early exits for LifetimeMap::new
and ConstAndTypeMap::new
. Would you prefer this?
enum Replacement {
Generic { lifetime_map: LifetimeMap, const_and_type_map: ConstAndTypeMap },
Plain,
}
let replacement = if let Some(alias_generics) = alias.generic_param_list() {
if alias_generics.generic_params().next().is_none() {
cov_mark::hit!(no_generics_params);
return None;
}
let instance_args =
alias_instance.syntax().descendants().find_map(ast::GenericArgList::cast);
Replacement::Generic {
lifetime_map: LifetimeMap::new(&instance_args, &alias_generics)?,
const_and_type_map: ConstAndTypeMap::new(&instance_args, &alias_generics)?,
}
} else {
Replacement::Plain
};
let target = alias_instance.syntax().text_range();
acc.add(
AssistId("inline_type_alias", AssistKind::RefactorInline),
"Inline type alias",
target,
|builder| {
let replacement_text = match replacement {
Replacement::Generic { lifetime_map, const_and_type_map } => {
create_replacement(&lifetime_map, &const_and_type_map, &concrete_type)
}
Replacement::Plain => concrete_type.to_string(),
};
builder.replace(target, replacement_text);
},
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ye that would work
bors r+ |
I'm working towards implementing #10881, but I'd like to get this in first with earlier feedback.
Is
inline_type_alias
a good enough name? I guess the follow up assist would be calledinline_type_alias_into_all_users
based on that.