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

Extract into Function does not create a generic function with constraints when extracting generic code #7636

Closed
gbutler69 opened this issue Feb 11, 2021 · 1 comment
Labels
A-assists E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now

Comments

@gbutler69
Copy link

With the new "Extract into Function" code action that recently landed, although you can extract generic code, it does not make the new function generic (does not include the Generic parameters within brackets preceding the function name) and it does not include the generic constrains in the where clause. This makes the use of "Extract into Function" with generic code sub-optimal.

@Veykril Veykril added A-assists S-actionable Someone could pick this issue up and work on it right now labels Feb 11, 2021
DorianListens added a commit to DorianListens/rust-analyzer that referenced this issue Jun 16, 2022
This change attempts to resolve issue rust-lang#7637: Extract into Function does not
create a generic function with constraints when extracting generic code.

In `FunctionBody::analyze_container`, when the ancestor matches `ast::Fn`, we
can perserve both the `generic_param_list` and the `where_clause`. These can
then be included in the newly extracted function output via `format_function`.

From what I can tell, the only other ancestor type that could potentially have
a generic param list would be `ast::ClosureExpr`. In this case, we perserve the
`generic_param_list`, but no where clause is ever present.

In this inital implementation, all the generic params and where clauses from
the parent function will be copied to the newly extracted function. An obvious
improvement would be to filter this output in some way to only include generic
parameters that are actually used in the function body.

fixes rust-lang#7636
DorianListens added a commit to DorianListens/rust-analyzer that referenced this issue Jun 16, 2022
This change attempts to resolve issue rust-lang#7637: Extract into Function does not
create a generic function with constraints when extracting generic code.

In `FunctionBody::analyze_container`, when the ancestor matches `ast::Fn`, we
can perserve both the `generic_param_list` and the `where_clause`. These can
then be included in the newly extracted function output via `format_function`.

From what I can tell, the only other ancestor type that could potentially have
a generic param list would be `ast::ClosureExpr`. In this case, we perserve the
`generic_param_list`, but no where clause is ever present.

In this inital implementation, all the generic params and where clauses from
the parent function will be copied to the newly extracted function. An obvious
improvement would be to filter this output in some way to only include generic
parameters that are actually used in the function body.

fixes rust-lang#7636
DorianListens added a commit to DorianListens/rust-analyzer that referenced this issue Jun 16, 2022
This change attempts to resolve issue rust-lang#7637: Extract into Function does not
create a generic function with constraints when extracting generic code.

In `FunctionBody::analyze_container`, when the ancestor matches `ast::Fn`, we
can perserve both the `generic_param_list` and the `where_clause`. These can
then be included in the newly extracted function output via `format_function`.

From what I can tell, the only other ancestor type that could potentially have
a generic param list would be `ast::ClosureExpr`. In this case, we perserve the
`generic_param_list`, but no where clause is ever present.

In this inital implementation, all the generic params and where clauses from
the parent function will be copied to the newly extracted function. An obvious
improvement would be to filter this output in some way to only include generic
parameters that are actually used in the function body.

fixes rust-lang#7636
@Veykril Veykril added the E-unknown It's unclear if the issue is E-hard or E-easy without digging in label Jun 22, 2022
DorianListens added a commit to DorianListens/rust-analyzer that referenced this issue Jul 13, 2022
This change attempts to resolve issue rust-lang#7636: Extract into Function does not
create a generic function with constraints when extracting generic code.

In `FunctionBody::analyze_container`, we now traverse the `ancestors` in search
of `AnyHasGenericParams`, and attach any `GenericParamList`s and `WhereClause`s
we find to the `ContainerInfo`.

Later, in `format_function`, we collect all the `GenericParam`s and
`WherePred`s from the container, and filter them to keep only types matching
`TypeParam`s used within the newly extracted function body or param list. We
can then include the new `GenericParamList` and `WhereClause` in the new
function definition.

This change only impacts `TypeParam`s. `LifetimeParam`s and `ConstParam`s are
out of scope for this change.
bors added a commit that referenced this issue Jul 14, 2022
fix: Support generics in extract_function assist

This change attempts to resolve issue #7636: Extract into Function does not
create a generic function with constraints when extracting generic code.

In `FunctionBody::analyze_container`, we now traverse the `ancestors` in search
of `AnyHasGenericParams`, and attach any `GenericParamList`s and `WhereClause`s
we find to the `ContainerInfo`.

Later, in `format_function`, we collect all the `GenericParam`s and
`WherePred`s from the container, and filter them to keep only types matching
`TypeParam`s used within the newly extracted function body or param list. We
can then include the new `GenericParamList` and `WhereClause` in the new
function definition.

This change only impacts `TypeParam`s. `LifetimeParam`s and `ConstParam`s are
out of scope for this change.

I've never contributed to this project before, but I did try to follow the style guide. I believe that this change represents an improvement over the status quo, but I think it's also fair to argue that it doesn't fully "fix" the linked issue. I'm totally open to merging this as is, or going further to try to make a more complete solution. Also: if there are other unit or integration tests I should add, please let me know where to look!
Veykril pushed a commit to Veykril/rust-analyzer that referenced this issue Aug 9, 2022
This change attempts to resolve issue rust-lang#7636: Extract into Function does not
create a generic function with constraints when extracting generic code.

In `FunctionBody::analyze_container`, we now traverse the `ancestors` in search
of `AnyHasGenericParams`, and attach any `GenericParamList`s and `WhereClause`s
we find to the `ContainerInfo`.

Later, in `format_function`, we collect all the `GenericParam`s and
`WherePred`s from the container, and filter them to keep only types matching
`TypeParam`s used within the newly extracted function body or param list. We
can then include the new `GenericParamList` and `WhereClause` in the new
function definition.

This change only impacts `TypeParam`s. `LifetimeParam`s and `ConstParam`s are
out of scope for this change.
@lnicola
Copy link
Member

lnicola commented Aug 27, 2024

Fixed in #12556.

@lnicola lnicola closed this as completed Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

3 participants