scalameta / metals-feature-requests

Issue tracker for Metals feature requests
37 stars 4 forks source link

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

Open carlosedp opened 4 months ago

carlosedp commented 4 months ago

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 commented 3 months ago

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 commented 3 months ago

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

carlosedp commented 3 months ago

@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 commented 3 months ago

Apparently, https://github.com/scalameta/metals/pull/4218 did that but I can't manage it to work...

tgodzik commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

That should be replaced as with adjust type :thinking:

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