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

Emit @Generated("...") with a link back to the generator on all top-level classes #924

Open
kluever opened this issue Aug 16, 2022 · 1 comment

Comments

@kluever
Copy link
Contributor

kluever commented Aug 16, 2022

Very often it's hard to trace back from generated code to the codegen that produced it. At Google, codegens very often make up the "long tail" of any migration, so being able to easily identify where the code is being generated is hugely important. Thus, we're trying to enforce that all generated code is tagged with @Generated (specifically, javax.annotation.processing.Generated), and includes a reference back to the generator that emitted it.

Is it possible to have javapoet do this automatically for top-level classes?

@JakeWharton
Copy link
Member

It's a tough problem, for sure.

One problem with anything automatic here is how we gain access to the "who" of the generator. Each file spec is a value type, so there's no central factory or authority which represents all files in the compilation/generation unit to which we can register the "who".

The library can also be used both from within an annotation processor but also standalone in a JVM tool or IntelliJ plugin to produce code making it harder to automatically infer anything. We could maybe stack walk up to the processor subtype for annotation processors, to then class containing the main method for the standalone tool, but I have no idea how reliable that would be and no idea what to do for IntelliJ plugins which have lots of entry points.

A final hiccup about doing something automatically is that the @Generated annotation (as you probably know) has a bit of a storied past in that it moved incompatibly between 8 and 9 to facilitate module splits. Additionally, when compiling for Android targets neither version of the annotation will be present unless added to the application's classpath through an external dependency (it's never on the bootstrap classpath equivalent). As a result, when I do @Generated in an annotation processor it ends up looking like this: https://github.com/cashapp/InflationInject/blob/250b14d066509e1c1e647ddcfaad57070f32fff1/inflation-inject-processor/src/main/java/app/cash/inject/inflation/processor/internal/generatedAnnotation.kt#L30-L43. Figuring out the source version to pick the correct type and then resolving whether or not that type is valid are two things that JavaPoet can't really do on its own.

I'm not sure if you've tried anything else yet, but there's definitely other places to potentially intercept this problem and trying to help fix it:

  • You could use a Filer wrapper that (unfortunately) re-parses the Java and asserts the presence of the annotation with increasing degrees of finger-wagging over time. This would also be a good stack walker usage opportunity as whoever is interacting with the filer is very close to blame for the source being written. This requires a library with the filer wrapper and then a one-liner change inside your JDK to always wrap before invoking the processor.

  • You could deny-list the various static factories on the specs and introduce your own centralized factory for them which can only be obtained by providing generator information.

    Something like

    var specs = SpecGenerator.forProcessor(this, "some optional comment")
    // ...later...
    var file = specs.fileBuilder(..)
      .addType(specs.typeBuilder(..)
        .build())
      .build()

I'm sure there's others I'm not thinking of.

Definitely open to other suggestions here, but I'm not seeing how we could easily do this automatically while also being so general-purpose as a tool.

If we wanted to do something in JavaPoet, the best I can think of is registering an AnnotationSpec with the FileSpec.Builder which is put on top of each top-level type it contains at write-time (we can't do it at add time because we break referential equality in the public list of types that is exposed).

var file = FileSpec.builder(..)
    .generatedTypeAnnotation(..)
    .addType(..)
    .build()

You would still have to deny-list FileSpec.builder() and provide something like FileSpecs.builder() which required the AnnotationSpec (and internally invoked the regular builder() + generatedTypeAnnotation() methods), but that's not too bad.

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

No branches or pull requests

2 participants