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

MiMa doesn't check the generic signature of a class/method/field #40

Closed
dotta opened this issue Apr 2, 2013 · 6 comments
Closed

MiMa doesn't check the generic signature of a class/method/field #40

dotta opened this issue Apr 2, 2013 · 6 comments
Milestone

Comments

@dotta
Copy link
Contributor

dotta commented Apr 2, 2013

Say you have

class A {
  def foo(): (String,Int) = ("",0)
}

And you evolve it to

class A {
  def foo(): (Int, String) = (0, "")
}

The signature of method foo doesn't change, i.e.,

% javap -private A 
Compiled from "A.scala"
public class A extends java.lang.Object implements scala.ScalaObject{
    public scala.Tuple2 foo();
    public A();
}

MiMa doesn't report any incompatibility in the above case.

One reason for this is that MiMa doesn't currently check the generic signature. However, even if it would, we can still find instances where the generic signature is the same but you still end up with a ClassCastException at runtime. Hence, a good fix for this isn't straightforward.

@Sciss
Copy link

Sciss commented Apr 2, 2013

Which would be a case where, with the type parameters check, you still end up with a ClassCastException? In any case, I think having more warnings is better than less, because that way you don't end up thinking that your new version is a drop in replacement....

dotta added a commit to dotta/migration-manager that referenced this issue Apr 2, 2013
@dwijnand
Copy link
Collaborator

👍

Another example is trying to evolve a case class. MiMa doesn't warm about the binary incompatible change to unapply.

@dotta
Copy link
Contributor Author

dotta commented Aug 21, 2015

@dwijnand Is that related to the generic signature? Can you provide an example (with also the generated bytecode).

@dwijnand
Copy link
Collaborator

Sure.

Before

sealed case class Foo(a: String, b: String)
% javap -cp target/scala-2.11/classes -private Foo
Compiled from "Foo.scala"
public class Foo implements scala.Product,scala.Serializable {
  private final java.lang.String a;
  private final java.lang.String b;
  public static scala.Option<scala.Tuple2<java.lang.String, java.lang.String>> unapply(Foo);
  public static Foo apply(java.lang.String, java.lang.String);
  public static scala.Function1<scala.Tuple2<java.lang.String, java.lang.String>, Foo> tupled();
  public static scala.Function1<java.lang.String, scala.Function1<java.lang.String, Foo>> curried();
  public java.lang.String a();
  public java.lang.String b();
  public Foo copy(java.lang.String, java.lang.String);
  public java.lang.String copy$default$1();
  public java.lang.String copy$default$2();
  public java.lang.String productPrefix();
  public int productArity();
  public java.lang.Object productElement(int);
  public scala.collection.Iterator<java.lang.Object> productIterator();
  public boolean canEqual(java.lang.Object);
  public int hashCode();
  public java.lang.String toString();
  public boolean equals(java.lang.Object);
  public Foo(java.lang.String, java.lang.String);
}
% javap -cp target/scala-2.11/classes -private Foo$
Compiled from "Foo.scala"
public final class Foo$ extends scala.runtime.AbstractFunction2<java.lang.String, java.lang.String, Foo> implements scala.Serializable {
  public static final Foo$ MODULE$;
  public static {};
  public final java.lang.String toString();
  public Foo apply(java.lang.String, java.lang.String);
  public scala.Option<scala.Tuple2<java.lang.String, java.lang.String>> unapply(Foo);
  private java.lang.Object readResolve();
  public java.lang.Object apply(java.lang.Object, java.lang.Object);
  private Foo$();
}

After

diff --git a/Foo.scala b/Foo.scala
index 60177d3..ff47732 100644
--- a/Foo.scala
+++ b/Foo.scala
@@ -1 +1,10 @@
-sealed case class Foo(a: String, b: String)
+sealed case class Foo(a: String, b: String, c: String) {
+  def this(a: String, b: String) = this(a, b, "")
+
+  def copy(a: String, b: String): Foo = copy(a, b, c)
+  def copy(a: String = a, b: String = b, c: String = c) = new Foo(a, b, c)
+}
+
+object Foo extends scala.runtime.AbstractFunction2[String, String, Foo] {
+  def apply(a: String, b: String) = new Foo(a, b)
+}
sealed case class Foo(a: String, b: String, c: String) {
  def this(a: String, b: String) = this(a, b, "")

  def copy(a: String, b: String): Foo = copy(a, b, c)
  def copy(a: String = a, b: String = b, c: String = c) = new Foo(a, b, c)
}

object Foo extends scala.runtime.AbstractFunction2[String, String, Foo] {
  def apply(a: String, b: String) = new Foo(a, b)
}
> mimaReportBinaryIssues
[info] Resolving default#l2mima_2.11;0.1.0 ...
[info] Compiling 1 Scala source to /Users/dnw/Desktop/l2mima/target/scala-2.11/classes...
[info] Resolving org.scala-lang#scala-library;2.11.7 ...
[info] l2mima: found 0 potential binary incompatibilities
[success] Total time: 1 s, completed 21-Aug-2015 13:13:04
% javap -cp target/scala-2.11/classes -private Foo
Compiled from "Foo.scala"
public class Foo implements scala.Product,scala.Serializable {
  private final java.lang.String a;
  private final java.lang.String b;
  private final java.lang.String c;
  public static scala.Option<scala.Tuple3<java.lang.String, java.lang.String, java.lang.String>> unapply(Foo);
  public static Foo apply(java.lang.String, java.lang.String, java.lang.String);
  public static Foo apply(java.lang.String, java.lang.String);
  public static scala.Function1<scala.Tuple2<java.lang.String, java.lang.String>, Foo> tupled();
  public static scala.Function1<java.lang.String, scala.Function1<java.lang.String, Foo>> curried();
  public java.lang.String a();
  public java.lang.String b();
  public java.lang.String c();
  public Foo copy(java.lang.String, java.lang.String);
  public Foo copy(java.lang.String, java.lang.String, java.lang.String);
  public java.lang.String copy$default$1();
  public java.lang.String copy$default$2();
  public java.lang.String copy$default$3();
  public java.lang.String productPrefix();
  public int productArity();
  public java.lang.Object productElement(int);
  public scala.collection.Iterator<java.lang.Object> productIterator();
  public boolean canEqual(java.lang.Object);
  public int hashCode();
  public java.lang.String toString();
  public boolean equals(java.lang.Object);
  public Foo(java.lang.String, java.lang.String, java.lang.String);
  public Foo(java.lang.String, java.lang.String);
}
% javap -cp target/scala-2.11/classes -private Foo$
Compiled from "Foo.scala"
public final class Foo$ extends scala.runtime.AbstractFunction2<java.lang.String, java.lang.String, Foo> implements scala.Serializable {
  public static final Foo$ MODULE$;
  public static {};
  public Foo apply(java.lang.String, java.lang.String);
  public Foo apply(java.lang.String, java.lang.String, java.lang.String);
  public scala.Option<scala.Tuple3<java.lang.String, java.lang.String, java.lang.String>> unapply(Foo);
  private java.lang.Object readResolve();
  public java.lang.Object apply(java.lang.Object, java.lang.Object);
  private Foo$();
}
% diff -u Foo.before.txt Foo.after.txt
--- Foo.before.txt  2015-08-21 13:15:03.000000000 +0100
+++ Foo.after.txt   2015-08-21 13:15:26.000000000 +0100
@@ -2,15 +2,20 @@
 public class Foo implements scala.Product,scala.Serializable {
   private final java.lang.String a;
   private final java.lang.String b;
-  public static scala.Option<scala.Tuple2<java.lang.String, java.lang.String>> unapply(Foo);
+  private final java.lang.String c;
+  public static scala.Option<scala.Tuple3<java.lang.String, java.lang.String, java.lang.String>> unapply(Foo);
+  public static Foo apply(java.lang.String, java.lang.String, java.lang.String);
   public static Foo apply(java.lang.String, java.lang.String);
   public static scala.Function1<scala.Tuple2<java.lang.String, java.lang.String>, Foo> tupled();
   public static scala.Function1<java.lang.String, scala.Function1<java.lang.String, Foo>> curried();
   public java.lang.String a();
   public java.lang.String b();
+  public java.lang.String c();
   public Foo copy(java.lang.String, java.lang.String);
+  public Foo copy(java.lang.String, java.lang.String, java.lang.String);
   public java.lang.String copy$default$1();
   public java.lang.String copy$default$2();
+  public java.lang.String copy$default$3();
   public java.lang.String productPrefix();
   public int productArity();
   public java.lang.Object productElement(int);
@@ -19,5 +24,6 @@
   public int hashCode();
   public java.lang.String toString();
   public boolean equals(java.lang.Object);
+  public Foo(java.lang.String, java.lang.String, java.lang.String);
   public Foo(java.lang.String, java.lang.String);
 }
--- Foo$.before.txt 2015-08-21 13:18:03.000000000 +0100
+++ Foo$.after.txt  2015-08-21 13:18:03.000000000 +0100
@@ -2,9 +2,9 @@
 public final class Foo$ extends scala.runtime.AbstractFunction2<java.lang.String, java.lang.String, Foo> implements scala.Serializable {
   public static final Foo$ MODULE$;
   public static {};
-  public final java.lang.String toString();
   public Foo apply(java.lang.String, java.lang.String);
-  public scala.Option<scala.Tuple2<java.lang.String, java.lang.String>> unapply(Foo);
+  public Foo apply(java.lang.String, java.lang.String, java.lang.String);
+  public scala.Option<scala.Tuple3<java.lang.String, java.lang.String, java.lang.String>> unapply(Foo);
   private java.lang.Object readResolve();
   public java.lang.Object apply(java.lang.Object, java.lang.Object);
   private Foo$();

@dwijnand
Copy link
Collaborator

By the way, a workaround to this, suggested by Paul Phillips here is to add to object Foo:

  def unapply(o: Any): Option[(String, String)] =
    o match {
      case foo: Foo => Some((foo.a, foo.b))
      case _        => None
    }

Which changes the original

public scala.Option<scala.Tuple2<java.lang.String, java.lang.String>> unapply(Foo);

to

public scala.Option<scala.Tuple2<java.lang.String, java.lang.String>> unapply(java.lang.Object);

and adds another unapply for Tuple3.

@dwijnand
Copy link
Collaborator

dwijnand commented Jun 9, 2019

Fixed in #299, to the extend possible with the bytecode alone and without "Scala working mode" (#34), i.e. an unpickler (which perhaps we should close because it's been reasoned to be a "wontfix" for MiMa, sadly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants