diff --git a/ScipDotnet.Tests/ExternalSymbolsTests.cs b/ScipDotnet.Tests/ExternalSymbolsTests.cs new file mode 100644 index 0000000..5d1899e --- /dev/null +++ b/ScipDotnet.Tests/ExternalSymbolsTests.cs @@ -0,0 +1,146 @@ +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)); + + var declared = DeclaredSymbols(index); + + 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)); + } + + // 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). + 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..6809a44 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); @@ -263,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; @@ -299,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; @@ -323,6 +357,43 @@ 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, + // 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));