Skip to content

Commit

Permalink
Support configurable @generated annotation types (#259)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjamin-bader authored Nov 2, 2018
1 parent 78f0a79 commit 00bbd2e
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 74 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
language: java
jdk:
- oraclejdk8
- openjdk10

sudo: false
install: echo './gradlew check will install dependencies'
Expand Down
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions thrifty-compiler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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
* ...
Expand All @@ -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.
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
9 changes: 8 additions & 1 deletion thrifty-integration-tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}


Expand All @@ -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"
]
}
Expand All @@ -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"
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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) {

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 }
Expand All @@ -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 {
Expand Down Expand Up @@ -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())
}
}
Expand Down
Loading

0 comments on commit 00bbd2e

Please sign in to comment.