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

INT: disable SubstituteAssociatedTypeIntention for type aliases #6032

Merged

Conversation

Kobzol
Copy link
Member

@Kobzol Kobzol commented Aug 30, 2020

SubstituteAssociatedTypeIntention has also worked for type aliases. It is OK in basic cases, but for generic type aliases (which cannot occur currently for associated types on stable - this is the infamous GAT feature) it doesn't work properly. There are some issues with propagating type arguments and also with handling the alias itself - it's only properly supported for ADTs currently. So for now I think that we should disable this intention for type aliases.

Fixes: #6030

@Kobzol Kobzol added the fix Pull requests that fix some bug(s) label Aug 30, 2020
@Kobzol Kobzol force-pushed the int-subst-assoc-type-no-type-alias branch from 287d35b to d95d9d1 Compare August 30, 2020 21:46
@vlad20012 vlad20012 added this to the v130 milestone Aug 31, 2020
@vlad20012
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 31, 2020

Build succeeded:

@bors bors bot merged commit 0e79d2a into intellij-rust:master Aug 31, 2020
@Kobzol Kobzol deleted the int-subst-assoc-type-no-type-alias branch August 31, 2020 10:02
bors bot added a commit that referenced this pull request Jul 2, 2021
7345: INT: enable SubstituteAssociatedTypeIntention for type aliases r=vlad20012 a=Kobzol

This PR adds support for substituting type aliases to `SubstituteAssociatedTypeIntention`. This was previously disabled in #6032 due to problems with generic parameters (#6030).

I used the approach hinted by @vlad20012 [here](#6030 (comment)), it seems to work fine.

However, there is still a problem with paths using type aliases that are in expression context and that have inferred type arguments (see the commented test). In this situation, the resolved type is just a new type argument placeholder (e.g. `T`), but it's not resolved to the actual type yet (https://github.com/intellij-rust/intellij-rust/blob/master/src/main/kotlin/org/rust/lang/core/resolve/ref/RsPathReferenceImpl.kt#L234).

I need a way to somehow get the inferred result of the type from type inference. I can get the type of the function call `Foo::new` (`fn() -> S<i32>`), but I need the type of `Foo` (`S<i32>`). Is there some existing way of doing that?

changelog: The intention that enables you to substitute associated types now works on all type aliases.

Co-authored-by: Jakub Beránek <berykubik@gmail.com>
bors bot added a commit that referenced this pull request Jul 2, 2021
7345: INT: enable SubstituteAssociatedTypeIntention for type aliases r=vlad20012 a=Kobzol

This PR adds support for substituting type aliases to `SubstituteAssociatedTypeIntention`. This was previously disabled in #6032 due to problems with generic parameters (#6030).

I used the approach hinted by @vlad20012 [here](#6030 (comment)), it seems to work fine.

However, there is still a problem with paths using type aliases that are in expression context and that have inferred type arguments (see the commented test). In this situation, the resolved type is just a new type argument placeholder (e.g. `T`), but it's not resolved to the actual type yet (https://github.com/intellij-rust/intellij-rust/blob/master/src/main/kotlin/org/rust/lang/core/resolve/ref/RsPathReferenceImpl.kt#L234).

I need a way to somehow get the inferred result of the type from type inference. I can get the type of the function call `Foo::new` (`fn() -> S<i32>`), but I need the type of `Foo` (`S<i32>`). Is there some existing way of doing that?

changelog: The intention that enables you to substitute associated types now works on all type aliases.

Co-authored-by: Jakub Beránek <berykubik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests that fix some bug(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Substitute associated type" intention works for regular type aliases
2 participants