Bug overriding varargs method defined both in Scala class and Java interface, while also narrowing return type

[mkurz]

reproduction steps

(see #11944 (comment) for minimal repro)

Clone my fork of the Play Framework and checkout the addAttrs branch (according pull request):

git clone git@github.com:mkurz/playframework.git
git checkout addAttrs

Try to compile:

sbt compile

problem

Because of this commit compilation fails:

[error] ./core/play/src/main/scala/play/core/j/JavaHelpers.scala:258:7: incompatible type in overriding
[error] def addAttrs(entries: play.libs.typedmap.TypedEntry[_]*): play.mvc.Http.Request (defined in trait Request)
[error]   with override def addAttrs(entries: play.libs.typedmap.TypedEntry[_]*): play.mvc.Http.RequestHeader (defined in class RequestHeaderImpl);
[error]  found   : (entries: play.libs.typedmap.TypedEntry[_]*)play.mvc.Http.RequestHeader
[error]  required: (entries: play.libs.typedmap.TypedEntry[_]*)play.mvc.Http.Request
[error] class RequestImpl(request: Request[RequestBody]) extends RequestHeaderImpl(request) with JRequest {
[error]       ^

I am pretty sure this is a bug in the Scala compiler.

All the commit does it adds the method addAttrs with a varargs param to Java interfaces and Scala classes that extend from those interfaces (and from each other).
If you look at the commit you will see there is an existing addAttr method (without s and the end) already, just above the addAttrs method I want to add. This existing method follows the exact same scheme (e.g. return type) like the method I want to add. For addAttr the code compiles, addAttrs makes it fail. That's why I think this has to be a bug in the Scala compiler which is caused by the varargs param.

Effects Scala 2.13.1 and 2.12.11.
I am using AdoptOpenJDK 11, also tried with 8.

expectation

Code compiles.

[NthPortal]

Scala can handle overriding Java varargs with Scala varargs.

I'm going to illustrate the problem by replacing varargs with their corresponding implementations (and translate everything to Scala).

trait JRequest {
  def addAttrs(entries: Array[play.libs.typedmap.TypedEntry[_]])play.mvc.Http.Request
}
class RequestHeaderImpl {
  def addAttrs(entries: Seq[play.libs.typedmap.TypedEntry[_]])play.mvc.Http.RequestHeader
}

then RequestImpl tries to inherit from both, and override the varargs method to return play.mvc.Http.Request. but those two addAttrs methods are not compatible, so it can't override both.

I believe that is the issue here. someone else can jump in and correct me if I'm way off mark

[mkurz]

@NthPortal Is there maybe a chance to workaround this with the help of the @varargs annotation? I tried but couldn't make it work unfortunately...

[NthPortal]

@mkurz no, that annotation is just to make the compiler generate a java-compatible varargs forwarder, which is done automatically if you're overriding a java varargs method

[mkurz]

...which is done automatically if you're overriding a java varargs method

I knew it generates an array forwarder method, but didn't know such a method is generated automatically anyway when overriding java varargs (so @varargs wouldn't make any difference), interesting.
So I guess there is no workaround for this problem right now?

[NthPortal]

so, a possible workaround would be to create a Scala trait that extends JRequest and overrides the method with an abstract implementation

// you can really name this trait whatever
// sealed so that lack of implementation can't be accidentally used elsewhere
sealed trait RequestImplHelper extends JRequest {
  override def addAttrs(entries: play.libs.typedmap.TypedEntry[_]*)play.mvc.Http.Request = ???
}

this (in theory) will create a trait that has an addAttrs method that takes Scala varargs instead of (technically in addition to) the Java varargs. then, you ought to be able to to extend RequestImplHelper and RequestHeaderImpl at the same time, since they both have an addAttrs method that takes Scala varargs. hopefully.

class RequestImpl(request: Request[RequestBody])
    extends RequestHeaderImpl(request)
    with RequestImplHelper { ... }

of course, now that you're adding a new trait, you're jumping into bincompat hell, so good luck

[NthPortal]

I'm actually going to leave this ticket open, on the thought that there is still something that can be improved here.

at the very least, we can improve the error message so it indicates that the two incompatible method signatures use different kinds of varargs.

however, it's not actually obvious to me that you shouldn't be able to override two different kinds of varargs at once. it seems like Scala otherwise does its best to paper over the difference, so shouldn't it do that here too?

[NthPortal]

More minimal repro:

// T1.java
package test;

import scala.collection.immutable.Seq;

interface T1 {
	Seq<String> va(String... strings);
}

// L.java
package test;

import scala.collection.immutable.List;

interface L extends T1 {
	List<String> va(String... strings);
}

// R_T2.scala
package test

class R extends T1 {
  override def va(strings: String*): Seq[String] = strings
}

class T2 extends R with L {
    override def va(strings: String*): List[String] = strings.toList
}

$ scalac T1.java L.java R_T2.scala
R_T2.scala:7: error: incompatible type in overriding
def va(strings: String*): List[String] (defined in trait L)
  with override def va(strings: String*): Seq[String] (defined in class R);
 found   : (strings: String*)Seq[String]
 required: (strings: String*)List[String]
class T2 extends R with L {
      ^
one error found

---

minimal repro with workaround: replace R_T2.scala with R_L2_T2.scala

// R_L2_T2.scala
package test

class R extends T1 {
  override def va(strings: String*): Seq[String] = strings
}

sealed trait L2 extends L {
  override def va(strings: String*): List[String] = ???
}

class T2 extends R with L2 {
    override def va(strings: String*): List[String] = strings.toList
}

[dwijnand]

There has to be 4 methods in play (🤡 ): the Seq returning, the List returning, and their varargs-forwarding buddies. My guess is the part of the compiler that deals with making sure the forwarder is emitted isn't remembering about covariant return types.

[som-snytt]

There's also the "concrete overrides abstract" issue. In Scala 3, this was changed to follow Java scala/scala3#4770 but I don't have all the plates in the air to see how it interacts with this ticket.

Edit:

-- Error: R_T2.scala:10:17 -----------------------------------------------------
10 |    override def va(strings: String*): List[String] = strings.toList
   |                 ^
   |error overriding method va in class R of type (strings: Seq[String]): Seq[String];
   |  method va of type (strings: Seq[String]): List[String] needs `override` modifier
1 error found

similarly

➜  t11944 ~/scala3-3.0.0-M1/bin/scalac -d /tmp *.java fix.scala
-- Error: fix.scala:13:15 ------------------------------------------------------
13 |  override def va(strings: String*): List[String] = strings.toList
   |               ^
   |error overriding method va in trait L2 of type (strings: Seq[String]): List[String];
   |  method va of type (strings: Seq[String]): List[String] needs `override` modifier
1 error found