Signature checks fail using value classes & upgrading to 2.12.9+ or 2.13.1+

[travisbrown]

If you publish this code with Scala 2.12.8:

class Foo(val value: String) extends AnyVal

case class Bar(foo: Foo)

And then change the Scala version to 2.12.9 (or 10) and run sbt mimaReportBinaryIssues, you'll get these errors:

[error]  * method unapply(Bar)scala.Option in object Bar has a different signature in current version, where it is (LBar;)Lscala/Option<LFoo;>; rather than (LBar;)Lscala/Option<Ljava/lang/String;>;
[error]    filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("Bar.unapply")
[error]  * static method unapply(Bar)scala.Option in class Bar has a different signature in current version, where it is (LBar;)Lscala/Option<LFoo;>; rather than (LBar;)Lscala/Option<Ljava/lang/String;>;
[error]    filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("Bar.unapply")
[error]  * static method andThen(scala.Function1)scala.Function1 in class Bar has a different signature in current version, where it is <A:Ljava/lang/Object;>(Lscala/Function1<LBar;TA;>;)Lscala/Function1<LFoo;TA;>; rather than <A:Ljava/lang/Object;>(Lscala/Function1<LBar;TA;>;)Lscala/Function1<Ljava/lang/String;TA;>;
[error]    filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("Bar.andThen")
[error]  * static method compose(scala.Function1)scala.Function1 in class Bar has a different signature in current version, where it is <A:Ljava/lang/Object;>(Lscala/Function1<TA;LFoo;>;)Lscala/Function1<TA;LBar;>; rather than <A:Ljava/lang/Object;>(Lscala/Function1<TA;Ljava/lang/String;>;)Lscala/Function1<TA;LBar;>;
[error]    filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("Bar.compose")
[error] java.lang.RuntimeException: unapply_test: Binary compatibility check failed!

The issue seems to be the value class in the synthetic method signatures.

[dwijnand]

Yeah, that was a bug fixed in 2.12.9, one that ideally MiMa could recognise and avoid the false positive.

[diesalbla]

We have found a similar problem in https://github.com/Banno/vault4s/ between versions v.5.1.0 and v.5.2.0:

• In that case we also have an inner AnyVal and the build for the latter would start showing similar errors.
• I checked with scalap the binaries of the v5.1.0 version, and they do not show it to be using String, but rather the AnyVal wrapper in the binary.

@dwijnand When you say that it was a bug fixed in 2.12.9, would that be the version of the scalac compiler, or the scalap de-compiler?

[dwijnand]

The fix was in the compiler (scala/scala#8127). My understanding is that scalap shows you what's in ScalaSignature rather than the (erased) method descriptor or the (Java) (generic) signature, the latter of which is what Harrison fixed and MiMa analyses (edit: MiMa analyses the descriptor - thanks Arnout).

[raboof]

My understanding is that scalap shows you what's in ScalaSignature rather than the (erased) method descriptor or the (Java) (generic) signature, the latter of which is what Harrison fixed and MiMa analyses

For context: by default javap shows something that looks like Java code, with generics:

$ javap ActorSystem.class
public abstract class akka.actor.ActorSystem implements akka.actor.ActorRefFactory,akka.actor.Class
icActorSystemProvider {
  ...
  public abstract <T extends akka.actor.Extension> T registerExtension(akka.actor.ExtensionId<T>);
  ...
}

You can get the full details with javap -v:

public abstract <T extends akka.actor.Extension> T registerExtension(akka.actor.ExtensionId<T>);
    descriptor: (Lakka/actor/ExtensionId;)Lakka/actor/Extension;
    flags: (0x0401) ACC_PUBLIC, ACC_ABSTRACT
    Signature: #192                         // <T::Lakka/actor/Extension;>(Lakka/actor/ExtensionId<TT;>;)TT;
    MethodParameters:
      Name                           Flags
      ext                            final

MiMa mainly uses the descriptor, which as you can see does not take into account generic parameters. If there is an incompatibility in the descriptor, it's almost guaranteed that use of that method would lead to an error at run time.

Specifically for the IncompatibleSignatureProblem check, MiMa looks at the Signature. If there is an incompatibility in the Signature, this may or may not actually lead to errors at run time. So if you get IncompatibleSignatureProblem warnings, it is definitely useful to double-check whether they are problematic, but it is perfectly possible that they can be safely excluded.

[raboof]

I checked with scalap the binaries of the v5.1.0 version, and they do not show it to be using String, but rather the AnyVal wrapper in the binary.

If I look at com/banno/vault/transit/CipherText.class, the published v5.1.0 jars do look off:

$ javap com/banno/vault/transit/CipherText.class | grep eqCipher
  public static cats.kernel.Eq<java.lang.String> eqCipherText();

Though indeed when I check out the tag from git it looks ok:

$ javap ./core/target/scala-2.12/classes/com/banno/vault/transit/CipherText.class | grep eqCipher
  public static cats.kernel.Eq<com.banno.vault.transit.CipherText> eqCipherText();

It looks like the published 5.1.0 artifacts were built with scala <2.12.9 (even though build.sbt asks for 2.12.10)?

Summarizing it seems mima correctly flagged incompatible changes to the generic signature, but these were caused by old Scala versions declaring wrong signatures (scala/scala#8127), so in this case we can be sure no-one relied on the old (incorrect) signatures.

Not sure if there is a lot we can do about it from the mima side?

[diesalbla]

@raboof Just for my ease of mind... if someone writes a fili library, that depends on Vault4s 5.1.0, fetches that binary, compiles his binary jar to target 5.1.0, publishes it; and then someone uses that fili binary into her app, and then he gets an eviction of 5.1.0 to 5.2.0... would they suffer a binary backwards-incompatibility exception?

In other words, do we have a true positive here between the current jars for 5.1.0 and the current jars for 5.2.0?

[dwijnand]

No, CipherText is and always has been a value class, which therefore was boxed and not-boxed as required - nothing's changed there. The only difference is now the bytecode using it has the right generic signatures for javac - which for a library called vault4s isn't going to be life-changing. 😆

[raboof]

@diesalbla no, I think it can be broken down as follows:

• 5.1.0 was compiled with scala <2.12.9, which produced a (wrong) Eq<String> signature for a method that actually compared CipherText objects
• Anyone who would have called that method and, based on that generic as seen by javac (scalac is separate), expected to be usable with Strings, would have encountered errors (because it in fact works with CipherText objects
• 5.2.0 is compiled with scala >=2.12.9, which produces a (correct) Eq<CipherText> signature
• MiMa reports that if there was anyone who relied on the Eq<String> signature before, they would be in trouble now because it changed to Eq<CipherText>.
• But there is no-one that relied on that signature, because it was wrong and wouldn't have worked in the first place