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

After upgrading to kotlin 1.5.0, the kotlin-parcelize generated code started to be reported as public by binary-compatibility-validator #55

Closed
4brunu opened this issue May 31, 2021 · 6 comments · Fixed by #60

Comments

@4brunu
Copy link

4brunu commented May 31, 2021

Hi,

I'm using kotlin-parcelize plugin to generate the parcelable code in the internal data classes in my project.

Let's consider the following kotlin data class, that is marked as internal.

@Parcelize
internal data class Country (
    val code: String,
    val name: String
) : Parcelable

I have made the following tests with binary-compatibility-validator 0.5.0.
And there is a difference when using kotlin 1.5.0 and 1.4.32.

Before upgrading to kotlin 1.5.0, using kotlin 1.4.32, the command ./gradlew teleconsultationandroid:apiDump wouldn't return any output related to the class Country.
And to help debug this issue, here is the compiled code of Country in kotlin 1.4.32.

Compiled `Country` data class using kotlin 1.4.32. import android.os.Parcel; import android.os.Parcelable; import kotlin.Metadata; import kotlin.jvm.internal.Intrinsics; import kotlinx.parcelize.Parcelize; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable;

@parcelize
@metadata(...)
/* compiled from: Country.kt */
public final class Country implements Parcelable {
public static final Parcelable.Creator CREATOR = new Creator();
@NotNull
private final String code;
@NotNull
private final String name;

@Metadata(bv = {1, 0, 3}, d1 = {}, d2 = {}, k = 3, mv = {1, 4, 2})
public static class Creator implements Parcelable.Creator<Country> {
    @Override // android.os.Parcelable.Creator
    @NotNull
    public final Country createFromParcel(@NotNull Parcel parcel) {
        Intrinsics.checkNotNullParameter(parcel, "in");
        return new Country(parcel.readString(), parcel.readString());
    }

    @Override // android.os.Parcelable.Creator
    @NotNull
    public final Country[] newArray(int i) {
        return new Country[i];
    }
}

public Country(@NotNull String str, @NotNull String str2) {
    Intrinsics.checkNotNullParameter(str, "code");
    Intrinsics.checkNotNullParameter(str2, "name");
    this.code = str;
    this.name = str2;
}

public static /* synthetic */ Country copy$default(Country country, String str, String str2, int i, Object obj) {
    if ((i & 1) != 0) {
        str = country.code;
    }
    if ((i & 2) != 0) {
        str2 = country.name;
    }
    return country.copy(str, str2);
}

@NotNull
public final String component1() {
    return this.code;
}

@NotNull
public final String component2() {
    return this.name;
}

@NotNull
public final Country copy(@NotNull String str, @NotNull String str2) {
    Intrinsics.checkNotNullParameter(str, "code");
    Intrinsics.checkNotNullParameter(str2, "name");
    return new Country(str, str2);
}

public int describeContents() {
    return 0;
}

public boolean equals(@Nullable Object obj) {
    if (this != obj) {
        if (obj instanceof Country) {
            Country country = (Country) obj;
            if (!Intrinsics.areEqual(this.code, country.code) || !Intrinsics.areEqual(this.name, country.name)) {
                return false;
            }
        }
        return false;
    }
    return true;
}

@NotNull
public final String getCode() {
    return this.code;
}

@NotNull
public final String getName() {
    return this.name;
}

public int hashCode() {
    int i = 0;
    String str = this.code;
    int hashCode = (str != null ? str.hashCode() : 0) * 31;
    String str2 = this.name;
    if (str2 != null) {
        i = str2.hashCode();
    }
    return hashCode + i;
}

@NotNull
public String toString() {
    return "Country(code=" + this.code + ", name=" + this.name + ")";
}

public void writeToParcel(@NotNull Parcel parcel, int i) {
    Intrinsics.checkNotNullParameter(parcel, "parcel");
    parcel.writeString(this.code);
    parcel.writeString(this.name);
}

}

But after upgrading to kotlin 1.5.0, the command ./gradlew teleconsultationandroid:apiDump started returing the following output.

public final class com/sample/project/Country$Creator : android/os/Parcelable$Creator {
	public fun <init> ()V
	public synthetic fun createFromParcel (Landroid/os/Parcel;)Ljava/lang/Object;
	public final fun createFromParcel (Landroid/os/Parcel;)L com/sample/project/Country;
	public synthetic fun newArray (I)[Ljava/lang/Object;
	public final fun newArray (I)[Lcom/sample/project/Country;
}

And here is the output of the compiled code in kotlin 1.5.0.

Compiled `Country` data class using kotlin 1.5.0. import android.os.Parcel; import android.os.Parcelable; import kotlin.Metadata; import kotlin.jvm.internal.Intrinsics; import kotlinx.parcelize.Parcelize; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable;

@parcelize
@metadata(...)
/* compiled from: Country.kt */
public final class Country implements Parcelable {
@NotNull
public static final Parcelable.Creator CREATOR = new Creator();
@NotNull
private final String code;
@NotNull
private final String name;

@Metadata(bv = {1, 0, 3}, d1 = {}, d2 = {}, k = 3, mv = {1, 5, 1})
/* compiled from: Country.kt */
public static final class Creator implements Parcelable.Creator<Country> {
    @Override // android.os.Parcelable.Creator
    @NotNull
    public final Country createFromParcel(@NotNull Parcel parcel) {
        Intrinsics.checkNotNullParameter(parcel, "parcel");
        return new Country(parcel.readString(), parcel.readString());
    }

    @Override // android.os.Parcelable.Creator
    @NotNull
    public final Country[] newArray(int i) {
        return new Country[i];
    }
}

public Country(@NotNull String str, @NotNull String str2) {
    Intrinsics.checkNotNullParameter(str, "code");
    Intrinsics.checkNotNullParameter(str2, "name");
    this.code = str;
    this.name = str2;
}

public static /* synthetic */ Country copy$default(Country country, String str, String str2, int i, Object obj) {
    if ((i & 1) != 0) {
        str = country.code;
    }
    if ((i & 2) != 0) {
        str2 = country.name;
    }
    return country.copy(str, str2);
}

@NotNull
public final String component1() {
    return this.code;
}

@NotNull
public final String component2() {
    return this.name;
}

@NotNull
public final Country copy(@NotNull String str, @NotNull String str2) {
    Intrinsics.checkNotNullParameter(str, "code");
    Intrinsics.checkNotNullParameter(str2, "name");
    return new Country(str, str2);
}

public int describeContents() {
    return 0;
}

public boolean equals(@Nullable Object obj) {
    if (this == obj) {
        return true;
    }
    if (!(obj instanceof Country)) {
        return false;
    }
    Country country = (Country) obj;
    if (!Intrinsics.areEqual(this.code, country.code)) {
        return false;
    }
    return Intrinsics.areEqual(this.name, country.name);
}

@NotNull
public final String getCode() {
    return this.code;
}

@NotNull
public final String getName() {
    return this.name;
}

public int hashCode() {
    return (this.code.hashCode() * 31) + this.name.hashCode();
}

@NotNull
public String toString() {
    return "Country(code=" + this.code + ", name=" + this.name + ')';
}

public void writeToParcel(@NotNull Parcel parcel, int i) {
    Intrinsics.checkNotNullParameter(parcel, "out");
    parcel.writeString(this.code);
    parcel.writeString(this.name);
}

}

Is this as issue of binary-compatibility-validator?
Thanks

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented May 31, 2021

Hi, either the compilation strategy between Kotlin 1.4.x and 1.5.0 regarding parcelize has been changed or this is indeed the issue of BCV.

I'm not very familiar with parcelize, so if you could provide a sample project that reproduces the problem (e.g. I only update Kotlin/BCV version and the output of apiDump changes), it would significantly simplify the speed of the investigation and fix (if there will be any)

@4brunu
Copy link
Author

4brunu commented May 31, 2021

I will create one, thanks for responding so quickly 👍

@4brunu
Copy link
Author

4brunu commented May 31, 2021

Here is the sample project

https://github.com/4brunu/kotlin-binary-compatibility-validator-issue

To dump the API, just run make or ./gradlew mylibrary:apiDump and check the result file under mylibrary/api/mylibrary.api.

Here is the api file with kotlin 1.4.32

Commit

mylibrary.api

Here is the api file with kotlin 1.5.0

Commit

mylibrary.api

qwwdfsad added a commit that referenced this issue Jun 2, 2021
Parcelize bug explanation:

With Kotlin 1.4.x, the corresponding $Creator class has been generated properly as inner public class and his visibility was generated by internal outer class

With Kotlin 1.5.0, $Creator is generated as local class within <clinit> block. Local classes do not have the corresponding 'outer_class_info_index' attribute and thus their visibility cannot be dominated by the outer class. But they do not constitute public API anyway. The bug was in incorrect detection on whether the class is local.

Our isLocal check has been broken, but we had an additional guard in the code and also checked kotlinx.metadata.Flag.IS_LOCAL. Parcelize-generated classes lack such metadata and that guard also had failed

Fixes #55
qwwdfsad added a commit that referenced this issue Jun 4, 2021
Parcelize bug explanation:

With Kotlin 1.4.x, the corresponding $Creator class has been generated properly as an inner public class and its visibility was generated by internal outer class

With Kotlin 1.5.0, $Creator is generated as local class within <clinit> block. Local classes do not have the corresponding 'outer_class_info_index' attribute and thus their visibility cannot be dominated by the outer class. But they do not constitute public API anyway. The bug was in incorrect detection on whether the class is local.

Our isLocal check has been broken, but we had an additional guard in the code and also checked kotlinx.metadata.Flag.IS_LOCAL. Parcelize-generated classes lack such metadata and that guard also had failed

Fixes #55
@4brunu
Copy link
Author

4brunu commented Nov 21, 2022

This PR #60 didn't fixed this issue.
I tried with Kotlin 1.7.20 and binary-compatibility-validator 0.12.1 and it didn't worked.
Can we reopen this issue please?

@4brunu
Copy link
Author

4brunu commented Nov 21, 2022

@qwwdfsad or should I open a new one?

@qwwdfsad
Copy link
Collaborator

Please open a new one with a reproducer as well

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

Successfully merging a pull request may close this issue.

2 participants