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

Ambiguous behavior for ClassNameGenerator::generateClassName #28517

Closed
bclozel opened this issue May 24, 2022 · 2 comments
Closed

Ambiguous behavior for ClassNameGenerator::generateClassName #28517

bclozel opened this issue May 24, 2022 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented May 24, 2022

ClassNameGenerator has two variants for the generateClassName feature. One that takes a target Class<?> and another that takes a target String. I'm a bit confused by the concept of a target here and I think we should explain that a bit more in the javadocs.

These methods also have different behavior:

ClassName first = this.generator.generateClassName(java.io.InputStream.class, "bytes");
// will result in java.io.InputStream__Bytes

ClassName first = this.generator.generateClassName("java.io.InputStream", "bytes");
// will result in __.JavaIoInputStream__Bytes

Even if the difference of behavior is intended, the name and docs are very similar. Also, a common use case for the ClassNameGenerator is about generating sources in a specific package to work around visibility issues. The second variant can make this case more difficult to achieve.

As a side note, this class is also referring to the now defunct @see GeneratedClassName.

cc @philwebb @snicoll

@bclozel bclozel added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels May 24, 2022
@bclozel bclozel added this to the 6.0.0-M5 milestone May 24, 2022
@philwebb
Copy link
Member

The String variant was added to support generation where there isn't a single class that can be linked. I think it's currently only used in BeanRegistrationsAotContribution. We should probably rename that method to make it clearer that there is no target class.

I know @snicoll was wondering about the use of the __ package in general, perhaps we can make the generated BeanRegistrations class be in the same package as the @SpringBootApplication class then we can drop the string version entirely.

The ClassNameGenerator should be removed, we dropped that class during the prototype work.

@snicoll
Copy link
Member

snicoll commented Jun 20, 2022

The second method that takes a String rather than a Class<?> has been removed in 4bd33cb. I am going to look at the Javadoc and see if we can clarify

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants