Skip to content

feat: Add an assist for inlining all type alias uses #13036

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 3 commits into from
Aug 18, 2022

Conversation

sancho20021
Copy link
Contributor

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

Related Issues

Partially fixes #10881

@sancho20021
Copy link
Contributor Author

One test (inline_uses_across_files_fails) does not pass, and I don't understand why:
Source:

//- /lib.rs
mod foo;
type $0I = i32;

//- /foo.rs
use super::I;
fn foo() {
    let _: I = 0;
}

Expected output:

//- /lib.rs
mod foo;
type I = i32;

//- /foo.rs
use super::I;
fn foo() {
    let _: i32 = 0;
}

Actual:

use super::I;
fn foo() {
    let _: i32 = 0;
}

I'm not familiar with the way it is being tested, @Veykril, maybe you can help.

@Veykril
Copy link
Member

Veykril commented Aug 16, 2022

The output will only render files that have changed content, so lib.rs is not being re-rendered since it doesn't contain changes/

.into_iter()
.filter_map(|file_ref| match file_ref.name {
ast::NameLike::NameRef(path_type) => {
path_type.syntax().ancestors().find_map(ast::PathType::cast)
Copy link
Member

Choose a reason for hiding this comment

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

This might hit some paths that it shouldn't right now, like T::Assoc where T is what we inline. In this case we should just go with path_type.syntax().ancestors().nth(3).and_then(ast::Pathtype::cast). We take the third ancestor because of the following hierarchy, NameRef -> PathSegment -> Path -> PathType. As this assist currently only supports single segment path types, so we should make sure to only pick those here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, fixed!

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Aug 18, 2022

📌 Commit 313b004 has been approved by Veykril

It is now in the queue for this repository.

bors added a commit that referenced this pull request 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
Copy link
Contributor

bors commented Aug 18, 2022

⌛ Testing commit 313b004 with merge 02466b2...

@bors
Copy link
Contributor

bors commented Aug 18, 2022

💔 Test failed - checks-actions

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

@bors retry

@bors
Copy link
Contributor

bors commented Aug 18, 2022

⌛ Testing commit 313b004 with merge 5543dd8...

@bors
Copy link
Contributor

bors commented Aug 18, 2022

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 5543dd8 to master...

@ice1000
Copy link
Contributor

ice1000 commented Sep 3, 2022

The first two "not supported" are now supported in #13091

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.

Feature request: Inline type alias
4 participants