Skip to content

feat: implement 'convert to named lambda parameters' code action #6669

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

Conversation

KacperFKorban
Copy link
Collaborator

@KacperFKorban KacperFKorban commented Aug 12, 2024

Associated dotty PR: scala/scala3#22799

Add a code action that converts a wildcard lambda to a lambda with named parameters. It supports Scala 3 only at the moment.

e.g.
converttonamedlambdaparameters
converttonamedlambdaparameters1

Disclaimer: I'm not sure what I'm doing 😃

@KacperFKorban KacperFKorban force-pushed the convert-to-named-lambda-params branch 2 times, most recently from 6fd2149 to 601c8f7 Compare August 12, 2024 16:28
@KacperFKorban KacperFKorban marked this pull request as draft August 13, 2024 09:18
@KacperFKorban KacperFKorban force-pushed the convert-to-named-lambda-params branch from 601c8f7 to 0eabc2e Compare August 13, 2024 09:38
@tgodzik
Copy link
Contributor

tgodzik commented Aug 13, 2024

I wonder if we need to add a new method for this. Ideally each new code action feature would be able to reuse an existing method. Would it possible to implements it with the scalameta parser only? We could use go to type definition on wildcards to obtain its type symbol and construct a new name out of it.

Alternatively, we could add a more universal method such as (ActionType interface should be binary compatible I think 🤔 otherwise we can use a string and some separate config parameter)

CompletableFuture<List<TextEdit>> codeAction(OffsetParams params, ActionType actionType) {

Adding a new method for each code action causes the interfaces to bloat and more complex to the users, which is why I am hesitant to continue with that.

@KacperFKorban
Copy link
Collaborator Author

Using only the scalameta parser would be ideal, but my concern is that it isn't that obvious how to find the reference of the correct wildcard parameter. With pc, I can just use the parameter symbol. I'll try to reimplement it using just trees and we'll see how it goes.

@tgodzik
Copy link
Contributor

tgodzik commented Aug 13, 2024

but my concern is that it isn't that obvious how to find the reference of the correct wildcard parameter

There is only one, no? Not sure if I understand 🤔

@KacperFKorban
Copy link
Collaborator Author

I mean that there could be multiple wildcard params for one lambda and there can be nested wildcard lambdas. Plus the rules for resolving a wildcard param into its owning lambda are not 100% clear to me. In pc I can reference the parameters of a lambda and use their symbols, because it is already done in the Scala compiler's parser. But the tree representing a wildcard lambda in scalameta is just a node AnonymousFunction with a single field -- body.

@tgodzik
Copy link
Contributor

tgodzik commented Aug 13, 2024

Ach, if it's more difficult with the scalameta parser then do not worry about that. I would however try to add a more universal interface for code actions

@KacperFKorban KacperFKorban force-pushed the convert-to-named-lambda-params branch from 0eabc2e to 746a923 Compare November 5, 2024 20:31
@KacperFKorban KacperFKorban force-pushed the convert-to-named-lambda-params branch from 746a923 to 24383a3 Compare March 13, 2025 17:57
KacperFKorban added a commit to scala/scala3 that referenced this pull request May 1, 2025
Associated Metals PR: scalameta/metals#6669

Add a code action that converts a wildcard lambda to a lambda with named
parameters. It supports Scala 3 only at the moment.

e.g.

![converttonamedlambdaparameters](https://github.com/user-attachments/assets/93561630-b626-4cff-8248-806e4c32744a)

![converttonamedlambdaparameters1](https://github.com/user-attachments/assets/292ad14f-be8b-4535-b4e4-45819f14da9e)
@KacperFKorban KacperFKorban force-pushed the convert-to-named-lambda-params branch from 24383a3 to 9ae0a02 Compare May 2, 2025 08:33
@KacperFKorban KacperFKorban marked this pull request as ready for review May 5, 2025 06:59
@KacperFKorban KacperFKorban requested a review from kasiaMarek May 5, 2025 07:00
tgodzik pushed a commit to scala/scala3-lts that referenced this pull request May 5, 2025
…#22799)

Associated Metals PR: scalameta/metals#6669

Add a code action that converts a wildcard lambda to a lambda with named
parameters. It supports Scala 3 only at the moment.

e.g.

![converttonamedlambdaparameters](https://github.com/user-attachments/assets/93561630-b626-4cff-8248-806e4c32744a)

![converttonamedlambdaparameters1](https://github.com/user-attachments/assets/292ad14f-be8b-4535-b4e4-45819f14da9e)
[Cherry-picked 43118de]
@KacperFKorban KacperFKorban force-pushed the convert-to-named-lambda-params branch from 84e8e7d to 0091b56 Compare May 6, 2025 07:25
Copy link
Member

@kasiaMarek kasiaMarek left a comment

Choose a reason for hiding this comment

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

lgtm

@tgodzik tgodzik merged commit 7bdbe16 into scalameta:main May 6, 2025
48 of 49 checks passed
@KacperFKorban KacperFKorban deleted the convert-to-named-lambda-params branch May 6, 2025 15:09
odersky pushed a commit to dotty-staging/dotty that referenced this pull request May 12, 2025
…#22799)

Associated Metals PR: scalameta/metals#6669

Add a code action that converts a wildcard lambda to a lambda with named
parameters. It supports Scala 3 only at the moment.

e.g.

![converttonamedlambdaparameters](https://github.com/user-attachments/assets/93561630-b626-4cff-8248-806e4c32744a)

![converttonamedlambdaparameters1](https://github.com/user-attachments/assets/292ad14f-be8b-4535-b4e4-45819f14da9e)
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.

3 participants