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

Make the code action "Insert type annotation" become "Insert or update type annotation" #386

Open
carlosedp opened this issue Jun 3, 2024 · 7 comments

Comments

@carlosedp
Copy link

Is your feature request related to a problem? Please describe.

Currently Metals can add the type annotation to methods, variables, etc but in case you need to change the return type (Eg. in a refactor), you need to delete manually the type and ask Metals to re-add the new one.

Describe the solution you'd like

It would be nice if the Quick Action (code action) could be "Insert or update type annotation" that would replace the current annotation with the new one inferred by the compiler.

Describe alternatives you've considered

Manually removing the existing annotation and re-adding via existing action.

Additional context

No response

Search terms

type annotation

@tgodzik
Copy link
Contributor

tgodzik commented Jun 11, 2024

This should be fairly easy to do if anyone is interested. We would have to change https://github.com/scalameta/metals/blob/3326953a7fe3f2e7efdae3b724fe0ab10c926778/metals/src/main/scala/scala/meta/internal/metals/codeactions/InsertInferredType.scala#L87 to also show the code action when the type is defined (just change the title in that case) and write tests in order to verify if that works)

@carlosedp
Copy link
Author

Sweet, thanks for the pointer... I'll take a look at it!

@carlosedp
Copy link
Author

@tgodzik any tip on how can I get the inferred type for the Term to compare with the existing one in inferTypeTitle so I can only show the message if they differ?

@carlosedp
Copy link
Author

Apparently, scalameta/metals#4218 did that but I can't manage it to work...

@tgodzik
Copy link
Contributor

tgodzik commented Jun 11, 2024

That's only on diagnostic, so you would need to save it. You can do something like:

      case Term.Param(_, _, tpe, _)  =>
        if (tpe.isEmpty)
          Some(insertType)
        else Some(updateType)

There is no way to find the type without compiling and getting diagnostics, so updateType would just be "Update type"

@carlosedp
Copy link
Author

I got there... the problem is that running the action when there's already a correct type, adds another type... like this:

image

Apparently the InferredTypeProvider is not removing the previous type or checking if it's the same... gonna take a look at it.

@tgodzik
Copy link
Contributor

tgodzik commented Jun 11, 2024

That should be replaced as with adjust type 🤔

Might be an issue here then https://github.com/scalameta/metals/blob/main/mtags/src/main/scala-2/scala/meta/internal/pc/InferredTypeProvider.scala#L87

We would maybe need to add a condition to not return anything in that case. There should also be the same class for Scala 3

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

No branches or pull requests

2 participants