From 1cd2eac13777762702de52608129f9d1f0198ba7 Mon Sep 17 00:00:00 2001 From: runchen0919 Date: Wed, 20 May 2026 22:20:41 +0800 Subject: [PATCH] fix: guard JDTUtils.searchDecompiledSources NPE via OSGi WeavingHook JDT.LS has an unfixed bug (eclipse-jdtls#3083) where JDTUtils.searchDecompiledSources() throws NPE in the MethodInvocation branch due to a missing finder.initialize() call. This crashes "Find References" and "Go to Implementation" when classpath JARs lack source attachments. This fix registers an OSGi WeavingHook that intercepts class loading of JDT.LS handler classes and wraps call sites to searchDecompiledSources in try-catch(NPE), returning Collections.emptyList() on catch. The approach patches callers (not JDTUtils itself) because JDTUtils is loaded during JDT.LS initialization before our bundle starts, while handler classes are lazily loaded on first LSP request. Key design choices: - Pre-scan classes with bare ClassVisitor before COMPUTE_FRAMES to avoid ClassNotFoundException in OSGi for unrelated classes - SafeClassWriter falls back to java/lang/Object when type resolution fails across bundle boundaries - Match by owner+method name only (no descriptor) for version resilience - ASM 9.7 embedded as Private-Package via bnd Co-Authored-By: Claude Opus 4.6 --- bazel-jdt-bridge/java-bridge/bnd.bnd | 2 + bazel-jdt-bridge/java-bridge/pom.xml | 12 ++ .../java/com/bazel/jdt/BazelActivator.java | 10 + .../java/com/bazel/jdt/JDTUtilsPatcher.java | 144 +++++++++++++ .../com/bazel/jdt/JDTUtilsPatcherTest.java | 142 ++++++++++++ docs/weaving-hook-npe-fix.md | 202 ++++++++++++++++++ 6 files changed, 512 insertions(+) create mode 100644 bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/JDTUtilsPatcher.java create mode 100644 bazel-jdt-bridge/java-bridge/src/test/java/com/bazel/jdt/JDTUtilsPatcherTest.java create mode 100644 docs/weaving-hook-npe-fix.md diff --git a/bazel-jdt-bridge/java-bridge/bnd.bnd b/bazel-jdt-bridge/java-bridge/bnd.bnd index 2778bd6..33db071 100644 --- a/bazel-jdt-bridge/java-bridge/bnd.bnd +++ b/bazel-jdt-bridge/java-bridge/bnd.bnd @@ -12,8 +12,10 @@ Import-Package: \ org.eclipse.core.runtime.jobs, \ org.eclipse.jdt.launching, \ org.osgi.framework, \ + org.osgi.framework.hooks.weaving, \ * Export-Package: com.bazel.jdt +Private-Package: org.objectweb.asm,org.objectweb.asm.signature Bundle-NativeCode: \ native/linux-x86_64/libbazel_jdt_core.so; osname=Linux; processor=x86_64, \ native/linux-aarch64/libbazel_jdt_core.so; osname=Linux; processor=aarch64, \ diff --git a/bazel-jdt-bridge/java-bridge/pom.xml b/bazel-jdt-bridge/java-bridge/pom.xml index fcb8cdc..12b109e 100644 --- a/bazel-jdt-bridge/java-bridge/pom.xml +++ b/bazel-jdt-bridge/java-bridge/pom.xml @@ -50,6 +50,18 @@ 3.23.0 provided + + org.osgi + osgi.core + 8.0.0 + provided + + + org.ow2.asm + asm + 9.7 + compile + junit junit diff --git a/bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/BazelActivator.java b/bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/BazelActivator.java index fea0252..350a17a 100644 --- a/bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/BazelActivator.java +++ b/bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/BazelActivator.java @@ -19,6 +19,8 @@ import org.eclipse.core.runtime.Status; import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; +import org.osgi.framework.hooks.weaving.WeavingHook; public class BazelActivator implements BundleActivator { private static final ILog LOG = Platform.getLog(BazelActivator.class); @@ -26,12 +28,16 @@ public class BazelActivator implements BundleActivator { Pattern.compile(".+_[0-9a-f]{4,}$"); private IResourceChangeListener invisibleProjectListener; + private ServiceRegistration weavingHookRegistration; @Override public void start(BundleContext context) throws Exception { LOG.log(new Status(IStatus.INFO, "com.bazel.jdt", "Bazel JDT Bridge bundle starting")); + weavingHookRegistration = context.registerService( + WeavingHook.class, new JDTUtilsPatcher(), null); + invisibleProjectListener = this::checkForInvisibleProjects; ResourcesPlugin.getWorkspace().addResourceChangeListener( invisibleProjectListener, IResourceChangeEvent.POST_CHANGE); @@ -39,6 +45,10 @@ public void start(BundleContext context) throws Exception { @Override public void stop(BundleContext context) throws Exception { + if (weavingHookRegistration != null) { + weavingHookRegistration.unregister(); + weavingHookRegistration = null; + } if (invisibleProjectListener != null) { ResourcesPlugin.getWorkspace().removeResourceChangeListener(invisibleProjectListener); invisibleProjectListener = null; diff --git a/bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/JDTUtilsPatcher.java b/bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/JDTUtilsPatcher.java new file mode 100644 index 0000000..5c88d43 --- /dev/null +++ b/bazel-jdt-bridge/java-bridge/src/main/java/com/bazel/jdt/JDTUtilsPatcher.java @@ -0,0 +1,144 @@ +package com.bazel.jdt; + +import java.util.logging.Level; +import java.util.logging.Logger; + +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.osgi.framework.hooks.weaving.WeavingHook; +import org.osgi.framework.hooks.weaving.WovenClass; + +public class JDTUtilsPatcher implements WeavingHook, Opcodes { + + private static final Logger LOG = Logger.getLogger(JDTUtilsPatcher.class.getName()); + + static final String JDTUTILS_INTERNAL_NAME = "org/eclipse/jdt/ls/core/internal/JDTUtils"; + static final String TARGET_METHOD = "searchDecompiledSources"; + private static final String TARGET_BUNDLE = "org.eclipse.jdt.ls.core"; + private static final String NPE_INTERNAL_NAME = "java/lang/NullPointerException"; + private static final String COLLECTIONS_INTERNAL_NAME = "java/util/Collections"; + + @Override + public void weave(WovenClass wovenClass) { + String bundleName = wovenClass.getBundleWiring().getBundle().getSymbolicName(); + if (!TARGET_BUNDLE.equals(bundleName)) { + return; + } + + byte[] original = wovenClass.getBytes(); + if (!containsTargetCallSite(original)) { + return; + } + + try { + byte[] patched = patchCallerClass(original); + if (patched != null) { + wovenClass.setBytes(patched); + LOG.info("Patched " + wovenClass.getClassName() + + ": wrapped searchDecompiledSources call site with NPE guard"); + } + } catch (Exception e) { + LOG.log(Level.WARNING, + "Failed to patch " + wovenClass.getClassName() + ", leaving class unmodified", e); + } + } + + static boolean containsTargetCallSite(byte[] classBytes) { + boolean[] found = {false}; + ClassReader reader = new ClassReader(classBytes); + reader.accept(new ClassVisitor(ASM9) { + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, + String signature, String[] exceptions) { + if (found[0]) return null; + return new MethodVisitor(ASM9) { + @Override + public void visitMethodInsn(int opcode, String owner, String mName, + String mDescriptor, boolean isInterface) { + if (opcode == INVOKESTATIC + && JDTUTILS_INTERNAL_NAME.equals(owner) + && TARGET_METHOD.equals(mName)) { + found[0] = true; + } + } + }; + } + }, ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES); + return found[0]; + } + + byte[] patchCallerClass(byte[] classBytes) { + ClassReader reader = new ClassReader(classBytes); + ClassWriter writer = new SafeClassWriter(reader, ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + boolean[] patched = {false}; + + ClassVisitor visitor = new ClassVisitor(ASM9, writer) { + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, + String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); + return new CallSiteWrappingVisitor(mv, patched); + } + }; + + reader.accept(visitor, 0); + return patched[0] ? writer.toByteArray() : null; + } + + static class CallSiteWrappingVisitor extends MethodVisitor { + private final boolean[] patched; + + CallSiteWrappingVisitor(MethodVisitor mv, boolean[] patched) { + super(ASM9, mv); + this.patched = patched; + } + + @Override + public void visitMethodInsn(int opcode, String owner, String name, + String descriptor, boolean isInterface) { + if (opcode == INVOKESTATIC + && JDTUTILS_INTERNAL_NAME.equals(owner) + && TARGET_METHOD.equals(name)) { + + Label tryStart = new Label(); + Label tryEnd = new Label(); + Label catchHandler = new Label(); + Label afterCatch = new Label(); + + mv.visitTryCatchBlock(tryStart, tryEnd, catchHandler, NPE_INTERNAL_NAME); + mv.visitLabel(tryStart); + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + mv.visitLabel(tryEnd); + mv.visitJumpInsn(GOTO, afterCatch); + mv.visitLabel(catchHandler); + mv.visitInsn(POP); + mv.visitMethodInsn(INVOKESTATIC, COLLECTIONS_INTERNAL_NAME, + "emptyList", "()Ljava/util/List;", false); + mv.visitLabel(afterCatch); + + patched[0] = true; + return; + } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + } + } + + private static class SafeClassWriter extends ClassWriter { + SafeClassWriter(ClassReader classReader, int flags) { + super(classReader, flags); + } + + @Override + protected String getCommonSuperClass(String type1, String type2) { + try { + return super.getCommonSuperClass(type1, type2); + } catch (Exception e) { + return "java/lang/Object"; + } + } + } +} diff --git a/bazel-jdt-bridge/java-bridge/src/test/java/com/bazel/jdt/JDTUtilsPatcherTest.java b/bazel-jdt-bridge/java-bridge/src/test/java/com/bazel/jdt/JDTUtilsPatcherTest.java new file mode 100644 index 0000000..7af0ad8 --- /dev/null +++ b/bazel-jdt-bridge/java-bridge/src/test/java/com/bazel/jdt/JDTUtilsPatcherTest.java @@ -0,0 +1,142 @@ +package com.bazel.jdt; + +import static org.junit.Assert.*; + +import java.lang.reflect.Method; +import java.util.List; + +import org.junit.Test; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +public class JDTUtilsPatcherTest implements Opcodes { + + private static final String SIMPLE_DESC = "()Ljava/util/List;"; + + @Test + public void patchCallerClass_wrapsCallSite() { + byte[] caller = buildCallerClass(); + JDTUtilsPatcher patcher = new JDTUtilsPatcher(); + byte[] patched = patcher.patchCallerClass(caller); + + assertNotNull("should patch class that calls searchDecompiledSources", patched); + assertFalse("patched bytes should differ", + java.util.Arrays.equals(caller, patched)); + } + + @Test + public void patchCallerClass_skipsClassWithNoCallSite() { + byte[] unrelated = buildUnrelatedClass(); + JDTUtilsPatcher patcher = new JDTUtilsPatcher(); + byte[] patched = patcher.patchCallerClass(unrelated); + + assertNull("should return null for class without searchDecompiledSources call", patched); + } + + @Test + public void patchCallerClass_skipsWhenOwnerDiffers() { + byte[] wrongOwner = buildCallerWithDifferentOwner(); + JDTUtilsPatcher patcher = new JDTUtilsPatcher(); + byte[] patched = patcher.patchCallerClass(wrongOwner); + + assertNull("should not patch calls to other classes", patched); + } + + @Test + public void patchedCallSite_returnsEmptyListInsteadOfNPE() throws Exception { + byte[] fakeJDTUtils = buildFakeJDTUtils(); + byte[] caller = buildCallerClass(); + + JDTUtilsPatcher patcher = new JDTUtilsPatcher(); + byte[] patchedCaller = patcher.patchCallerClass(caller); + assertNotNull(patchedCaller); + + TestClassLoader loader = new TestClassLoader(); + loader.define("org.eclipse.jdt.ls.core.internal.JDTUtils", fakeJDTUtils); + Class callerClass = loader.define("TestCaller", patchedCaller); + + Method method = callerClass.getMethod("callSearch"); + Object result = method.invoke(null); + + assertNotNull("patched call site should return non-null", result); + assertTrue("should return a List", result instanceof List); + assertTrue("should return empty list", ((List) result).isEmpty()); + } + + private byte[] buildCallerClass() { + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + cw.visit(V17, ACC_PUBLIC, "TestCaller", null, "java/lang/Object", null); + + MethodVisitor mv = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, + "callSearch", SIMPLE_DESC, null, null); + mv.visitCode(); + mv.visitMethodInsn(INVOKESTATIC, JDTUtilsPatcher.JDTUTILS_INTERNAL_NAME, + JDTUtilsPatcher.TARGET_METHOD, SIMPLE_DESC, false); + mv.visitInsn(ARETURN); + mv.visitMaxs(1, 0); + mv.visitEnd(); + + cw.visitEnd(); + return cw.toByteArray(); + } + + private byte[] buildFakeJDTUtils() { + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + cw.visit(V17, ACC_PUBLIC, + JDTUtilsPatcher.JDTUTILS_INTERNAL_NAME, null, "java/lang/Object", null); + + MethodVisitor mv = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, + JDTUtilsPatcher.TARGET_METHOD, SIMPLE_DESC, null, null); + mv.visitCode(); + mv.visitTypeInsn(NEW, "java/lang/NullPointerException"); + mv.visitInsn(DUP); + mv.visitLdcInsn("occurrences is null"); + mv.visitMethodInsn(INVOKESPECIAL, "java/lang/NullPointerException", + "", "(Ljava/lang/String;)V", false); + mv.visitInsn(ATHROW); + mv.visitMaxs(3, 0); + mv.visitEnd(); + + cw.visitEnd(); + return cw.toByteArray(); + } + + private byte[] buildUnrelatedClass() { + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + cw.visit(V17, ACC_PUBLIC, "UnrelatedClass", null, "java/lang/Object", null); + + MethodVisitor mv = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, + "doSomething", "()V", null, null); + mv.visitCode(); + mv.visitInsn(RETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + + cw.visitEnd(); + return cw.toByteArray(); + } + + private byte[] buildCallerWithDifferentOwner() { + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + cw.visit(V17, ACC_PUBLIC, "WrongOwnerCaller", null, "java/lang/Object", null); + + MethodVisitor mv = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, + "callSearch", SIMPLE_DESC, null, null); + mv.visitCode(); + mv.visitMethodInsn(INVOKESTATIC, "com/other/Utils", + JDTUtilsPatcher.TARGET_METHOD, SIMPLE_DESC, false); + mv.visitInsn(ARETURN); + mv.visitMaxs(1, 0); + mv.visitEnd(); + + cw.visitEnd(); + return cw.toByteArray(); + } + + private static class TestClassLoader extends ClassLoader { + Class define(String name, byte[] bytes) { + return defineClass(name, bytes, 0, bytes.length); + } + } +} diff --git a/docs/weaving-hook-npe-fix.md b/docs/weaving-hook-npe-fix.md new file mode 100644 index 0000000..300f4bb --- /dev/null +++ b/docs/weaving-hook-npe-fix.md @@ -0,0 +1,202 @@ +# WeavingHook NPE Fix — JDTUtils.searchDecompiledSources + +> Implemented: 2026-05-20 + +--- + +## 1. Problem + +JDT.LS has an unfixed bug ([eclipse-jdtls#3083](https://github.com/eclipse-jdtls/eclipse.jdt.ls/issues/3083)) in `JDTUtils.searchDecompiledSources()`. The `MethodInvocation` branch is missing a `finder.initialize()` call: + +```java +// JDTUtils.java (JDT.LS 1.58.0), around line 1830 +} else if (node instanceof MethodInvocation mi) { + SimpleName name = mi.getName(); + // BUG: missing finder.initialize(unit, name); +} +OccurrenceLocation[] occurrences = finder.getOccurrences(); +for (OccurrenceLocation occurrence : occurrences) { // NPE: occurrences is null +``` + +This causes a `NullPointerException` that crashes "Find References" and "Go to Implementation" requests entirely when the search hits a classpath JAR without source attachment. + +**Affected callers:** +- `ReferencesHandler$1.acceptSearchMatch()` — "Find References" +- `NavigateToDefinitionHandler` — "Go to Definition" + +Both enter the `searchDecompiledSources` code path when `classFile.getSourceRange() == null` (no source attachment). + +## 2. Solution Overview + +We use the OSGi standard **WeavingHook** API to patch the bytecode of classes that **call** `searchDecompiledSources`, wrapping the specific call site in a `try-catch(NullPointerException)` that returns `Collections.emptyList()` on catch. + +``` +┌──────────── Before (crashes) ──────────────────────┐ +│ │ +│ ReferencesHandler$1.acceptSearchMatch() │ +│ │ │ +│ └─ JDTUtils.searchDecompiledSources(...) │ +│ └─ NPE thrown → entire request fails │ +│ │ +├──────────── After (graceful degradation) ───────────┤ +│ │ +│ ReferencesHandler$1.acceptSearchMatch() │ +│ │ │ +│ ├─ try { │ +│ │ JDTUtils.searchDecompiledSources(...) │ +│ │ } catch (NullPointerException) { │ +│ │ return Collections.emptyList(); │ +│ │ } │ +│ │ │ +│ └─ Request completes with partial results │ +│ │ +└─────────────────────────────────────────────────────┘ +``` + +## 3. Why Patch Callers, Not JDTUtils Itself + +The initial approach was to patch `JDTUtils.searchDecompiledSources()` directly — wrapping the entire method body in try-catch. This failed because of class loading timing: + +``` +Timeline: + 19:07:53 JavaLanguageServerPlugin started → JDTUtils class loaded + 19:07:58 Our bundle starts → WeavingHook registered (too late for JDTUtils) + 19:09:39 NPE fires in searchDecompiledSources (unpatched) +``` + +**WeavingHook only intercepts first-time class loading** (`defineClass`). Since `JDTUtils` is a core utility class loaded during JDT.LS initialization (before our bundle is even installed), the hook never fires for it. + +**Handler classes are lazily loaded** — `ReferencesHandler`, `NavigateToDefinitionHandler`, and their inner classes are only loaded when the user first triggers the corresponding LSP request, well after our WeavingHook is registered. + +## 4. Implementation Details + +### 4.1 Files Changed + +| File | Change | +|------|--------| +| `java-bridge/pom.xml` | Added `org.ow2.asm:asm:9.7` (compile) and `org.osgi:osgi.core:8.0.0` (provided) | +| `java-bridge/bnd.bnd` | Added `org.osgi.framework.hooks.weaving` to Import-Package, `Private-Package` for ASM embedding | +| `java-bridge/src/main/java/com/bazel/jdt/JDTUtilsPatcher.java` | **New** — WeavingHook implementation | +| `java-bridge/src/main/java/com/bazel/jdt/BazelActivator.java` | Register/unregister WeavingHook service | +| `java-bridge/src/test/java/com/bazel/jdt/JDTUtilsPatcherTest.java` | **New** — 4 unit tests | + +### 4.2 JDTUtilsPatcher Architecture + +``` +┌────────────────── weave(WovenClass) ──────────────────────┐ +│ │ +│ 1. Bundle filter │ +│ └─ wovenClass.bundle != "org.eclipse.jdt.ls.core"? │ +│ → skip (most classes never reach step 2) │ +│ │ +│ 2. Pre-scan (read-only, no COMPUTE_FRAMES) │ +│ └─ containsTargetCallSite(bytes): │ +│ ClassReader + bare MethodVisitor scans for │ +│ INVOKESTATIC JDTUtils.searchDecompiledSources │ +│ using SKIP_DEBUG | SKIP_FRAMES flags │ +│ → No call site found? skip │ +│ │ +│ 3. Patch (only reached for classes with the call site) │ +│ └─ patchCallerClass(bytes): │ +│ ClassReader → ClassVisitor → CallSiteWrappingVisitor │ +│ → SafeClassWriter (COMPUTE_FRAMES + COMPUTE_MAXS) │ +│ │ +│ CallSiteWrappingVisitor.visitMethodInsn(): │ +│ When seeing INVOKESTATIC to searchDecompiledSources│ +│ → Emit: try { originalCall } catch (NPE) { │ +│ return Collections.emptyList(); │ +│ } │ +│ │ +│ 4. Apply │ +│ └─ wovenClass.setBytes(patchedBytes) │ +│ │ +└────────────────────────────────────────────────────────────┘ +``` + +### 4.3 The Pre-scan Optimization + +Without pre-scanning, `patchCallerClass()` runs `ClassWriter(COMPUTE_FRAMES)` on every class from the JDT.LS bundle. `COMPUTE_FRAMES` calls `getCommonSuperClass()` internally, which uses `Class.forName()` — this fails in OSGi when the target class references types not visible to our bundle's classloader (e.g., `org.gradle.tooling.model.GradleProject`, `org.eclipse.lsp4j.CompletionItem`). + +The pre-scan uses a bare `ClassVisitor` with no `ClassWriter`, so no frame computation occurs. Only classes confirmed to contain the call site proceed to the full transformation. + +### 4.4 SafeClassWriter + +Even with pre-scanning, the classes that DO contain the call site may reference types outside our bundle's visibility. `SafeClassWriter` overrides `getCommonSuperClass()` to fall back to `"java/lang/Object"` when type resolution fails, instead of throwing `TypeNotPresentException`. + +### 4.5 BazelActivator Integration + +```java +// In start(): +weavingHookRegistration = context.registerService( + WeavingHook.class, new JDTUtilsPatcher(), null); + +// In stop(): +if (weavingHookRegistration != null) { + weavingHookRegistration.unregister(); + weavingHookRegistration = null; +} +``` + +The hook is registered during `BazelActivator.start()`, which executes during `BundleUtils.loadBundles()`. At this point, handler classes have not yet been loaded (they are loaded on first LSP request), so the WeavingHook fires in time. + +## 5. Degradation Strategy + +``` +┌────────────────── Degradation Layers ─────────────────────┐ +│ │ +│ Layer 1: WeavingHook registration │ +│ ├─ Success → call site patched when handler loads │ +│ └─ Failure → bundle still works, NPE persists (current │ +│ behavior, no regression) │ +│ │ +│ Layer 2: Pre-scan + patch │ +│ ├─ Call site found → patch applied │ +│ └─ Not found → JDT.LS version changed method name, │ +│ no modification (silent skip) │ +│ │ +│ Layer 3: Runtime catch │ +│ ├─ NPE thrown → caught, returns emptyList() │ +│ │ "Find References" shows partial results │ +│ └─ No NPE → method returns normally (no overhead) │ +│ │ +│ If upstream fixes the bug: patch becomes a no-op │ +│ (try-catch still safe, just never triggers) │ +│ │ +└────────────────────────────────────────────────────────────┘ +``` + +## 6. Version Compatibility + +The patch is designed to be robust across JDT.LS versions: + +- **Method matching**: Only matches by owner class internal name (`org/eclipse/jdt/ls/core/internal/JDTUtils`) and method name (`searchDecompiledSources`). Does **not** match by descriptor, so signature changes (new parameters, different return type) are tolerated. +- **Call site wrapping**: Wraps the specific `INVOKESTATIC` instruction, not the entire method. Works regardless of the caller's method structure. +- **Silent skip**: If `searchDecompiledSources` is renamed or removed in a future version, the pre-scan finds no call sites and the hook is a no-op. +- **Upstream fix**: If the NPE bug is fixed upstream, the try-catch still compiles and loads correctly — it simply never catches anything. + +## 7. Testing + +Four unit tests in `JDTUtilsPatcherTest.java`: + +| Test | Verifies | +|------|----------| +| `patchCallerClass_wrapsCallSite` | ASM transformation produces different bytecode for a class calling `searchDecompiledSources` | +| `patchCallerClass_skipsClassWithNoCallSite` | Returns null (no modification) for unrelated classes | +| `patchCallerClass_skipsWhenOwnerDiffers` | Does not patch calls to methods with the same name on different classes | +| `patchedCallSite_returnsEmptyListInsteadOfNPE` | End-to-end: loads a fake JDTUtils that throws NPE, patches the caller, invokes it, and verifies `Collections.emptyList()` is returned | + +Run tests: +```bash +cd java-bridge && mvn test -Dtest=JDTUtilsPatcherTest +``` + +## 8. Approaches Considered and Rejected + +| Approach | Why Rejected | +|----------|-------------| +| **Stub source JAR generation** | Persistent edge cases with inner classes, anonymous classes, synthetic classes. Reverted after multiple iterations. | +| **OSGi Fragment** | Equinox `ClasspathManager` searches host classpath entries before fragment entries — fragments cannot replace existing host classes. | +| **Patch JDTUtils directly** | `JDTUtils` is loaded during JDT.LS initialization, before our bundle starts. WeavingHook never fires for already-loaded classes. | +| **Equinox ClassLoaderHook** | Internal API, requires framework extension configuration (`hookconfigurators.properties`), cannot be registered from a normal bundle. | +| **Upstream PR** | Outside project control, uncertain timeline. | +| **Disable `includeDecompiledSources`** | Globally disables decompiled source browsing — too large a UX degradation. |