From 640b968b59771645caf4ebf93424c6660adca883 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 16 Jul 2026 09:54:24 +0200 Subject: [PATCH] Rebuild the assembly-tree namespaces eagerly, as the WPF host did With "Use nested namespace structure" enabled, most namespaces never appeared in the tree, and the first expand of a large assembly lagged. Both come from the same regression: the Avalonia assembly-tree nodes were written from scratch as lazy scaffolding, not ported from the WPF design, and lost the single eager build the WPF host used. A NamespaceTreeNode filters as Recurse/MatchAndRecurse, so the filter cascade computes its IsHidden as "all children hidden" -- vacuously true for an empty child set. The lazy build attached each namespace node while it was still empty, latching intermediate namespaces (those that hold only sub-namespaces, e.g. System.Collections) hidden and stranding everything beneath them. The cascade also force-loads every namespace node's children anyway, so the per-node laziness avoided no work: it rescanned the whole TypeDefinitions table once per namespace node. Restore release/10.1's structure: AssemblyTreeNode builds the entire namespace band in one pass over the module's top-level types, populates each node before attaching it, and keeps two indexes -- full namespace name -> node and type handle -> node -- so FindNamespaceNode/FindTypeNode are O(1) and correct at any nesting depth. TreeNodeLocator.FindTypeNode (hyperlink clicks, search activation, JumpToType) delegates to that index instead of walking children by display name, which never matched in nested mode. NamespaceTreeNode goes back to a dumb label holder and re-escapes its display label via ILAmbience.EscapeName. The band is built from the module's type system, like 10.1's, not from raw metadata: each TypeTreeNode holds the resolved ITypeDefinition it renders from, so painting a cell no longer re-enters the settings-keyed type-system cache the way master's lazy node did on every Text/Icon/ Filter read -- each of which rebuilt an effective-settings object and took its lock. Ordering the pass by full ReflectionName is also what interleaves a namespace's types and its sub-namespaces into one alphabetical run (a sub-namespace attaches when its first descendant type is reached, landing at its own alphabetical slot among the sibling types); grouping all types ahead of all namespaces was a visible departure from the WPF order. Two deliberate departures from a literal 10.1 copy: keep the global- namespace "-" node, and keep the cached IsPublicAPI getter. Both index dictionaries are cleared on rebuild so a nested/flat toggle leaves no stale entries. Holding resolved entities means the tree has to be rebuilt when a setting changes the type system. Only one compilation is cached per module, keyed on the effective decompiler settings, so a language- version or decompiler-option change drops it and would otherwise leave every node pointing at a discarded compilation -- stale labels, icons and filters, and the C# 14 extension-block nodes shown against the wrong version. AssemblyTreeModel reloads the loaded assemblies when the computed TypeSystemOptions actually change (Display-only settings never do, and cost nothing), then restores the selected node from its path -- which re-expands its ancestors on the way to revealing it -- the way Refresh does. The WPF host got this for free: its modal Options dialog rebuilt the tree on close, where the Avalonia page applies live. Assisted-by: Claude:claude-opus-4-8:Claude Code --- .../AssemblyList/NamespaceTreeNodeTests.cs | 63 ++++ .../AssemblyList/NestedNamespaceTreeTests.cs | 268 ++++++++++++++++++ .../AssemblyList/TypeSystemStalenessTests.cs | 119 ++++++++ ILSpy/AssemblyTree/AssemblyTreeModel.cs | 42 +++ ILSpy/AssemblyTree/TreeNodeLocator.cs | 13 +- ILSpy/TreeNodes/AssemblyTreeNode.cs | 122 ++++---- ILSpy/TreeNodes/NamespaceTreeNode.cs | 39 +-- ILSpy/TreeNodes/TypeTreeNode.cs | 74 ++--- 8 files changed, 604 insertions(+), 136 deletions(-) create mode 100644 ILSpy.Tests/AssemblyList/NamespaceTreeNodeTests.cs create mode 100644 ILSpy.Tests/AssemblyList/NestedNamespaceTreeTests.cs create mode 100644 ILSpy.Tests/AssemblyList/TypeSystemStalenessTests.cs diff --git a/ILSpy.Tests/AssemblyList/NamespaceTreeNodeTests.cs b/ILSpy.Tests/AssemblyList/NamespaceTreeNodeTests.cs new file mode 100644 index 000000000..045a999cd --- /dev/null +++ b/ILSpy.Tests/AssemblyList/NamespaceTreeNodeTests.cs @@ -0,0 +1,63 @@ +// 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.Threading.Tasks; + +using Avalonia.Headless.NUnit; + +using ICSharpCode.ILSpy.AssemblyTree; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests; + +[TestFixture] +public class NamespaceTreeNodeTests +{ + [AvaloniaTest] + public async Task Text_Escapes_Characters_That_Cannot_Be_Displayed() + { + // Namespace names come straight out of the metadata string heap, which permits whitespace + // and control characters that would corrupt the tree row if rendered raw. The label is + // escaped for display; Name and FullName stay raw because they are the lookup keys used + // against metadata. + + var (_, vm) = await TestHarness.BootAsync(3); + var module = vm.AssemblyTreeModel.FindNode("System.Linq") + .LoadedAssembly.GetMetadataFileOrNull()!; + + var node = new NamespaceTreeNode("Weird\tNamespace", module); + + Assert.That(node.Text.ToString(), Is.EqualTo("Weird\\u0009Namespace")); + Assert.That(node.Name, Is.EqualTo("Weird\tNamespace"), "Name stays raw -- it is a lookup key"); + Assert.That(node.FullName, Is.EqualTo("Weird\tNamespace"), "FullName stays raw -- it is a lookup key"); + } + + [AvaloniaTest] + public async Task Text_Renders_The_Global_Namespace_As_Dash() + { + var (_, vm) = await TestHarness.BootAsync(3); + var module = vm.AssemblyTreeModel.FindNode("System.Linq") + .LoadedAssembly.GetMetadataFileOrNull()!; + + var node = new NamespaceTreeNode(string.Empty, module); + + Assert.That(node.Text.ToString(), Is.EqualTo("-")); + } +} diff --git a/ILSpy.Tests/AssemblyList/NestedNamespaceTreeTests.cs b/ILSpy.Tests/AssemblyList/NestedNamespaceTreeTests.cs new file mode 100644 index 000000000..2bf7c53a9 --- /dev/null +++ b/ILSpy.Tests/AssemblyList/NestedNamespaceTreeTests.cs @@ -0,0 +1,268 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +using Avalonia.Headless.NUnit; + +using ICSharpCode.Decompiler.TypeSystem; +using ICSharpCode.ILSpyX; + +using ICSharpCode.ILSpy; +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.AssemblyTree; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests; + +/// +/// Nested-namespace mode ("Use nested namespace structure"): the tree must expose every namespace, +/// and the namespace/type lookup primitives must resolve at any nesting depth. +/// +[TestFixture] +public class NestedNamespaceTreeTests +{ + /// + /// Expands an assembly with nested-namespace mode on. The assembly node is expanded first so it + /// is a realised, visible row: the filter cascade only runs for children of a visible parent, and + /// that cascade is what computes . + /// + static async Task<(AssemblyTreeModel Model, AssemblyTreeNode Assembly)> BootNestedAsync(string assemblyName) + { + var (window, vm) = await TestHarness.BootAsync(3); + var pane = await window.WaitForComponent(); + await pane.WaitForComponent(); + + AppComposition.Current.GetExport().DisplaySettings.UseNestedNamespaceNodes = true; + + var assemblyNode = vm.AssemblyTreeModel.FindNode(assemblyName); + assemblyNode.IsExpanded = true; + assemblyNode.EnsureLazyChildren(); + return (vm.AssemblyTreeModel, assemblyNode); + } + + static void ResetNestedMode() + => AppComposition.Current.GetExport().DisplaySettings.UseNestedNamespaceNodes = false; + + static IEnumerable DescendantNamespaces(NamespaceTreeNode node) + { + yield return node; + foreach (var child in node.Children.OfType()) + { + foreach (var descendant in DescendantNamespaces(child)) + yield return descendant; + } + } + + [AvaloniaTest] + public async Task Nested_Mode_Does_Not_Hide_Namespaces_That_Hold_Only_Child_Namespaces() + { + // A namespace that contains no types of its own but does contain sub-namespaces -- e.g. + // "System.Collections" in an assembly that only ships "System.Collections.Generic" types -- + // is a pure intermediate node. It must still be shown: hiding it makes every namespace + // underneath it unreachable in the tree, which is what "not all namespaces are showing" + // looks like to the user. + + try + { + var (_, assemblyNode) = await BootNestedAsync("System.Linq"); + + var intermediates = assemblyNode.Children.OfType() + .SelectMany(DescendantNamespaces) + .Where(ns => ns.Children.OfType().Any()) + .ToList(); + + Assert.That(intermediates, Is.Not.Empty, + "the fixture assembly must contain at least one namespace with sub-namespaces, " + + "otherwise this test asserts nothing"); + + var hidden = intermediates.Where(ns => ns.IsHidden).Select(ns => ns.FullName).ToList(); + Assert.That(hidden, Is.Empty, + "namespaces holding sub-namespaces must stay visible; hiding them strands every " + + "namespace below them"); + } + finally + { + ResetNestedMode(); + } + } + + [AvaloniaTest] + public async Task Nested_Mode_Interleaves_Types_And_Sub_Namespaces_Alphabetically() + { + // A namespace holding both types and sub-namespaces lists them as one alphabetical + // sequence -- "Collections" (namespace) sits between "Buffers" and "Console" (types) -- + // rather than grouping all types ahead of all namespaces. The band is built in a single + // pass over the types ordered by full name, and a sub-namespace node is attached when its + // first descendant is reached, which lands it at its own alphabetical position. + + try + { + var (_, assemblyNode) = await BootNestedAsync(TreeNavigation.CoreLibName); + + var system = assemblyNode.Children.OfType() + .SelectMany(DescendantNamespaces) + .Single(ns => ns.FullName == "System"); + + // Without both kinds present there is no interleaving to observe and the ordering + // assertion below would hold vacuously. + Assert.That(system.Children.OfType(), Is.Not.Empty, + "the fixture's System namespace must declare types of its own"); + Assert.That(system.Children.OfType(), Is.Not.Empty, + "the fixture's System namespace must contain sub-namespaces"); + + // ToString() is the ordering key on both node kinds: ReflectionName for a type, + // the full dotted path for a namespace. + var keys = system.Children.Select(child => child.ToString()).ToList(); + Assert.That(keys, Is.Ordered.Using(NaturalStringComparer.Instance), + "types and sub-namespaces share one alphabetical sequence; grouping the types " + + "ahead of the namespaces breaks the ordering the WPF host had"); + } + finally + { + ResetNestedMode(); + } + } + + [AvaloniaTest] + public async Task FindNamespaceNode_Resolves_A_Nested_Namespace() + { + // FindNamespaceNode is the lookup primitive behind "--navigateto N:..." and namespace + // navigation. It takes a full dotted name and must resolve it at any depth -- in nested mode + // the node for "System.Collections.Generic" is three levels down, not an assembly child. + + try + { + var (_, assemblyNode) = await BootNestedAsync("System.Linq"); + + var ns = assemblyNode.FindNamespaceNode("System.Collections.Generic"); + + Assert.That(ns, Is.Not.Null, + "a full dotted namespace name must resolve in nested mode, where the matching node " + + "is a descendant rather than a direct child of the assembly node"); + Assert.That(ns!.FullName, Is.EqualTo("System.Collections.Generic")); + Assert.That(ns.Name, Is.EqualTo("Generic"), + "in nested mode the display label is the last segment only"); + } + finally + { + ResetNestedMode(); + } + } + + [AvaloniaTest] + public async Task FindTypeNode_Resolves_A_Type_In_A_Nested_Namespace() + { + // FindTypeNode backs hyperlink clicks, search-result activation and JumpToType. A type whose + // namespace has more than one segment must resolve in nested mode too. + + try + { + var (_, assemblyNode) = await BootNestedAsync("System.Linq"); + + var module = assemblyNode.LoadedAssembly.GetMetadataFileOrNull()!; + var typeSystem = (MetadataModule)module.GetTypeSystemOrNull()!.MainModule; + var nestedType = typeSystem.TopLevelTypeDefinitions + .First(t => t.Namespace.Contains('.') && t.Namespace != "System.Linq"); + + var node = assemblyNode.FindTypeNode(nestedType); + + Assert.That(node, Is.Not.Null, + $"the tree node for '{nestedType.ReflectionName}' must resolve in nested mode"); + Assert.That(node!.Handle, + Is.EqualTo((System.Reflection.Metadata.TypeDefinitionHandle)nestedType.MetadataToken)); + } + finally + { + ResetNestedMode(); + } + } + + [AvaloniaTest] + public async Task FindTreeNode_Resolves_A_Type_Reference_In_A_Nested_Namespace() + { + // The reference-to-node lookup behind hyperlink clicks in the decompiler view, search-result + // activation and JumpToType. It goes through TreeNodeLocator rather than calling + // AssemblyTreeNode.FindTypeNode directly, so it needs its own coverage in nested mode. + + try + { + var (model, assemblyNode) = await BootNestedAsync("System.Linq"); + + var module = assemblyNode.LoadedAssembly.GetMetadataFileOrNull()!; + var typeSystem = (MetadataModule)module.GetTypeSystemOrNull()!.MainModule; + var nestedType = typeSystem.TopLevelTypeDefinitions + .First(t => t.Namespace.Contains('.') && t.Namespace != "System.Linq"); + + var node = model.FindTreeNode(nestedType); + + Assert.That(node, Is.InstanceOf(), + $"a reference to '{nestedType.ReflectionName}' must resolve to its tree node in nested mode"); + Assert.That(((TypeTreeNode)node!).Handle, + Is.EqualTo((System.Reflection.Metadata.TypeDefinitionHandle)nestedType.MetadataToken)); + } + finally + { + ResetNestedMode(); + } + } + + [AvaloniaTest] + public async Task Nested_Namespace_Is_Hidden_When_All_Of_Its_Types_Are_Filtered_Out() + { + // Nested namespace nodes stay subject to the filter cascade: a namespace whose types are all + // filtered out by ShowApiLevel must disappear along with them, rather than linger as an empty + // row. Opting namespace nodes out of the cascade (by reporting FilterResult.Match) would make + // the display bug go away too, but at the cost of this behaviour -- hence the coverage. + + var settings = AppComposition.Current.GetExport().SessionSettings.LanguageSettings; + var original = settings.ShowApiLevel; + try + { + var (_, assemblyNode) = await BootNestedAsync("System.Linq"); + + // FxResources.* holds only internal resource-string types, so PublicOnly empties it. + var fxResources = assemblyNode.Children.OfType() + .SingleOrDefault(ns => ns.FullName == "FxResources"); + Assert.That(fxResources, Is.Not.Null, + "the fixture assembly must expose the non-public FxResources namespace in nested mode"); + Assert.That(fxResources!.IsHidden, Is.False, "it is visible while every API level is shown"); + + settings.ShowApiLevel = ApiVisibility.PublicOnly; + assemblyNode.RefreshRealizedFilter(); + + Assert.That(fxResources.IsHidden, Is.True, + "a namespace left with no visible types must be hidden by the filter cascade"); + + settings.ShowApiLevel = ApiVisibility.All; + assemblyNode.RefreshRealizedFilter(); + + Assert.That(fxResources.IsHidden, Is.False, + "and it must come back when the API level is widened again"); + } + finally + { + settings.ShowApiLevel = original; + ResetNestedMode(); + } + } +} diff --git a/ILSpy.Tests/AssemblyList/TypeSystemStalenessTests.cs b/ILSpy.Tests/AssemblyList/TypeSystemStalenessTests.cs new file mode 100644 index 000000000..a154ab776 --- /dev/null +++ b/ILSpy.Tests/AssemblyList/TypeSystemStalenessTests.cs @@ -0,0 +1,119 @@ +// 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 ICSharpCode.Decompiler.TypeSystem; + +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.AssemblyTree; +using ICSharpCode.ILSpy.Languages; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests; + +/// +/// The tree resolves each type once, when its assembly's namespace band is built, and holds the +/// resulting entity. Those entities belong to one compilation, and only one is cached per module -- +/// keyed on the effective decompiler settings. Anything that changes those settings drops the cached +/// compilation, so the tree has to re-resolve or it is left holding entities from a compilation that +/// no longer exists. +/// +[TestFixture] +public class TypeSystemStalenessTests +{ + static TypeTreeNode FirstTypeNode(AssemblyTreeNode assembly) + => assembly.Children.OfType() + .SelectMany(ns => ns.Children) + .OfType() + .First(); + + [AvaloniaTest] + public async Task Changing_The_Language_Version_Re_Resolves_The_Tree_Against_The_New_Type_System() + { + var (_, vm) = await TestHarness.BootAsync(3); + var languageService = AppComposition.Current.GetExport(); + var originalVersion = languageService.CurrentVersion; + + try + { + var assembly = vm.AssemblyTreeModel.FindNode(TreeNavigation.CoreLibName); + assembly.IsExpanded = true; + assembly.EnsureLazyChildren(); + + var before = FirstTypeNode(assembly).TypeDefinition.Compilation; + + // C# 1 switches off the language features that map onto TypeSystemOptions (dynamic, + // tuples, nullable annotations, ...), so the module's cached compilation is rebuilt. + var oldest = languageService.CurrentLanguage.LanguageVersions.First(); + Assert.That(oldest, Is.Not.EqualTo(originalVersion), + "the fixture language must offer a version other than the active one, or this test " + + "changes nothing"); + languageService.CurrentVersion = oldest; + + var after = FirstTypeNode(assembly).TypeDefinition.Compilation; + Assert.That(after, Is.Not.SameAs(before), + "the tree must re-resolve its entities after the language version changes the effective " + + "decompiler settings; holding the old compilation's entities leaves every label, icon " + + "and filter rendering against a type system the rest of the app has already dropped"); + } + finally + { + languageService.CurrentVersion = originalVersion; + } + } + + [AvaloniaTest] + public async Task Rebuilding_After_A_Language_Version_Change_Restores_The_Selected_Node() + { + var (_, vm) = await TestHarness.BootAsync(3); + var languageService = AppComposition.Current.GetExport(); + var originalVersion = languageService.CurrentVersion; + + try + { + var model = vm.AssemblyTreeModel; + var assembly = model.FindNode(TreeNavigation.CoreLibName); + assembly.IsExpanded = true; + assembly.EnsureLazyChildren(); + + var selected = FirstTypeNode(assembly); + model.SelectedItem = selected; + var pathBefore = AssemblyTreeModel.GetPathForNode(selected); + + languageService.CurrentVersion = languageService.CurrentLanguage.LanguageVersions.First(); + + // The rebuild replaces the node objects, so identity cannot survive -- the path must. + Assert.That(model.SelectedItem, Is.Not.Null, + "the rebuild must not drop the selection: the node the user was looking at has to come back"); + Assert.That(AssemblyTreeModel.GetPathForNode(model.SelectedItem), Is.EqualTo(pathBefore), + "the same node, identified by path, must be selected again after the tree is rebuilt"); + Assert.That(model.SelectedItem, Is.Not.SameAs(selected), + "and it must be the freshly resolved node, not the one holding the dropped compilation's entity"); + } + finally + { + languageService.CurrentVersion = originalVersion; + } + } +} diff --git a/ILSpy/AssemblyTree/AssemblyTreeModel.cs b/ILSpy/AssemblyTree/AssemblyTreeModel.cs index d0aab0cd1..1f6a27edd 100644 --- a/ILSpy/AssemblyTree/AssemblyTreeModel.cs +++ b/ILSpy/AssemblyTree/AssemblyTreeModel.cs @@ -153,6 +153,10 @@ namespace ICSharpCode.ILSpy.AssemblyTree this.settingsService = settingsService; this.languageService = languageService; languageService.PropertyChanged += (_, e) => { + // The language version feeds the effective decompiler settings, and through those the + // type system the tree's nodes hold their entities from. + if (e.PropertyName is nameof(LanguageService.CurrentLanguage) or nameof(LanguageService.CurrentVersion)) + RebuildIfTypeSystemOptionsChanged(); if (e.PropertyName == nameof(LanguageService.CurrentLanguage) && Root != null) NotifyTextChanged(Root); }; @@ -252,8 +256,42 @@ namespace ICSharpCode.ILSpy.AssemblyTree NotifyTextChanged(child); } + // The type system each tree node resolved its entity from is keyed on the effective decompiler + // settings, and only one is cached per module: the moment those options change, the cached + // compilation is dropped and rebuilt, leaving every node holding an entity from a compilation + // that no longer exists. Rebuilding the loaded assembly nodes re-resolves them against the new + // one. Keyed on the options rather than the settings themselves because the Options page is + // live-apply -- most toggles (and every Display setting) leave the type system alone, and those + // must not cost a rebuild. + TypeSystemOptions? lastTypeSystemOptions; + + void RebuildIfTypeSystemOptionsChanged() + { + if (Root == null) + return; + var options = DecompilerTypeSystem.GetOptions(settingsService.CreateEffectiveDecompilerSettings()); + if (lastTypeSystemOptions == options) + return; + lastTypeSystemOptions = options; + + // The rebuild replaces every node below an assembly, leaving the selection pointing at one + // that is no longer in the tree. Re-establish it from its path, the way Refresh does. That + // restores the expansion too: revealing the selected node expands its ancestors on the way + // to centring it. + var path = GetPathForNode(SelectedItem); + foreach (var assembly in Root.Children.OfType()) + assembly.ReloadChildren(); + OnPropertyChanged(nameof(Root)); + if (path is { Length: > 0 }) + SelectNode(FindNodeByPath(path, returnBestMatch: true)); + } + void OnSettingsChanged(object? sender, Util.SettingsChangedEventArgs e) { + // A decompiler option can change the type system the tree's entities came from; the + // Display buckets below never do. + if (sender is Decompiler.DecompilerSettings or Options.DisplaySettings) + RebuildIfTypeSystemOptionsChanged(); if (sender is not Options.DisplaySettings) return; if (Root == null) @@ -502,6 +540,10 @@ namespace ICSharpCode.ILSpy.AssemblyTree using (AppEnv.AppLog.Phase("new AssemblyListTreeNode")) assemblyListTreeNode = new AssemblyListTreeNode(list); Root = assemblyListTreeNode; + // Baseline for RebuildIfTypeSystemOptionsChanged: whatever this tree's nodes will resolve + // their entities against. Recorded here rather than on the first settings change, so that a + // change arriving before any other has something to compare against. + lastTypeSystemOptions = DecompilerTypeSystem.GetOptions(settingsService.CreateEffectiveDecompilerSettings()); AppEnv.AppLog.Mark("Root assigned"); ScheduleBackgroundLoadSweep(list); } diff --git a/ILSpy/AssemblyTree/TreeNodeLocator.cs b/ILSpy/AssemblyTree/TreeNodeLocator.cs index 98f5791fb..693fe68ba 100644 --- a/ILSpy/AssemblyTree/TreeNodeLocator.cs +++ b/ILSpy/AssemblyTree/TreeNodeLocator.cs @@ -170,20 +170,15 @@ namespace ICSharpCode.ILSpy.AssemblyTree .FirstOrDefault(a => a.LoadedAssembly.GetMetadataFileOrNull() == module); if (assembly == null) return null; - assembly.EnsureLazyChildren(); var nesting = new Stack(); for (var current = type; current != null; current = current.DeclaringTypeDefinition) nesting.Push(current); - var top = nesting.Pop(); - var ns = assembly.Children.OfType() - .FirstOrDefault(n => n.Name == (top.Namespace ?? string.Empty)); - if (ns == null) - return null; - ns.EnsureLazyChildren(); - var typeNode = ns.Children.OfType() - .FirstOrDefault(t => t.Handle == top.MetadataToken); + // The assembly node indexes every top-level type it built, so this resolves regardless of + // how deep the type's namespace nests. Nested types are not in that index -- they are + // loaded lazily by their declaring type's node -- so walk the remaining chain by handle. + var typeNode = assembly.FindTypeNode(nesting.Pop()); while (typeNode != null && nesting.Count > 0) { typeNode.EnsureLazyChildren(); diff --git a/ILSpy/TreeNodes/AssemblyTreeNode.cs b/ILSpy/TreeNodes/AssemblyTreeNode.cs index 942de3377..651016973 100644 --- a/ILSpy/TreeNodes/AssemblyTreeNode.cs +++ b/ILSpy/TreeNodes/AssemblyTreeNode.cs @@ -43,14 +43,27 @@ using ICSharpCode.ILSpy.AppEnv; using ICSharpCode.ILSpy.Controls.TreeView; using ICSharpCode.ILSpy.Languages; +using TypeDefinitionHandle = System.Reflection.Metadata.TypeDefinitionHandle; + namespace ICSharpCode.ILSpy.TreeNodes { + /// + /// Tree node representing an assembly. + /// This class is responsible for loading both namespace and type nodes. + /// public sealed class AssemblyTreeNode : ILSpyTreeNode, IRichTextNode { readonly LoadedAssembly assembly; string? loadError; MetadataFile? cachedModule; + // Full (unescaped) namespace name -> node, and type handle -> node. Both are filled by the + // single pass in LoadChildren and are what make the Find* lookups O(1) and correct at any + // nesting depth: in nested-namespace mode the node for "System.Collections.Generic" is a + // descendant, not a child, so walking Children by name cannot find it. + readonly Dictionary namespaces = new(StringComparer.Ordinal); + readonly Dictionary typeDict = new(); + public LoadedAssembly LoadedAssembly => assembly; /// @@ -353,31 +366,25 @@ namespace ICSharpCode.ILSpy.TreeNodes public override bool IsAutoLoaded => assembly.IsAutoLoaded; /// - /// Finds the for the given namespace string, or - /// null if no children are loaded yet or the namespace has no top-level types - /// in this assembly. + /// Finds the for the given full (unescaped) namespace + /// string, at any nesting depth, or null if the namespace has no top-level types in + /// this assembly. The empty string resolves to the global-namespace node. /// public NamespaceTreeNode? FindNamespaceNode(string namespaceName) { ArgumentNullException.ThrowIfNull(namespaceName); EnsureLazyChildren(); - return Children.OfType().FirstOrDefault(ns => ns.Name == namespaceName); + return namespaces.GetValueOrDefault(namespaceName); } /// /// Finds the for the given top-level type definition. - /// Walks the assembly's namespaces (loading them as needed) and matches by - /// . /// public TypeTreeNode? FindTypeNode(ITypeDefinition type) { ArgumentNullException.ThrowIfNull(type); - var ns = FindNamespaceNode(type.Namespace); - if (ns == null) - return null; - ns.EnsureLazyChildren(); - var handle = (System.Reflection.Metadata.TypeDefinitionHandle)type.MetadataToken; - return ns.Children.OfType().FirstOrDefault(n => n.Handle == handle); + EnsureLazyChildren(); + return typeDict.GetValueOrDefault((TypeDefinitionHandle)type.MetadataToken); } protected override void LoadChildren() @@ -438,56 +445,67 @@ namespace ICSharpCode.ILSpy.TreeNodes if (module.Resources.Any()) Children.Add(new ResourceListTreeNode(module)); - var metadata = module.Metadata; - // Every top-level namespace string in the module — INCLUDING the empty string for - // types declared at module scope. The empty namespace becomes a NamespaceTreeNode - // whose Text renders as "-"; without that path, global-namespace types (every PE's - // pseudo-type plus any user-declared ones) would have no parent node to - // live under, and the long-standing tree shape would break. - var namespaces = metadata.TypeDefinitions - .Where(t => metadata.GetTypeDefinition(t).GetDeclaringType().IsNil) - .Select(t => metadata.GetString(metadata.GetTypeDefinition(t).Namespace)) - .Distinct() - .OrderBy(ns => ns, NaturalStringComparer.Instance); - - if (TryGetUseNestedNamespaceNodes()) - { - // Build the nested chain: every dotted namespace string becomes a node whose - // parent is the namespace one segment shorter. Intermediate ancestors that - // don't appear in the namespaces list themselves (e.g. an assembly that has - // "System.Collections.Generic" but no types directly in "System") still get - // created — EnsureNested walks the parent chain. - var byFullName = new Dictionary(StringComparer.Ordinal); - foreach (var ns in namespaces) - EnsureNested(ns, byFullName); - } - else - { - foreach (var ns in namespaces) - Children.Add(new NamespaceTreeNode(ns, module)); - } + namespaces.Clear(); + typeDict.Clear(); + bool useNestedStructure = TryGetUseNestedNamespaceNodes(); - NamespaceTreeNode EnsureNested(string fullNs, Dictionary byFullName) + // The band is built from the type system rather than raw metadata: every TypeTreeNode is + // handed the resolved ITypeDefinition it renders from, so painting a cell never has to + // re-enter the settings-keyed type-system cache. Resolving the module's types is what the + // tree ends up doing anyway the moment a namespace is expanded. + if (module.GetTypeSystemWithCurrentOptionsOrNull()?.MainModule is not MetadataModule mainModule) + return; + + // One pass over the module's top-level types builds the entire namespace band, ordered by + // full reflection name. Sorting by the full name is what interleaves a namespace's types + // and its sub-namespaces into one alphabetical run: a sub-namespace node is attached the + // moment its first descendant type is reached, which lands it at its own alphabetical + // position among the sibling types. + // + // Every namespace string is represented — INCLUDING the empty string for types declared at + // module scope, which becomes a node whose Text renders as "-"; without it the global + // namespace's types (every PE's pseudo-type plus any user-declared ones) would + // have no parent node and the long-standing tree shape would break. + foreach (var type in mainModule.TopLevelTypeDefinitions + .OrderBy(t => t.ReflectionName, NaturalStringComparer.Instance)) + { + var namespaceNode = GetOrCreateNamespaceTreeNode(type.Namespace); + var typeNode = new TypeTreeNode(type, module); + typeDict[(TypeDefinitionHandle)type.MetadataToken] = typeNode; + namespaceNode.Children.Add(typeNode); + } + + // Attach the roots last, once they are fully populated. The filter cascade computes a + // node's IsHidden as "all children are hidden", which is vacuously true for an empty + // child collection — so a node attached while still empty latches hidden, stranding + // every namespace below it. + var roots = namespaces.Values + .Where(ns => ns.Children.Count > 0 && ns.Parent == null) + .OrderBy(ns => ns.Name, NaturalStringComparer.Instance) + .ToList(); + foreach (var ns in roots) + Children.Add(ns); + + NamespaceTreeNode GetOrCreateNamespaceTreeNode(string namespaceName) { - if (byFullName.TryGetValue(fullNs, out var existing)) + if (namespaces.TryGetValue(namespaceName, out var existing)) return existing; - int dot = fullNs.LastIndexOf('.'); NamespaceTreeNode node; - if (dot < 0) + int lastDot = useNestedStructure ? namespaceName.LastIndexOf('.') : -1; + if (lastDot < 0) { - // Top-level: display equals full name. The empty-namespace case lands here - // too, mapping to the "-" display. - node = new NamespaceTreeNode(fullNs, module); - Children.Add(node); + // Flat mode, or a single-segment namespace: the display label is the full name. + node = new NamespaceTreeNode(namespaceName, module); } else { - var parent = EnsureNested(fullNs.Substring(0, dot), byFullName); - var displayName = fullNs.Substring(dot + 1); - node = new NamespaceTreeNode(displayName, fullNs, module); + // Nested mode: hang the node off the namespace one segment shorter, creating + // that ancestor first if the module declares no types directly in it. + var parent = GetOrCreateNamespaceTreeNode(namespaceName.Substring(0, lastDot)); + node = new NamespaceTreeNode(namespaceName.Substring(lastDot + 1), namespaceName, module); parent.Children.Add(node); } - byFullName[fullNs] = node; + namespaces.Add(namespaceName, node); return node; } } diff --git a/ILSpy/TreeNodes/NamespaceTreeNode.cs b/ILSpy/TreeNodes/NamespaceTreeNode.cs index 67b06a763..2f1d6917d 100644 --- a/ILSpy/TreeNodes/NamespaceTreeNode.cs +++ b/ILSpy/TreeNodes/NamespaceTreeNode.cs @@ -20,6 +20,7 @@ using System; using System.Linq; using ICSharpCode.Decompiler; +using ICSharpCode.Decompiler.IL; using ICSharpCode.Decompiler.Metadata; using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.ILSpyX; @@ -29,6 +30,11 @@ using ICSharpCode.ILSpy.Languages; namespace ICSharpCode.ILSpy.TreeNodes { + /// + /// Namespace node. The loading of the type nodes is handled by the parent AssemblyTreeNode, + /// which builds the whole namespace band in a single pass and populates each node before + /// attaching it to the tree. + /// public sealed class NamespaceTreeNode : ILSpyTreeNode { readonly string name; @@ -63,10 +69,12 @@ namespace ICSharpCode.ILSpy.TreeNodes this.name = displayName ?? throw new ArgumentNullException(nameof(displayName)); this.fullName = fullName ?? throw new ArgumentNullException(nameof(fullName)); this.module = module ?? throw new ArgumentNullException(nameof(module)); - LazyLoading = true; } - public override object Text => name.Length == 0 ? "-" : name; + // Namespace names come from the metadata string heap, which permits whitespace and control + // characters that would corrupt the tree row if rendered raw. Escaping is display-only: + // Name and FullName stay raw because they are the keys used to look namespaces up. + public override object Text => name.Length == 0 ? "-" : ILAmbience.EscapeName(name); public override object Icon => Images.Namespace; @@ -77,21 +85,6 @@ namespace ICSharpCode.ILSpy.TreeNodes // nested-namespace setting. public override string ToString() => fullName; - protected override void LoadChildren() - { - var metadata = module.Metadata; - var types = metadata.TypeDefinitions - .Where(t => { - var td = metadata.GetTypeDefinition(t); - return td.GetDeclaringType().IsNil - && metadata.GetString(td.Namespace) == fullName; - }) - .OrderBy(t => metadata.GetString(metadata.GetTypeDefinition(t).Name), NaturalStringComparer.Instance); - - foreach (var t in types) - Children.Add(new TypeTreeNode(t, module)); - } - public override void Decompile(Language language, ITextOutput output, DecompilationOptions options) { // Enumerate via the type system matching this run's settings so the namespace @@ -107,18 +100,12 @@ namespace ICSharpCode.ILSpy.TreeNodes language.DecompileNamespace(name, types, output, options); } - // A namespace counts as public-API iff at least one type it contains is public-API. - // Forces lazy children once so the aggregate is available before the cell template - // queries it for the gray-foreground binding; the result is cached because types - // don't change accessibility at runtime. Mirrors WPF's AssemblyTreeNode.SetPublicAPI - // recursive walk. + // A namespace counts as public-API iff at least one type it contains is public-API. The + // result is cached because types don't change accessibility at runtime. bool? cachedIsPublicAPI; public override bool IsPublicAPI { get { - if (cachedIsPublicAPI is { } cached) - return cached; - EnsureLazyChildren(); - cachedIsPublicAPI = Children.OfType().Any(c => c.IsPublicAPI); + cachedIsPublicAPI ??= Children.OfType().Any(c => c.IsPublicAPI); return cachedIsPublicAPI.Value; } } diff --git a/ILSpy/TreeNodes/TypeTreeNode.cs b/ILSpy/TreeNodes/TypeTreeNode.cs index ac7a1a687..008e93a9d 100644 --- a/ILSpy/TreeNodes/TypeTreeNode.cs +++ b/ILSpy/TreeNodes/TypeTreeNode.cs @@ -35,39 +35,38 @@ namespace ICSharpCode.ILSpy.TreeNodes { public sealed class TypeTreeNode : ILSpyTreeNode, IMemberTreeNode { + readonly ITypeDefinition typeDefinition; readonly TypeDefinitionHandle handle; readonly MetadataFile module; public TypeDefinitionHandle Handle => handle; public MetadataFile Module => module; - // IEntity for the wrapped type. Resolution is lazy and may return null when the - // type system can't be built (e.g. broken assemblies); callers must handle null. - public IEntity? Member => ResolveTypeDefinition(); + public ITypeDefinition TypeDefinition => typeDefinition; - public TypeTreeNode(TypeDefinitionHandle handle, MetadataFile module) + public IEntity Member => typeDefinition; + + /// + /// The type is resolved once, by the parent 's single pass over + /// the module's type system, and held for the node's lifetime. Every member below reads it + /// without touching the per-module type-system cache again: that cache is keyed on the current + /// decompiler settings, so re-resolving on each property read would rebuild an effective + /// settings object and take its lock for every cell the tree paints. + /// + public TypeTreeNode(ITypeDefinition typeDefinition, MetadataFile module) { - this.handle = handle; + this.typeDefinition = typeDefinition ?? throw new ArgumentNullException(nameof(typeDefinition)); this.module = module ?? throw new ArgumentNullException(nameof(module)); + this.handle = (TypeDefinitionHandle)typeDefinition.MetadataToken; LazyLoading = true; } - public override object Text { - get { - var typeDef = ResolveTypeDefinition(); - string baseText = typeDef != null - ? Language.TypeToString(typeDef, ConversionFlags.None) - : module.Metadata.GetString(module.Metadata.GetTypeDefinition(handle).Name); - return baseText + GetSuffixString(handle); - } - } + public override object Text + => Language.TypeToString(typeDefinition, ConversionFlags.None) + GetSuffixString(handle); public override object Icon { get { - var typeDef = ResolveTypeDefinition(); - if (typeDef == null) - return Images.Class; - var baseImage = typeDef.Kind switch { + var baseImage = typeDefinition.Kind switch { TypeKind.Interface => Images.Interface, TypeKind.Struct or TypeKind.Void => Images.Struct, TypeKind.Delegate => Images.Delegate, @@ -75,22 +74,16 @@ namespace ICSharpCode.ILSpy.TreeNodes _ => Images.Class, }; return Images.GetIcon(baseImage, - Images.GetOverlay(typeDef.Accessibility), typeDef.IsStatic); + Images.GetOverlay(typeDefinition.Accessibility), typeDefinition.IsStatic); } } public override bool CanExpandRecursively => true; public override void Decompile(Language language, ITextOutput output, DecompilationOptions options) - { - var typeDef = ResolveTypeDefinition(); - if (typeDef != null) - language.DecompileType(typeDef, output, options); - else - language.WriteCommentLine(output, "(could not resolve type)"); - } + => language.DecompileType(typeDefinition, output, options); - public override bool IsPublicAPI => ResolveTypeDefinition()?.Accessibility switch { + public override bool IsPublicAPI => typeDefinition.Accessibility switch { Accessibility.Public or Accessibility.Protected or Accessibility.ProtectedOrInternal => true, _ => false, }; @@ -99,12 +92,9 @@ namespace ICSharpCode.ILSpy.TreeNodes { if (settings.ShowApiLevel == ApiVisibility.PublicOnly && !IsPublicAPI) return FilterResult.Hidden; - var typeDef = ResolveTypeDefinition(); - if (typeDef == null) - return FilterResult.Match; - if (settings.SearchTermMatches(typeDef.Name)) + if (settings.SearchTermMatches(typeDefinition.Name)) { - if (settings.ShowApiLevel == ApiVisibility.All || LanguageService.CurrentLanguage.ShowMember(typeDef)) + if (settings.ShowApiLevel == ApiVisibility.All || LanguageService.CurrentLanguage.ShowMember(typeDefinition)) return FilterResult.Match; else return FilterResult.Hidden; @@ -117,11 +107,7 @@ namespace ICSharpCode.ILSpy.TreeNodes // Stable identity for SessionSettings.ActiveTreeViewPath. ReflectionName is // language-independent. - public override string ToString() - { - var typeDef = ResolveTypeDefinition(); - return typeDef?.ReflectionName ?? module.Metadata.GetString(module.Metadata.GetTypeDefinition(handle).Name); - } + public override string ToString() => typeDefinition.ReflectionName; // Sealed / static / value-type / enum / delegate cannot be the base of another class, // so a DerivedTypes child would always show up empty. Suppress it for those kinds. @@ -135,19 +121,9 @@ namespace ICSharpCode.ILSpy.TreeNodes }; } - ITypeDefinition? ResolveTypeDefinition() - { - var typeSystem = module.GetTypeSystemWithCurrentOptionsOrNull(); - if (typeSystem == null) - return null; - return ((MetadataModule)typeSystem.MainModule).GetDefinition(handle); - } - protected override void LoadChildren() { - var typeDef = ResolveTypeDefinition(); - if (typeDef == null) - return; + var typeDef = typeDefinition; // Inheritance-relation siblings come first so they sit above the type's own members. // BaseTypes is skipped for System.Object (no upstream chain) and for value types' @@ -166,7 +142,7 @@ namespace ICSharpCode.ILSpy.TreeNodes foreach (var nestedType in typeDef.NestedTypes .OrderBy(t => t.Name, NaturalStringComparer.Instance)) { - Children.Add(new TypeTreeNode((TypeDefinitionHandle)nestedType.MetadataToken, module)); + Children.Add(new TypeTreeNode(nestedType, module)); } // C# 14 explicit-extension declaration blocks surface as their own container nodes