Fix/external symbols#114
Open
thgbrb wants to merge 2 commits into
Open
Conversation
Reference occurrences for symbols defined in external NuGet/BCL packages
were emitted without a matching SymbolInformation: external_symbols was
never populated and SymbolInformation was only emitted for in-source
definitions. As a result every cross-package reference dangled, which
scip lint reports as "no matching SymbolInformation in external symbols
or any document".
Collect every referenced external-package symbol while indexing and
declare it in Index.external_symbols with minimal hover documentation
(no relationships, to avoid recursing across the BCL type graph),
deduplicated against in-source definitions. Index-local ("nuget . .")
symbols are intentionally out of scope.
Add ExternalSymbolsTests as a mini scip-lint regression test asserting
every genuine-external occurrence resolves.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Records implement IEquatable<T> implicitly and framework base/interface hierarchies expand transitively to external types that never appear textually in the source, so those relationship targets never passed through VisitOccurrence and were never collected into Index.external_symbols. scip lint flagged them as "has a relationship to <symbol>, but couldn't find sourcegraph#2 in external symbols or some other document". Collect external relationship targets (BaseType chain, AllInterfaces, OverriddenMethod, InterfaceImplementations) via the same minimal SymbolInformation path used for reference occurrences (hover documentation only, no relationships, to avoid recursing across the BCL type graph). Add EveryExternalRelationshipTargetHasSymbolInformation regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement
external_symbolsto fix dangling cross-package referencesSummary
This PR populates
Index.external_symbolswithSymbolInformationfor symbols defined in external NuGet/BCL packages that are referenced by indexed source files. Before this change, the field was never written to, which left every cross-package reference orphaned and unusable for code navigation.Fixes #62. Addresses the user-visible complaint behind #3.
Measured on a real-world .NET 9 + EF Core 9 codebase (1,263 source documents, 361,764 occurrences):
The remaining counts are edge cases involving deeper type-graph paths (transitive ancestors of generic constructions). They could be addressed by walking the full transitive hierarchy when collecting from the relationship path, at the cost of larger indexes; this PR keeps the collection one level deep.
Relationship to existing issues
#62 "C# indexing doesn't work properly". Same bug. The maintainer's reproduction on
dotnet/eshopshows references to global symbols missing because theirSymbolInformationwas never written. This PR resolves it.#3 "Missing System.Runtime core assembly". The user-visible complaint (BCL navigation broken) is the same and is resolved by this PR. The specific mechanical detail in that issue (the literal
<Missing Core Assembly>placeholder) no longer exists in the current code, so #3 partially reflects an older resolution path; the current scip-dotnet resolvesSystem.Runtimeidentity correctly and only the emission was missing.Limitations
nuget . .errors (mostly namespaces) are intentionally untouched and need a separate design decision.net8.0/net9.0was not performed in this PR; the snapshot suite runs under whichever frameworkdotnet testselects on the build machine (here,net10.0). The fix is framework-agnostic.