The Javac DOM parser does not include the generated Lombok code

[testforstephen]

The Javac DOM converter relies on the JCCompilationUnit produced by JavacTask.parse() job as the source AST, which only includes explicitly declared members in the source file. In contrast, Lombok methods are injected into the JCTree during the JavacTask.analyze() phase, which triggers annotation processing. So the AST generated by the DOM parser is an incomplete tree and omit the Lombok-generated methods.

[mickaelistria]

I think it would be fine that, when lombok is enabled, we call analyze() in parse before converting the DOM.

[mickaelistria]

https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/pull/771 might help. @testforstephen Can you please check?

[mickaelistria]

@testforstephen so can we close this one now?

[testforstephen]

https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/pull/771 does not fix it, the issue still exists.

[testforstephen]

https://github.com/user-attachments/assets/d95e6d04-c5b7-4af6-89b3-aba10ddc1478

If you trigger code completion on a Java file that defines the lombok annotation (ModelLombokValue), the completion works because the ECJ generates the AST correctly for the current file.

But if you work it with cross file case, the completion does not suggest the lombok methods (ModelLombokValue) from the referenced class (ModelJava).

[mickaelistria]

Note that lombok requires even more --add-opens just to not crash: --add-opens jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED. After that, I tried the following project:

WithLombok.java

package withLombok;
public class WithLombok {
    @lombok.Getter
    private int a;
}

and Other.java

package withLombok;
public class Other {
    void lol() {
        new WithLombok().getA();
    }
}

The IDE reports getA cannot be resolved. I then try to get it working with the javac CLI as we're basically mimicking it

$ javac -proc:full -cp /home/mistria/.m2/repository/org/projectlombok/lombok/1.18.32/lombok-1.18.32.jar -sourcepath . withLombok/Other.java
withLombok/Other.java:5: error: cannot find symbol
        new WithLombok().getA();
                        ^
  symbol:   method getA()
  location: class WithLombok
1 error

it seems like even with CLI, lombok is not applied to other classes in the source path (if I build the 2 classes simultaneously, it's fine, but just trying to build one with the other as source doesn't work). Is there a particular option of javac to cascade APT to other source files? If so, we'll need to configure it in the IDE too.

[mickaelistria]

Ok, it seems like -proc:only is a good fit for JavacCompilationUnitResolver. I'm merging a patch to provide that. Would that work for build too?

[mickaelistria]

So I tried -proc:only which seems like a good fit for JavacCompilationUnitResolver. I'm merging a patch to enable it. Do you think we'd need it for the JavacCompiler too?

[testforstephen]

I can share more context on how ECJ supports Lombok annotation. The Lombok team implements a java agent for ECJ, that is kind of hack with the ECJ. The agent will dynamically replace some ECJ internal classes with Lombok implemented version in the JVM loading. This allows the Lombok custom logic can be invoked during parsing AST with ECJ. That's why we need to add a vmArgs (e.g. -javaagent:/to/the/path/of/lombok.jar) to the startup CLI of JDT language server to get support for Lombok with ECJ.

For Javac, I think we only need to add lombok.jar to the classpath or the annotation processor path, it will get the Lombok support automatically. This is the same way how the build tools support it.

[testforstephen]

For the issue above I observed, I only enabled Javac for basic operations and compilation, not for code completion. It still uses the ECJ to serve the completion feature. On debugging, I can see that the default CompletionEngine uses ECJ parser to parse current working file ModelJava, then resolve the referenced Lombok file (ModelLombokValue) to its SourceTypeBinding. The problem is the resolved SourceTypeBinding didn't contain the generated Lombok methods. But for the same project, if I don't enable any Javac features, the code completion can work well for the Lombok reference case.

[mickaelistria]

It seems like https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/commit/871e2a8eb93360d3471709f35363ce22ae5c04ba has improved (resolved?) this case then Can you please retry with latest head?

[testforstephen]

Nope. Still not working with the latest head.

Have a look at how ECJ resolves source binding from LookupEnvironment, it first retrieves the SourceTypeElementInfo from the Java model JavaElement, and then this SourceTypeElementInfo is converted into a CompilationUnitDeclaration to build the SourceTypeBindings.

In the case of code completion, although we use the default CompletionEngine based on the ECJ parser, the binding resolution still reuses source element information from the JavaElement models. Since the JavaElement is populated by the DOMToModelPopulator using Javac, it omits elements generated by Lombok. This omission is what causes the code completion to fail in cross-file scenarios.

[testforstephen]

It seems like 871e2a8 has improved (resolved?) this case then

I don't see how adding -proc:only takes any difference with this case. At another commit https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/commit/8ed2e07ceffea954c3d7721cbdf0044821fbfc2b, I have added lombok.jar to annotation processor path and leave the option as null (which means -proc:all), it has enabled lombok annotation processing in DOM resolver.

[mickaelistria]

https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/pull/831 could be the right fix.

[mickaelistria]

Please try again a next build as I merged #831

[testforstephen]

A few NPEs. You can use the sample project https://github.com/che-samples/lombok-project-sample to reproduce it.

!ENTRY org.eclipse.jdt.core 4 0 2024-09-14 12:13:27.868
!MESSAGE Internal failure while parsing or converting AST for unit /outputCleaner/src/main/java/org/codesolt/model/ModelLombokValue.java

!ENTRY org.eclipse.jdt.core 4 0 2024-09-14 12:13:27.874
!MESSAGE startPos = -1 and length is 9.
This breaks the rule that length must be 0 if startPosition is negative. Affected Node:
class org.eclipse.jdt.core.dom.SimpleName: getLogger
!STACK 0
java.lang.IllegalArgumentException: startPos = -1 and length is 9.
This breaks the rule that length must be 0 if startPosition is negative. Affected Node:
class org.eclipse.jdt.core.dom.SimpleName: getLogger
    at lombok.eclipse.agent.PatchDiagnostics.setSourceRangeCheck(PatchDiagnostics.java:43)
    at org.eclipse.jdt.core.dom.ASTNode.setSourceRange(ASTNode.java)
    at org.eclipse.jdt.core.dom.JavacConverter.convertExpressionImpl(JavacConverter.java:1360)
    at org.eclipse.jdt.core.dom.JavacConverter.convertExpression(JavacConverter.java:1854)
    at org.eclipse.jdt.core.dom.JavacConverter.createVariableDeclarationFragment(JavacConverter.java:1115)
    at org.eclipse.jdt.core.dom.JavacConverter.convertFieldDeclaration(JavacConverter.java:1125)
    at org.eclipse.jdt.core.dom.JavacConverter.convertBodyDeclaration(JavacConverter.java:771)
    at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:617)
    at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:537)
    at org.eclipse.jdt.core.dom.JavacConverter.convertBodyDeclaration(JavacConverter.java:768)
    at org.eclipse.jdt.core.dom.JavacConverter.lambda$populateCompilationUnit$2(JavacConverter.java:188)
    at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215)
    at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
    at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
    at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
    at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:636)
    at org.eclipse.jdt.core.dom.JavacConverter.populateCompilationUnit(JavacConverter.java:190)
    at org.eclipse.jdt.core.dom.JavacCompilationUnitResolver.parse(JavacCompilationUnitResolver.java:638)
    at org.eclipse.jdt.core.dom.JavacCompilationUnitResolver.toCompilationUnit(JavacCompilationUnitResolver.java:459)
    at org.eclipse.jdt.core.dom.ASTParser.internalCreateASTCached(ASTParser.java:1263)
    at org.eclipse.jdt.core.dom.ASTParser.lambda$0(ASTParser.java:1142)
    at org.eclipse.jdt.internal.core.JavaModelManager.cacheZipFiles(JavaModelManager.java:5770)
    at org.eclipse.jdt.core.dom.ASTParser.internalCreateAST(ASTParser.java:1142)
    at org.eclipse.jdt.core.dom.ASTParser.createAST(ASTParser.java:882)
    at org.eclipse.jdt.internal.core.CompilationUnit.buildStructure(CompilationUnit.java:218)
    at org.eclipse.jdt.internal.core.Openable.generateInfos(Openable.java:245)
    at org.eclipse.jdt.internal.core.JavaElement.openWhenClosed(JavaElement.java:585)
    at org.eclipse.jdt.internal.core.BecomeWorkingCopyOperation.executeOperation(BecomeWorkingCopyOperation.java:43)
    at org.eclipse.jdt.internal.core.JavaModelOperation.run(JavaModelOperation.java:739)
    at org.eclipse.jdt.internal.core.JavaModelOperation.runOperation(JavaModelOperation.java:804)
    at org.eclipse.jdt.internal.core.CompilationUnit.becomeWorkingCopy(CompilationUnit.java:159)
    at org.eclipse.jdt.internal.core.CompilationUnit.becomeWorkingCopy(CompilationUnit.java:166)
    at org.eclipse.jdt.ls.core.internal.handlers.BaseDocumentLifeCycleHandler.handleOpen(BaseDocumentLifeCycleHandler.java:429)
    at org.eclipse.jdt.ls.core.internal.handlers.BaseDocumentLifeCycleHandler.didOpen(BaseDocumentLifeCycleHandler.java:348)
    at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.didOpen(JDTLanguageServer.java:875)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$recursiveFindRpcMethods$0(GenericEndpoint.java:65)
    at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.notify(GenericEndpoint.java:160)
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleNotification(RemoteEndpoint.java:231)
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:198)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:185)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:97)
    at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:114)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
    at java.base/java.lang.Thread.run(Thread.java:1575)

[mickaelistria]

I've reverted the patch about lombok because it its current state it was breaking many JDT-LS tests: https://ci.eclipse.org/ls/job/jdt-ls-javac/546/testReport/

[mickaelistria]

The reason for revert is that both JavacTask.analyze() and Lombok tend to add extra content in the dom, that doesn't really exist. Some of them need to be ignored (eg default empty constructor for Javac), some of them need to be shown in AST (eg Lombok stuff). The criterion I was using was the position, which is usually working well; but which seems to be incorrect for some cases highlighted in JDT-LS.