Skip to content

Lombok & VSCode - #3082

Open
rnc wants to merge 2 commits into
diffplug:mainfrom
rnc:ISSUE2795-2
Open

rnc wants to merge 2 commits into
diffplug:mainfrom
rnc:ISSUE2795-2

Conversation

@rnc

@rnc rnc commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Please DO NOT FORCE PUSH. Don't worry about messy history, it's easier to do code review if we can tell what happened after the review, and force pushing breaks that.

Please make sure that your PR allows edits from maintainers. Sometimes it's faster for us to just fix something than it is to describe how to fix it.

Allow edits from maintainers

After creating the PR, please add a commit that adds a bullet-point under the [Unreleased] section of CHANGES.md, plugin-gradle/CHANGES.md, and plugin-maven/CHANGES.md which includes:

  • a summary of the change
  • either
    • a link to the issue you are resolving (for small changes)
    • a link to the PR you just created (for big changes likely to have discussion)

If your change only affects a build plugin, and not the lib, then you only need to update the plugin-foo/CHANGES.md for that plugin.

If your change affects lib in an end-user-visible way (fixing a bug, updating a version) then you need to update CHANGES.md for both the lib and all build plugins. Users of a build plugin shouldn't have to refer to lib to see changes that affect them.

This makes it easier for the maintainers to quickly release your changes :)


@nedtwigg Unfortunately after trying on other repositories I've found further issues with Lombok & VSCode (when working with spotless) after #3038.

I am not sure whether you will accept this as it adds a dependency - but only for compileOnly scope, not runtime.

Summary of changes:

While 3038 started the foundational changes, there were still two categories of failures remaining:

1. NoSuchMethodError on PatchFixesHider$PatchFixes methods with concrete ECJ types

Lombok transplants call sites into ECJ bytecode with exact descriptors referencing concrete ECJ types — e.g. invokestatic PatchFixesHider$PatchFixes.isGenerated(Lorg/eclipse/jdt/internal/compiler/ast/ASTNode;)Z. PR #3038's stub declared isGenerated(Object), which the JVM rejects at link time with NoSuchMethodError because the descriptor doesn't match. Similarly, several other PatchFixes methods (getSourceEndFixed, getRealNodeSource, isBlockedVisitorAndGenerated) referenced ECJ types that were missing, and LombokDeps had no method bodies at all.

2. NoClassDefFoundError for eclipse/agent/ portal classes

ECJ's patched methods reference PatchDiagnostics, PatchValEclipsePortal, PatchDelegatePortal, PatchFixesShadowLoaded, PatchJavadoc, and EclipseLoaderPatcherTransplants directly by method call. synthesiseEmptyClass can satisfy class loading but not method lookup — when ECJ calls PatchDiagnostics.setSourceRangeCheck(...) the JVM finds the synthesised empty class has no such method and throws NoSuchMethodError.


Changes in this PR

lib/build.gradle.kts (build file migrated to Kotlin DSL since #3038)

  • Add "lombokStubsCompileOnly"("org.eclipse.jdt:org.eclipse.jdt.core:3.46.0") — needed so PatchFixesHider.java can compile against the exact ECJ types whose descriptors lombok transplants into ECJ bytecode. This is compileOnly; it adds zero runtime dependency to spotless-lib.

lib/src/lombokStubs/java/lombok/launch/PatchFixesHider.java — rewritten

  • PatchFixes.isGenerated split into three overloads with concrete ECJ types: (org.eclipse.jdt.core.dom.ASTNode), (org.eclipse.jdt.internal.compiler.ast.ASTNode), (org.eclipse.jdt.core.IMember) — matching the exact invokestatic descriptors lombok transplants
  • PatchFixes.isBlockedVisitorAndGenerated updated to use concrete ASTNode/ASTVisitor types
  • PatchFixes.getSourceEndFixed and getRealNodeSource updated to use concrete ECJ ASTNode types
  • Added missing PatchFixes methods: fixRetrieveIdentifierEndPosition, fixRetrieveEllipsisStartPosition, fixRetrieveStartBlockPosition
  • LombokDeps body filled in: addLombokNotesToEclipseAboutDialog, runPostCompiler (×3 overloads)
  • Transform methods declared throws IOException to match transplanted descriptors

6 new stub source files for eclipse/agent/ portal classes — these need real method bodies (not synthesiseEmptyClass) because ECJ's patched bytecode calls their methods directly:

New file Key methods
PatchDiagnostics.java setSourceRangeCheck(Object,int,int) → false
PatchValEclipsePortal.java copyInitializationOf… (no-ops)
PatchDelegatePortal.java handleDelegateForType → false, addGeneratedDelegateMethods → []
PatchFixesShadowLoaded.java addLombokNotes… → original, runPostCompiler (×3)
PatchJavadoc.java getHTMLContentFromSource → original
EclipseLoaderPatcherTransplants.java overrideLoadDecide → false, overrideLoadResult → null

FeatureClassLoaderLombokStubsTest.java — expanded from 11 to 17 tests

  • patchFixesHider_patchFixesCanBeLoaded → replaced by patchFixesHider_patchFixes_isGeneratedOverloadsExist, which parses the compiled .class file's constant pool directly (hand-written JVMS §4.4 byte parser, no external libraries) to verify the three concrete ECJ method descriptors are present — without triggering reflective type resolution of ECJ classes absent from the test classpath
  • 6 new tests for the eclipse/agent/ stubs, each verifying the method exists, is static, and returns the correct sentinel value
  • unknownLombokClass_* tests updated to use EclipsePatcher (which genuinely has no stub) rather than PatchDiagnostics (which now has one)

@rnc
rnc marked this pull request as ready for review September 16, 2026 11:31
Comment thread lib/build.gradle.kts
"zjsonPatchCompileOnly"(libs.zjsonpatch)
// lombokStubs – needs ECJ types to match the exact method descriptors that lombok
// transplants into ECJ's ASTConverter, ASTNode, etc.
"lombokStubsCompileOnly"("org.eclipse.jdt:org.eclipse.jdt.core:3.46.0")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving forward, how would you like us to maintain this 3.46.0 version? Should we bump it to latest whenever a new version is released? Should we pin it here until/unless you open a PR that bumps it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants