From 487ac72ebbae18241c1c4d287ef3a39f45760b6d Mon Sep 17 00:00:00 2001 From: Thiago Borba Date: Tue, 9 Jun 2026 15:26:50 -0300 Subject: [PATCH 1/2] fix: populate Index.external_symbols for cross-package references 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) --- ScipDotnet.Tests/ExternalSymbolsTests.cs | 105 +++++++++++++++++++++++ ScipDotnet/IndexCommandHandler.cs | 11 +++ ScipDotnet/ScipDocumentIndexer.cs | 58 ++++++++++++- ScipDotnet/ScipProjectIndexer.cs | 8 +- ScipDotnet/ScipSymbol.cs | 9 ++ 5 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 ScipDotnet.Tests/ExternalSymbolsTests.cs diff --git a/ScipDotnet.Tests/ExternalSymbolsTests.cs b/ScipDotnet.Tests/ExternalSymbolsTests.cs new file mode 100644 index 0000000..acdaad9 --- /dev/null +++ b/ScipDotnet.Tests/ExternalSymbolsTests.cs @@ -0,0 +1,105 @@ +using System.Diagnostics; +using Scip; +using Index = Scip.Index; + +namespace ScipDotnet.Tests; + +/// +/// Regression test for dangling cross-package occurrences. +/// +/// SCIP requires that every occurrence referencing a symbol that is defined in an +/// external package have a matching in +/// Index.external_symbols (the same invariant scip lint enforces). +/// +/// Before the external-symbols fix, scip-dotnet emitted reference occurrences for +/// NuGet/BCL symbols (e.g. System.Runtime) but never populated +/// external_symbols, so ~60% of occurrences in a real-world index dangled. +/// +[TestFixture] +public class ExternalSymbolsTests +{ + [Test] + public void EveryGenuineExternalOccurrenceHasSymbolInformation() + { + var inputDirectory = Path.Join(RootDirectory(), "snapshots", "input", "syntax"); + var indexFile = IndexDirectory(inputDirectory); + var index = Index.Parser.ParseFrom(File.ReadAllBytes(indexFile)); + + // The set of symbols for which a SymbolInformation exists, either as an + // in-document definition or as an external symbol. + var declared = new HashSet(); + foreach (var document in index.Documents) + { + foreach (var info in document.Symbols) + { + declared.Add(info.Symbol); + } + } + foreach (var info in index.ExternalSymbols) + { + declared.Add(info.Symbol); + } + + var dangling = new SortedSet(); + foreach (var document in index.Documents) + { + foreach (var occurrence in document.Occurrences) + { + if (IsGenuineExternal(occurrence.Symbol) && !declared.Contains(occurrence.Symbol)) + { + dangling.Add(occurrence.Symbol); + } + } + } + + Assert.That(dangling, Is.Empty, + "Occurrences reference external-package symbols that have no SymbolInformation " + + "in external_symbols or any document:\n " + string.Join("\n ", dangling)); + } + + // A "genuine external" symbol is a global symbol that resolves to a real NuGet/BCL + // package. Index-local symbols use the "scip-dotnet nuget . . " package placeholder + // and are intentionally excluded here (their dangling references are a separate issue). + private static bool IsGenuineExternal(string symbol) => + symbol.StartsWith("scip-dotnet nuget ") + && !symbol.StartsWith("scip-dotnet nuget . . "); + + private static string IndexDirectory(string directory) + { + var framework = $"net{Environment.Version.Major}.0"; + var arguments = $"run --project ScipDotnet --framework {framework} -- index --working-directory {directory}"; + var process = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = "dotnet", + Arguments = arguments, + WorkingDirectory = RootDirectory() + } + }; + process.Start(); + process.WaitForExit(); + if (process.ExitCode != 0) + { + Assert.Fail($"non-zero exit code {process.ExitCode} indexing {directory}\ndotnet {arguments}"); + } + + return Path.Join(directory, "index.scip"); + } + + private static string RootDirectory() + { + var process = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = "git", + Arguments = "rev-parse --show-toplevel", + UseShellExecute = false, + RedirectStandardOutput = true + } + }; + process.Start(); + return process.StandardOutput.ReadToEnd().Trim(); + } +} diff --git a/ScipDotnet/IndexCommandHandler.cs b/ScipDotnet/IndexCommandHandler.cs index f4d8e8c..0e667d2 100644 --- a/ScipDotnet/IndexCommandHandler.cs +++ b/ScipDotnet/IndexCommandHandler.cs @@ -110,6 +110,17 @@ private static async Task ScipIndex(IHost host, IndexCommandOptions options) options.Logger.LogWarning("Indexing finished without error but no documents were indexed."); } + // Declare every referenced external-package symbol in Index.external_symbols so that + // cross-package references resolve. Skip any symbol that also has an in-source + // definition (possible under --allow-global-symbol-definitions). Order by symbol for + // deterministic output. + foreach (var external in indexer.ExternalSymbols + .Where(entry => !indexer.DefinedSymbols.Contains(entry.Key)) + .OrderBy(entry => entry.Key, StringComparer.Ordinal)) + { + index.ExternalSymbols.Add(external.Value); + } + await File.WriteAllBytesAsync(options.Output.FullName, index.ToByteArray()); options.Logger.LogInformation("done: {OptionsOutput} {TimeElapsed}", options.Output, stopwatch.Elapsed.ToFriendlyString()); diff --git a/ScipDotnet/ScipDocumentIndexer.cs b/ScipDotnet/ScipDocumentIndexer.cs index 57a6e6d..80e0c73 100644 --- a/ScipDotnet/ScipDocumentIndexer.cs +++ b/ScipDotnet/ScipDocumentIndexer.cs @@ -15,6 +15,15 @@ public class ScipDocumentIndexer private int _localCounter; private readonly Dictionary _globals; private readonly Dictionary _locals = new(SymbolEqualityComparer.Default); + + // Index-wide accounting shared across every document, used to populate + // Index.external_symbols. `_externalSymbols` maps a SCIP symbol string to the + // SymbolInformation we will emit for a referenced external-package symbol. + // `_definedSymbols` is the set of symbols that already have an in-source definition; + // it lets us avoid duplicating a symbol in external_symbols (relevant when + // --allow-global-symbol-definitions assigns a real package name to a source symbol). + private readonly Dictionary _externalSymbols; + private readonly HashSet _definedSymbols; private readonly string _markdownCodeFenceLanguage; // Custom formatting options to render symbol documentation. Feel free to tweak these parameters. @@ -59,11 +68,15 @@ public class ScipDocumentIndexer public ScipDocumentIndexer( Document doc, IndexCommandOptions options, - Dictionary globals) + Dictionary globals, + Dictionary externalSymbols, + HashSet definedSymbols) { _doc = doc; _options = options; _globals = globals; + _externalSymbols = externalSymbols; + _definedSymbols = definedSymbols; _markdownCodeFenceLanguage = _doc.Language == "C#" ? "cs" : "vb"; } @@ -226,7 +239,8 @@ public void VisitOccurrence(ISymbol? symbol, Location location, bool isDefinitio symbolRole |= (int)SymbolRole.Definition; } - var scipSymbol = CreateScipSymbol(symbol).Value; + var scip = CreateScipSymbol(symbol); + var scipSymbol = scip.Value; var occurrence = new Occurrence { Symbol = scipSymbol, @@ -238,9 +252,23 @@ public void VisitOccurrence(ISymbol? symbol, Location location, bool isDefinitio occurrence.Range.Add(range); } - if (!isDefinition) return; + if (!isDefinition) + { + // This occurrence references a symbol that is not defined in the indexed + // source. If it belongs to an external NuGet/BCL package, record a minimal + // SymbolInformation so the reference is resolvable via Index.external_symbols. + // Without this, every cross-package reference dangles (scip lint reports + // "no matching SymbolInformation in external symbols or any document"). + if (scip.IsExternalPackageSymbol() && !_externalSymbols.ContainsKey(scipSymbol)) + { + _externalSymbols[scipSymbol] = CreateExternalSymbolInformation(symbol, scipSymbol); + } + + return; + } // Emit SymbolInformation for this definition occurrence. + _definedSymbols.Add(scipSymbol); var info = new SymbolInformation { Symbol = scipSymbol }; _doc.Symbols.Add(info); @@ -323,6 +351,30 @@ public void VisitOccurrence(ISymbol? symbol, Location location, bool isDefinitio } } + // Builds the SymbolInformation emitted into Index.external_symbols for a referenced + // external-package symbol. We intentionally emit only the symbol and its hover + // documentation: unlike in-source definitions we do NOT walk base types / interfaces, + // because doing so would recurse across the entire BCL type graph and generate an + // unbounded number of additional external symbols. + private SymbolInformation CreateExternalSymbolInformation(ISymbol symbol, string scipSymbol) + { + var info = new SymbolInformation { Symbol = scipSymbol }; + + var symbolSignature = symbol.ToDisplayString(_format); + if (symbolSignature.Length > 0) + { + info.Documentation.Add($"```{_markdownCodeFenceLanguage}\n{symbolSignature}\n```"); + } + + var symbolDocumentation = symbol.GetDocumentationCommentXml(); + if (symbolDocumentation?.Length > 0) + { + info.Documentation.Add(symbolDocumentation); + } + + return info; + } + // Returns explicitly and implicitly implemented interface methods by the given symbol method. // The Roslyn API has a `ExplicitInterfaceImplementations` that does not return implicitly implemented // methods. diff --git a/ScipDotnet/ScipProjectIndexer.cs b/ScipDotnet/ScipProjectIndexer.cs index 1f1debf..6b2307c 100644 --- a/ScipDotnet/ScipProjectIndexer.cs +++ b/ScipDotnet/ScipProjectIndexer.cs @@ -18,6 +18,12 @@ public ScipProjectIndexer(ILogger logger) => private ILogger Logger { get; } + // Index-wide collections populated while documents are indexed. After indexing + // finishes, the caller copies ExternalSymbols (minus DefinedSymbols) into + // Index.external_symbols so that cross-package references are resolvable. + public Dictionary ExternalSymbols { get; } = new(); + public HashSet DefinedSymbols { get; } = new(); + private void Restore(IndexCommandOptions options, FileInfo project) { var isSolution = project.Extension.Equals(".sln", StringComparison.OrdinalIgnoreCase) @@ -144,7 +150,7 @@ await host.Services.GetRequiredService() } else { - var symbolFormatter = new ScipDocumentIndexer(doc, options, globals); + var symbolFormatter = new ScipDocumentIndexer(doc, options, globals, ExternalSymbols, DefinedSymbols); var root = await document.GetSyntaxRootAsync(); if (language == "C#") { diff --git a/ScipDotnet/ScipSymbol.cs b/ScipDotnet/ScipSymbol.cs index a6471e4..b54d56d 100644 --- a/ScipDotnet/ScipSymbol.cs +++ b/ScipDotnet/ScipSymbol.cs @@ -16,6 +16,15 @@ private ScipSymbol(string value) => public bool IsLocal() => Value.StartsWith("local "); + // SCIP symbol prefix shared by every global (package-scoped) symbol this tool emits. + private const string PackagePrefix = "scip-dotnet nuget "; + + // True if this symbol resolves to a real external NuGet/BCL package, i.e. a global + // symbol that is NOT the index-local package placeholder (`scip-dotnet nuget . . `). + // These are the symbols that must be declared in Index.external_symbols. + public bool IsExternalPackageSymbol() => + Value.StartsWith(PackagePrefix) && !Value.StartsWith(IndexLocalPackage.Value); + public static ScipSymbol Global(ScipSymbol owner, SymbolDescriptor descriptor) => new(owner.Value + DescriptorString(descriptor)); From 8f98e59a20628d0f0e49d7879e2f05680093634f Mon Sep 17 00:00:00 2001 From: Thiago Borba Date: Tue, 9 Jun 2026 15:45:09 -0300 Subject: [PATCH 2/2] fix: declare external symbols referenced only via relationships Records implement IEquatable 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 , but couldn't find #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) --- ScipDotnet.Tests/ExternalSymbolsTests.cs | 69 +++++++++++++++++++----- ScipDotnet/ScipDocumentIndexer.cs | 35 +++++++++--- 2 files changed, 82 insertions(+), 22 deletions(-) diff --git a/ScipDotnet.Tests/ExternalSymbolsTests.cs b/ScipDotnet.Tests/ExternalSymbolsTests.cs index acdaad9..5d1899e 100644 --- a/ScipDotnet.Tests/ExternalSymbolsTests.cs +++ b/ScipDotnet.Tests/ExternalSymbolsTests.cs @@ -25,20 +25,7 @@ public void EveryGenuineExternalOccurrenceHasSymbolInformation() var indexFile = IndexDirectory(inputDirectory); var index = Index.Parser.ParseFrom(File.ReadAllBytes(indexFile)); - // The set of symbols for which a SymbolInformation exists, either as an - // in-document definition or as an external symbol. - var declared = new HashSet(); - foreach (var document in index.Documents) - { - foreach (var info in document.Symbols) - { - declared.Add(info.Symbol); - } - } - foreach (var info in index.ExternalSymbols) - { - declared.Add(info.Symbol); - } + var declared = DeclaredSymbols(index); var dangling = new SortedSet(); foreach (var document in index.Documents) @@ -57,6 +44,60 @@ public void EveryGenuineExternalOccurrenceHasSymbolInformation() + "in external_symbols or any document:\n " + string.Join("\n ", dangling)); } + // Every external-package symbol referenced as a relationship target (e.g. the + // implicitly implemented IEquatable on a record, or a transitively inherited + // interface that never appears textually in the source) must also be declared in + // external_symbols. Otherwise scip lint reports: + // "has a relationship to , but couldn't find #2 in external symbols ...". + [Test] + public void EveryExternalRelationshipTargetHasSymbolInformation() + { + var inputDirectory = Path.Join(RootDirectory(), "snapshots", "input", "syntax"); + var indexFile = IndexDirectory(inputDirectory); + var index = Index.Parser.ParseFrom(File.ReadAllBytes(indexFile)); + + var declared = DeclaredSymbols(index); + + var dangling = new SortedSet(); + foreach (var document in index.Documents) + { + foreach (var info in document.Symbols) + { + foreach (var relationship in info.Relationships) + { + if (IsGenuineExternal(relationship.Symbol) && !declared.Contains(relationship.Symbol)) + { + dangling.Add(relationship.Symbol); + } + } + } + } + + Assert.That(dangling, Is.Empty, + "SymbolInformation relationships target external-package symbols that have no " + + "SymbolInformation in external_symbols or any document:\n " + string.Join("\n ", dangling)); + } + + // The set of symbols for which a SymbolInformation exists, either as an + // in-document definition or as an external symbol. + private static HashSet DeclaredSymbols(Index index) + { + var declared = new HashSet(); + foreach (var document in index.Documents) + { + foreach (var info in document.Symbols) + { + declared.Add(info.Symbol); + } + } + foreach (var info in index.ExternalSymbols) + { + declared.Add(info.Symbol); + } + + return declared; + } + // A "genuine external" symbol is a global symbol that resolves to a real NuGet/BCL // package. Index-local symbols use the "scip-dotnet nuget . . " package placeholder // and are intentionally excluded here (their dangling references are a separate issue). diff --git a/ScipDotnet/ScipDocumentIndexer.cs b/ScipDotnet/ScipDocumentIndexer.cs index 80e0c73..6809a44 100644 --- a/ScipDotnet/ScipDocumentIndexer.cs +++ b/ScipDotnet/ScipDocumentIndexer.cs @@ -291,33 +291,35 @@ public void VisitOccurrence(ISymbol? symbol, Location location, bool isDefinitio var baseType = namedTypeSymbol.BaseType; while (baseType != null) { - var baseTypeSymbol = CreateScipSymbol(baseType).Value; - if (IsIgnoredRelationshipSymbol(baseTypeSymbol)) + var baseTypeScip = CreateScipSymbol(baseType); + if (IsIgnoredRelationshipSymbol(baseTypeScip.Value)) { break; } info.Relationships.Add(new Relationship { - Symbol = baseTypeSymbol, + Symbol = baseTypeScip.Value, IsImplementation = true }); + CollectExternalRelationship(baseType, baseTypeScip); baseType = baseType.BaseType; } foreach (var interfaceSymbol in namedTypeSymbol.AllInterfaces) { - var interfaceSymbolSymbol = CreateScipSymbol(interfaceSymbol).Value; - if (IsIgnoredRelationshipSymbol(interfaceSymbolSymbol)) + var interfaceScip = CreateScipSymbol(interfaceSymbol); + if (IsIgnoredRelationshipSymbol(interfaceScip.Value)) { continue; } info.Relationships.Add(new Relationship { - Symbol = interfaceSymbolSymbol, + Symbol = interfaceScip.Value, IsImplementation = true }); + CollectExternalRelationship(interfaceSymbol, interfaceScip); } break; @@ -327,23 +329,27 @@ public void VisitOccurrence(ISymbol? symbol, Location location, bool isDefinitio var overriddenMethod = methodSymbol.OverriddenMethod; while (overriddenMethod != null) { + var overriddenScip = CreateScipSymbol(overriddenMethod); info.Relationships.Add(new Relationship { - Symbol = CreateScipSymbol(overriddenMethod).Value, + Symbol = overriddenScip.Value, IsImplementation = true, IsReference = true }); + CollectExternalRelationship(overriddenMethod, overriddenScip); overriddenMethod = overriddenMethod.OverriddenMethod; } foreach (var interfaceMethod in ScipDocumentIndexer.InterfaceImplementations(methodSymbol)) { + var interfaceMethodScip = CreateScipSymbol(interfaceMethod); info.Relationships.Add(new Relationship { - Symbol = CreateScipSymbol(interfaceMethod).Value, + Symbol = interfaceMethodScip.Value, IsImplementation = true, IsReference = true }); + CollectExternalRelationship(interfaceMethod, interfaceMethodScip); } break; @@ -351,6 +357,19 @@ public void VisitOccurrence(ISymbol? symbol, Location location, bool isDefinitio } } + // Collects an external-package symbol that appears only as a relationship target + // (e.g. an implicitly implemented IEquatable on a record, or a transitive base + // class / inherited interface that never appears textually in the source and so is + // never seen by VisitOccurrence). Without this, the relationship dangles + // (scip lint: "has a relationship to , couldn't find #2"). + private void CollectExternalRelationship(ISymbol related, ScipSymbol scip) + { + if (scip.IsExternalPackageSymbol() && !_externalSymbols.ContainsKey(scip.Value)) + { + _externalSymbols[scip.Value] = CreateExternalSymbolInformation(related, scip.Value); + } + } + // Builds the SymbolInformation emitted into Index.external_symbols for a referenced // external-package symbol. We intentionally emit only the symbol and its hover // documentation: unlike in-source definitions we do NOT walk base types / interfaces,