diff --git a/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs b/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs index ff059de21..834fdc40c 100644 --- a/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs +++ b/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs @@ -1225,6 +1225,216 @@ namespace ModreqParams "GetIdString on found entity does not match the input ID string"); } + /// + /// Every ID Roslyn generates for the fixture must resolve, and resolve back to the same + /// ID. Driving this from the Roslyn map rather than a hand-picked list is what makes it + /// cover the generic shapes exhaustively: generic types of each arity, generic methods, + /// members whose signatures mention `0/``0, and types nested to three levels where the + /// nesting mixes generic and non-generic (Gen`1.Inner, Gen`1.InnerGen`1.Deepest`1). + /// + [Test] + public void FindEntity_RoundTripsEveryIdRoslynGenerates() + { + var module = decompilerTypeSystem.MainModule.MetadataFile; + var unresolved = new List(); + var mismatched = new List(); + var threw = new List(); + + var drifted = new List(); + + foreach (string id in roslynIdMap.Keys.OrderBy(k => k, StringComparer.Ordinal)) + { + // Namespaces have no entity to find, and Roslyn also emits IDs for the + // referenced assemblies; only the fixture's own entities can be found in the + // module under test. + if (id.Length < 2 || id[1] != ':' || !"TMPFE".Contains(id[0])) + continue; + EntityHandle handle; + try + { + (_, handle) = IdStringProvider.FindEntity(id, new[] { module }); + } + catch (ReflectionNameParseException ex) + { + threw.Add($"{id} -> {ex.Message}"); + continue; + } + // Roslyn emits IDs for members it declares implicitly - a struct's parameterless + // constructor, for one - which the compiler never writes to metadata. Those must + // resolve to nothing; resolving them to a same-named sibling is the drift this + // guards against, and a record struct's primary constructor is a live example. + bool implicitlyDeclared = roslynIdMap[id].IsImplicitlyDeclared; + if (handle.IsNil) + { + if (IsFixtureId(id) && !implicitlyDeclared) + unresolved.Add(id); + continue; + } + string roundTripped = IdStringProvider.GetIdString(module, handle); + if (implicitlyDeclared) + { + if (roundTripped != id) + drifted.Add($"{id} -> {roundTripped}"); + continue; + } + if (roundTripped != id) + mismatched.Add($"{id} -> {roundTripped}"); + } + + Assert.Multiple(() => { + Assert.That(unresolved, Is.Empty, "IDs Roslyn generates that FindEntity cannot resolve"); + Assert.That(mismatched, Is.Empty, "IDs that resolve to an entity with a different ID"); + Assert.That(threw, Is.Empty, "IDs Roslyn generates that FindEntity rejects as malformed"); + Assert.That(drifted, Is.Empty, "IDs of members that never reach metadata, resolved to a different member"); + }); + } + + /// + /// True for an ID naming an entity declared by the fixture itself, as opposed to one of + /// the assemblies it references. + /// + static bool IsFixtureId(string id) + { + string name = id.Substring(2); + return !name.StartsWith("System.", StringComparison.Ordinal) + && !name.StartsWith("Microsoft.", StringComparison.Ordinal); + } + + /// + /// The id grammar is exact, so resolution is too: an id naming a signature no member has, + /// or leaving one off entirely, resolves to nothing rather than to a same-named sibling. + /// Tolerating what a human leaves out is 's job, and + /// keeping it out of here is what lets cref-following trust the answer. + /// + // a signature no member has + [TestCase("M:Acme.Widget.M1(System.Int32)")] + [TestCase("M:Acme.Widget.M6(System.String)")] + [TestCase("M:Acme.Widget.op_Explicit(Acme.Widget)~System.Byte")] + // no such member at all + [TestCase("M:Acme.Widget.NoSuchMember")] + [TestCase("F:Acme.Widget.noSuchField")] + // an arity no member has, and one left off entirely + [TestCase("M:Acme.UseList.GetValues``2")] + [TestCase("M:Acme.UseList.GetValues")] + // a parameter list left off + [TestCase("M:Acme.Widget.M1")] + [TestCase("P:Acme.Widget.Item")] + // a type arity left off + [TestCase("T:Acme.MyList")] + [TestCase("M:Acme.MyList.Test")] + public void FindEntity_ResolvesOnlyWhatTheIdExactlyNames(string idString) + { + var (_, handle) = IdStringProvider.FindEntity( + idString, new[] { decompilerTypeSystem.MainModule.MetadataFile }); + Assert.That(handle.IsNil, Is.True, $"'{idString}' must not resolve to any member"); + } + + #region DocumentationIdSearch - the omission-tolerant ladder + + ImmutableArray Search(string idString) + { + var (_, handles) = DocumentationIdSearch.Find( + idString, new[] { decompilerTypeSystem.MainModule.MetadataFile }); + return handles; + } + + string[] SearchIds(string idString) + { + var module = decompilerTypeSystem.MainModule.MetadataFile; + return Search(idString).Select(h => IdStringProvider.GetIdString(module, h)).OrderBy(x => x, StringComparer.Ordinal).ToArray(); + } + + /// + /// Rung 1: an id that spells everything out names exactly one entity, and the ladder must + /// not loosen past it. + /// + [TestCase("M:Acme.Widget.M1(System.Char,System.Single@,Acme.ValueType@,System.Int32@)")] + [TestCase("T:Acme.MyList`1")] + [TestCase("M:Acme.UseList.GetValues``1(``0)")] + [TestCase("T:DeepNesting.GenericLevel1`1.GenericLevel2`1.GenericLevel3`1")] + public void Search_ExactIdNamesOneEntity(string idString) + { + Assert.That(Search(idString), Has.Length.EqualTo(1), idString); + } + + /// + /// Rung 2: the parameter list may be left off, naming the whole member group. + /// + [Test] + public void Search_ParameterListMayBeLeftOff() + { + Assert.That(SearchIds("M:Overloads.OverloadResolution.M"), Has.Length.EqualTo(5)); + Assert.That(SearchIds("P:Acme.Widget.Item"), Has.Length.EqualTo(2)); + Assert.That(SearchIds("M:Acme.Widget.M1"), + Is.EqualTo(new[] { "M:Acme.Widget.M1(System.Char,System.Single@,Acme.ValueType@,System.Int32@)" })); + } + + /// + /// Rung 3: generic arities may be left off - on the member, on the declaring type, and on + /// any level of a nested type. Knowing an arity is the same problem as knowing an overload + /// count, and a backtick does not survive being typed at a shell prompt unquoted. + /// + [TestCase("M:Acme.UseList.GetValues", "M:Acme.UseList.GetValues``1(``0)")] + [TestCase("T:Acme.MyList", "T:Acme.MyList`1")] + [TestCase("M:Acme.MyList.Test", "M:Acme.MyList`1.Test(`0)")] + [TestCase("T:DeepNesting.GenericLevel1.GenericLevel2.GenericLevel3", "T:DeepNesting.GenericLevel1`1.GenericLevel2`1.GenericLevel3`1")] + [TestCase("T:DeepNesting.GenericLevel1`1.GenericLevel2.GenericLevel3", "T:DeepNesting.GenericLevel1`1.GenericLevel2`1.GenericLevel3`1")] + public void Search_GenericArityMayBeLeftOff(string idString, string expected) + { + Assert.That(SearchIds(idString), Is.EqualTo(new[] { expected })); + } + + /// + /// A stated arity still has to be right: leaving one off asks for any, giving a wrong one + /// asks for something that does not exist. + /// + [TestCase("M:Acme.UseList.GetValues``2")] + [TestCase("T:Acme.MyList`9")] + [TestCase("M:Acme.Widget.M1(System.Int32)")] + [TestCase("M:Acme.Widget.NoSuchMember")] + public void Search_StatedDetailMustStillMatch(string idString) + { + Assert.That(Search(idString), Is.Empty, idString); + } + + /// + /// The "T:"/"M:" prefix may be left off, and the declaring type may be named by the tail + /// of its path rather than in full - both are what a person or a tool actually types. + /// + [TestCase("Acme.UseList.GetValues", "M:Acme.UseList.GetValues``1(``0)")] + [TestCase("UseList.GetValues", "M:Acme.UseList.GetValues``1(``0)")] + [TestCase("MyList.Test", "M:Acme.MyList`1.Test(`0)")] + [TestCase("GenericLevel2.GenericLevel3", "T:DeepNesting.GenericLevel1`1.GenericLevel2`1.GenericLevel3`1")] + public void Search_PrefixAndNamespaceMayBeLeftOff(string idString, string expected) + { + Assert.That(SearchIds(idString), Is.EqualTo(new[] { expected })); + } + + /// + /// Arity may be given the way a cref or C# spells it, which is both what people reach for + /// and what survives a shell prompt. + /// + [TestCase("T:Acme.MyList{T}")] + [TestCase("T:Acme.MyList")] + [TestCase("MyList{T}")] + public void Search_AcceptsBracketedGenericArguments(string idString) + { + Assert.That(SearchIds(idString), Is.EqualTo(new[] { "T:Acme.MyList`1" })); + } + + /// + /// A name can read as a nested type or as a member; both are searched, and both are + /// reported rather than one being guessed at. + /// + [Test] + public void Search_WithoutAPrefixReportsBothReadings() + { + Assert.That(SearchIds("Acme.Widget.NestedClass"), Is.EqualTo(new[] { "T:Acme.Widget.NestedClass" })); + Assert.That(SearchIds("Acme.Widget.Width"), Has.Length.EqualTo(1)); + } + + #endregion + [TestCase("T:Acme.MyList{")] [TestCase("T:Acme.MyList{System.Int32")] [TestCase("T:Acme.MyList{Acme.MyList{System.Int32}")] diff --git a/ICSharpCode.Decompiler/Documentation/DocumentationIdSearch.cs b/ICSharpCode.Decompiler/Documentation/DocumentationIdSearch.cs new file mode 100644 index 000000000..761ce4f35 --- /dev/null +++ b/ICSharpCode.Decompiler/Documentation/DocumentationIdSearch.cs @@ -0,0 +1,386 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Reflection.Metadata; +using System.Text; + +using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.TypeSystem; + +namespace ICSharpCode.Decompiler.Documentation +{ + /// + /// Finds the entities a hand-written documentation id was aiming at. + /// + /// + /// + /// The id grammar is exact, and implements it exactly: that is + /// what keeps cref-following honest, because an id in a documentation file is machine-written + /// and means one member. This is the other half - the ids people and tools type by hand at a + /// command line, where being made to spell out a parameter list or a generic arity means + /// knowing the answer before asking the question. + /// + /// + /// Matching is a ladder, loosening one thing at a time and stopping at the first rung that + /// matches anything: + /// + /// + /// the exact id, as the grammar defines it; + /// the id without its parameter list, naming a member group; + /// the id with generic arities left off the declaring type, the member, or both. + /// + /// + /// A rung can match several entities, and every one of them is returned: which to present is + /// the caller's decision, and hiding the others would hide that the id was ambiguous. An id + /// that does spell out a signature never leaves the first rung, so stating a signature no + /// member has still finds nothing rather than drifting to a same-named sibling. + /// + /// + public static class DocumentationIdSearch + { + /// + /// Finds every entity the given id names, in the first module that matches at all. + /// Returns an empty group if no module does. + /// + public static (MetadataFile Module, ImmutableArray Handles) Find(string idString, IReadOnlyList modules) + { + if (idString == null) + throw new ArgumentNullException(nameof(idString)); + if (modules == null) + throw new ArgumentNullException(nameof(modules)); + + var (exactModule, exactHandle) = TryExact(idString, modules); + if (!exactHandle.IsNil) + return (exactModule, ImmutableArray.Create(exactHandle)); + + var queries = Parse(idString); + if (queries.Count == 0) + return (null, ImmutableArray.Empty); + + // Rung 2 requires the id to name the declaring type in full; rung 3 lets it name only + // the tail of that path, so "Dictionary.Add" finds the member without the namespace. + foreach (bool suffixMatch in new[] { false, true }) + { + foreach (var module in modules) + { + if (module == null) + continue; + var matches = ImmutableArray.CreateBuilder(); + foreach (var query in queries) + CollectInModule(module, query, suffixMatch, matches); + if (matches.Count > 0) + return (module, Distinct(matches)); + } + } + + return (null, ImmutableArray.Empty); + } + + static (MetadataFile, EntityHandle) TryExact(string idString, IReadOnlyList modules) + { + // Only a well-formed id can be exact, and IdStringProvider throws rather than + // returning nothing for one that is not. + if (idString.Length < 2 || idString[1] != ':') + return (null, default); + try + { + return IdStringProvider.FindEntity(idString, modules); + } + catch (ReflectionNameParseException) + { + return (null, default); + } + } + + static ImmutableArray Distinct(ImmutableArray.Builder matches) + { + var seen = new HashSet(); + var result = ImmutableArray.CreateBuilder(matches.Count); + foreach (var handle in matches) + { + if (seen.Add(handle)) + result.Add(handle); + } + return result.ToImmutable(); + } + + /// A name with the generic arity the id stated for it, or -1 for none. + readonly struct Part + { + public Part(string name, int arity) + { + Name = name; + Arity = arity; + } + + public string Name { get; } + public int Arity { get; } + } + + struct Query + { + public char Kind { get; set; } + public Part[] TypePath { get; set; } + public string MemberName { get; set; } + public int MemberArity { get; set; } + } + + /// + /// Turns an id into the queries it could plausibly mean. A missing "X:" prefix leaves the + /// kind open, and the final dot is then ambiguous between a nested type name and a member + /// name - both readings are returned, and both are searched. + /// + static List Parse(string idString) + { + var queries = new List(); + string rest = idString; + char kind = '\0'; + if (rest.Length > 2 && rest[1] == ':') + { + kind = rest[0]; + if (kind is not ('T' or 'M' or 'P' or 'F' or 'E')) + return queries; + rest = rest.Substring(2); + } + + // An id that spells out a signature is exact or nothing, and exact has been tried. + if (rest.IndexOf('(') >= 0 || rest.IndexOf('~') >= 0) + return queries; + + var path = SplitPath(rest); + if (path.Count == 0) + return queries; + + if (kind is '\0' or 'T') + queries.Add(new Query { Kind = 'T', TypePath = path.ToArray() }); + + if (kind != 'T' && path.Count >= 2) + { + var member = path[path.Count - 1]; + var declaring = path.GetRange(0, path.Count - 1).ToArray(); + // '#ctor'/'#cctor' are the id spelling of the metadata names '.ctor'/'.cctor'. + string memberName = member.Name.Replace('#', '.'); + foreach (char memberKind in kind == '\0' ? new[] { 'M', 'P', 'F', 'E' } : new[] { kind }) + { + queries.Add(new Query { + Kind = memberKind, + TypePath = declaring, + MemberName = memberName, + MemberArity = member.Arity, + }); + } + } + + return queries; + } + + /// + /// Splits a dotted name into parts, reading the generic arity of each from whichever + /// spelling it was given: the id form (Dictionary`2, M``1) or the cref and + /// C# forms (Dictionary{TKey,TValue}, Dictionary<TKey,TValue>). The + /// bracketed forms are what a person reaches for, and unlike a backtick they survive being + /// typed at a shell prompt. + /// + static List SplitPath(string text) + { + var parts = new List(); + int i = 0; + while (i <= text.Length) + { + int start = i; + int arity = -1; + var name = new StringBuilder(); + while (i < text.Length && text[i] != '.') + { + char c = text[i]; + if (c == '`') + { + int digits = i + 1; + while (digits < text.Length && text[digits] == '`') + digits++; + int numberStart = digits; + while (digits < text.Length && char.IsDigit(text[digits])) + digits++; + if (digits == numberStart) + return new List(); + arity = int.Parse(text.Substring(numberStart, digits - numberStart)); + i = digits; + continue; + } + if (c is '{' or '<') + { + int close = MatchingBracket(text, i); + if (close < 0) + return new List(); + arity = CountArguments(text, i + 1, close); + i = close + 1; + continue; + } + name.Append(c); + i++; + } + if (name.Length == 0 && start == i) + return new List(); + parts.Add(new Part(name.ToString(), arity)); + if (i >= text.Length) + break; + i++; // the '.' + } + return parts; + } + + static int MatchingBracket(string text, int open) + { + int depth = 0; + for (int i = open; i < text.Length; i++) + { + if (text[i] is '{' or '<') + depth++; + else if (text[i] is '}' or '>' && --depth == 0) + return i; + } + return -1; + } + + static int CountArguments(string text, int start, int end) + { + if (start >= end) + return 0; + int depth = 0, count = 1; + for (int i = start; i < end; i++) + { + if (text[i] is '{' or '<') + depth++; + else if (text[i] is '}' or '>') + depth--; + else if (text[i] == ',' && depth == 0) + count++; + } + return count; + } + + static void CollectInModule(MetadataFile module, Query query, bool suffixMatch, ImmutableArray.Builder matches) + { + var metadata = module.Metadata; + foreach (var typeHandle in metadata.TypeDefinitions) + { + var typeDef = metadata.GetTypeDefinition(typeHandle); + if (!TypeMatches(metadata, typeDef, query.TypePath, suffixMatch)) + continue; + if (query.Kind == 'T') + matches.Add(typeHandle); + else + CollectMembers(metadata, typeDef, query, matches); + } + } + + /// + /// Compares a type's namespace-and-nesting path against the id's, part by part. A part + /// that states an arity must match it; one that leaves it off matches any, which is what + /// lets "Dictionary" find "Dictionary`2". With the id need + /// only name the tail of the path, so a namespace may be shortened or dropped. + /// + static bool TypeMatches(MetadataReader metadata, TypeDefinition typeDef, Part[] wanted, bool suffixMatch) + { + var actual = new List(); + var current = typeDef; + while (true) + { + actual.Insert(0, SplitArity(metadata.GetString(current.Name))); + var declaring = current.GetDeclaringType(); + if (declaring.IsNil) + break; + current = metadata.GetTypeDefinition(declaring); + } + string ns = metadata.GetString(current.Namespace); + if (ns.Length > 0) + { + var namespaceParts = ns.Split('.'); + for (int i = namespaceParts.Length - 1; i >= 0; i--) + actual.Insert(0, new Part(namespaceParts[i], -1)); + } + + if (suffixMatch ? actual.Count < wanted.Length : actual.Count != wanted.Length) + return false; + int offset = actual.Count - wanted.Length; + for (int i = 0; i < wanted.Length; i++) + { + var a = actual[offset + i]; + if (a.Name != wanted[i].Name) + return false; + if (wanted[i].Arity >= 0 && a.Arity != wanted[i].Arity) + return false; + } + return true; + } + + static Part SplitArity(string metadataName) + { + int tick = metadataName.IndexOf('`'); + if (tick < 0) + return new Part(metadataName, 0); + return int.TryParse(metadataName.Substring(tick + 1), out int arity) + ? new Part(metadataName.Substring(0, tick), arity) + : new Part(metadataName, 0); + } + + static void CollectMembers(MetadataReader metadata, TypeDefinition typeDef, Query query, ImmutableArray.Builder matches) + { + bool NameMatches(StringHandle candidate) => metadata.StringComparer.Equals(candidate, query.MemberName); + + switch (query.Kind) + { + case 'F': + foreach (var handle in typeDef.GetFields()) + { + if (NameMatches(metadata.GetFieldDefinition(handle).Name)) + matches.Add(handle); + } + break; + case 'M': + foreach (var handle in typeDef.GetMethods()) + { + var method = metadata.GetMethodDefinition(handle); + if (!NameMatches(method.Name)) + continue; + if (query.MemberArity >= 0 && method.GetGenericParameters().Count != query.MemberArity) + continue; + matches.Add(handle); + } + break; + case 'P': + foreach (var handle in typeDef.GetProperties()) + { + if (NameMatches(metadata.GetPropertyDefinition(handle).Name)) + matches.Add(handle); + } + break; + case 'E': + foreach (var handle in typeDef.GetEvents()) + { + if (NameMatches(metadata.GetEventDefinition(handle).Name)) + matches.Add(handle); + } + break; + } + } + } +} diff --git a/ICSharpCode.ILSpyCmd.Tests/MemberOptionTests.cs b/ICSharpCode.ILSpyCmd.Tests/MemberOptionTests.cs index 4e6f22cff..745875931 100644 --- a/ICSharpCode.ILSpyCmd.Tests/MemberOptionTests.cs +++ b/ICSharpCode.ILSpyCmd.Tests/MemberOptionTests.cs @@ -63,6 +63,40 @@ namespace ICSharpCode.ILSpyCmd.Tests Assert.That(result.Output, Does.Contain("int Add(int a, int b)")); } + /// + /// The short form is what a user reaches for, because knowing the parameter list means + /// knowing the overload count beforehand. Where it names one member, it just works. + /// + [Test] + public async Task ShortFormWithoutSignatureDecompilesTheMember() + { + var result = await RunAsync(testAssemblyPath, "--disable-updatecheck", + "-m", "M:ICSharpCode.ILSpyCmd.Tests.MemberOptionSample.Add"); + + Assert.That(result.ExitCode, Is.EqualTo(0), result.Error); + Assert.That(result.Output, Does.Contain("int Add(int a, int b)")); + Assert.That(result.Output, Does.Not.Contain("names 2 members")); + } + + /// + /// The short form of an overloaded member names the whole group. Every member is shown - + /// making the user re-run with a full signature would defeat the point of accepting the + /// short form - with a comment saying the ID was ambiguous and what it matched. + /// + [Test] + public async Task ShortFormOfOverloadedMemberDecompilesEveryMember() + { + var result = await RunAsync(testAssemblyPath, "--disable-updatecheck", + "-m", "M:ICSharpCode.ILSpyCmd.Tests.MemberOptionSample.Scale"); + + Assert.That(result.ExitCode, Is.EqualTo(0), result.Error); + Assert.That(result.Output, Does.Contain("int Scale(int value)")); + Assert.That(result.Output, Does.Contain("string Scale(string value)")); + Assert.That(result.Output, Does.Contain("names 2 members")); + Assert.That(result.Output, Does.Contain("M:ICSharpCode.ILSpyCmd.Tests.MemberOptionSample.Scale(System.Int32)")); + Assert.That(result.Output, Does.Contain("M:ICSharpCode.ILSpyCmd.Tests.MemberOptionSample.Scale(System.String)")); + } + [Test] public async Task UnknownMemberReportsError() { @@ -134,5 +168,15 @@ namespace ICSharpCode.ILSpyCmd.Tests public void Unrelated() { } + + public int Scale(int value) + { + return value * 2; + } + + public string Scale(string value) + { + return value + value; + } } } diff --git a/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs b/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs index 15e74dab4..82c1208a8 100644 --- a/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs +++ b/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs @@ -18,6 +18,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.ComponentModel.DataAnnotations; using System.IO; using System.IO.Compression; @@ -681,17 +682,24 @@ Examples: if (MemberIdString != null) { - if (!TryResolveMember(decompiler.TypeSystem, MemberIdString, out EntityHandle handle, out string error)) + if (!TryResolveMembers(decompiler.TypeSystem, MemberIdString, out var handles, out string error)) { Console.Error.WriteLine(error); return ProgramExitCodes.EX_DATAERR; } - if (handle.Kind != HandleKind.MethodDefinition) + // The short form of an overloaded method names the whole group; dumping every + // body beats picking one of them silently. + var resolved = handles.Where(h => h.Kind == HandleKind.MethodDefinition).ToArray(); + if (resolved.Length == 0) { Console.Error.WriteLine($"'{MemberIdString}' does not name a method; ILAst exists for method bodies only."); return ProgramExitCodes.EX_DATAERR; } - methods = new[] { mainModule.GetDefinition((MethodDefinitionHandle)handle) }; + if (resolved.Length > 1) + { + Console.Error.WriteLine($"'{MemberIdString.Trim()}' names {resolved.Length} methods; the ILAst of each is written below."); + } + methods = resolved.Select(h => mainModule.GetDefinition((MethodDefinitionHandle)h)).ToArray(); } else if (TypeName != null) { @@ -808,13 +816,34 @@ Examples: { CSharpDecompiler decompiler = GetDecompiler(assemblyFileName); - if (!TryResolveMember(decompiler.TypeSystem, idOrToken, out EntityHandle handle, out string error)) + if (!TryResolveMembers(decompiler.TypeSystem, idOrToken, out var handles, out string error)) { Console.Error.WriteLine(error); return ProgramExitCodes.EX_DATAERR; } - output.Write(decompiler.DecompileAsString(handle)); + // A short-form id names an overload group. Showing every member beats making the + // user re-run with a full signature, but the output must say so: otherwise several + // members arrive with nothing explaining why more than one was asked for. + if (handles.Length > 1) + { + var metadataFile = decompiler.TypeSystem.MainModule.MetadataFile; + output.WriteLine($"// '{idOrToken.Trim()}' names {handles.Length} members; all of them are shown below."); + foreach (var member in handles) + { + output.WriteLine($"// {metadataFile.GetIdString(member)}"); + } + output.WriteLine(); + } + + bool first = true; + foreach (var member in handles) + { + if (!first) + output.WriteLine(); + output.Write(decompiler.DecompileAsString(member)); + first = false; + } ReportDecompilationErrors(assemblyFileName, decompiler.Errors); return 0; } @@ -828,7 +857,24 @@ Examples: /// static bool TryResolveMember(IDecompilerTypeSystem typeSystem, string idOrToken, out EntityHandle handle, out string error) { - handle = default; + if (!TryResolveMembers(typeSystem, idOrToken, out var handles, out error)) + { + handle = default; + return false; + } + handle = handles[0]; + return true; + } + + /// + /// As , but reports every member the reference names. A + /// documentation id written without a parameter list names an overload group, and the + /// short form is what a user reaches for: spelling out the signature means knowing the + /// overload count beforehand, which is the thing they came here to find out. + /// + static bool TryResolveMembers(IDecompilerTypeSystem typeSystem, string idOrToken, out ImmutableArray handles, out string error) + { + handles = ImmutableArray.Empty; error = null; string trimmed = idOrToken.Trim(); @@ -855,32 +901,55 @@ Examples: error = $"Metadata token {trimmed} does not reference a type or member of this module."; return false; } - handle = candidate; + handles = ImmutableArray.Create(candidate); return true; } - IEntity entity; + var mainModule = typeSystem.MainModule.MetadataFile; + ImmutableArray found; try { - entity = IdStringProvider.FindEntity(trimmed, new SimpleTypeResolveContext(typeSystem.MainModule)); + (_, found) = DocumentationIdSearch.Find(trimmed, new[] { mainModule }); } catch (ReflectionNameParseException ex) { error = $"'{trimmed}' is not a valid documentation id string: {ex.Message}"; return false; } - if (entity == null || entity.MetadataToken.IsNil) + if (found.IsEmpty) { - error = $"Member '{trimmed}' was not found in this module. Expected an XML documentation id string (e.g. \"M:System.String.Concat(System.String,System.String)\") or a metadata token (e.g. 0x06000005)."; + // "It exists, but not here" is worth saying: naming the assembly it does live in + // tells the user which one to point at, where a bare not-found leaves them + // guessing whether they mistyped the id. + if (ResolveElsewhere(typeSystem, trimmed) is { } elsewhere) + { + error = $"Member '{trimmed}' is defined in '{elsewhere.AssemblyName}', not in this module."; + return false; + } + error = $"Member '{trimmed}' was not found in this module. Expected an XML documentation id string (e.g. \"M:System.String.Concat(System.String,System.String)\") or a metadata token (e.g. 0x06000005). The parameter list and generic arities may be left off."; return false; } - if (entity.ParentModule != typeSystem.MainModule) + handles = found; + return true; + } + + /// + /// The module that defines the given id, when it is not the one being decompiled. Only an + /// exact id is tried: the loose ladder exists to help someone name a member of the module + /// in front of them, not to go hunting through its references. + /// + static IModule ResolveElsewhere(IDecompilerTypeSystem typeSystem, string idString) + { + try + { + var entity = IdStringProvider.FindEntity(idString, new SimpleTypeResolveContext(typeSystem.MainModule)); + if (entity != null && entity.ParentModule != typeSystem.MainModule) + return entity.ParentModule; + } + catch (ReflectionNameParseException) { - error = $"Member '{trimmed}' is defined in '{entity.ParentModule?.AssemblyName}', not in this module."; - return false; } - handle = entity.MetadataToken; - return true; + return null; } /// diff --git a/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs b/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs index f91ecdaa7..336935b52 100644 --- a/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs +++ b/ILSpy.Tests/Commands/CommandLineArgumentsTests.cs @@ -111,6 +111,86 @@ public class CommandLineArgumentsTests ((object?)vm.AssemblyTreeModel.SelectedItem).Should().BeNull(); } + [AvaloniaTest] + public async Task NavigateTo_Accepts_A_Member_Id_Without_Its_Signature() + { + // 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, 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); + + 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 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("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] + public async Task NavigateTo_Falls_Back_To_The_Loaded_Assembly_When_The_Id_Does_Not_Resolve() + { + // An ID that names nothing must not leave the tree on an empty selection with no + // indication of what happened: the assembly the user asked to open is still the best + // answer, and it is what opening it without --navigateto would have selected. + + // Arrange - boot, then ask to open an assembly the default list does not contain. + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); + vm.AssemblyTreeModel.SelectedItems.Clear(); + + string path = typeof(CommandLineArgumentsTests).Assembly.Location; + var args = CommandLineArguments.Create(new[] { path, "--navigateto", "M:No.Such.Type.NoSuchMember" }); + + // Act. + await vm.AssemblyTreeModel.HandleCommandLineArgumentsAsync(args); + + // Assert - the requested assembly is selected. + ((object?)vm.AssemblyTreeModel.SelectedItem).Should().NotBeNull( + "an unresolvable target must fall back to the assembly that was opened"); + vm.AssemblyTreeModel.SelectedItem!.GetType().Should().Be(typeof(AssemblyTreeNode)); + vm.AssemblyTreeModel.SelectedItem!.ToString().Should().Be(path); + } + [AvaloniaTest] public async Task NavigateTo_Skips_A_Missing_Session_Assembly_Instead_Of_Crashing() { @@ -140,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; } ///