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

Type mismatch calling interface with method override #12892

Open
tpetillot opened this issue Oct 16, 2023 · 9 comments · May be fixed by scala/scala#10580
Open

Type mismatch calling interface with method override #12892

tpetillot opened this issue Oct 16, 2023 · 9 comments · May be fixed by scala/scala#10580
Labels
fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) java interop
Milestone

Comments

@tpetillot
Copy link

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

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

tpetillot commented Oct 17, 2023

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

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

lrytz commented Oct 18, 2023

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

@lrytz no it actually changed in Dotty.

@lrytz
Copy link
Member

lrytz commented Oct 18, 2023

OK I must be confusing some things.

  • Is this change observable within Scala only, without Java declarations?
  • Do you have any history of the change at hand?

The 3.4 spec still says

First, a concrete definition always overrides an abstract definition

@som-snytt
Copy link

som-snytt commented Oct 18, 2023

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

lrytz commented Oct 19, 2023

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) java interop
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants