eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
157 stars 124 forks source link

Eclipse impedes developing annotation processors by refusing to use them in the workspace #1384

Open garretwilson opened 12 months ago

garretwilson commented 12 months ago

This ticket was originally opened as eclipse-m2e/m2e-core#1550, but I was told it belongs here.

Summary: Projects imported into Eclipse using m2e that use an annotation processor, which is itself defined in other project open in the same workspace, do not invoke the annotation processor when compiling unless the project defining the annotation processor is first closed in the workspace.

I'm using a standard Eclipse EE 2023-09 installation. I have a Maven project that defines an annotation processor, which I have imported into Eclipse using m2e. The processor processes the @Foo annotation. The project looks like this:

The processor so far is pretty bare-bones:

@SupportedAnnotationTypes("com.example.Foo")
public class FooProcessor extends AbstractProcessor {

  @Override
  public SourceVersion getSupportedSourceVersion() {
    return SourceVersion.latestSupported();
  }

  @Override
  public boolean process(final Set<? extends TypeElement> annotations, final RoundEnvironment roundEnv) {
    java.awt.Toolkit.getDefaultToolkit().beep();
    System.out.println("***** Annotation processing!");
    processingEnv.getMessager().printMessage(NOTE, "Processing round; processing %d annotations ...".formatted(annotations.size()));
    return true;
  }

}

Now if I run mvn clean compile on foo-aggregate, when maven compiles the test-project subproject I hear a beep and Maven prints:

***** Annotation processing!
***** Annotation processing!
[INFO] Processing round; processing 1 annotations ...
[INFO] Processing round; processing 0 annotations ...

This is as I would expect.

In Eclipse, I have found that there is a Preferences > Maven > Annotation Processing > Select Annotation Processing Mode setting which by default is set to "Do not automatically configure/execute annotation processing from pom.xml". I have changed this to "Automatically configure JDT API …". So we should be all set.

However when I import the foo-aggregate project into Eclipse using m2e and compile the test-project subproject, I see no annotation processing output in the "Error Log" view tab; nor do I hear a beep.

In fact someone noted this six years ago in an answer on Stack Overflow:

… If the project containing the actual annotation processor (so NOT the "client") is in the same workspace as the "client" project, then m2e-apt will simply ignore your annotation processor; I don't know why. Closing your annotation processor project would be enough in this case (you don't have to delete it from workspace). …

So it seems impossible to have a subproject that uses an annotation processing defined in the same multimodule aggregate project that defines the annotation processor in a separate subproject.

But it's worse than that—it's not confined to the same aggregate project. If I create a completely separate bar-project that uses the @Foo annotation and lists foo-processor-provider as a dependency in the POM, the same bug appears. I imported bar-project into the same workspace in Eclipse, and there are still no messages or beep when compiling bar-project.

So I closed the foo-aggregate project (making sure I had used mvn install from the command line so that the annotation processor would still appear in Maven's local repository), and sure enough—I get two lines of in the Error Log view tab, and I hear a beep! (Note that the lines in the Error Log appear in reverse order in the tab because that's how the tab lists messages.)

Processing round; processing 0 annotations ... Processing round; processing 1 annotations ...

The behavior gets even more complicated than that:

Why must I close the project in which the annotation processor is defined in order to use it in another project in the same workspace in Eclipse? (This will significantly impede my ability to develop an annotation processor using Eclipse.)

garretwilson commented 12 months ago

Someone referenced an old ticket, Bug 259230 - Annotation processors do not pick up the class path from the project, in which in the comments people were saying how bad this behavior is and asking for it to be reopened.

stephan-herrmann commented 11 months ago

From the old bug I gather, that this design is motivated by the following goal: Eclipse should not class-load any .class files in the workspace, as this will lock the files (possibly until Eclipse is shut down), and conflicts with compilation (of the processor project) trying to replace those locked files.

Is my understanding correct? Does that problem still exist?

If so, what would be a desirable strategy?

Note, that Eclipse invokes the compiler in-process, we never fork a new process for this.

garretwilson commented 11 months ago

I have little knowledge of the underlying mechanisms of Eclipse compiler locking and class loading. But I'm not understanding a comment you made:

… Eclipse should not class-load any .class files in the workspace …

On its face that confuses me, because obviously Eclipse loads .class files open in the workspace—otherwise Eclipse could never run unit tests the dependencies of which were open in the workspace and themselves being compiled by Eclipse.

Are you referring to the .class files of the compiled annotation processor? If so, how is opening those target/classes/*AnnotationProcessor.class files any different than opening /target/classes/*AnnotationProcessor.class from any other dependency open in the workspace in another project?

Or maybe you're referring to opening the .class files of the compiled versions of the new source files that the annotation processor generated? If so, I'm guessing those .class files end up in target/classes/*.class as well, so how would loading them be any different than loading other class files?

I must not be understanding what you're getting at.

In my mind, at the time project B is being compiled, the annotation processors in project A have already been compiled and are available in target/classes/*.class, so why can't project B use those .class file just like it would any other .class files, e.g. when running JUnit tests in the IDE?

stephan-herrmann commented 11 months ago

The difference is loading vs. classloading:

We have no problems directly reading any files, .class or other. Open the file, read the contents, close it again. Fine.

But we don't have full control over what happens when we let a ClassLoader read the file. And since JDT is supposed to invoke an annotation processor, we need to do the equivalent of Class.forName(<nameOfYourAnnotationProcessorClass>). Inside that call we have little to no control about file handling. I'm essentially guessing that this is the problem, but that's how I read the old bugzilla.

@jarthana could you add some detail about how (and possibly where) this class loading happens in JDT/APT?

jarthana commented 11 months ago

@jarthana could you add some detail about how (and possibly where) this class loading happens in JDT/APT?

I must admit, I never studied these in details. These were decided long before I took over and were never touched since. Having said that, I think the class loading happens in AnnotationProcessorFactoryLoader.loadFactories().

stephan-herrmann commented 11 months ago

I think the class loading happens in AnnotationProcessorFactoryLoader.loadFactories().

so we are clearly using dedicated URLClassLoaders.

A quick web search shows:

"Recently" (2017) calls to close() where added in JDT via bug 514121.

Given that a workspace build locks the workspace anyway (all of it or per project?), it shouldn't hurt that during the build any referenced class files of the annotation processor cannot be modified. Due to the close() call they should be released at the end of the build.

To me this looks doable, where probably the first step would be to implement a new ProjectFactoryContainer extends FactoryContainer that is able to provide the URLs of all locations that need to be passed to a new URLClassLoader, where "all locations" obviously needs to cover all dependencies of that project, too.

There may be more to it regarding the configuration to use such a annotation-processor-in-a-project (JDT/Core and JDT/UI), but no rocket science if you ask me.

stephan-herrmann commented 11 months ago

Someone referenced an old ticket, Bug 259230 - Annotation processors do not pick up the class path from the project, in which in the comments people were saying how bad this behavior is and asking for it to be reopened.

The quoted decision in Bug 259230 was made in 2008. Given that URLClassLoader.close() was introduced in Java 7 (2011) we may conclude that the decision was well justified at the time it was made, but nowadays we have the option to change the design :)