skinny85 / jilt

Java annotation processor library for auto-generating Builder (including Staged Builder) pattern classes
Other
240 stars 13 forks source link

Meta-annotations support #14

Closed thmasker closed 4 months ago

thmasker commented 8 months ago

Hi, I'm using this library's @Builder in quite a lot of classes with the same arguments:

@Builder(setterPrefix = "with", style = BuilderStyle.STAGED, factoryMethod = "builder")

I was thinking of creating an annotation to avoid this code repetition, but I see this annotation cannot be used as meta-annotation. I lack enough knowledge to know if it is possible to implement this functionality, but I think it would be really nice if possible

skinny85 commented 8 months ago

Hi @thmasker,

thanks a lot for opening the issue!

To summarize, you'd want to do something like:

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.SOURCE)
@Builder(
    setterPrefix = "with",
    style = BuilderStyle.STAGED,
    factoryMethod = "builder")
public @interface MyBuilder {
}

And then use it like so:

@MyBuilder
public MyValueClass {
    // ...
}

Did I understand your intentions correctly?

Thanks, Adam

thmasker commented 8 months ago

Thatโ€™s exactly what I wanna do

skinny85 commented 8 months ago

Sweet. I think that's a very reasonable ask, and a lot of other annotation processor libraries work this way.

I'll work on this feature this month. I'll update this issue as I know more.

thmasker commented 8 months ago

Thanks! Iโ€™m open to contribute as wellโ€ฆ

skinny85 commented 8 months ago

Thanks for being open to that ๐Ÿ™‚. Do you have any experience with implementing meta-annotations? I have to confess I don't, but I can do some research ๐Ÿ™‚.

thmasker commented 8 months ago

I don't either, but I will do some research too, and try to implement it

thmasker commented 8 months ago

I've done a PR (https://github.com/skinny85/jilt/pull/15). I tried to link the issue, but was not able to

skinny85 commented 8 months ago

Thank you! I've commented on the PR, and edited its title and description to link it with this issue.

skinny85 commented 7 months ago

This has been fixed in Jilt release 1.5.

I'm resolving this issue, please comment if you run into this problem again, and I'll be happy to reopen.

thmasker commented 7 months ago

Thanks! Just so you know, I'm right now testing it and it works. However, there is a case in which it isn't. Since I'm using the same builder annotation for several projects, I've created my own builder annotation on a common Maven project, shared between all the rest of the projects. In this case, builders are not generated.

skinny85 commented 7 months ago

@thmasker would you mind creating a small GitHub repo with a reproduction? Iโ€™d be easier for me to investigate that way.

thmasker commented 7 months ago

I will, asap

skinny85 commented 7 months ago

Thank you!

thmasker commented 7 months ago

jilt-common includes the common builder jilt-test tries to use it

thmasker commented 7 months ago

Another interesting feature would be to add @Documented in both @Builder and @BuilderInterfaces. At least I'd find it useful in the case of meta-annotations. Anyway, this is a minor update and I'm already happy with the library. Just so you may consider it...

skinny85 commented 7 months ago

Remind me, what does @Documented do again? ๐Ÿ˜›

thmasker commented 7 months ago

It would make @Builder and/or @BuilderInterfaces annotations to appear in JavaDocs in its annotated elements. I think it would be really useful if Jilt's annotations are used as meta-annotations. This is a more practical explanation.

A more technical explanation can be found here: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/annotation/Documented.html

thmasker commented 7 months ago

A thought just popped in right before going to bed: it might have to do with the retention policy. I will test tomorrow if possible

skinny85 commented 7 months ago

It would make @Builder and/or @BuilderInterfaces annotations to appear in JavaDocs in its annotated elements. I think it would be really useful if Jilt's annotations are used as meta-annotations. This is a more practical explanation.

A more technical explanation can be found here: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/annotation/Documented.html

Sure. We can add it, I don't have any objections to that.

thmasker commented 7 months ago

Follow up: I tested it and obviously with RetentionPolicy.SOURCE annotations are lost after compiling the common project, so no processing takes place on dependent projects. I also tested it with RetentionPolicy.CLASS and RetentionPolicy.RUNTIME. In these cases, annotations are preserved, but no processing is made. I guess that's because the common project is compiled and dependent projects use those compiled files. So far, I don't see how to fix this, although I will continue to investigate...

skinny85 commented 7 months ago

Actually, CLASS preserves the annotations in the compiled .class files (hence the name).

thmasker commented 7 months ago

You are right! I edited the comment to make sense. It was a typo

skinny85 commented 7 months ago

I did some digging on my own, and the problem is that the process() method of the annotation processor is not even called when a class is annotated just with @MyBuilder (without @Builder) in the consuming project.

I'm wondering whether the solution is that you have to annotate your class with both, but the Jilt annotation processor recognizes that a class is "double annotated" (through the meta-annotation) with @Builder, and it merges the attributes correctly (so, @MyBuilder attributes will take precedence over @Builder one).

So, given:

@Builder(setterPrefix = "with", style = BuilderStyle.STAGED, factoryMethod = "builder")
public @interface MyBuilder {
}

If you then have:

@MyBuilder
@Builder
public final class MyClass {
    // ...
}

Then it's like if you annotated MyClass with @Builder(setterPrefix = "with", style = BuilderStyle.STAGED, factoryMethod = "builder"). So, while we do need to have 2 annotations, the attributes of @Builder are not repeated, so @MyBuilder does serve its basic purpose.

Thoughts on this @thmasker?

thmasker commented 7 months ago

To be honest I'd rather redo my custom annotation per project. I see your proposal more like a tweaking. It seems there is no other way at the moment, tho. But I'd really like to know your opinion on it as well since you are far more experienced than me. Would you apply that solution on your projects?

skinny85 commented 7 months ago

I agree with you ๐Ÿ™‚. While it's not the worst, it is a little awkward. I would probably also just define the annotation inside my project, and abandon the idea of putting it into a shared library.

thmasker commented 7 months ago

Maybe coding a processor for the custom annotation in the shared library would work. Although it is also a bit of work, maybe simply extending or copying Jilt's would be enough... now this is a challenge for me haha. I can't rest until I find a solution.

skinny85 commented 7 months ago

I think that's absolutely insane, but hey, go for it - don't let me stop you! ๐Ÿ˜ƒ

skinny85 commented 4 months ago

@thmasker I assume with #20 done, we can safely close this issue.

Let me know if you run into any problems related to this, and I'd be happy to reopen.

thmasker commented 4 months ago

Hi @skinny85. First of all, thanks for releasing the new version! But I'm afraid this problem is not solved... Meta-annotations still fails to work when using a common library for multiple projects

skinny85 commented 4 months ago

Well, I think the only thing we can do here is https://github.com/skinny85/jilt/issues/14#issuecomment-2021956294, and we both agreed we probably don't want to go there, so I'm not sure there's much left to do here...

thmasker commented 4 months ago

I guess you are right