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

scalafmt failing #1680

Closed
SethTisue opened this issue Jul 4, 2023 · 12 comments
Closed

scalafmt failing #1680

SethTisue opened this issue Jul 4, 2023 · 12 comments
Assignees

Comments

@SethTisue
Copy link
Member

SethTisue commented Jul 4, 2023

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 scala/scala#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
Copy link
Member Author

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

@som-snytt
Copy link

[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
Copy link
Member Author

SethTisue commented Jul 11, 2023

Jenkins is currently chewing on the sbt 1.9.1->2 upgrade. after that, #1681 should get us to where we would be able to pull in a Scalameta fix

fyi @kitbellew

@SethTisue
Copy link
Member Author

@lrytz does this give you cold feet about scala/scala#10392 ?

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

@som-snytt
Copy link

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

@SethTisue
Copy link
Member Author

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

@SethTisue
Copy link
Member Author

yup that did it 🎉 , thx @som-snytt

@alexander-klimov
Copy link

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
Copy link

kitbellew commented Jun 25, 2024

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
Copy link

alexander-klimov commented Jun 25, 2024

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
Copy link

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
Copy link

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 😅

mbland added a commit to mbland/rules_scala that referenced this issue Oct 1, 2024
As it turns out, we're using a very old version of Scalafmt that breaks
completely:

```txt
ERROR: rules_scala/test/scalafmt/BUILD:43:20:
  ScalaFmt test/scalafmt/test/scalafmt/formatted/formatted-test.scala.fmt.output failed:
  (Exit 1): scalafmt failed: error executing ScalaFmt command
  (from target //test/scalafmt:formatted-test)
bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/scala/scalafmt/scalafmt
  '--jvm_flag=-Dfile.encoding=UTF-8' ... (remaining 1 argument skipped)

java.util.NoSuchElementException: last of empty IndexedSeq
```

This matches:

- scala/community-build#1680

Which mentions apparent fixes in:

- scalameta/scalameta#3235
- scalameta/scalafmt#3581

So the fix is to update Scalafmt, but given how we don't use
rules_jvm_external, that means a lot of manual updates to
third_party/repositories/scala_*.bzl. There doesn't appear to be a way
to automate this; there's no indication that the most recent update was
automated in any way:

- bazelbuild#1543

So I'll plow through all the JARs and make the necessary changes:

- https://mvnrepository.com/artifact/org.scalameta/scalafmt-core

I can't update scala_2_11, since there's not a more recent compatible
version. But the changes to scala_2_12 and scala_2_13 should be similar,
and the changes to 2_13 should apply to 3_{1,2,3,4} as well.

I had to hack .scalafmt.conf and ScalafmtWorker.scala a bit. This may
make it incompatible with 2.11.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants