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

scalafmt failing #1680

Closed SethTisue closed 1 year ago

SethTisue commented 1 year ago

after #1677

-# June 3, 2023
-nightly=2.13.12-bin-1a2373b
+# July 3, 2023
+nightly=2.13.12-bin-7c63166

Bisecting shows the problem to have started with https://github.com/scala/scala/pull/10392 ("IndexedSeq.head throws NoSuchElementException")

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

[scalafmt] [error] Failed tests:
[scalafmt] [error]  org.scalafmt.FidelityTest
[scalafmt] [error]  org.scalafmt.FormatTests
[scalafmt] Caused by: java.util.NoSuchElementException: last of empty IndexedSeq
SethTisue commented 1 year ago

@som-snytt not obvious me to what the cause might be. I'll keep digging.

som-snytt commented 1 year ago

[warn] 1 deprecation (since 18); re-run with -deprecation for details

I think this is our first time seeing Java since.

Nice comment is cold comfort:

  * NOTE(olafurpg). The pattern match in this file has gotten out of hand. It's
  * difficult even for myself to keep track of what's going on

It turns out that Scalameta has (where IndexedSeqOptimized is a dummy trait for 2.13)

@data class Tokens private (
    private val tokens: Array[Token],
    private val start: Int,
    private val end: Int
) extends immutable.IndexedSeq[Token] with IndexedSeqOptimized[Token] {
  def apply(idx: Int): Token = tokens(start + idx)
  def length: Int = end - start

head is defined as apply(0) (the old implementation) and last is inherited, which used to be apply(length-1) but now checks isEmpty first. Since start is typically an index into the large tokens array, apply(-1) does not throw but generally supplies a scala.meta.tokens.Token$Space.

Probably that is just a bug in Scalameta, and the additional bug in Scalafmt is not to assume non-empty tokens (see added filter):

defnBeforeTemplate(leftOwner).filter(_.tokens.nonEmpty).map { x =>
  val policyEnd = Policy.End.On(x.tokens.last)
  delayedBreakPolicy(policyEnd)(forceNewlineBeforeExtends)
}

or what have you.

For a while, I assumed the problem would be brittle inheritance in IndexedSeq, which may still be an issue. Not sure if the following comment just means compatibility between 2.12 and 2.13, etc.

  /* Both head and headOption need to be implemented here due to
   * binary incompatibility caused by
   * https://github.com/scala/scala/commit/b20dd00b11f06c14c823d277cdfb58043a2586fc
   */
  override def head: Token = apply(0)
SethTisue commented 1 year ago

Jenkins is currently chewing on the sbt 1.9.1->2 upgrade. after that, https://github.com/scala/community-build/pull/1681 should get us to where we would be able to pull in a Scalameta fix

fyi @kitbellew

SethTisue commented 1 year ago

@lrytz does this give you cold feet about https://github.com/scala/scala/pull/10392 ?

while you ponder that, I'll try making Som's suggested changes in scalacommunitybuild forks of scalameta and scalafmt.

som-snytt commented 1 year ago

@SethTisue revealing bugs in library code is a good thing. I'm using "good thing" in its generic, non-trademarked sense.

SethTisue commented 1 year ago

ah, @som-snytt is way ahead of me... I'm just now seeing https://github.com/scalameta/scalameta/issues/3235 and https://github.com/scalameta/scalafmt/pull/3581. I'll give those a try

SethTisue commented 1 year ago

yup that did it 🎉 , thx @som-snytt

alexander-klimov commented 5 months ago

What do I need to do to get around this with Scala 3.3.3 and SBT 1.10 ? I'm getting this exact error when using Scalafmt 🤔

kitbellew commented 5 months ago

What do I need to do to get around this with Scala 3.3.3 and SBT 1.10 ? I'm getting this exact error when using Scalafmt 🤔

@alexander-klimov александр, scalafmt не собирается под scala3 (но может форматировать scala3, если собран под 2.12/2.13). в этом был вопрос?

alexander-klimov commented 5 months ago

What do I need to do to get around this with Scala 3.3.3 and SBT 1.10 ? I'm getting this exact error when using Scalafmt 🤔

@alexander-klimov александр, scalafmt не собирается под scala3 (но может форматировать scala3, если собран под 2.12/2.13). в этом был вопрос?

I've just installed Scalafmt using Coursier as described here, and when running it to format my Scala 3 repository, this error - java.util.NoSuchElementException: last of empty IndexedSeq - is thrown many times, and it only formats some of the files (presumably those that don't trigger the error). I'm not sure how I'm supposed to use it with Scala 3 sources.

Note: sbt scalafmtAll runs just fine.

kitbellew commented 5 months ago

What do I need to do to get around this with Scala 3.3.3 and SBT 1.10 ? I'm getting this exact error when using Scalafmt 🤔

@alexander-klimov александр, scalafmt не собирается под scala3 (но может форматировать scala3, если собран под 2.12/2.13). в этом был вопрос?

I've just installed Scalafmt using Coursier as described here, and when running it to format my Scala 3 repository, this error - java.util.NoSuchElementException: last of empty IndexedSeq - is thrown many times, and it only formats some of the files (presumably those that don't trigger the error). I'm not sure how I'm supposed to use it with Scala 3 sources.

Note: sbt scalafmtAll runs just fine.

in that case, can you please submit a reproducible bug report in the appropriate repository?

alexander-klimov commented 5 months ago

What do I need to do to get around this with Scala 3.3.3 and SBT 1.10 ? I'm getting this exact error when using Scalafmt 🤔

@alexander-klimov александр, scalafmt не собирается под scala3 (но может форматировать scala3, если собран под 2.12/2.13). в этом был вопрос?

I've just installed Scalafmt using Coursier as described here, and when running it to format my Scala 3 repository, this error - java.util.NoSuchElementException: last of empty IndexedSeq - is thrown many times, and it only formats some of the files (presumably those that don't trigger the error). I'm not sure how I'm supposed to use it with Scala 3 sources. Note: sbt scalafmtAll runs just fine.

in that case, can you please submit a reproducible bug report in the appropriate repository?

Sure thing. I only wrote in this ticket because I've not found tickets on other repos 😅