scala / community-build

Scala 2 community build — a corpus of open-source repos built against Scala nightlies
Apache License 2.0
139 stars 59 forks source link

unfork scalameta, update Metals #1700

Closed SethTisue closed 1 year ago

SethTisue commented 1 year ago

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1380/ https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1381/

SethTisue commented 1 year ago

scapegoat is failing, but it looks like https://github.com/scapegoat-scala/scapegoat/pull/789 will land soon and then I can try again

SethTisue commented 1 year ago

Metals is failing because they're still on Scalameta 4.8.3. They have an open PR from Scala Steward to upgrade to 4.8.10 at https://github.com/scalameta/metals/pull/5617 . I just commented there expressing hope that it could be merged.

SethTisue commented 1 year ago

https://github.com/scalameta/metals/pull/5671 (successor to 5617) has been merged

SethTisue commented 1 year ago

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1441/ queued

SethTisue commented 1 year ago

in a local test, scapegoat is now giving a single test failure (com.sksamuel.scapegoat.inspections.unnecessary.UnnecessaryConversionTest), instead of a compilation failure

and in Metals, the ever-fragile tests.pc.CompletionSuite fails

let's see what Jenkins says, but this seems promising

SethTisue commented 1 year ago

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1441/artifact/logs/metals-build.log

SethTisue commented 1 year ago

I'm going to go ahead and merge this and worry about the Metals test failures separately

(it's not always easy to get all the stars to align on Scalameta version, so if they're aligned, seize the day!)

SethTisue commented 1 year ago

I'm going to go ahead and merge this and worry about the Metals test failures separately

@kasiaMarek I'm hoping you might have some insight into the pc.CompletionSuite test failures I'm seeing at https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1453/artifact/logs/metals-build.log ...?

I have verified that the problem is reproducible in current head of the metals repo with ++2.13.13-bin-bf45e19! — to run cross/testOnly *pc.CompletionSuite I have to first locally publish scalameta 4.8.12 for that Scala version

I thought maybe some particular Scala 2.13 PR we merged might have caused the change, but I tried bisecting and I found that every 2.13.13-bin-... version fails, even the very first 2.13.13 nightly where we hadn't changed anything substantive, we only bumped the reference compiler version

that makes me suspect there might still be something in the Metals repo that is parsing the Scala version and behaving differently according to what it finds? does that seem possible to you? I did some poking around in the code but it wasn't apparent to me where it might be

I also tried the test case in the REPL with scala-cli -S 2.13.13-bin-bf45e19 and I don't see the spurious List showing up, the completion seems as expected, same as in 2.13.12:

Welcome to Scala 2.13.13-bin-bf45e19 (OpenJDK 64-Bit Server VM, Java 17.0.9).

scala> type TTT[A <: Int] = List[A]
type TTT

scala> val t: TT<TAB>

TT completes to TTT as expected, and pressing the tab key a second time produces type TTT[A <: <?>] = TTT, same as in 2.13.12

hoping you'll have some insight here — I'm stumped

kasiaMarek commented 1 year ago

@kasiaMarek I'm hoping you might have some insight into the pc.CompletionSuite test failures I'm seeing at https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1453/artifact/logs/metals-build.log ...?

I'll take a look, it's indeed odd.

kasiaMarek commented 1 year ago

It's exactly how you expected, @SethTisue, https://github.com/scalameta/metals/pull/5799 should fix it.

SethTisue commented 1 year ago

confirmed working in community build. thank you!