Make the CharSequence wrappers in Predef non-implicit, for JDK 15

[dwijnand]

This change is binary compatible, but not source compatible. Users will need to update callsites to explicitly convert to CharSequence, since the conversions are no longer implicit.

• ArrayCharSequence is no longer needed and will be deprecated in 2.12.13 and 2.13.5. Instead, use java.nio.CharBuffer.wrap to convert an Array[Char] to a CharSequence.
• SeqCharSequence remains useful, but now must be explicitly constructed.

Why was this necessary? In JDK 15 CharSequence has an isEmpty method with a default implementation, which clashes with our Array[Char]#isEmpty, IndexedSeq[Char]#isEmpty, as well as our StringBuilder and reflect's Name.

Fixes scala/bug#12172

[smarter]

IndexedSeq[Char]#isEmpty

isEmpty is defined directly on Seq, so I don't think that an implicit conversion also defining isEmpty leads to a clash.

[dwijnand]

I added it mostly to get rid of the other way CharSequence, which we don't control, can break us.

[SethTisue]

do we want to deprecate these and suggest people write new ...?

   def SeqCharSequence(sequenceOfChars: scala.collection.IndexedSeq[Char]): SeqCharSequence = new SeqCharSequence(sequenceOfChars)
   def ArrayCharSequence(arrayOfChars: Array[Char]): ArrayCharSequence = new ArrayCharSequence(arrayOfChars)

in the ArrayCharSequence case, note that you can call java.nio.CharBuffer.wrap instead. which tempts me to suggest we deprecate ArrayCharSequence in its entirety

[smarter]

do we want to deprecate these and suggest people write new ...?

Bit of a mixed message with http://dotty.epfl.ch/docs/reference/other-new-features/creator-applications.html :)

in the ArrayCharSequence case, note that you can call java.nio.CharBuffer.wrap instead. which tempts me to suggest we deprecate ArrayCharSequence in its entirety

Yep that's what I suggested in the previous PR too.

[dwijnand]

do we want to deprecate these and suggest people write new ...?

Bit of a mixed message with http://dotty.epfl.ch/docs/reference/other-new-features/creator-applications.html :)

Just SeqCharSequence(myCharList).substring(..) without the new.

I agree to deprecating ArrayCharSequence and pointing to CharBuffer wrap.

[SethTisue]

community build results for 2.13.4-bin-0230f94-SNAPSHOT are in.

my conclusion: this stuff is rarely used. it is okay to go ahead with this.

details follow:

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2065/
https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2066/
https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2067/
https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2068/

scala-parser-combinators needed changes to 3 call sites in 3 different source files: scala/scala-parser-combinators#306 — it is using SeqCharSequence in a way that actually kind of makes sense in my glance at it (namely, to avoid copying)

scalatest was just using ArrayCharSequence in a single call site in a single source file, and only in an currently silly/unnecessary way (perhaps justified at some point in the distant past? idk): SethTisue/scalatest@2f6a176

I had to fix those (and, separately, scalatest-3-0) in order to get a usable run.

The remaining failures are: FAILURES (UNEXPECTED): twirl,scalafix,scoverage; BLOCKING DOWNSTREAM: twirl (12), scalafix (1)

the twirl failure is actually about that the exhaustivity changes and not about this. so that leaves scalafix and scoverage

in scalafix, four call sites in a single source file is affected; in scoverage, two call sites in a single source file

this all seems well within the range of acceptability to me

[smarter]

One thing that I don't think has been discussed: should this be backported to 2.12? Or perhaps the original idea of hacking implicit search would be more appropriate there? Or Java 15+ should just be declared unsupported? (Might be problematic given that Java 17 is not that far away and will be an LTS release).

[dwijnand]

Yeah, we'll want to backport this to 2.12, though it's not as critical as 2.13.4 for it to be in 2.12.13, IMO. PR's welcome.

[lrytz]

Yeah, we'll want to backport this to 2.12

I agree.

SeqCharSequence(myCharList).substring(..) without the new.

A good replacement would be asJavaCharSequence in https://github.com/scala/scala/blob/2.13.x/src/library/scala/jdk/CollectionConverters.scala, but binary compatibility. scala-library-next? Or a ticket to remember for the next std lib?

I agree to deprecating ArrayCharSequence and pointing to CharBuffer wrap.

That wasn't done in the end, it seems?

[SethTisue]

PR deprecating ArrayCharSequence on 2.12.x, also to be forward-merged (at some point) to 2.13.x: #9321

PR backporting this PR to 2.12.x: #9322

[sjrd]

But first I want to ask @sjrd: does Scala.js support java.nio.CharBuffer.wrap? Because if it doesn't, that could be a reason not to deprecate ArrayCharSequence.

Yes, it's supported. I even already used it as a replacement in one of our tests that was relying on ArrayCharSequence. ;)

[retronym]

Another way the source incompatibility can be experienced, for the record.

Welcome to Scala 2.12.12 (Java HotSpot(TM) 64-Bit Server VM, Java 14).
Type in expressions for evaluation. Or try :help.

scala> object X { val sb = new StringBuilder(); println(sb.length())}
defined object X

Welcome to Scala 2.12.13-20201208-103432-471ea9d (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_144).
Type in expressions for evaluation. Or try :help.

scala> object X { val sb = new StringBuilder(); println(sb.length())}
<console>:11: error: Int does not take parameters
object X { val sb = new StringBuilder(); println(sb.length())}
                                                          ^