scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Type mismatch calling interface with method override #12892

Open tpetillot opened 11 months ago

tpetillot commented 11 months ago

Reproduction steps

Scala version: 2.13.12

Java interfaces:

interface Parent {
  default Object bug() {
    return "Hello world";
  }
}

interface Child extends Parent {
  @Override
  String bug();
}

Problem

Do compile in Java:

public class MainJava {
  public static void main(String[] args) {
      Child child = null;
      String value = child.bug();
  }
}

Do not compile in Scala:

object MainScala {
  def main(args: Array[String]): Unit = {
    val child: Child = null
    val value: String = child.bug()
  }
}

Result:

[error] type mismatch;
[error] found   : Object
[error] required: String
[error]     val value: String = child.bug()

Java and Scala implementation should have the same behavior.

som-snytt commented 11 months ago

In Scala 2, concrete definitions override abstract declarations, even when defined in Java.

That changed in Scala 3, so this compiles under dotty.

Not sure whether Scala 2 should complain about the types, but possibly it's just a limitation.

tpetillot commented 11 months ago

@som-snytt thanks for your response.

As background, this limitation disrupts compatibility with the builder mechanism in aws-sdk-java-v2

SdkPresigner.java#L81C9-L81C9

S3Presigner.java#L589

Since version 2.21.0 released 5 days ago.

sugmanue commented 11 months ago

+1 this is a bug report for the S3Presigner.java mentioned by @tpetillot. Here is a small reproducible test case that breaks with Scala 2.

Using the following Java interfaces and class,

public interface InterfaceA {

    InterfaceA anotherMethod();

    default InterfaceA defaultedMethod() {
        return this;
    }
}

public interface InterfaceB extends InterfaceA {

    @Override
    InterfaceB anotherMethod();

    @Override
    InterfaceB defaultedMethod();
}

public class InterfaceBImpl implements InterfaceB {

    @Override
    public InterfaceB anotherMethod() {
        return this;
    }

    @Override
    public InterfaceB defaultedMethod() {
        return this;
    }

    public InterfaceB doSomething() {
        return this;
    }

    public static InterfaceBImpl instance() {
        return new InterfaceBImpl();
    }
}

This method compiles and works fine in Java.

public static void main(String[] args) {
    InterfaceBImpl.instance()
            .doSomething()
            .anotherMethod()
            .defaultedMethod()
            .anotherMethod();       
}

Whereas using Scala:

  def getInterfaceB(): InterfaceB =
    InterfaceBImpl
      .instance()
      .doSomething()
      .anotherMethod()
      .defaultedMethod()
      .anotherMethod()

I get an error similar to

/home/sugmanue/ScalaBug.scala:80:21: type mismatch;
 found   : test.InterfaceA
 required: test.InterfaceB
      .anotherMethod()?
test.InterfaceA <: test.InterfaceB?
false
 one error found
lrytz commented 11 months ago

In Scala 2, concrete definitions override abstract declarations, even when defined in Java.

That changed in Scala 3, so this compiles under dotty.

Did it change in Scala 3?

scala> trait T { def m: T = this }
trai// defined trait T

scala> trait U extends T { override def m: U }
-- [E164] Declaration Error: ---------------------------------------------------
1 |trait U extends T { override def m: U }
  |      ^
  | error overriding method m in trait U of type => U;
  |   method m in trait T of type => T has incompatible type
  | (Note that method m in trait U of type => U is abstract,
  | and is therefore overridden by concrete method m in trait T of type => T)
  |
  | longer explanation available when compiling with `-explain`
1 error found

You're right that the test works in Scala 3, so probably it just supports Java better?

som-snytt commented 11 months ago

@lrytz no it actually changed in Dotty.

lrytz commented 11 months ago

OK I must be confusing some things.

The 3.4 spec still says

First, a concrete definition always overrides an abstract definition

som-snytt commented 11 months ago

We know the spec is a big fat lie!

There is an early ticket where Allan Renucci (spelling) notes the discrepancy, and then it was adopted "because that is how Java does it". At some point, I noted that it was not like "sipped", but absorbed by osmosis, but I think it was intended all along to remedy the defect in Scala 2, where it was a limitation.

(I understand the limitation is about tracking bounds in overrides or something, but I never went back as I intended to do, to understand exactly what.)

(As LeVar Burton says, "Don't take my word for it." I think I'm awake right now?)

lrytz commented 11 months ago

After looking at this a bunch more I find it confusing how things work on Scala 3.

Here it tells me B.f overrides A.f:

scala> trait A { def f: String = "" }
scala> trait B extends A { def f: Object }
-- [E164] Declaration Error: ---------------------------------------------------
1 |trait B extends A { def f: Object }
  |                        ^
  |                   error overriding method f in trait A of type => String;
  |                     method f of type => Object has incompatible type

But then it tells me A.f overrides B.f:

scala> trait A { def f: Object = "" }
scala> trait B extends A { override def f: String }
-- [E164] Declaration Error: ---------------------------------------------------
1 |trait B extends A { override def f: String }
  |      ^
  |error overriding method f in trait B of type => String;
  |  method f in trait A of type => Object has incompatible type
  |(Note that method f in trait B of type => String is abstract,
  |and is therefore overridden by concrete method f in trait A of type => Object)

Anyway.. like @dwijnand I don't see the problem with Scala 2's approach, except for supporting Java (this ticket). Maybe we can change the lookup rules for Java?

The override checking should be unaffected:

interface A { default A m() { return this; } }
interface B extends A { @Override B m(); }

new B {} in Scala gives

  |error overriding method m in trait B of type (): B;
  |  method m in trait A of type (): A has incompatible type
  |(Note that method m in trait B of type (): B is abstract,
  |and is therefore overridden by concrete method m in trait A of type (): A)
xuwei-k commented 11 months ago

🤔