From 8bf93eae1ff278a27045f742cfedb69090c5f9d2 Mon Sep 17 00:00:00 2001 From: Ben Bader Date: Fri, 2 Oct 2020 23:05:36 -0600 Subject: [PATCH] Remove all support for '@Generated' annotations (#402) --- README.md | 9 --- thrifty-compiler/README.md | 2 - .../thrifty/compiler/ThriftyCompiler.kt | 65 ------------------- .../thrifty/gradle/ThriftyExtension.kt | 13 ---- .../microsoft/thrifty/gradle/ThriftyTask.kt | 3 - thrifty-integration-tests/build.gradle | 3 - .../ThriftyCodeGenerator.kt | 26 +------- .../thrifty/kgen/KotlinCodeGenerator.kt | 38 ----------- 8 files changed, 2 insertions(+), 157 deletions(-) diff --git a/README.md b/README.md index 61a9485..b7a14e6 100644 --- a/README.md +++ b/README.md @@ -464,7 +464,6 @@ java -jar thrifty-compiler.jar \ --experimental-kt-builderless-structs \ --kt-file-per-type \ --omit-file-comments \ - --omit-generated-annotations \ ... ``` @@ -492,14 +491,6 @@ 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 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 Thrifty owes an enormous debt to Square and the Wire team; without them, this project would not exist. Thanks! diff --git a/thrifty-compiler/README.md b/thrifty-compiler/README.md index db3e046..d8ad51b 100644 --- a/thrifty-compiler/README.md +++ b/thrifty-compiler/README.md @@ -24,9 +24,7 @@ Option | Description `--nullability-annotation-type=[none,android-support,androidx]` | Optional, defaults to `none`. Specifies whether or not Android nullability annotations will be added for generated fields (`@NonNull` and `@Nullable`) and, if so, which types - those in `android.support.annotation` or `androidx.annotation`. `--parcelable` | Optional. Structs, unions, and exceptions will have implementations of Parcelable generated. `--omit-file-comments` | Optional. When set, don't add file comments to generated files. -`--omit-generated-annotations` | Optional. When set, don't add @Generated annotations to generated types. `--list-type=[classname]` | A java.util.List implementation to be used wherever lists are instantiated in generated code. `--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 53e071c..bf47339 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 @@ -117,19 +117,6 @@ import java.util.ArrayList * 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). - * * `--experimental-kt-builder-required-ctor` is optional. When specified, Generate struct Builder * constructor with required parameters, and marks empty Builder constructor as deprecated. Helpful * when needing a compile time check that required parameters are supplied to the struct. This @@ -146,36 +133,10 @@ 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(canBeFile = false, canBeDir = true) .required() @@ -230,19 +191,6 @@ class ThriftyCompiler { help = "When set, don't add file comments to generated files") .flag(default = false) - val omitGeneratedAnnotations: Boolean by option("--omit-generated-annotations", - 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 kotlinEmitJvmName: Boolean by option("--kt-emit-jvmname", help = "When set, emit @JvmName annotations") .flag(default = false) @@ -269,18 +217,7 @@ class ThriftyCompiler { help = "When set, unknown values found when decoding will throw an exception. Otherwise, it uses null/default values.") .flag("--no-fail-on-unknown-enum-values", default = true) - 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) @@ -346,7 +283,6 @@ class ThriftyCompiler { gen.nullabilityAnnotationType(nullabilityAnnotationType) gen.emitFileComment(!omitFileComments) gen.emitParcelable(emitParcelable) - gen.emitGeneratedAnnotations(generatedAnnotationClassName) gen.failOnUnknownEnumValues(failOnUnknownEnumValues) gen.generate(outputDirectory) @@ -354,7 +290,6 @@ class ThriftyCompiler { private fun generateKotlin(schema: Schema) { val gen = KotlinCodeGenerator(nameStyle) - .emitGeneratedAnnotations(generatedAnnotationClassName) if (nullabilityAnnotationType != NullabilityAnnotationType.NONE) { TermUi.echo("Warning: Nullability annotations are unnecessary in Kotlin and will not be generated") diff --git a/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyExtension.kt b/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyExtension.kt index 0395053..5540918 100644 --- a/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyExtension.kt +++ b/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyExtension.kt @@ -140,11 +140,6 @@ sealed class ThriftOptions { var mapType: String? = null private set - @Input - @Optional - var generatedAnnotationType: String? = null - private set - @Input var parcelable: Boolean = false @@ -198,14 +193,6 @@ sealed class ThriftOptions { this.mapType = clazz.canonicalName!! } - fun generatedAnnotationType(clazz: Class<*>) { - generatedAnnotationType(clazz.canonicalName) - } - - fun generatedAnnotationType(type: String?) { - this.generatedAnnotationType = type - } - fun allowUnknownEnumValues(allow: Boolean) { this.allowUnknownEnumValues = allow } diff --git a/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyTask.kt b/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyTask.kt index 0ad0c19..f8ca3a2 100644 --- a/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyTask.kt +++ b/thrifty-gradle-plugin/src/main/kotlin/com/microsoft/thrifty/gradle/ThriftyTask.kt @@ -113,7 +113,6 @@ open class ThriftyTask @Inject constructor( private fun generateJavaThrifts(schema: Schema, options: JavaThriftOptions) { val gen = ThriftyCodeGenerator(schema, options.namePolicy).apply { emitFileComment(true) - emitGeneratedAnnotations(options.generatedAnnotationType) emitParcelable(options.parcelable) failOnUnknownEnumValues(!options.allowUnknownEnumValues) @@ -162,8 +161,6 @@ open class ThriftyTask @Inject constructor( } } - emitGeneratedAnnotations(options.generatedAnnotationType) - options.listType?.let { listClassName(it) } options.setType?.let { setClassName(it) } options.mapType?.let { mapClassName(it) } diff --git a/thrifty-integration-tests/build.gradle b/thrifty-integration-tests/build.gradle index acccd27..6f04493 100644 --- a/thrifty-integration-tests/build.gradle +++ b/thrifty-integration-tests/build.gradle @@ -56,7 +56,6 @@ def compileTestThrift = tasks.register("compileTestThrift", JavaExec) { t -> args = [ "--out=$projectDir/build/generated-src/thrifty/java", - "--generated-annotation-type=native", "$projectDir/ClientThriftTest.thrift"] } @@ -73,7 +72,6 @@ def kompileTestThrift = tasks.register("kompileTestThrift", JavaExec) { t -> "--map-type=java.util.LinkedHashMap", "--set-type=java.util.LinkedHashSet", "--list-type=java.util.ArrayList", - "--generated-annotation-type=native", "$projectDir/ClientThriftTest.thrift" ] } @@ -87,7 +85,6 @@ def kompileCoroutineTestThrift = tasks.register("kompileCoroutineTestThrift", Ja args = [ "--out=$projectDir/build/generated-src/thrifty/kotlin", "--lang=kotlin", - "--generated-annotation-type=native", "--kt-coroutine-clients", "--experimental-kt-builderless-structs", "$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 96aa50a..6fdb546 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 @@ -72,9 +72,6 @@ class ThriftyCodeGenerator { private var emitParcelable: Boolean = false private var emitFileComment = true private var failOnUnknownEnumValues = true - private var generatedAnnotationType: ClassName? = null - private val emitGeneratedAnnotations: Boolean - get() = generatedAnnotationType != null constructor(schema: Schema, namingPolicy: FieldNamingPolicy = FieldNamingPolicy.DEFAULT) { @@ -119,10 +116,6 @@ class ThriftyCodeGenerator { return this } - fun emitGeneratedAnnotations(annotationTypeName: String?) = apply { - this.generatedAnnotationType = annotationTypeName?.let { ClassName.bestGuess(it) } - } - fun usingTypeProcessor(typeProcessor: TypeProcessor): ThriftyCodeGenerator { this.typeProcessor = typeProcessor return this @@ -188,17 +181,9 @@ class ThriftyCodeGenerator { } private fun assembleJavaFile(packageName: String, spec: TypeSpec, location: Location? = null): JavaFile? { - val annotatedSpec = if (emitGeneratedAnnotations) { - spec.toBuilder() - .addAnnotation(generatedAnnotation()) - .build() - } else { - spec - } - val processedSpec = typeProcessor?.let { processor -> - processor.process(annotatedSpec) ?: return null - } ?: annotatedSpec + processor.process(spec) ?: return null + } ?: spec val file = JavaFile.builder(packageName, processedSpec) .skipJavaLangImports(true) @@ -214,13 +199,6 @@ class ThriftyCodeGenerator { return file.build() } - private fun generatedAnnotation(): AnnotationSpec { - return AnnotationSpec.builder(generatedAnnotationType!!) - .addMember("value", "\$S", ThriftyCodeGenerator::class.java.name) - .addMember("comments", "\$S", "https://github.com/microsoft/thrifty") - .build() - } - private fun buildStruct(type: StructType): TypeSpec { val packageName = type.getNamespaceFor(NamespaceScope.JAVA) val structTypeName = ClassName.get(packageName, type.name) 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 3ddd3af..63f8863 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 @@ -197,7 +197,6 @@ class KotlinCodeGenerator( var processor: KotlinTypeProcessor = NoTypeProcessor var outputStyle: OutputStyle = OutputStyle.FILE_PER_NAMESPACE - var generatedAnnotationType: ClassName? = null fun filePerNamespace(): KotlinCodeGenerator = apply { outputStyle = OutputStyle.FILE_PER_NAMESPACE } fun filePerType(): KotlinCodeGenerator = apply { outputStyle = OutputStyle.FILE_PER_TYPE } @@ -231,10 +230,6 @@ class KotlinCodeGenerator( this.coroutineServiceClients = true } - fun emitGeneratedAnnotations(type: String?): KotlinCodeGenerator = apply { - this.generatedAnnotationType = type?.let { ClassName.bestGuess(type) } - } - fun emitJvmName(): KotlinCodeGenerator = apply { this.emitJvmName = true } @@ -359,7 +354,6 @@ class KotlinCodeGenerator( internal fun generateEnumClass(enumType: EnumType): TypeSpec { val typeBuilder = TypeSpec.enumBuilder(enumType.name) - .addGeneratedAnnotation() .addProperty(PropertySpec.builder("value", INT) .jvmField() .initializer("value") @@ -415,8 +409,6 @@ class KotlinCodeGenerator( internal fun generateDataClass(schema: Schema, struct: StructType): TypeSpec { val structClassName = ClassName(struct.kotlinNamespace, struct.name) val typeBuilder = TypeSpec.classBuilder(structClassName).apply { - addGeneratedAnnotation() - if (struct.fields.isNotEmpty()) { addModifiers(KModifier.DATA) } @@ -541,8 +533,6 @@ class KotlinCodeGenerator( val structClassName = ClassName(struct.kotlinNamespace, struct.name) val typeBuilder = TypeSpec.classBuilder(structClassName).apply { - addGeneratedAnnotation() - addModifiers(KModifier.SEALED) if (struct.isDeprecated) addAnnotation(makeDeprecated()) @@ -1514,7 +1504,6 @@ class KotlinCodeGenerator( val typeName = type.typeName val propName = allocator.newName(constant.name, constant) val propBuilder = PropertySpec.builder(propName, typeName) - .addGeneratedAnnotation() if (constant.isDeprecated) propBuilder.addAnnotation(makeDeprecated()) if (constant.hasJavadoc) propBuilder.addKdoc("%L", constant.documentation) @@ -1843,8 +1832,6 @@ class KotlinCodeGenerator( serviceType.extendsService?.let { baseType -> addSuperinterface(baseType.typeName) } - - addGeneratedAnnotation() } val allocator = nameAllocators[serviceType] @@ -1892,8 +1879,6 @@ class KotlinCodeGenerator( superclass(baseClassName) addSuperinterface(ClassName(serviceType.kotlinNamespace, serviceType.name)) - addGeneratedAnnotation() - // If any servces extend this, then this needs to be open. if (schema.services.any { it.extendsService == serviceType }) { addModifiers(KModifier.OPEN) @@ -1943,8 +1928,6 @@ class KotlinCodeGenerator( serviceType.extendsService?.let { addSuperinterface(it.typeName) } - - addGeneratedAnnotation() } val allocator = nameAllocators[serviceType] @@ -1986,8 +1969,6 @@ class KotlinCodeGenerator( superclass(baseClassName) addSuperinterface(ClassName(serviceType.kotlinNamespace, serviceType.name)) - addGeneratedAnnotation() - // If any servces extend this, then this needs to be open. if (schema.services.any { it.extendsService == serviceType }) { addModifiers(KModifier.OPEN) @@ -2306,29 +2287,10 @@ class KotlinCodeGenerator( .build() } - private fun generatedAnnotation(): AnnotationSpec { - return AnnotationSpec.builder(generatedAnnotationType!!) - .addMember("value = [%S]", KotlinCodeGenerator::class.java.name) - .addMember("comments = %S", "https://github.com/microsoft/thrifty") - .build() - } - private fun suppressUnusedParam(): AnnotationSpec { return AnnotationSpec.builder(Suppress::class) .addMember("%S", "UNUSED_PARAMETER") .build() } - - private fun TypeSpec.Builder.addGeneratedAnnotation(): TypeSpec.Builder = apply { - if (generatedAnnotationType != null) { - addAnnotation(generatedAnnotation()) - } - } - - private fun PropertySpec.Builder.addGeneratedAnnotation(): PropertySpec.Builder = apply { - if (generatedAnnotationType != null) { - addAnnotation(generatedAnnotation()) - } - } }