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

Add TableBuilder API for Scala (and others) compatibility #33

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

Flowdalic
Copy link

@Flowdalic Flowdalic commented Oct 25, 2021

Add TableBuilder API, which is a Java-shim over Picnic's Table.Builder
API. Unfortunately Table.Builder is not accessible from Scala (and
probably other JVM-based languages). The Scala incompatibility of
Table.Builder comes from the fact that Scala does not seem to
understand Kotlins @set:JvmSynthetic annotation. And since this
annotation is used in Picnic to hide Kotlin setter methods from
Java (and, in turn from Scala), the Scala compiler failes to resolve a
method reference:

[error] Example.scala:37:14: ambiguous reference to overloaded definition,
[error] both method setColumnSpan in class TableBuilder of type (x$1: Int): jakewharton.picnic.Cell.Builder
[error] and method setColumnSpan in class TableBuilder of type (x$1: Int): Unit
[error] match argument types (Int)
[error] .setColumnSpan(4)

As additional benefit, the new TableBuilder API creates much less noisy
code compared to the existing Table.Builder, since it avoids the 'new'
keyword and nexted builders.

Compare Table.Builder

.setHeader(new TableSection.Builder()
    .setCellStyle(new CellStyle.Builder()
        .setBorder(true)
        .setAlignment(BottomLeft)
        .build())
    .addRow(new Row.Builder()
        .addCell(new Cell.Builder("APK")
            .setRowSpan(2)
            .build())
        .addCell(new Cell.Builder("compressed")
            .setColumnSpan(3)
            .setStyle(new CellStyle.Builder()
                .setAlignment(BottomCenter)
                .build())
            .build())
        .addCell(new Cell.Builder("uncompressed")
            .setColumnSpan(3)
            .setStyle(new CellStyle.Builder()
                .setAlignment(BottomCenter)
                .build())
            .build())
        .build())
     .addRow("old", "new", "diff", "old", "new", "diff")
     .build())

with TableBuilder

.withHeader()
    .withCellStyle()
        .setBorder(true)
        .setAlignment(BottomLeft)
        .endCellStyle()
    .addRow()
        .addCell("APK")
            .setRowSpan(2)
            .endCell()
        .addCell("compressed")
            .setColumnSpan(3)
            .withCellStyle()
                .setAlignment(BottomCenter)
                .endCellStyle()
            .endCell()
        .addCell("uncompressed")
            .setColumnSpan(3)
            .withCellStyle()
                .setAlignment(BottomCenter)
                .endCellStyle()
            .endCell()
        .endRow()
    .addRow("old", "new", "diff", "old", "new", "diff")
    .endHeader()

Note that the Animal Sniffer Android API bump from 21 to 24 is due to TableBuilder's usage of java.util.function.Consumer in a single method. I often use this method from Scala, but if you don't want to bump Picnic's Android requirements, then could write an explicit interface for the method.

Add TableBuilder API, which is a Java-shim over Picnic's Table.Builder
API. Unfortunately Table.Builder is not accessible from Scala (and
probably other JVM-based languages). The Scala incompatibility of
Table.Builder comes from the fact that Scala does not seem to
understand Kotlins @set:JvmSynthetic annotation. And since this
annotation is used in Picnic to hide Kotlin setter methods from
Java (and, in turn from Scala), the Scala compiler failes to resolve a
method reference:

[error] Example.scala:37:14: ambiguous reference to overloaded definition,
[error] both method setColumnSpan in class TableBuilder of type (x$1: Int): jakewharton.picnic.Cell.Builder
[error] and  method setColumnSpan in class TableBuilder of type (x$1: Int): Unit
[error] match argument types (Int)
[error]             .setColumnSpan(4)

As additional benefit, the new TableBuilder API creates much less noisy
code compared to the existing Table.Builder, since it avoids the 'new'
keyword and nexted builders.

Compare Table.Builder

.setHeader(new TableSection.Builder()
    .setCellStyle(new CellStyle.Builder()
        .setBorder(true)
        .setAlignment(BottomLeft)
        .build())
    .addRow(new Row.Builder()
        .addCell(new Cell.Builder("APK")
            .setRowSpan(2)
            .build())
        .addCell(new Cell.Builder("compressed")
            .setColumnSpan(3)
            .setStyle(new CellStyle.Builder()
                .setAlignment(BottomCenter)
                .build())
            .build())
        .addCell(new Cell.Builder("uncompressed")
            .setColumnSpan(3)
            .setStyle(new CellStyle.Builder()
                .setAlignment(BottomCenter)
                .build())
            .build())
        .build())

with TableBuilder

.withHeader()
    .withCellStyle()
        .setBorder(true)
        .setAlignment(BottomLeft)
        .endCellStyle()
    .addRow()
        .addCell("APK")
            .setRowSpan(2)
            .endCell()
        .addCell("compressed")
            .setColumnSpan(3)
            .withCellStyle()
                .setAlignment(BottomCenter)
                .endCellStyle()
            .endCell()
        .addCell("uncompressed")
            .setColumnSpan(3)
            .withCellStyle()
                .setAlignment(BottomCenter)
                .endCellStyle()
            .endCell()
        .endRow()
    .addRow("old", "new", "diff", "old", "new", "diff")
    .endHeader()
@Flowdalic
Copy link
Author

Friendly reminder. Is this something you would be interested in? Otherwise I may factor the code out into an extra library.

@Flowdalic
Copy link
Author

@JakeWharton another friendly reminder. Please let me know if you are interested in this PR.

@Flowdalic
Copy link
Author

Published source code at https://github.com/Flowdalic/picnic-table-builder and the library artifact to Maven Central: https://search.maven.org/artifact/eu.geekplace/picnic-table-builder

@JakeWharton
Copy link
Owner

The JvmSynthetic annotation sets the ACC_SYNTHETIC flag in the classfile which means a language should ignore that method as something callable. This is the same mechanism that Java uses to do covariant return type overrides and to implement generic return types so I'm extremely surprised that Scala does not work here. That seems like a Scala bug, and I'm very hesitant to add such a large API surface solely to work around it.

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 this pull request may close these issues.

None yet

2 participants