From 00bbd2e0a9879a166d7a56d3222c72a85636ac8e Mon Sep 17 00:00:00 2001 From: Ben Bader Date: Fri, 2 Nov 2018 14:13:02 -0700 Subject: [PATCH] Support configurable @Generated annotation types (#259) --- .travis.yml | 1 + README.md | 8 +- thrifty-compiler/README.md | 1 + .../thrifty/compiler/ThriftyCompiler.kt | 83 ++++++++++++++++++- thrifty-integration-tests/build.gradle | 9 +- .../ThriftyCodeGenerator.kt | 11 +-- .../thrifty/gen/ThriftyCodeGeneratorTest.kt | 40 --------- .../thrifty/kgen/KotlinCodeGenerator.kt | 13 ++- .../thrifty/kgen/KotlinCodeGeneratorTest.kt | 18 ---- 9 files changed, 110 insertions(+), 74 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7b4da1ca0..3314ade8b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,7 @@ language: java jdk: - oraclejdk8 + - openjdk10 sudo: false install: echo './gradlew check will install dependencies' diff --git a/README.md b/README.md index d0c5b5f5e..de2707b06 100644 --- a/README.md +++ b/README.md @@ -481,7 +481,13 @@ The final new flag is `--kt-file-per-type`. Thrifty's convention is to generate `--omit-file-comments` suppresses the comment that prefixes each autogenerated Java file. -Types generated by Thrifty are annotated with `@javax.annotation.Generated` by default; this can be disabled with the `--omit-generated-annotations` flag. +Types generated by Thrifty are annotated with a `@Generated` annotation by default; this can be disabled with the `--omit-generated-annotations` flag. The actual type of the annotation may be specified with a `--use-generated-annotation=[jdk8|jdk9|native]` argument. By default, we assume `jdk8`, i.e. `javax.annotation.Generated`, as this is what's supported in the Android toolchain ()and will be for the foreseeable future). + +##### About @Generated annotations + +In short, from Java 5-8, everyone has used `javax.annotation.Generated`; this class was replaced in Java 9+ with `javax.annotation.processing.Generated`. This means that code generated for JDK8 may not compile on JDK9, and vice-versa. The solution is to either suppress `@Generated` annotations entirely, or to be explicit about which annotation we want to use. + +For Android, the compiler's defaults are good enough. If you are going to use generated code with JRE 9+, make sure to use `--use-generated-type=jdk9`. You almost _never_ want `--use-generated-type=native`; it exists to support running our integration tests in a convenient way in multiple environments, and will probably only cause you pain if you try to use it yourself. ### Thanks diff --git a/thrifty-compiler/README.md b/thrifty-compiler/README.md index a35d9f92b..a6288a294 100644 --- a/thrifty-compiler/README.md +++ b/thrifty-compiler/README.md @@ -28,3 +28,4 @@ Option | Description `--set-type=[classname]` | A java.util.Set implementation to be used wherever sets are instantiated in generated code. `--map-type=[classname]` | A java.util.Map implementation, as above. `--kt-file-per-type` | Specifies that one .kt file should be generated per Kotlin type. The default is for all code to be written to a single file. +`--generated-annotation-type=[jdk8,jdk9,native]` | Optional, defaults to `jdk8`. Specifies which `@Generated` annotation type to use - `javax.annotation.Generated`, `javax.annotation.processing.Generated`, or whichever type is present in the Java runtime at compile-time. diff --git a/thrifty-compiler/src/main/kotlin/com/microsoft/thrifty/compiler/ThriftyCompiler.kt b/thrifty-compiler/src/main/kotlin/com/microsoft/thrifty/compiler/ThriftyCompiler.kt index 80b003c5b..89121dc03 100644 --- a/thrifty-compiler/src/main/kotlin/com/microsoft/thrifty/compiler/ThriftyCompiler.kt +++ b/thrifty-compiler/src/main/kotlin/com/microsoft/thrifty/compiler/ThriftyCompiler.kt @@ -59,6 +59,7 @@ import java.util.ArrayList * [--use-android-annotations] * [--omit-file-comments] * [--omit-generated-annotations] + * [--generated-annotation-type=[jdk8|jdk9|native] * file1.thrift * file2.thrift * ... @@ -81,6 +82,38 @@ import java.util.ArrayList * class name when instantiating map-typed values. Defaults to [java.util.HashMap]. * Android users will likely wish to substitute `android.support.v4.util.ArrayMap`. * + * `--lang=[java|kotlin]` is optional, defaulting to Java. When provided, the + * compiler will generate code in the specified language. + * + * `--kt-file-per-type` is optional. When specified, one Kotlin file will be generated + * for each top-level generated Thrift type. When absent (the default), all generated + * types in a single package will go in one file named `ThriftTypes.kt`. Implies + * `--lang=kotlin`. + * + * `--parcelable` is optional. When provided, generated types will contain a + * `Parcelable` implementation. Kotlin types will use the `@Parcelize` extension. + * + * `--use-android-annotations` is optional. When specified, generated Java classes + * will have `@android.support.annotation.Nullable` or `@android.support.annotation.NotNull` + * annotations, as appropriate. Has no effect on Kotlin code. + * + * `--omit-file-comments` is optional. When specified, no file-header comment is generated. + * The default behavior is to prefix generated files with a comment indicating that they + * are generated by Thrifty, and should probably not be modified by hand. + * + * `--omit-generated-annotations` is optional. When specified, generated types will not + * have `@Generated` annotations applied. The default behavior is to annotate all generated + * types with `javax.annotation.Generated`. + * + * `--generated-annotatation-type=[jdk8|jdk9|native]` is optional, defaulting to `jdk8`. + * This option controls the type of `@Generated` annotation added to generated types. The default + * of `jdk8` results in `javax.annotation.Generated`. `jdk9` results in + * `javax.annotation.processing.Generated`. `native` will use whichever version is present + * in the Java runtime used to run the compiler. This option is a sad consequence of the decision + * to repackage the `@Generated` annotation starting in Java 9. Code generated with one annotation + * will not compile on Java versions that do not have this annotation. Because Thrifty is intended + * for use on Android, we default to jdk8 (Android doesn't support JDK9 and probably never* will). + * * If no .thrift files are given, then all .thrift files located on the search path * will be implicitly included; otherwise only the given files (and those included by them) * will be compiled. @@ -92,10 +125,36 @@ class ThriftyCompiler { KOTLIN } + private object GeneratedAnnotationTypes { + const val jdk8 = "javax.annotation.Generated" + const val jdk9 = "javax.annotation.processing.Generated" + } + private val cli = object : CliktCommand( name = "thrifty-compiler", help = "Generate Java or Kotlin code from .thrift files" ) { + + + private val javaVersion: Double by lazy { + val javaVersionText = System.getProperty("java.version") + val versionNumberExpr = Regex("^(\\d+(\\.\\d+)?).*") + val matcher = versionNumberExpr.toPattern().matcher(javaVersionText) + if (!matcher.matches()) { + error("Incomprehensible Java version: $javaVersionText") + } + + val versionNumberText = matcher.group(1) + versionNumberText.toDouble() + } + + private val nativeGeneratedAnnotation: String by lazy { + when { + javaVersion <= 1.8 -> GeneratedAnnotationTypes.jdk8 + else -> GeneratedAnnotationTypes.jdk9 + } + } + val outputDirectory: Path by option("-o", "--out", help = "the output directory for generated files") .path(fileOkay = false, folderOkay = true) .required() @@ -135,6 +194,15 @@ class ThriftyCompiler { help = "When set, @Generated annotations will be suppressed") .flag(default = false) + val generatedAnnotationType: String by option( + "--generated-annotation-type", + help = "JDK 9 repackaged the traditional @Generated annotation. The current platform's annotation is used by default, unless overridden with this option") + .choice( + "jdk8" to GeneratedAnnotationTypes.jdk8, + "jdk9" to GeneratedAnnotationTypes.jdk9, + "native" to nativeGeneratedAnnotation) + .default("javax.annotation.Generated") + val kotlinFilePerType: Boolean by option( "--kt-file-per-type", help = "Generate one .kt file per type; default is one per namespace.") .flag(default = false) @@ -149,7 +217,18 @@ class ThriftyCompiler { .path(exists = true, fileOkay = true, folderOkay = false, readable = true) .multiple() + private val generatedAnnotationClassName: String? by lazy { + when { + omitGeneratedAnnotations -> null + else -> generatedAnnotationType + } + } + override fun run() { + if (javaVersion > 1.8 && generatedAnnotationClassName == GeneratedAnnotationTypes.jdk8) { + TermUi.echo("WARNING: You are using Java $javaVersion, but generating code annotated with $generatedAnnotationClassName") + } + val loader = Loader() for (thriftFile in thriftFiles) { loader.addThriftFile(thriftFile) @@ -207,14 +286,14 @@ class ThriftyCompiler { gen.emitAndroidAnnotations(emitNullabilityAnnotations) gen.emitFileComment(!omitFileComments) gen.emitParcelable(emitParcelable) - gen.emitGeneratedAnnotations(!omitGeneratedAnnotations) + gen.emitGeneratedAnnotations(generatedAnnotationClassName) gen.generate(outputDirectory) } private fun generateKotlin(schema: Schema) { val gen = KotlinCodeGenerator(nameStyle) - .emitGeneratedAnnotations(!omitGeneratedAnnotations) + .emitGeneratedAnnotations(generatedAnnotationClassName) if (emitNullabilityAnnotations) { TermUi.echo("Warning: Nullability annotations are unnecessary in Kotlin and will not be generated") diff --git a/thrifty-integration-tests/build.gradle b/thrifty-integration-tests/build.gradle index b6b4e0aea..589728408 100644 --- a/thrifty-integration-tests/build.gradle +++ b/thrifty-integration-tests/build.gradle @@ -62,7 +62,12 @@ task compileTestThrift(type: Exec) { dependsOn jarTask executable 'java' - args = ['-jar', jarTask.archivePath.absolutePath, "--out=$projectDir/build/generated-src/thrifty/java", "$projectDir/ClientThriftTest.thrift"] + args = [ + '-jar', + jarTask.archivePath.absolutePath, + "--out=$projectDir/build/generated-src/thrifty/java", + "--generated-annotation-type=native", + "$projectDir/ClientThriftTest.thrift"] } @@ -84,6 +89,7 @@ task kompileTestThrift(type: Exec) { "--map-type=java.util.LinkedHashMap", "--set-type=java.util.LinkedHashSet", "--list-type=java.util.ArrayList", + "--generated-annotation-type=native", "$projectDir/ClientThriftTest.thrift" ] } @@ -103,6 +109,7 @@ task kompileCoroutineTestThrift(type: Exec) { jarTask.archivePath.absolutePath, "--out=$projectDir/build/generated-src/thrifty/kotlin", "--lang=kotlin", + "--generated-annotation-type=native", "--kt-coroutine-clients", "$projectDir/CoroutineClientTest.thrift" ] diff --git a/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt b/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt index 1d51305fe..d12636733 100644 --- a/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt +++ b/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt @@ -59,7 +59,6 @@ import java.time.format.DateTimeFormatter import java.util.ArrayList import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger -import javax.annotation.Generated class ThriftyCodeGenerator { @@ -72,7 +71,9 @@ class ThriftyCodeGenerator { private var emitAndroidAnnotations: Boolean = false private var emitParcelable: Boolean = false private var emitFileComment = true - private var emitGeneratedAnnotations = true + private var generatedAnnotationType: ClassName? = null + private val emitGeneratedAnnotations: Boolean + get() = generatedAnnotationType != null constructor(schema: Schema, namingPolicy: FieldNamingPolicy = FieldNamingPolicy.DEFAULT) { @@ -117,8 +118,8 @@ class ThriftyCodeGenerator { return this } - fun emitGeneratedAnnotations(emitGeneratedAnnotations: Boolean): ThriftyCodeGenerator = apply { - this.emitGeneratedAnnotations = emitGeneratedAnnotations + fun emitGeneratedAnnotations(annotationTypeName: String?) = apply { + this.generatedAnnotationType = annotationTypeName?.let { ClassName.bestGuess(it) } } fun usingTypeProcessor(typeProcessor: TypeProcessor): ThriftyCodeGenerator { @@ -208,7 +209,7 @@ class ThriftyCodeGenerator { } private fun generatedAnnotation(): AnnotationSpec { - return AnnotationSpec.builder(Generated::class.java) + return AnnotationSpec.builder(generatedAnnotationType!!) .addMember("value", "\$S", ThriftyCodeGenerator::class.java.name) .addMember("comments", "\$S", "https://github.com/microsoft/thrifty") .build() diff --git a/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt b/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt index 4a2a4ea9d..d4e627e34 100644 --- a/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt +++ b/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt @@ -264,12 +264,6 @@ class ThriftyCodeGeneratorTest { assertThat(file.toString()).isEqualTo(""" package byte_consts; - import javax.annotation.Generated; - - @Generated( - value = "com.microsoft.thrifty.gen.ThriftyCodeGenerator", - comments = "https://github.com/microsoft/thrifty" - ) public final class Constants { public static final byte I8 = (byte) 123; @@ -293,12 +287,6 @@ class ThriftyCodeGeneratorTest { assertThat(file.toString()).isEqualTo(""" package short_consts; - import javax.annotation.Generated; - - @Generated( - value = "com.microsoft.thrifty.gen.ThriftyCodeGenerator", - comments = "https://github.com/microsoft/thrifty" - ) public final class Constants { public static final short INT = (short) 0xFF; @@ -322,12 +310,6 @@ class ThriftyCodeGeneratorTest { assertThat(file.toString()).isEqualTo(""" package int_consts; - import javax.annotation.Generated; - - @Generated( - value = "com.microsoft.thrifty.gen.ThriftyCodeGenerator", - comments = "https://github.com/microsoft/thrifty" - ) public final class Constants { public static final int INT = 12345; @@ -351,12 +333,6 @@ class ThriftyCodeGeneratorTest { assertThat(file.toString()).isEqualTo(""" package long_consts; - import javax.annotation.Generated; - - @Generated( - value = "com.microsoft.thrifty.gen.ThriftyCodeGenerator", - comments = "https://github.com/microsoft/thrifty" - ) public final class Constants { public static final long LONG = 0xFFFFFFFFFFL; @@ -407,12 +383,6 @@ class ThriftyCodeGeneratorTest { val expectedFormat = """ package sigils.consts; - import javax.annotation.Generated; - - @Generated( - value = "com.microsoft.thrifty.gen.ThriftyCodeGenerator", - comments = "https://github.com/microsoft/thrifty" - ) public final class Constants { /** * This comment has ${"$"}Dollar ${"$"}Signs @@ -453,15 +423,9 @@ class ThriftyCodeGeneratorTest { val expected = """ package sigils.enums; - import javax.annotation.Generated; - /** * ${"$"}Sigil here */ - @Generated( - value = "com.microsoft.thrifty.gen.ThriftyCodeGenerator", - comments = "https://github.com/microsoft/thrifty" - ) public enum TestEnum { /** * ${"$"}Good, here's another @@ -541,10 +505,6 @@ class ThriftyCodeGeneratorTest { /** * ${"$"}A ${"$"}B ${"$"}C ${"$"}D ${"$"}E */ - @Generated( - value = "com.microsoft.thrifty.gen.ThriftyCodeGenerator", - comments = "https://github.com/microsoft/thrifty" - ) public final class Foo implements Struct { """.trimRawString() diff --git a/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt b/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt index 85f85384f..ab3b89f2e 100644 --- a/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt +++ b/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt @@ -86,7 +86,6 @@ import com.squareup.kotlinpoet.jvm.jvmField import com.squareup.kotlinpoet.jvm.jvmStatic import okio.ByteString import java.io.IOException -import javax.annotation.Generated private object Tags { val ADAPTER = "RESERVED:ADAPTER" @@ -186,7 +185,7 @@ class KotlinCodeGenerator( var processor: KotlinTypeProcessor = NoTypeProcessor var outputStyle: OutputStyle = OutputStyle.FILE_PER_NAMESPACE - var emitGeneratedAnnotations = true + var generatedAnnotationType: ClassName? = null fun filePerNamespace(): KotlinCodeGenerator = apply { outputStyle = OutputStyle.FILE_PER_NAMESPACE } fun filePerType(): KotlinCodeGenerator = apply { outputStyle = OutputStyle.FILE_PER_TYPE } @@ -212,8 +211,8 @@ class KotlinCodeGenerator( this.coroutineServiceClients = true } - fun emitGeneratedAnnotations(shouldEmit: Boolean): KotlinCodeGenerator = apply { - this.emitGeneratedAnnotations = shouldEmit + fun emitGeneratedAnnotations(type: String?): KotlinCodeGenerator = apply { + this.generatedAnnotationType = type?.let { ClassName.bestGuess(type) } } private object NoTypeProcessor : KotlinTypeProcessor { @@ -1878,20 +1877,20 @@ class KotlinCodeGenerator( } private fun generatedAnnotation(): AnnotationSpec { - return AnnotationSpec.builder(Generated::class.java) + return AnnotationSpec.builder(generatedAnnotationType!!) .addMember("value = [%S]", KotlinCodeGenerator::class.java.name) .addMember("comments = %S", "https://github.com/microsoft/thrifty") .build() } private fun TypeSpec.Builder.addGeneratedAnnotation(): TypeSpec.Builder = apply { - if (emitGeneratedAnnotations) { + if (generatedAnnotationType != null) { addAnnotation(generatedAnnotation()) } } private fun PropertySpec.Builder.addGeneratedAnnotation(): PropertySpec.Builder = apply { - if (emitGeneratedAnnotations) { + if (generatedAnnotationType != null) { addAnnotation(generatedAnnotation()) } } diff --git a/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt b/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt index c96381ce0..a55367add 100644 --- a/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt +++ b/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt @@ -238,16 +238,11 @@ class KotlinCodeGeneratorTest { "${generate(thrift).single()}" shouldBe """ |package test.typedefs | - |import javax.annotation.Generated |import kotlin.Int |import kotlin.collections.Map | |typealias Weights = Map | - |@Generated( - | value = ["com.microsoft.thrifty.kgen.KotlinCodeGenerator"], - | comments = "https://github.com/microsoft/thrifty" - |) |val WEIGHTS: Weights = mapOf(1 to 2) | """.trimMargin() @@ -293,16 +288,11 @@ class KotlinCodeGeneratorTest { |package test.map_consts | |import android.support.v4.util.ArrayMap - |import javax.annotation.Generated |import kotlin.Int |import kotlin.String |import kotlin.collections.List |import kotlin.collections.Map | - |@Generated( - | value = ["com.microsoft.thrifty.kgen.KotlinCodeGenerator"], - | comments = "https://github.com/microsoft/thrifty" - |) |val Maps: Map> = ArrayMap>(2).apply { | put(1, emptyList()) | put(2, listOf("foo")) @@ -323,18 +313,10 @@ class KotlinCodeGeneratorTest { val file = generate(thrift) { coroutineServiceClients() } file.single().toString() should contain(""" - |@Generated( - | value = ["com.microsoft.thrifty.kgen.KotlinCodeGenerator"], - | comments = "https://github.com/microsoft/thrifty" - |) |interface Svc { | suspend fun doSomething(foo: Int): Int |} | - |@Generated( - | value = ["com.microsoft.thrifty.kgen.KotlinCodeGenerator"], - | comments = "https://github.com/microsoft/thrifty" - |) |class SvcClient(protocol: Protocol, listener: AsyncClientBase.Listener) : AsyncClientBase(protocol, listener), | Svc { | override suspend fun doSomething(foo: Int): Int = suspendCoroutine { cont ->