fix: restore lombok.config resolution for the Lombok agent - #3858
Conversation
Lombok maps the workspace-relative file name reported by ECJ to an absolute file system location through ResourcesPlugin, in EclipseAST.getAbsoluteFileLocation0(). Lombok's classes are loaded by a ShadowClassLoader whose parent is the bundle class loader that first requested a lombok.* class, which is the bundle hosting ECJ. Since ECJ was split out into org.eclipse.jdt.core.compiler.batch, that bundle declares no Import-Package and no Require-Bundle, so org.eclipse.core.runtime.IPath cannot be loaded and the lookup fails with NoClassDefFoundError. Lombok then falls back to resolving the workspace relative name against the current working directory, which points at a file that does not exist, so FileSystemSourceCache.forUri gives up and every lombok.config key silently falls back to its default. Add a fragment of org.eclipse.jdt.core.compiler.batch that contributes Import-Package: org.eclipse.core.resources, org.eclipse.core.runtime to the host bundle, so the agent woven into ECJ can resolve lombok.config again. Fixes redhat-developer/vscode-java#4461 Fixes redhat-developer/vscode-java#4467 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Changyong Gong <chagon@microsoft.com>
45d1e6e to
20e1541
Compare
|
@datho7561 Can you take a look? That CI failure looks like a flaky failure. Can you retry it for me? |
|
Yeah it looks like a flakey test, I'll rerun it and try to get to reviewing this today. |
|
Reading through this issue, I think this should be fixed upstream in Lombok instead of doing hacks here to try and get it to work. That's what's been done in the past (See projectlombok/lombok#3347). If lombok wants to access org.eclipse.core.resources from ECJ, it should contribute the hacks necessary to get that working, and based off the previous fix it should be able to do that. |
|
See https://github.com/projectlombok/lombok/pull/3563/changes for reference maybe? That seems to be specifically targetting jdt.core for loading; we'd need to do org.eclipse.core.resources. |
|
You're right — this belongs upstream in Lombok. The existing fix from projectlombok/lombok#3563 reflects I'll close this PR and move the fix upstream. |
Problem
lombok.configis never applied by jdt.ls. None of the keys take effect —lombok.accessors.chain,lombok.equalsandhashcode.callsuper,lombok.copyableAnnotations, etc. all silently fall back to their defaults, while the same project compiles correctly withjavac/Maven.Reported downstream as redhat-developer/vscode-java#4461 and redhat-developer/vscode-java#4467.
Root cause
Lombok resolves which
lombok.configapplies to a source file inEclipseAST.getAbsoluteFileLocation0(). ECJ hands it a workspace-relative name such as/myproject/src/main/java/com/example/Main.java, which Lombok maps to a real disk location viaResourcesPlugin.getWorkspace().getRoot().getFile(new Path(path)).getLocationURI().Lombok's classes are loaded by a
ShadowClassLoaderwhose parent is the bundle class loader that first requested alombok.*class — the bundle hosting ECJ. Since ECJ was split out intoorg.eclipse.jdt.core.compiler.batch, that bundle's manifest declares onlyExport-Package: noImport-Package, noRequire-Bundle. Soorg.eclipse.core.runtime.IPathcannot be loaded, and the workspace-based resolution fails:Lombok catches that and falls back to
new File(fileName).getAbsoluteFile(), resolving the workspace-relative name against the process working directory. That path does not exist, soFileSystemSourceCache.forUrithrows and the resolver returns no configuration at all — every key gets its default.Change
Adds
org.eclipse.jdt.ls.compiler.batch.fragment, a fragment oforg.eclipse.jdt.core.compiler.batchcontributing:This restores class visibility for agents woven into ECJ without modifying the host bundle. The fragment carries no code — it exists purely for its manifest.
Registered in the root
pom.xml, both.productfiles,category.xml, and the three dev.launchconfigs. The test runtime picks it up viaextraRequirementsinorg.eclipse.jdt.ls.tests/pom.xml.Test
LombokConfigurationTestimports the newmaven/mavenlombokconfigfixture, whose rootlombok.configsetslombok.accessors.chain=true, and asserts the generated setter returns the declaring type rather thanvoid. It fails without the fragment:and passes with it. The test no-ops when
jdt.ls.lombok.disabledis set or when the Lombok agent isn't present, matching the existing Lombok tests.Note for upstream JDT
Any Eclipse-based product weaving an agent into ECJ hits this, not just jdt.ls — the Eclipse IDE included. A
DynamicImport-PackageorEclipse-BuddyPolicy: dependentonorg.eclipse.jdt.core.compiler.batchwould fix it at the source; happy to open an issue against eclipse.jdt.core if that's preferred over carrying the fragment here.