diff --git a/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs b/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs index f7eb75a0c..336935b52 100644 --- a/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs +++ b/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs @@ -114,26 +114,54 @@ public class CommandLineArgumentsTests [AvaloniaTest] public async Task NavigateTo_Accepts_A_Member_Id_Without_Its_Signature() { - // A cref may name a member without a parameter list ("M:System.Linq.Enumerable.Where"), - // which is what a user reaches for on the command line and what an xml doc comment - // allows. The short form names the whole overload group, so any of its members is a - // correct landing spot. + // A cref may name a member without a parameter list, which is what a user reaches for: + // spelling out the signature means knowing the overload count beforehand. Where the + // short form names exactly one member, it selects that member. - // Arrange - boot. + // Arrange - boot, and open an assembly with a member that has no overloads. var window = AppComposition.Current.GetExport(); window.Show(); var vm = (MainWindowViewModel)window.DataContext!; await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); - var args = CommandLineArguments.Create(new[] { "--navigateto", "M:System.Linq.Enumerable.Where" }); + string path = typeof(CommandLineArgumentsTests).Assembly.Location; + var args = CommandLineArguments.Create(new[] { + path, "--navigateto", "M:ICSharpCode.ILSpy.Tests.NavigateToSample.OnlyOne" }); // Act. await vm.AssemblyTreeModel.HandleCommandLineArgumentsAsync(args); - // Assert - selection landed on one of the Where overloads. + // Assert - selection landed on the member itself. ((object?)vm.AssemblyTreeModel.SelectedItem).Should().NotBeNull(); vm.AssemblyTreeModel.SelectedItem!.GetType().Should().Be(typeof(MethodTreeNode)); - ((MethodTreeNode)vm.AssemblyTreeModel.SelectedItem!).MethodDefinition.Name.Should().Be("Where"); + ((MethodTreeNode)vm.AssemblyTreeModel.SelectedItem!).MethodDefinition.Name.Should().Be("OnlyOne"); + } + + [AvaloniaTest] + public async Task NavigateTo_Short_Form_Of_An_Overloaded_Member_Selects_Every_Overload() + { + // The short form of an overloaded member names the whole group, and no single overload + // is a better answer than its siblings. Selecting all of them shows every one without + // leaving the member level: falling back to the declaring type would bury the group in + // a large type's decompilation, and picking one would hide that there was a choice. + + // Arrange - boot, and open an assembly with an overloaded member. + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); + + string path = typeof(CommandLineArgumentsTests).Assembly.Location; + var args = CommandLineArguments.Create(new[] { + path, "--navigateto", "M:ICSharpCode.ILSpy.Tests.NavigateToSample.Overloaded" }); + + // Act. + await vm.AssemblyTreeModel.HandleCommandLineArgumentsAsync(args); + + // Assert - every overload is selected, and nothing else. + vm.AssemblyTreeModel.SelectedItems.Should().HaveCount(2); + vm.AssemblyTreeModel.SelectedItems.Should().AllSatisfy(node => + ((MethodTreeNode)node).MethodDefinition.Name.Should().Be("Overloaded")); } [AvaloniaTest] @@ -192,3 +220,16 @@ public class CommandLineArgumentsTests vm.AssemblyTreeModel.SelectedItem!.ToString().Should().Be("System.Linq.Enumerable"); } } + +/// +/// Fixture for --navigateto: one member with no overloads, and one with several, so the short +/// form of a member ID can be exercised in both shapes. +/// +public class NavigateToSample +{ + public void OnlyOne(int a, int b) { } + + public void Overloaded(int a) { } + + public void Overloaded(string a) { } +} diff --git a/ILSpy/AssemblyTree/AssemblyTreeModel.cs b/ILSpy/AssemblyTree/AssemblyTreeModel.cs index ac237d341..1a77c1987 100644 --- a/ILSpy/AssemblyTree/AssemblyTreeModel.cs +++ b/ILSpy/AssemblyTree/AssemblyTreeModel.cs @@ -726,21 +726,30 @@ namespace ICSharpCode.ILSpy.AssemblyTree ? new List(newlyLoaded) : AssemblyList?.GetAssemblies().ToList() ?? new List(); - if (args.NavigateTo is { Length: > 0 } navigateTo) - await NavigateOnLaunchAsync(navigateTo, relevant); - else if (newlyLoaded.Count == 1 && FindAssemblyNode(newlyLoaded[0]) is { } singleNode) + // Only a target that actually resolved gets to own the selection. An ID naming + // nothing falls through to the same single-assembly selection that opening the + // file without --navigateto would have made, rather than leaving the tree empty + // with no indication of what went wrong. + bool navigationHandled = args.NavigateTo is { Length: > 0 } navigateTo + && await NavigateOnLaunchAsync(navigateTo, relevant); + if (!navigationHandled && newlyLoaded.Count == 1 && FindAssemblyNode(newlyLoaded[0]) is { } singleNode) SelectNode(singleNode); // Search-pane wiring lands with task 6. Until then the arg parses but is a no-op // rather than crashing. } - async Task NavigateOnLaunchAsync(string navigateTo, IList relevant) + /// + /// Navigates to the given target. Returns false if it named nothing, leaving the + /// selection for the caller to fill in. + /// + async Task NavigateOnLaunchAsync(string navigateTo, IList relevant) { // "none" is a sentinel used by the WPF VS add-in to suppress initial navigation — - // the real target arrives later via IPC. + // the real target arrives later via IPC. Nothing else may claim the selection + // either, so this counts as handled. if (navigateTo == "none") - return; + return true; if (navigateTo.StartsWith("N:", StringComparison.Ordinal)) { @@ -757,10 +766,10 @@ namespace ICSharpCode.ILSpy.AssemblyTree if (nsNode != null) { SelectNode(nsNode); - return; + return true; } } - return; + return false; } // A gone or unreadable assembly resolves to null and is skipped by the entity search @@ -768,29 +777,61 @@ namespace ICSharpCode.ILSpy.AssemblyTree foreach (var asm in relevant) await asm.GetMetadataFileOrNullAsync().ConfigureAwait(true); - var entity = await Task.Run(() => FindEntityInRelevantAssemblies(navigateTo, relevant)); - if (entity != null) + var group = await Task.Run(() => FindEntitiesInRelevantAssemblies(navigateTo, relevant)); + if (group.Count == 0) + return false; + // The short form of an overloaded member names the whole group, and no single + // overload answers it better than its siblings. Selecting all of them shows every + // one while staying at the member level, where the group is what the user was + // pointing at; picking one would hide that there was anything to pick. + var nodes = new List(group.Count); + foreach (var entity in group) { - var node = FindTreeNode(entity); - if (node != null) - SelectNode(node); + if (FindTreeNode(entity) is { } found) + nodes.Add(found); } + if (nodes.Count == 0) + return false; + SelectNodes(nodes); + return true; } internal static IEntity? FindEntityInRelevantAssemblies(string navigateTo, IEnumerable relevantAssemblies) + { + var group = FindEntitiesInRelevantAssemblies(navigateTo, relevantAssemblies); + return group.Count == 0 ? null : group[0]; + } + + /// + /// Resolves a navigation target to every entity it names. A member ID written without + /// its signature names an overload group; the caller decides how to present one. + /// + internal static IReadOnlyList FindEntitiesInRelevantAssemblies(string navigateTo, IEnumerable relevantAssemblies) { // Reference assemblies are skipped so the search keeps looking for another // assembly that might have a usable definition. IReadOnlyList modules = [.. from asm in relevantAssemblies let mod = asm.GetMetadataFileOrNull() where mod != null && !mod.IsReferenceAssembly() select mod]; - var (module, handle) = IdStringProvider.FindEntity(navigateTo, modules); - if (module == null || handle.IsNil) - (module, handle) = FindMemberViaTypeForwarders(navigateTo, modules); - if (module == null || handle.IsNil) - return null; - var metadataModule = module.GetLoadedAssembly().GetTypeSystemOrNull()?.MainModule as MetadataModule; - if (metadataModule == null) - return null; - return metadataModule.ResolveEntity(handle); + // The id came from a command line, so it is searched with the omission-tolerant + // ladder rather than resolved exactly: a parameter list or a generic arity that has + // to be spelled out is one the caller had to know before asking. + var (module, handles) = DocumentationIdSearch.Find(navigateTo, modules); + if (module == null || handles.IsEmpty) + { + var (forwardedModule, handle) = FindMemberViaTypeForwarders(navigateTo, modules); + if (forwardedModule == null || handle.IsNil) + return []; + module = forwardedModule; + handles = [handle]; + } + if (module.GetLoadedAssembly().GetTypeSystemOrNull()?.MainModule is not MetadataModule metadataModule) + return []; + var entities = new List(handles.Length); + foreach (var handle in handles) + { + if (metadataModule.ResolveEntity(handle) is { } entity) + entities.Add(entity); + } + return entities; } ///