REF: Don't insert default type parameters by Implement Member actions

[mchernyavsky]

Fixes #6186

Depends on #6948.

changelog: Don't insert default type parameters by Implement Member actions

[Undin]

@mchernyavsky could you resolve merge conflicts?

[mchernyavsky]

@Undin Done.

[Kobzol]

I wonder if skipUnchangedDefaultTypeArguments should just be true by default?

[Undin]

The current implementation doesn't work with type aliases. For example, for the following code:

mod bar {
    use std::collections::hash_map::RandomState;

    pub(crate) type HashMap<T, K = i32, S = RandomState> = std::collections::HashMap<T, K, S>;
}

trait Foo {
    fn foo() -> bar::HashMap<String>;
}

impl Foo for i32 {
}

Implement memebers produces

use crate::bar::HashMap;

mod bar {
    use std::collections::hash_map::RandomState;

    pub(crate) type HashMap<T, K = i32, S = RandomState> = std::collections::HashMap<T, K, S>;
}

trait Foo {
    fn foo() -> bar::HashMap<String>;
}

impl Foo for i32 {
    fn foo() -> HashMap<String, i32, RandomState> {
        unimplemented!()
    }
}

although I expect

use crate::bar::HashMap;

mod bar {
    use std::collections::hash_map::RandomState;

    pub(crate) type HashMap<T, K = i32, S = RandomState> = std::collections::HashMap<T, K, S>;
}

trait Foo {
    fn foo() -> bar::HashMap<String>;
}

impl Foo for i32 {
    fn foo() -> HashMap<String> {
        unimplemented!()
    }
}

[Undin]

Agree with @Kobzol about default value
Let's make it true by default.
Also, it would be great to add test(s) for each feature where importTypeReferencesFromElements is used to check that we don't import something unexpected. For example, change signature also use it

[Undin]

BTW do we know cases when skipUnchangedDefaultTypeArguments is actually needed?
If no, maybe it's better just not to add this argument at all and change import logic for all cases?

[Undin]

Note, I don't insist to make this investigation in this PR, i.e. we can investigate all other cases separately and decide if we need this param and which default value should be used or even we can drop it at all

[mchernyavsky]

Maybe it make sense to change skipUnchangedDefaultTypeArguments to true for Ty.renderInsertionSafe, but not for Ty.render? And remove this parameter for importTypeReferencesFromElements / getTypeReferencesInfoFromTys?

[Undin]

Probably. But it requires checking each feature that uses importing that may complicate this PR. But it's up to you

[mchernyavsky]

@Undin Fixed.

[Undin]

@mchernyavsky tests fail

[mchernyavsky]

@Undin fixed.

[mchernyavsky]

@Undin Rebased on master.

[Undin]

👍
bors r+

[bors]

This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried.

[bors]

Build succeeded:

• check