Fix missing "Go to Symbol" results in Java projects

[eirikbakke]

For the past years, the "Go to Symbol" feature has appeared broken for me; searching for the name of a class that exists would frequently turn up no results. Today I managed to reproduce the problem. I put Claude Opus 4.6 on the task of investigating the issue, and had it produce both a fix and a regression test.

The problem was that ".sig" files were written to the cache using a .class format with various NetBeans enhancements, but then loaded again in a way that failed when these enhancements were encountered. Only classes with certain contents would be affected, though here is a rather trivial example class that couldn't be found with "Go to Symbol" prior to the fix in this commit:

public class DifficultClass19 {
  private static final String SOMECONST_STR = "hello";
  public void someMethod() { }
}

(If you try searching for this class in Go to Symbol, you may still get one hit in lowercase "difficultclass19". That's from the text-based Lucene index, which is used as a fallback when no Java symbols appear.)

The fix is in AsyncJavaSymbolDescriptor. I have confirmed manually that the new JavaSymbolProviderTest fails without the fix in AsyncJavaSymbolDescriptor, and passes after the fix has been applied.

An unrelated potential race condition in Go to Symbol's ContentProviderImpl is also fixed.

The commit/PR description above was written by me, Eirik. The code in this commit is by Claude Opus 4.6, written under my supervision. I am including Claude's summary of the investigation and fix below, as it is quite informative.

I'm putting a "do not merge" label on this until I've had more human feedback on this and tested the patch in my working IDE for a while.

Claude's Summary (AI-generated, though useful)

Fix Go to Symbol silently dropping Java results.

AsyncJavaSymbolDescriptor.resolve() is meant to enrich Lucene-indexed Java symbol hits with real javac TypeElements. Since 2015 it has silently been returning an empty collection for every Java type whose per-root .sig file used any NetBeans-specific class-file encoding, which made Go to Symbol effectively broken for Java in many real projects: Lucene-backed entries flashed briefly in the list (lowercased, no icon) and were then deleted by Models.MutableListModelImpl#descriptorChanged, which treats an empty resolve() result as "delete this row". Clicking a surviving entry also did nothing, because the old open() went through the same empty resolve().

Root cause: resolve() built its JavacTask by calling JavacTool.create().getTask(...) directly, which does not pre-register any of NetBeans' javac context enhancers. NetBeans .sig files (the per-root cached form of indexed Java classes, written by NBClassWriter) use a relaxed/extended class-file format that only NBClassReader knows how to parse correctly -- stock javac's ClassReader rejects them with BadClassFile, which propagates as CompletionFailure. ElementUtils.getTypeElementByBinaryName silently catches the CompletionFailure and returns null, so resolve() falls through to its empty-list return path. The exact rejection message varies by which NB-specific feature the .sig used; for one observed real-world case the .sig contained a ConstantValue attribute on a non-static-final String field, and stock javac rejected it with "variable of type 'java.lang.String' cannot have a constant value, but has one specified". The boot classpath, java.lang.Object loadability, and supertype walking are not involved -- the old code can perfectly well find Object via the running JDK's auto-discovered system modules; it just can't read the per-root .sig file.

History (verified against the pre-donation NetBeans repo):

• 2015-05-18 (pre-Apache commit jtulach/netbeans-releases@0f2c3e3d) AsyncJavaSymbolDescriptor.java introduced. resolve() goes through JavaSource.create(cpInfo).runUserActionTask(...), which routes via JavacParser and pre-registers the full NB javac context (NBClassReader, NBAttr, NBEnter, NBMemberEnter, NBResolve, NBClassFinder, NBJavaCompiler, NBClassWriter, etc.). Correct.

• 2015-05-19 (pre-Apache commit jtulach/netbeans-releases@445add56) "Speed up of javac resolution". Replaces the JavaSource/CompilationController call with a hand-rolled JavacTool.create().getTask(..., new RootChange(), ...) to skip the JavaSource setup overhead. NB context enhancer pre-registration is lost in the process; the resulting javac task uses stock ClassReader. This is the root-cause regression.

• 2015-07-28 (pre-Apache commit jtulach/netbeans-releases@6b7a248a) "#253081:NullPointerException at AsyncJavaSymbolDescriptor.resolve". An NPE in resolve() is reported in the wild only ~10 weeks after the 2015-05-19 regression -- consistent with the new code path being immediately broken on at least some configurations, although the exact trigger is not derivable from the diff alone. The fix wraps the (already-broken) reflective ClassReader.packages/classes cleanup hack so it returns Collections.emptyMap() instead of null, hiding the NPE without addressing why the reflective cleanup failed in the first place.

• 2016-12-12 (pre-Apache commit jtulach/netbeans-releases@499db956) "Fixed Go To Symbol does not work with new javac". The commit message itself confirms that by late 2016 Go to Symbol was known to be broken with the JDK 9 javac. The attempted fix only switches the (still-broken) reflective hack from the now-non-existent ClassReader.packages/classes fields to a Symtab parameter, but still does ClassReader.class.getDeclaredField(...), so on JDK 9+ it remains a silent no-op. The underlying defect -- using stock javac with no NB context enhancers -- is not addressed, and Go to Symbol stays broken. This is the strongest direct evidence in the history that the bug has been live and acknowledged since at least 2016.

The defect has been in place for just under eleven years.

The fix replaces the whole hand-rolled construction -- including the long-broken reflective ClassReader.packages/classes cleanup hack (a silent no-op since those fields moved to Symtab on JDK 9, and the origin of the one-time WARNING "packages"/"classes" logged from AsyncJavaSymbolDescriptor at every JDK 9+ startup) -- with a single JavacParser.createJavacTask(ClasspathInfo.create(root), ...) call, matching the idiom JavaBinaryIndexer and ModuleNames.parseModuleName already use. JavacParser.createJavacTask routes through the NB context enhancer pre-registration path, so the resulting javac task uses NBClassReader and friends and can read the per-root .sig files. About 150 lines of dead code and the startup WARNINGs go away. See the comments in the file for the defense-in-depth fallbacks retained in resolve() and open().

A new regression test, JavaSymbolProviderTest.testAsyncDescriptorResolvesForIndexedType, indexes a single-class source root whose source is intentionally rich (a lambda, generics, a static-final String constant, and a separate annotation type) so that the .sig file the indexer writes is empirically rejected by stock javac's ClassReader -- a trivial empty class produces a .sig vanilla javac reads without trouble, so the richer source is required. The test then asks JavaSymbolProvider to look up the class, triggers AsyncJavaSymbolDescriptor's async resolve() by calling getSymbolName() on the returned descriptor, and asserts that the resulting descriptor-change event carries a non-empty, enriched replacement that is not the original AsyncJavaSymbolDescriptor instance. It passes against the fix and fails against the pre-fix code path (verified end-to-end by reverting AsyncJavaSymbolDescriptor.java only and re-running the test). The exact attribute in the test class's .sig that trips stock ClassReader has not been pinned down -- it is enough to know that the differential exists and is reliably reproduced.

Also fixes a small, unrelated race in ContentProviderImpl.Worker: model.remove and model.add, previously issued as two separate background-thread invokeInEDT calls, are now performed inside the existing invokeLater runnable and guarded by !isCanceled, so they happen atomically within one EDT turn.