Ideally avaje-inject-generator should not depend on avaje-inject

[agentgt]

A lot of folks are not aware of this but you should almost never use the actual annotation classes in an annotation processor even your own. BTW I only know of couple libraries that generally do this correctly: MapStruct, JStachio (mine) and I believe Micronaut (so don't feel bad). It is also probably why no one knows about Hickory (or the new MapStruct gemtools).

So why is it problematic? Well the annotation processor is not guaranteed to have classes loaded and if your annotation references other classes (e.g. has a Class<?> parameter ) that are not yours you will have issues especially and I mean especially if you are dealing with modularized projects.

So why have you guys not run into issues? Well because both Maven and I think gradle as well will simply move whatever dependencies that are in <annotationProcessorPaths> into the classpath during compilation. Otherwise all libraries that are referenced with requires (static or not) in module-info (and not in the annotationProcessorPaths) will be put in the module-path.

The above is one the reasons why <annotationProcessorPaths> is required for modular projects but it is seriously limited and technically not correct either: https://issues.apache.org/jira/browse/MCOMPILER-391 and https://issues.apache.org/jira/browse/MCOMPILER-412

Otherwise if avaje-inject-generator does not have dependencies just having avaje-inject-generator on the classpath will work and you would not need to do <annotationProcessorPaths> for modular projects and this is one of the big advantages to doing it the painful. BTW this is also why lots of annotation processors shade their deps but you can't really do that for annotations.

So how do you access your own annotations without using them directly? You have to load the TypeElement.

avaje-inject/inject-generator/src/main/java/io/avaje/inject/generator/Processor.java

Line 118 in 12eba7f

readScopes(roundEnv.getElementsAnnotatedWith(Scope.class));

Instead of Scope.class you are going to pass a TypeElement. To get the TypeElement you can either get it from the call process function or just call Elements.getTypeElement: https://docs.oracle.com/en/java/javase/11/docs/api/java.compiler/javax/lang/model/util/Elements.html#getTypeElement(java.lang.CharSequence)

Anyway that is easy and fine however it becomes a pain in the ass to get annotation parameters. That is where Hickory comes in (or mapstruct gemtools but you would have to shade it).
https://javadoc.io/static/com.jolira/hickory/1.0.0/net/java/dev/hickory/prism/package-summary.html

Hickory is an annotation processor that makes what are called prisms which you can access your annotations as though you are accessing them directly.

Anyway I normally would not bother even bringing all this up as yeah there is a work around ... manually register annotation processors...

But the project appears to not heavily dependent on its annotation core lib and is not a big change. In fact I could probably do a PR and do it myself if you are interested.

Furthermore it is technically the right way to do it and will prevent problems in the future as more folks embrace modularity.

[SentryMan]

annotation references other classes (e.g. has a Class<?> parameter ) you get a problem

Do you mean the whole MirroredTypeException thing where you need to catch and use the type mirror? Or something else?

Hickory

do you have a code sample I could view to see how accessing annotations works?

In fact I could probably do a PR and do it myself if you are interested.

Feel free my guy, I kinda want to see how it'd look.

[agentgt]

Do you mean the whole MirroredTypeException thing where you need to catch and use the type mirror? Or something else?

Yep. You can of course catch that and call whatever the method on it that gives you the mirrors. I didn't check to see if you are doing that.

Of course the above becomes moot if you use TypeMirror/TypeElement.

do you have a code sample I could view to see how accessing annotations works?

Yeah but just don't go look around too much in the code base or you will be horrified ( compiler module in the project is an older code base):

Registering prisms (you make the deps to the annotations provided and optional or shade the generated code like I did):

https://github.com/jstachio/jstachio/blob/main/compiler/jstachio-prisms/src/main/java/io/jstach/apt/prism/package-info.java

Here is some accessing JStacheInterfaces: https://github.com/jstachio/jstachio/blob/1648f5d5c724b427f286b059a0ef3e24000a5265/compiler/apt/src/main/java/io/jstach/apt/GenerateRendererProcessor.java#L212

However its confusing in my code base because I go lookup enclosing elements for annotations (findPrisms).

But to be honest the Javadoc on hickory explains it better than mode code:
https://javadoc.io/static/com.jolira/hickory/1.0.0/net/java/dev/hickory/prism/package-summary.html

[SentryMan]

So I just tried adding a module.info to my example project and got this error java.lang.NoClassDefFoundError: io/avaje/inject/spi/Module: io.avaje.inject.spi.Module pretty sure it's coming from over here.

Would the hickory thing fix that? Also I've been trying to figure out how to make it work for like an hour now. Do hickory and the prisms need to be in a separate module and shaded in? or can they work in the inject-generator module?

[agentgt]

Sadly Hickory is so old it does not have a module so I think that might be the issue. That is why in jstachio I shaded it in.

Otherwise I might be misunderstanding the problem. Let me reread your comment tomorrow and I’ll follow up.

[SentryMan]

Caused by: java.lang.NoClassDefFoundError: io/avaje/inject/spi/Module at io.avaje.inject.generator.ExternalProvider.init(ExternalProvider.java:18)

Don't think the problem is with hickory, the prisms generate fine. Module is an interface type from the core avaje-inject. When I make it not a provided dependency it compiles. See this branch.

Essentially, we use the serviceloader to auto detect modules at compile time. Eliminating the need to explicitly define @InjectModules. The generator has a hard dependency on the core inject.

[rbygrave]

should almost never use the actual annotation classes in an annotation processor

Yes. Now I'm sure I hit this with another annotation processor querybean-generator which generates query beans for ebean orm ... and I'm a tad confused because I'm sure I previously had avaje-inject-generator all working with modules BUT it definitely is not working today with later versions of maven, plugins, java etc.

The generator has a hard dependency on the core inject.

Well we can change that. We can change to not use the ServiceLoader but instead read the services files, get the element names of the other modules, and then read the provided components etc from @DependencyMeta (if we have that access via classpath which we sometimes don't and then we use the inject-maven-plugin) ... so we can remove the hard dependency on avaje-inject (although the Plugin doesn't come with the same meta data as yet).

Looking at the annotations, the only annotation that has attributes we read is @InjectModule and the rest are just markers. That suggests to me that it might be worth trying to go the TypeElement/TypeMirror route.

[agentgt]

@SentryMan I'm sorry I misread your comment. I understand the problem now. Let me try forking later today off your branch.

[SentryMan]

read the services files

Yeah, I'm gonna need a hint on how to do this. Filer can't read multiple resources from dependencies, and I tried doing something wacky with URL classloader and scanning jars for the service files, but no dice. Maybe we should just lean on the inject mvn plugin entirely for auto requires.

[agentgt]

I haven't looked at the code yet but I will say if you are offering an SPI (an interface) for hooking into code generation it really should not be in the avaje annotation module (however a quick glance at Module and that does not appear to be the case).

Hickory would help you get things out Module safely with our referencing the actual class.

If you are generating META-INF/services I'm not sure what the problem is there however for modular applications that will not work but usually the idea and I think the avaje doc says this is you just have to manually plug in some provider of providers in.

That is the SPI is more like a factory that returns a list of implementations and not a single implementation so that one only has to make one module-info entry instead.

e.g. the SPI should be more like:

public interface ModuleProvider {
   List<Module> modules();
} 

Then you generate one standard ModuleProvider implementation that the user then references in their module-info.

But that is just my quick opinion.

[SentryMan]

If you are generating META-INF/services I'm not sure what the problem is there however for modular applications that will not work but usually the idea and I think the avaje doc says this is you just have to manually plug in some provider of providers in.

This is a new feature to autodetect avaje modules from dependencies without the need to explicitly define them in @InjectModules. The module class is used at runtime to build the bean scope. If you want to quickly debug, you can use this test class to step through the processor code

[agentgt]

I'm looking at the code now. I really need to fork hickory and making it proper module. That is why you had so many problems initially. I'll do that later today.

I was able to avoid that by making my annotation processor module not have a module-info and then generating the module-info with moditect.

That is the annotation processor is classpath compiled by maven and the IDE.

[agentgt]

@SentryMan I'm working off your branch. Give me hopefully like an hour and I think I should have it possibly working.

[agentgt]

@SentryMan I see the major problem with ExternalProvider.

@rbygrave I'm not sure I know fully how it Module is supposed to work but I assume implementations are generated by the generator including the META-INF/services registration of Module? That is it is not a true SPI for end users correct?

I recommend that Module become an annotation and preferable not Module as that name has huge conflict.

Instead of generating Module implementations you will generate classes that have a Module annotation on them.

The Module annotation has all the methods on the interface.

The user then create a package and package-info lets call it "gen" using your documented example

@Modules({org.example.gen.ExampleModule})
package org.example.gen;

ExampleModule has the new Module annotation on it and is generated.

Then in module-info we have something like:

module org.example {
  requires io.avaje.inject;
  //provides io.avaje.inject.spi.Module with org.example.ExampleModule;
  //Instead we open 
  open org.example.gen;
}

Then you go fetch the annotation mirrors on org.example.gen in the annotation processor.

However I have a feeling the issue is cross compile lookup.

A project I know that does DI across modules and compile time is inverno so I might go check exactly how they deal with discovery across modules. I'm guessing module annotations unlike package annotations are generally public but maybe not.

[SentryMan]

Modules cannot become an annotation because it is used at runtime to wire beans. in any case, we can take out this part and use the inject maven plugin to read the provided classes.

[agentgt]

OK I will put a dummy implementation in for now and push my code so you can get see what I did.

[SentryMan]

I pushed some changes to my branch, the avaje inject part works on my example repo( jsonb is still failing) but the avaje tests are failing

[SentryMan]

OK I will put a dummy implementation in for now and push my code so you can get see what I did.

oh nice did you get tests to pass?

[agentgt]

Plugin is another broken one. That one I'm fairly sure can be fixed as an annotation.

The problem with these patterns of using the ServiceLoader is that the appear they are only used to get access to static stuff.

Does Module implementations do dynamic stuff or is it just a catalog?

[SentryMan]

They wire all the beans into a beanscope. The plugin thing can also be resolved by the inject mvn plugin.

Real quick clone this guy execute the run.sh script and see the generated module in target/generated sources.

[SentryMan]

Do you have a discord or something? this would be much easier to explain over a call.

[agentgt]

Yeah so far it looks like our code matches up. I'm still doing the to prism dance.

I haven't run the tests yet. Sadly I have kind of run out of time today :(

One thing I did and I'll just check it in is move prisms to their own module and then made that module an automatic module.

This just makes it easier for shitty IDEs like Eclipse (which yes I was using for this).
It also makes it so you don't have to use the weird maven third party annotation processor plugin.

I'll look at your changes later tonight. Sorry.

[SentryMan]

can you push your stuff so I can see it?

Eclipse

Ahh, another man of distinguished taste

[agentgt]

https://github.com/agentgt/avaje-inject

Like an imbecile I pushed the changes into master. I have a single test failing. The branch is prism. Sorry I have to go now. ttl

[agentgt]

https://github.com/SentryMan/avaje-inject/compare/prism...agentgt:avaje-inject:prism?expand=1

The only major difference is that I didn't deal with the service loader problems and I made a separate package for prisms which you could in theory shade.

I'll try to answer stuff later via mobile.

Cheers

[SentryMan]

your prism branch doesn't build [ERROR] (package javax.lang.model.element is declared in module java.compiler, but module io.avaje.inject.prism does not read it)

[agentgt]

Yeah maven multimodule really hates mixture of automatic and module-info.

I fixed it with moditect I think but I'm getting a strange error now about the maven ajave inject plugin.

EDIT

That is try my recent push.

BTW the changes would have worked if you built inject-prisms first but that is not ideal.

[SentryMan]

I just put hickory on the generator mvn compiler plugin itself and it works, no extra prism module required.

[agentgt]

The issue is you can’t have a

requires hickory

it is not a real module.

[agentgt]

The easiest thing to do is have the generator not have a module-info

Then shade in the prism module.

Then use moditect to generate a module-info for the generator.

[SentryMan]

So I was looking at this SO thread and I understand the problem with regular libs having a conflict of required modules. In those cases though, most people expect to use the library classes in their application/library code directly.

As I understand it, the only situation I'm seeing where this would become a problem for a pure annotation processor is when two conditions are met.

1. somebody explicitly requires io.avaje.inject.generator for their own library/application (could you even do this if exports io.avaje.inject.generator is not defined? Also all of the classes are final and none of the classes are public except the processor so why would you?)
2. A new library that also happens to be called hickory comes out and also doesn't name its module. (Hickory has remained the same since 2010, a newer library doesn't have an excuse not to define modules)

Please inform me if I have missed something, I haven't interacted with Java modules all that much so this is pretty interesting.

[agentgt]

somebody explicitly requires io.avaje.inject.generator for their own library/application (could you even do this if exports io.avaje.inject.generator is not defined? Also all of the classes are final and none of the classes are public except the processor so why would you?)

Correct for now. In current practice no one should every require the generator. I actually talk about that in my libraries javadoc: https://jstach.io/jstachio/#maven_classpath

It also works because the current annotation processor tools in build system do not use the modulepath

Cut if you do not want to follow link:

This works because maven will jstachio-apt on the classpath instead of the module path during compilation provided you DO NOT put requires io.jstach.apt in your module-info which you should never do in general.

However if they fix: https://issues.apache.org/jira/browse/MCOMPILER-412

Then you are fucked if someone runs it from the module-path. The generator will not be considered a true module and will try to load hickory. So yeah its a bad idea.

The ideal solution for ease of IDE usage (btw that non standard annotation plugin.. I always find it problematic with eclipse) and to avoid compiler warnings is to follow what I did in my library and mostly what I have already done in the branch with the exception of shading.

https://github.com/jstachio/jstachio/blob/main/compiler/apt/pom.xml

1. Have the prisms in a separate module. This is done in my branch. module-info is mostly optional here provided you do next step
2. remove module-info from inject-generator
3. shade prisms module (I mean maven module) into inject-generator
4. use plugin to generate module-info.java for inject-generator

The other reason to separate out the prisms into their own module is that it is pretty damn easy to accidentally import something from avaje-inject annotation module by accident.

You might be able to keep a fake module-info in inject-generator but you still need to replace it to remove the prisms library reference. Again I use moditect for that.

I helped @jknack modularize jooby and he has a nice little hack to copy the real module-info and remove things you do not want (I tagged him for credit on the awesome idea I wish I had come up with). See the ant plugin where he removes shaded references:

jooby-project/jooby@a19d316

Anyway I can add the shading stuff later today if you like. Its usually good practice to shade all deps (well ignoring the annotations as that won't work) for annotation processors. MapStruct and I think Immutables does it (as well as of course Jooby).

[SentryMan]

if they fix MCOMPILER-412

They certainly aren't rushing.

The generator will not be considered a true module and will try to load hickory.

See my question is whether loading hickory will actually become a problem. Are there really so many libraries called hickory defaulting to automatic modules? hickory itself is unlikely to change.

shading stuff

would like to see that, we'll probably need to do it for avaje http

[agentgt]

See my question is whether loading hickory will actually become a problem. Are there really so many libraries called hickory defaulting to automatic modules? hickory itself is unlikely to change.

Yes because people do use other build tools that use the module path for annotation processing. There is no guarantee the jar will be called "hickory".

There is a little known fact that you can make maven do the name of the jar as: groupId-artifactId to avoid conflicts (that would obviously make hickory not called hickory).

The name of the jar is not guaranteed. In fact in the olden days when you had assholes who would name jar artifacts like "core" I had a conflict with packaging and the assholes response was to configure maven to do the un-canonical above but kind of supported maven feature (as you can see I'm still salty about it).

Regardless it is the right thing to do just like naming artifacts prefixed with the project is the right thing to do.

And if you look at the bug at the bottom someone even found a workaround to make the annotation processor in maven aka maven compiler bug use the modulepath.

I'm having a feeling the hesitation is the work needed to integrate my changes with yours? Or is it the fake module called prisms? There is tricks to make that not even get deployed to sonatype if you are worried about it.

I guess what is the hesistation?

[SentryMan]

I guess what is the hesistation?

It's my sense of humor I suppose. Aside from hickory being easy to use, I just think it would be hilarious to have an annotation processor itself have an annotation processor directly on the same module. So I'm basically trying my best to see if we can get it into only the inject generator module. If it can't be done and we need a separate module it is what it is I guess.

[agentgt]

Also the constants of class name (Which I'm remiss that hickory doesn't put the damn classname as constant) can be generated in the prisms project:

See like this:

https://github.com/jstachio/jstachio/blob/main/compiler/jstachio-prisms/src/test/java/io/jstach/apt/prism/PrismsTest.java

Which will generate the following... well it doesn't but checks if it is equal and then you just copy and paste the class from the output of the test:

https://github.com/jstachio/jstachio/blob/main/compiler/jstachio-prisms/src/main/java/io/jstach/apt/prism/Prisms.java

I admit that isn't that strong of a reason but still having the extra prisms project is useful and we can call inject-generator-support if you prefer.

Aside from hickory being easy to use, I just think it would be hilarious to have an annotation processor itself have an annotation processor directly on the same module. So I'm basically trying my best to see if we can get it into only the inject generator module.

A lot of things are possible. It is probably possible but it will get nasty with lots of IDE breaking hacks. For example you already had to use an un-canonical third party annotation processor maven plugin and even then you still have to go cleanup the requires hickory reference.

Ignoring the whole modules for a second. Hickory won't run on Maven's annotationProcessorPaths or the non annotationProcessorPaths way because it should be an optional dependency (and not just provided). I'm not sure what happens with other build tools and that is why the safest thing is to make annotation processing jars have almost zero deps. You avoid a lot of issues that way.

Anyway we should shade the prism module though. Let know if you want me to do it.

[SentryMan]

already had to use an un-canonical third party annotation processor maven plugin and even then you still have to go cleanup the requires hickory reference.

I'm not using that dodgy plugin anymore, it works with the compiler plugin.

Hickory won't run on Maven's annotationProcessorPaths or the non annotationProcessorPaths way because it should be an optional dependency (

?? It works for me even though it's both optional and provided

Anyway we should shade the prism module though. Let know if you want me to do it.

Yeah sure, can you base it off my current branch though? or at least copy the External Provider stuff I added to make the service loading still work on non-modular projects.

[agentgt]

?? It works for me even though it's both optional and provided

Yeah I suppose it will work but there was some reason that I can't recall had issues with JDK 17. Anyway the major reason is to prevent someone from accidentally importing avaje-inject annotations stuff in the inject-generator.

Yeah sure, can you base it off my current branch though? or at least copy the External Provider stuff I added to make the service loading still work on non-modular projects.

Sure I'll pull your branch. Just give me some links to the ExternalProvider stuff and which branch you want me to pull.

[SentryMan]

this guy is the provider code, the branch is called prism

[agentgt]

After this we should talk about forking hickory and making avaje-prisms library to placate Rob's concerns.

What I will do is do it first fork it for my project and then you can just fork that and convince Rob to publish it to maven central.

The original code is BSD license so in theory it is compatible with apache.

EDIT its fun because the code doesn't exist on any source code repo anymore.. so yeah Rob has a point of concern :)

[SentryMan]

https://github.com/vietj/hickory this is one fork

[agentgt]

You can look at and copy that version minus pom as that was their addition (which would mean a copyright for notice on our pom) file but don't "github fork" it otherwise the attribution goes to the wrong author.

[SentryMan]

Made a fork, https://github.com/SentryMan/avaje-prisms. It's kinda wild how a java 6 library works so well even to this day.

[rbygrave]

I have created an empty repo - https://github.com/avaje/avaje-prisms

I didn't know what license to put on the repo so there isn't a LICENSE file there yet.

[rbygrave]

This is resolved by the PR - #277 ... which I squashed and merged via 064d8be

This is now in master and in the javax branch

[rbygrave]

So a massive thanks to both of you !! I think we've ended up with a really good result - love it !!

Cheers, Rob.

[rbygrave]

Unless I've missed something we can close this issue as resolved by PR #277 via 064d8be

[agentgt]

@SentryMan I ran out of time yesterday as I had some other obligations. Do you still need me to do the other module shade work or have you already done it?

[SentryMan]

Do we still need shading? Now that we have forked hickory and deployed with a set module name as io.avaje.prisms it's a real module now right?

[SentryMan]

@agentgt let me know if you want to add a feature to the prisms repo. (or if you are so inclined, feel free to raise a pr).

[agentgt]

Do we still need shading? Now that we have forked hickory and deployed with a set module name as io.avaje.prisms it's a real module now right?

No you do not for the most part for now. The only situation where it is problematic is if you were using avaje-prisms and something else that depends on it but that is an unlikely scenario.

The other issue is now possibly two annotation processors might run. I'm not sure on this one. That is avaje-inject and avaje-prisms.

@agentgt let me know if you want to add a feature to the prisms repo. (or if you are so inclined, feel free to raise a pr).

The only issue is ironically and this is super meta recursive irony but if the annotations are grouped with the processor then you have to put it in <annotationProcessorPaths> which is the case with prisms.

I was going to separate the annotations from the processor given both the above and the aforementioned possibility of two processors running but for now that is a minor inconvenience especially if I'm wrong about both processors running.

If you want to see if it runs just make a no arg constructor on the prisms and System.out some message.

[agentgt]

@agentgt let me know if you want to add a feature to the prisms repo. (or if you are so inclined, feel free to raise a pr).

An obvious feature is to put the Annotation class name as a public static final String literal.

[SentryMan]

The only issue is ironically and this is super meta recursive irony but if the annotations are grouped with the processor then you have to put it in which is the case with prisms.

If you want it to work on your annotation processor without having a separate mvn module, instead of doing -proc:none you just need to add

 
          <annotationProcessorPaths>
            <path>
              <groupId>io.avaje</groupId>
              <artifactId>avaje-prisms</artifactId>
              <version>1.1</version>
            </path>
          </annotationProcessorPaths>

The height of comedy.

The other issue is now possibly two annotation processors might run. I'm not sure on this one. That is avaje-inject and avaje-prisms.

I've tested with modules/non-modular and annotationPaths and it works as expected, only the inject-generator runs.

An obvious feature is to put the Annotation class name as a public static final String literal.

ok, is this to your liking? avaje/avaje-prisms#4

[agentgt]

Yes and here is my fork of hickory. I started it before @rbygrave did the fork but I did copy that shitty quote of mine on the readme and some of the How to use:

https://github.com/jstachio/pistachio

ok, is this to your liking? avaje/avaje-prisms#4

Yes.. and I will add similar feature.

[rbygrave]

before @rbygrave did the fork

I can't take any credit here. @SentryMan did all the work making the fork and then applying it to the AP's. I've been the glorified second set of eyes and touch up guy.

I've tested with modules/non-modular and annotationPaths and it works as expected, only the inject-generator runs.

Cool. As I see it, it wouldn't matter if in some future setup the avaje-prisms AP actually ran as well as it would have nothing to process / there would be no code compiled with the prisms annotations on it in this case.

[agentgt]

It's good that we now have two modernized hickories.

Mine will work if someone wants to run the annotation processoron the modulepath.

And avaje's fork will work on Java 8 and above (mine is compiled to 17).

[rbygrave]

And avaje's fork will work on Java 8 and above

Not anymore in that avaje-prisms is being bumped to Java 11 (as we are bumping avaje-jsonb to 11 and so the need for 8 is being removed). So avaje-prisms version 1.2 is going full module-info and using Java 11.