Browse Source

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
pull/3852/head
Siegfried Pammer 3 days ago committed by Siegfried Pammer
parent
commit
f2b51d2c34
  1. 95
      ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs
  2. 19
      ICSharpCode.Decompiler/Output/TextTokenWriter.cs
  3. 10
      ICSharpCode.Decompiler/TypeSystem/Implementation/LocalFunctionMethod.cs
  4. 83
      ILSpy.Tests/Editor/LocalReferenceHighlightTests.cs
  5. 3
      ILSpy/TextView/DecompilerTextView.axaml.cs

95
ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs

@ -52,12 +52,37 @@ namespace ICSharpCode.Decompiler.Tests.Output @@ -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<int, int> f = Identity<int>;
return Identity(42) + Identity("a").Length + f(0);
static T2 Identity<T2>(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 t)
{
return Echo(Echo(t));
static T2 Echo<T2>(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<bool> FoldStartDefaultCollapsed = new();
public int FoldEndCount;
@ -77,7 +102,10 @@ namespace ICSharpCode.Decompiler.Tests.Output @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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<IMethod>(), $"{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");
}
}
}
}

19
ICSharpCode.Decompiler/Output/TextTokenWriter.cs

@ -22,6 +22,7 @@ using System.Linq; @@ -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 @@ -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 @@ -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;

10
ICSharpCode.Decompiler/TypeSystem/Implementation/LocalFunctionMethod.cs

@ -79,7 +79,15 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation @@ -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> IMember.ExplicitlyImplementedInterfaceMembers => baseMethod.ExplicitlyImplementedInterfaceMembers;

83
ILSpy.Tests/Editor/LocalReferenceHighlightTests.cs

@ -0,0 +1,83 @@ @@ -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;
/// <summary>
/// Sample type decompiled by <see cref="LocalReferenceHighlightTests"/>: 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).
/// </summary>
public class GenericLocalFunctionSample
{
public T Run<T>(T t)
{
return Echo(Echo(t));
static T2 Echo<T2>(T2 value) => value;
}
}
/// <summary>
/// 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.
/// </summary>
[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<TypeTreeNode>(
"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<DecompilerTextView>().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");
}
}

3
ILSpy/TextView/DecompilerTextView.axaml.cs

@ -348,6 +348,9 @@ namespace ICSharpCode.ILSpy.TextView @@ -348,6 +348,9 @@ namespace ICSharpCode.ILSpy.TextView
/// document, so an HTML copy can include it on top of the xshd syntax colours.</summary>
internal RichTextModel? SemanticHighlightingModel => boundModel?.HighlightingModel;
/// <summary>The currently painted local-reference highlight marks (test observability).</summary>
internal IReadOnlyList<TextMarker> 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

Loading…
Cancel
Save