From f2b51d2c341e7635fa48aa4387ccc49ded84b21a Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 4 Jul 2026 09:15:05 +0200 Subject: [PATCH] Fix #2078: Generic local functions not highlighted properly LocalFunctionMethod.MemberDefinition returned the instance itself, so the declaration of a generic local function (whose base method is an identity specialization) and its use sites (whose base methods carry the use-site substitutions) never compared equal, and click-highlighting could not group them. Returning a wrapper around the unspecialized base method lets the token writer record the same definition object for the declaration and all use sites. Method-group references to local functions additionally need their own lookup, because GetSymbol() does not surface a MethodGroupResolveResult. The token-writer tests now keep the PEFile alive for the duration of each test: recorded references are type-system entities that lazily read from the PE image, and formatting one in an assertion message after disposal crashed with an AccessViolationException. Assisted-by: Claude:claude-fable-5:Claude Code --- .../Output/TextTokenWriterTests.cs | 95 +++++++++++++++++-- .../Output/TextTokenWriter.cs | 19 +++- .../Implementation/LocalFunctionMethod.cs | 10 +- .../Editor/LocalReferenceHighlightTests.cs | 83 ++++++++++++++++ ILSpy/TextView/DecompilerTextView.axaml.cs | 3 + 5 files changed, 197 insertions(+), 13 deletions(-) create mode 100644 ILSpy.Tests/Editor/LocalReferenceHighlightTests.cs diff --git a/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs b/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs index dff12179a..b47aefb45 100644 --- a/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs +++ b/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs @@ -52,12 +52,37 @@ namespace ICSharpCode.Decompiler.Tests.Output public override void Method() { } } + // Exercises local-reference output for local functions: the definition and every use site + // (specialized generic invocations and method-group references) must be written with equal + // reference objects, otherwise click-highlighting in the UI cannot match them (issue #2078). + internal class GenericLocalFunctionHost + { + public int Use() + { + Func f = Identity; + return Identity(42) + Identity("a").Length + f(0); + + static T2 Identity(T2 value) => value; + } + + // Inside a generic method, Roslyn prepends the enclosing method's type parameters to the + // compiler-generated method backing the local function, so every use site references a + // specialized method even when the local function's own type argument is a type parameter. + public T UseInGenericMethod(T t) + { + return Echo(Echo(t)); + + static T2 Echo(T2 value) => value; + } + } + [TestFixture] public class TextTokenWriterTests { sealed class ReferenceRecordingOutput : ITextOutput { public readonly List<(string Text, IMember Member)> MemberReferences = new(); + public readonly List<(string Text, object Reference, bool IsDefinition)> LocalReferences = new(); public readonly List FoldStartDefaultCollapsed = new(); public int FoldEndCount; @@ -77,7 +102,10 @@ namespace ICSharpCode.Decompiler.Tests.Output MemberReferences.Add((text, member)); } - public void WriteLocalReference(string text, object reference, bool isDefinition = false) { } + public void WriteLocalReference(string text, object reference, bool isDefinition = false) + { + LocalReferences.Add((text, reference, isDefinition)); + } public void MarkFoldStart(string collapsedText = "...", bool defaultCollapsed = false, bool isDefinition = false) { @@ -90,11 +118,17 @@ namespace ICSharpCode.Decompiler.Tests.Output } } - static List<(string Text, IMember Member)> DecompileAndCollectMemberReferences(Type type) + // The module must stay alive until the caller is done with the recorded reference + // objects: they are type-system entities that lazily read from the PE image, e.g. + // when NUnit formats an assertion message. + static PEFile OpenTestAssembly() + { + return new PEFile(typeof(TextTokenWriterTests).Assembly.Location); + } + + static ReferenceRecordingOutput DecompileAndCollectReferences(PEFile module, Type type) { - string assemblyPath = typeof(TextTokenWriterTests).Assembly.Location; - using var module = new PEFile(assemblyPath); - var resolver = new UniversalAssemblyResolver(assemblyPath, false, module.Metadata.DetectTargetFrameworkId()); + var resolver = new UniversalAssemblyResolver(module.FileName, false, module.Metadata.DetectTargetFrameworkId()); var settings = new DecompilerSettings(); var decompiler = new CSharpDecompiler(module, resolver, settings); var syntaxTree = decompiler.DecompileType(new FullTypeName(type.FullName)); @@ -102,7 +136,7 @@ namespace ICSharpCode.Decompiler.Tests.Output var output = new ReferenceRecordingOutput(); var tokenWriter = new TextTokenWriter(output, settings); syntaxTree.AcceptVisitor(new CSharpOutputVisitor(tokenWriter, settings.CSharpFormattingOptions)); - return output.MemberReferences; + return output; } static string FormatMember(IMember member) => $"{member.DeclaringType?.Name}.{member.Name}"; @@ -110,7 +144,8 @@ namespace ICSharpCode.Decompiler.Tests.Output [Test] public void OverrideKeywordReferencesTheOverriddenBaseMember() { - var references = DecompileAndCollectMemberReferences(typeof(OverrideLinkDerived)); + using var module = OpenTestAssembly(); + var references = DecompileAndCollectReferences(module, typeof(OverrideLinkDerived)).MemberReferences; var overrideTargets = references.Where(r => r.Text == "override").Select(r => FormatMember(r.Member)); Assert.That(overrideTargets, Is.EquivalentTo(new[] { @@ -122,7 +157,8 @@ namespace ICSharpCode.Decompiler.Tests.Output [Test] public void OverrideKeywordReferencesTheNearestOverriddenMember() { - var references = DecompileAndCollectMemberReferences(typeof(OverrideLinkSecondLevel)); + using var module = OpenTestAssembly(); + var references = DecompileAndCollectReferences(module, typeof(OverrideLinkSecondLevel)).MemberReferences; var overrideTargets = references.Where(r => r.Text == "override").Select(r => FormatMember(r.Member)); Assert.That(overrideTargets, Is.EquivalentTo(new[] { @@ -133,7 +169,8 @@ namespace ICSharpCode.Decompiler.Tests.Output [Test] public void VirtualAndAbstractModifiersAreNotReferences() { - var references = DecompileAndCollectMemberReferences(typeof(OverrideLinkBase)); + using var module = OpenTestAssembly(); + var references = DecompileAndCollectReferences(module, typeof(OverrideLinkBase)).MemberReferences; Assert.That(references.Where(r => r.Text is "virtual" or "abstract" or "override"), Is.Empty); } @@ -192,5 +229,45 @@ namespace ICSharpCode.Decompiler.Tests.Output Assert.That(output.FoldStartDefaultCollapsed, Has.Count.EqualTo(1)); Assert.That(output.FoldEndCount, Is.EqualTo(1)); } + + [Test] + public void GenericLocalFunctionDefinitionAndUsesShareEqualReferenceObjects() + { + using var module = OpenTestAssembly(); + var output = DecompileAndCollectReferences(module, typeof(GenericLocalFunctionHost)); + + Assert.Multiple(() => { + // Two invocations (with different inferred type arguments) and one method-group reference. + AssertLocalFunctionReferenceGroup(output, "Identity", expectedUses: 3); + // Two nested invocations whose type argument is the enclosing method's type parameter. + AssertLocalFunctionReferenceGroup(output, "Echo", expectedUses: 2); + }); + } + + // Runs inside Assert.Multiple: after a recorded assertion failure, bail out of the checks + // that would dereference the missing data instead of throwing. + static void AssertLocalFunctionReferenceGroup(ReferenceRecordingOutput output, string name, int expectedUses) + { + var references = output.LocalReferences.Where(r => r.Text == name).ToList(); + + var definitions = references.Where(r => r.IsDefinition).ToList(); + Assert.That(definitions, Has.Count.EqualTo(1), $"{name}: definition count"); + if (definitions.Count != 1) + return; + object definition = definitions[0].Reference; + Assert.That(definition, Is.InstanceOf(), $"{name}: definition reference type"); + if (definition is not IMethod method) + return; + Assert.That(method.IsLocalFunction, Is.True, $"{name}: IsLocalFunction"); + + var uses = references.Where(r => !r.IsDefinition).ToList(); + Assert.That(uses, Has.Count.EqualTo(expectedUses), $"{name}: use count"); + foreach (var use in uses) + { + Assert.That(use.Reference, Is.EqualTo(definition), $"{name}: use equals definition"); + Assert.That(definition, Is.EqualTo(use.Reference), $"{name}: definition equals use"); + Assert.That(use.Reference.GetHashCode(), Is.EqualTo(definition.GetHashCode()), $"{name}: hash codes"); + } + } } } diff --git a/ICSharpCode.Decompiler/Output/TextTokenWriter.cs b/ICSharpCode.Decompiler/Output/TextTokenWriter.cs index 7e6007145..67cac8870 100644 --- a/ICSharpCode.Decompiler/Output/TextTokenWriter.cs +++ b/ICSharpCode.Decompiler/Output/TextTokenWriter.cs @@ -22,6 +22,7 @@ using System.Linq; using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.CSharp.OutputVisitor; +using ICSharpCode.Decompiler.CSharp.Resolver; using ICSharpCode.Decompiler.CSharp.Syntax; using ICSharpCode.Decompiler.IL; using ICSharpCode.Decompiler.Semantics; @@ -157,11 +158,23 @@ namespace ICSharpCode.Decompiler return method + gotoStatement.Label; } + // Local-function references are recorded with the unspecialized definition, so that + // generic use sites (which carry specialized methods) and the declaration all share + // equal reference objects and can be matched for highlighting. if (node.Slot?.Kind == Slots.TargetExpression && node.Parent is InvocationExpression) { var symbol = node.Parent.GetSymbol(); - if (symbol is LocalFunctionMethod) - return symbol; + if (symbol is LocalFunctionMethod localFunction) + return localFunction.MemberDefinition; + } + + // Method-group references to local functions (delegate conversions) are annotated + // with a MethodGroupResolveResult, which GetSymbol() does not surface. A local + // function cannot be overloaded, so such a group contains exactly one method. + if (node.GetResolveResult() is MethodGroupResolveResult methodGroup + && methodGroup.Methods.FirstOrDefault() is LocalFunctionMethod methodGroupLocalFunction) + { + return methodGroupLocalFunction.MemberDefinition; } return null; @@ -205,7 +218,7 @@ namespace ICSharpCode.Decompiler { var localFunction = node.Parent.GetResolveResult() as MemberResolveResult; if (localFunction != null) - return localFunction.Member; + return localFunction.Member.MemberDefinition; } return null; diff --git a/ICSharpCode.Decompiler/TypeSystem/Implementation/LocalFunctionMethod.cs b/ICSharpCode.Decompiler/TypeSystem/Implementation/LocalFunctionMethod.cs index 0652551bc..94dccf7f2 100644 --- a/ICSharpCode.Decompiler/TypeSystem/Implementation/LocalFunctionMethod.cs +++ b/ICSharpCode.Decompiler/TypeSystem/Implementation/LocalFunctionMethod.cs @@ -79,7 +79,15 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation internal bool IsStaticLocalFunction { get; } - public IMember MemberDefinition => this; + public IMember MemberDefinition { + get { + var baseMethodDefinition = (IMethod)baseMethod.MemberDefinition; + if (ReferenceEquals(baseMethodDefinition, baseMethod)) + return this; + return new LocalFunctionMethod(baseMethodDefinition, Name, IsStaticLocalFunction, + NumberOfCompilerGeneratedParameters, NumberOfCompilerGeneratedTypeParameters); + } + } public IType ReturnType => baseMethod.ReturnType; IEnumerable IMember.ExplicitlyImplementedInterfaceMembers => baseMethod.ExplicitlyImplementedInterfaceMembers; diff --git a/ILSpy.Tests/Editor/LocalReferenceHighlightTests.cs b/ILSpy.Tests/Editor/LocalReferenceHighlightTests.cs new file mode 100644 index 000000000..cc4802f16 --- /dev/null +++ b/ILSpy.Tests/Editor/LocalReferenceHighlightTests.cs @@ -0,0 +1,83 @@ +// 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.Linq; +using System.Threading.Tasks; + +using Avalonia.Headless.NUnit; +using Avalonia.VisualTree; + +using AwesomeAssertions; + +using ICSharpCode.Decompiler.TypeSystem; +using ICSharpCode.ILSpy.TextView; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.TextView; + +/// +/// Sample type decompiled by : a generic local +/// function inside a generic method, so that every use site carries a specialized method +/// instance while the declaration carries the unspecialized one (issue #2078). +/// +public class GenericLocalFunctionSample +{ + public T Run(T t) + { + return Echo(Echo(t)); + + static T2 Echo(T2 value) => value; + } +} + +/// +/// Pins the local-reference highlighting for local functions: clicking any occurrence of a +/// generic local function must paint the definition and every use site as one group. +/// +[TestFixture] +public class LocalReferenceHighlightTests +{ + [AvaloniaTest] + public async Task Clicking_A_Generic_Local_Function_Use_Highlights_Definition_And_All_Uses() + { + var (window, vm) = await TestHarness.BootAsync(); + await vm.OpenAssemblyAsync(typeof(GenericLocalFunctionSample).Assembly.Location); + var typeNode = vm.AssemblyTreeModel.FindNode( + "ILSpy.Tests", + "ICSharpCode.ILSpy.Tests.TextView", + "ICSharpCode.ILSpy.Tests.TextView.GenericLocalFunctionSample"); + vm.AssemblyTreeModel.SelectNode(typeNode); + var tab = await vm.DockWorkspace.WaitForDecompiledTextAsync(); + var view = window.GetVisualDescendants().OfType().First(); + + var localFunctionSegments = tab.References! + .Where(r => r.IsLocal && r.Reference is IMethod { IsLocalFunction: true }) + .ToList(); + localFunctionSegments.Should().HaveCount(3, "the local function has one definition and two calls"); + localFunctionSegments.Count(r => r.IsDefinition).Should().Be(1); + + var use = localFunctionSegments.First(r => !r.IsDefinition); + view.OnReferenceClicked(use); + + view.LocalReferenceMarks.Select(m => m.StartOffset).Should().BeEquivalentTo( + localFunctionSegments.Select(s => s.StartOffset), + "clicking a use must highlight the definition and every use"); + } +} diff --git a/ILSpy/TextView/DecompilerTextView.axaml.cs b/ILSpy/TextView/DecompilerTextView.axaml.cs index 4e6fc469b..a7dc507cf 100644 --- a/ILSpy/TextView/DecompilerTextView.axaml.cs +++ b/ILSpy/TextView/DecompilerTextView.axaml.cs @@ -348,6 +348,9 @@ namespace ICSharpCode.ILSpy.TextView /// document, so an HTML copy can include it on top of the xshd syntax colours. internal RichTextModel? SemanticHighlightingModel => boundModel?.HighlightingModel; + /// The currently painted local-reference highlight marks (test observability). + internal IReadOnlyList LocalReferenceMarks => localReferenceMarks; + // ThemeManager.Current is a process-lived singleton, so subscribing to its ThemeChanged in // the constructor and never detaching would root every DecompilerTextView for the lifetime of // the process -- one leaked view per decompiler tab. Bind the handler to the visual-tree