Skip to content

Feature request: Inline type alias #10881

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

Closed
jplatte opened this issue Nov 28, 2021 · 9 comments · Fixed by #13036
Closed

Feature request: Inline type alias #10881

jplatte opened this issue Nov 28, 2021 · 9 comments · Fixed by #13036
Labels
A-assists C-feature Category: feature request good first issue S-actionable Someone could pick this issue up and work on it right now

Comments

@jplatte
Copy link
Contributor

jplatte commented Nov 28, 2021

Similar to "Inline function" / "Inline into all callers", it would be nice if there was "Inline typealias" and "Inline into all uses" for type aliases.

@Veykril Veykril added A-assists C-feature Category: feature request S-actionable Someone could pick this issue up and work on it right now labels Nov 28, 2021
@djc
Copy link

djc commented Dec 30, 2021

Was thinking about this yesterday, would’ve been nice for rustls/rustls#895.

@steven-joruk
Copy link
Contributor

I'll try to implement this.

bors bot added a commit that referenced this issue Mar 20, 2022
11690: feat: Add an assist for inlining type aliases r=Veykril a=steven-joruk

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 called `inline_type_alias_into_all_users` based on that.

![valid_inlines](https://user-images.githubusercontent.com/1277939/158020510-fed78b5c-4c7e-46d1-9151-3044a29b9990.gif)

![invalid_inlines](https://user-images.githubusercontent.com/1277939/158020516-8a2deb6d-c6ec-4adf-a15b-c514fc97dc43.gif)



Co-authored-by: Steven Joruk <[email protected]>
@0xPoe
Copy link
Member

0xPoe commented Jul 5, 2022

@Veykril It seems we can close this issue?

@steven-joruk
Copy link
Contributor

Hey @hi-rustin, #11690 only added inlining a single instance of a type alias use, this feature also requests inlining the definition to all users.

@sancho20021
Copy link
Contributor

I'm going to try this one

@sancho20021
Copy link
Contributor

Hi @steven-joruk! Currently I'm trying to implement inline_alias_uses.

You added a nice test:

// This must be supported in order to support "inline_alias_to_users" or
// whatever it will be called.
#[test]
fn alias_as_expression_ignored() {
check_assist_not_applicable(
inline_type_alias,
r#"
type A = Vec<u32>;
fn main() {
let a: A = $0A::new();
}
"#,
);
}

which shows that the current implementation can't inline such alias uses.

IIUC, this happens because ctx.find_node_at_offset::<ast::PathType>() invocation returns None, and we have no alias instance to work with.

Could you please explain, why ctx.find_node_at_offset behaves in such a way? This is my first issue in rust-analyzer so I have very little understanding of things 😄

@Veykril
Copy link
Member

Veykril commented Aug 8, 2022

An ast::PathType specifically is an ast::Path that appreas in type position, as an example let PathPat: PathType = PathExpr;.

So to support inlining on usages, we should be checking ctx.find_note_at_offset::<ast::Path>(), then check the parent node of that path and act accordingly.

@sancho20021
Copy link
Contributor

Besides calling an associated function on an aliased type, there seems to be a lot of other usages. (Enum variant, Record expression, etc)

What is more important that needs to be done first, cover as much usages as possible in inline_type_alias or implementing a basic version of inline_type_alias_uses that works only for PathType?

By the way, I'm not sure I understand, what does Path represent?

@Veykril
Copy link
Member

Veykril commented Aug 11, 2022

A Path is just something like foo::bar::Baz or foo on its own as well. PathType, PathPat, PathExpr etc just wrap a Path then depending on whether it is a type, pattern expression

It doesnt really matter what you improve herefirst, thats up to you

sancho20021 added a commit to sancho20021/rust-analyzer that referenced this issue Aug 16, 2022
bors added a commit that referenced this issue Aug 18, 2022
…ykril

feat: Add an assist for inlining all type alias uses

## Description
`inline_type_alias_uses` assist tries to inline all selected type alias occurrences.

### Currently
Type alias used in `PathType` position are inlined.

### Not supported
- Removing type alias declaration if all uses are inlined.
- Removing redundant imports after inlining all uses in the file.
- Type alias not in `PathType` position, such as:
  - `A::new()`
  - `let x = A {}`
  - `let bits = A::BITS`
  - etc.

## Demonstration

![example](https://user-images.githubusercontent.com/45790125/184905226-9cb8ac81-1439-4387-a13b-e18ad4ecf208.gif)

## Related Issues
Partially fixes #10881
@bors bors closed this as completed in 5543dd8 Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists C-feature Category: feature request good first issue S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants