Skip to content

Configuration property descriptions may be lost when building incrementally #28075

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

Open
wilkinsona opened this issue Sep 21, 2021 · 4 comments
Open
Labels
type: bug A general bug
Milestone

Comments

@wilkinsona
Copy link
Member

We have two @ConfigurationProperties annotated classes:

package com.example;

import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "more")
public class MoreProperties {
	
	/**
	 * Another example property.
	 */
	private String someProperty;

	public void setSomeProperty(String someProperty) {
		this.someProperty = someProperty;
	}

	public String getSomeProperty() {
		return this.someProperty;
	}

}
package com.example;

import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "some")
public class SomeProperties {
	
	/**
	 * An example property.
	 */
	private String someProperty;

	public void setSomeProperty(String someProperty) {
		this.someProperty = someProperty;
	}

	public String getSomeProperty() {
		return this.someProperty;
	}

}

compileJava produces the following metadata:

{
  "groups": [
    {
      "name": "more",
      "type": "com.example.MoreProperties",
      "sourceType": "com.example.MoreProperties"
    },
    {
      "name": "some",
      "type": "com.example.SomeProperties",
      "sourceType": "com.example.SomeProperties"
    }
  ],
  "properties": [
    {
      "name": "more.some-property",
      "type": "java.lang.String",
      "description": "Another example property.",
      "sourceType": "com.example.MoreProperties"
    },
    {
      "name": "some.some-property",
      "type": "java.lang.String",
      "description": "An example property.",
      "sourceType": "com.example.SomeProperties"
    }
  ],
  "hints": []
}

We then change the description of more.some-property to Another example property with a new description.. compileJava now produces the following metadata:

{
  "groups": [
    {
      "name": "more",
      "type": "com.example.MoreProperties",
      "sourceType": "com.example.MoreProperties"
    },
    {
      "name": "some",
      "type": "com.example.SomeProperties",
      "sourceType": "com.example.SomeProperties"
    }
  ],
  "properties": [
    {
      "name": "more.some-property",
      "type": "java.lang.String",
      "description": "Another example property with a new description.",
      "sourceType": "com.example.MoreProperties"
    },
    {
      "name": "some.some-property",
      "type": "java.lang.String",
      "sourceType": "com.example.SomeProperties"
    }
  ],
  "hints": []
}

The description of the property that was not changed, some.some-property, has been lost. Upon running clean compileJava it reappears.

@wilkinsona wilkinsona added the type: bug A general bug label Sep 21, 2021
@wilkinsona wilkinsona added this to the 2.4.x milestone Sep 21, 2021
@wilkinsona
Copy link
Member Author

wilkinsona commented Sep 24, 2021

During the second compilation, where the property's description disappears, this.env.getElementUtils().getDocComment(element) returns null. element is the field for the property. getElementUtils returns a com.sun.tools.javac.model.JavacElements.

The doc comment can't be found as com.sun.tools.javac.comp.Enter doesn't have an entry for the unaltered class which ultimately causes getTreeAndTopLevel(Element) to return null and getDocComment(Element) to also return null.

Looking at the output produced by Gradle when running with --debug, it's only the .java file that's been modified that is passed into the compiler. It's not yet clear to me why the other, unmodified type is then passed into the annotation processor.

@wilkinsona
Copy link
Member Author

wilkinsona commented Sep 24, 2021

Digging some more, Gradle's calling javax.tools.JavaCompiler.getTask(…) which states the following in its javadoc:

Note that annotation processing can process both the compilation units of source code to be compiled, passed with the compilationUnits parameter, as well as class files, whose names are passed with the classes parameter.

This is exactly what's happening in this case. The .java file that has been modified is passed in via the compilationUnits parameter while the .class file for the unmodified file is passed in via the classes parameter. As a result of the unmodified file being passed in via the classes parameter, its source model is unavailable making its javadoc comments null.

Based on the above, this is a bug in our annotation processor.

@wilkinsona
Copy link
Member Author

As far as I can tell, there's no way to tell if a TypeElement is backed by a .class file or a .java file without using compiler-specific API.

If we cast javax.lang.model.util.Elements to com.sun.tools.javac.model.JavacElements, we could call getTree(element). It'll return null for elements backed by a .class file. This is an internal of the JDK's compiler so it's quite risky to depend on it.

com.sun.source.util.Trees is an alternative. It's specific to the JDK's compiler, but it does appear to offer better stability guarantees. com.sun.source.util.Trees.instance(ProcessingEnvironment) will give us an instance and then I think we can call com.sun.source.util.Trees.getTree(Element). It'll return null for a TypeElement backed by a .class file.

@wilkinsona wilkinsona changed the title Configuration property descriptions may be lost when building incrementally with Gradle Configuration property descriptions may be lost when building incrementally Sep 30, 2021
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.5.x Nov 15, 2021
@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.6.x May 19, 2022
@wilkinsona wilkinsona self-assigned this Aug 1, 2022
@wilkinsona
Copy link
Member Author

wilkinsona commented Aug 1, 2022

Using com.sun.source.util.Trees to identify if the javadoc was unavailable seems to work. It makes it possible to differentiate between a property that has no description because one hasn't been provided and a property that has no description because the javadoc wasn't available during incremental compilation. The hope was that this would allow us to reuse the description from the previous metadata when the javadoc was unavailable. Unfortunately, this doesn't work as Gradle always deletes the spring-configuration-metadata.json file before incremental compilation begins.

I think we have two, perhaps three, options:

  1. find a way to preserve the previous metadata and make it available to the annotation processor
  2. stop declaring the annotation processor as incremental
  3. use something other than javadoc, for example an annotation with appropriate retention, to provide property descriptions

The second option would increase compile times. The third would require code changes to move away from javadoc to the new mechanism. I am going to explore the first option as, in theory, it offers a solution to the problem without affecting users.

@wilkinsona wilkinsona removed their assignment Sep 21, 2022
@wilkinsona wilkinsona modified the milestones: 2.6.x, 2.7.x Nov 24, 2022
fhirt added a commit to the-i-engineers/tie-gradle-plugins that referenced this issue Feb 17, 2023
cyrillzadra added a commit to the-i-engineers/tie-gradle-plugins that referenced this issue Mar 6, 2023
@philwebb philwebb modified the milestones: 2.7.x, 3.1.x Nov 8, 2023
@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.2.x Apr 26, 2024
@wilkinsona wilkinsona modified the milestones: 3.2.x, 3.3.x Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants