scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.08k stars 330 forks source link

Incorrect inferred method type argument in Scala 3 #4338

Open kpodsiad opened 2 years ago

kpodsiad commented 2 years ago

Describe the bug

//> using scala "3.2.0"

//> using lib "org.typelevel::cats-core:2.8.0"

import cats.data.StateT
import cats.syntax.all._
import cats.implicits._
import cats.Id

object Cats extends App {
  type IdState[A] = StateT[Id, Int, A]

  val test: IdState[Unit] =
    StateT.modify(old => old)

  test.runS(1)

}

Screenshot 2022-09-03 at 07 43 01

Expected behavior

StateT.modify has correct inferred type [Id, Int]

Operating system

macOS

Editor/Extension

VS Code

Version of Metals

0.11.8+72-6567f1a3-SNAPSHOT

Extra context or search terms

No response

tgodzik commented 2 years ago

Thanks for reporting, it seems in semanticdb we get: [13:4..13:17) => *[<?>, Int]

Which means we need to fix it in the compiler.

tanishiking commented 2 years ago

Probably related to https://github.com/scalameta/metals/issues/3935 I'll take a look :eyes:

tanishiking commented 2 years ago

Looks like this is caused by the lack of HKTypeLambda support in SemanticDB for Scala3, maybe it's time to working on it 👍

tanishiking commented 2 years ago

Realized synthetics for inferred type parameters have disappeared in 3.2.0 :cry: https://github.com/lampepfl/dotty/pull/15877

kpodsiad commented 2 years ago

What does that mean @tanishiking ?

tanishiking commented 2 years ago

inferred type decoration for StateT.modify won't be available with the latest version of Scala. (same applies to all the type apply like List(1.2.3) -> List[Int](1.2.3)).

Screen Shot 2022-09-13 at 15 46 10

It's regression, I'll take a look before working on lambda type support.

kpodsiad commented 2 years ago

It was a big PR, no one can review 8k lines of code 😞 Thanks @tanishiking

tgodzik commented 2 years ago

Thanks for finding this @tanishiking ! I asked Martin about it in https://github.com/lampepfl/dotty/pull/15877/files#r969241924 It's not cool that it contained so visible regressions and yet it was just merged.

tanishiking commented 2 years ago

Thanks, @tgodzik! Yeah, it (ignoring the regression test) shouldn't happen, but for this case, it might be easier to fix on top of the change made by Martin compared to revert it.

tgodzik commented 2 years ago

Thanks, @tgodzik! Yeah, it (ignoring the regression test) shouldn't happen, but for this case, it might be easier to fix on top of the change made by Martin compared to revert it.

Sure, I get that, but I think next time they should have us sign off on such changes.

tanishiking commented 2 years ago

Submit a PR https://github.com/lampepfl/dotty/pull/16031 to fix the regression