From 847324e836daae8e9b72a98aad64bc17facb03dc Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 4 Jul 2026 07:47:07 +0200 Subject: [PATCH] Fix #3845: Add option to expand XML documentation comments Doc-comment folds were created with defaultCollapsed hardcoded to true, so /// blocks always started collapsed. The new Display option is bridged into DecompilerSettings like the other expand flags and drives the fold's default state. Detecting the last line of a doc-comment block requires sibling navigation between trivia nodes, so trivia now carry parent, sibling list, and index state. That state lives on the Trivia subclass to keep AstNode's size unchanged, and the tree API treats trivia as a separate navigation space: Next/PrevSibling and Remove work within the owning list, Slot is null, ReplaceWith throws instead of aliasing the trivia index into the parent's child slots, GetNextNode/GetPrevNode stop at the list boundary, Clone and CopyAnnotationsFrom deep-copy and reparent trivia, and CheckInvariant validates the trivia lists. Assisted-by: Claude:claude-fable-5:Claude Code --- .../Output/TextTokenWriterTests.cs | 70 +++++- .../Syntax/TriviaTests.cs | 234 ++++++++++++++++++ ICSharpCode.Decompiler/CSharp/Annotations.cs | 5 + .../CSharp/Syntax/AstNode.cs | 134 +++++++++- .../CSharp/Syntax/GeneralScope/Trivia.cs | 14 ++ ICSharpCode.Decompiler/DecompilerSettings.cs | 14 ++ .../Output/TextTokenWriter.cs | 6 +- .../TextView/DisplaySettingsBridgeTests.cs | 14 +- ILSpy/Bookmarks/BookmarkViewState.cs | 3 + ILSpy/Options/DisplaySettingReactions.cs | 1 + ILSpy/Options/DisplaySettings.cs | 5 + ILSpy/Options/DisplaySettingsPanel.axaml | 2 + ILSpy/Properties/Resources.Designer.cs | 9 + ILSpy/Properties/Resources.resx | 3 + ILSpy/SettingsService.cs | 1 + 15 files changed, 499 insertions(+), 16 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/Syntax/TriviaTests.cs diff --git a/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs b/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs index e9db64ecf..dff12179a 100644 --- a/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs +++ b/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs @@ -23,6 +23,7 @@ using System.Reflection.Metadata; using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.CSharp.OutputVisitor; +using ICSharpCode.Decompiler.CSharp.Syntax; using ICSharpCode.Decompiler.Disassembler; using ICSharpCode.Decompiler.Metadata; using ICSharpCode.Decompiler.TypeSystem; @@ -57,6 +58,8 @@ namespace ICSharpCode.Decompiler.Tests.Output sealed class ReferenceRecordingOutput : ITextOutput { public readonly List<(string Text, IMember Member)> MemberReferences = new(); + public readonly List FoldStartDefaultCollapsed = new(); + public int FoldEndCount; public string IndentationString { get; set; } = "\t"; public void Indent() { } @@ -75,8 +78,16 @@ namespace ICSharpCode.Decompiler.Tests.Output } public void WriteLocalReference(string text, object reference, bool isDefinition = false) { } - public void MarkFoldStart(string collapsedText = "...", bool defaultCollapsed = false, bool isDefinition = false) { } - public void MarkFoldEnd() { } + + public void MarkFoldStart(string collapsedText = "...", bool defaultCollapsed = false, bool isDefinition = false) + { + FoldStartDefaultCollapsed.Add(defaultCollapsed); + } + + public void MarkFoldEnd() + { + FoldEndCount++; + } } static List<(string Text, IMember Member)> DecompileAndCollectMemberReferences(Type type) @@ -126,5 +137,60 @@ namespace ICSharpCode.Decompiler.Tests.Output Assert.That(references.Where(r => r.Text is "virtual" or "abstract" or "override"), Is.Empty); } + + static ReferenceRecordingOutput WriteLeadingTrivia(DecompilerSettings settings, params Comment[] comments) + { + var output = new ReferenceRecordingOutput(); + var writer = new TextTokenWriter(output, settings); + var node = new SyntaxTree(); + foreach (var comment in comments) + node.AddLeadingTrivia(comment); + foreach (var comment in comments) + { + writer.StartNode(comment); + writer.WriteComment(comment.CommentType, comment.Content); + writer.EndNode(comment); + } + return output; + } + + [TestCase(false, true)] + [TestCase(true, false)] + public void XmlDocumentationFoldDefaultFollowsSetting(bool expandXmlDocumentationComments, bool expectedDefaultCollapsed) + { + var settings = new DecompilerSettings { + ExpandXmlDocumentationComments = expandXmlDocumentationComments + }; + + var output = WriteLeadingTrivia(settings, + new Comment(" ", CommentType.Documentation), + new Comment(" ", CommentType.Documentation)); + + Assert.That(output.FoldStartDefaultCollapsed, Is.EqualTo(new[] { expectedDefaultCollapsed })); + Assert.That(output.FoldEndCount, Is.EqualTo(1)); + } + + [Test] + public void SingleDocumentationLineFollowedByRegularCommentOpensNoFold() + { + var output = WriteLeadingTrivia(new DecompilerSettings(), + new Comment(" single", CommentType.Documentation), + new Comment(" regular")); + + Assert.That(output.FoldStartDefaultCollapsed, Is.Empty); + Assert.That(output.FoldEndCount, Is.Zero); + } + + [Test] + public void DocumentationFoldClosesBeforeFollowingRegularComment() + { + var output = WriteLeadingTrivia(new DecompilerSettings(), + new Comment(" ", CommentType.Documentation), + new Comment(" ", CommentType.Documentation), + new Comment(" regular")); + + Assert.That(output.FoldStartDefaultCollapsed, Has.Count.EqualTo(1)); + Assert.That(output.FoldEndCount, Is.EqualTo(1)); + } } } diff --git a/ICSharpCode.Decompiler.Tests/Syntax/TriviaTests.cs b/ICSharpCode.Decompiler.Tests/Syntax/TriviaTests.cs new file mode 100644 index 000000000..1b0d3509e --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/Syntax/TriviaTests.cs @@ -0,0 +1,234 @@ +// 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.Linq; + +using ICSharpCode.Decompiler.CSharp; +using ICSharpCode.Decompiler.CSharp.Syntax; + +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests.Syntax +{ + /// + /// Trivia lives outside the child-slot space: it is reachable only through + /// /, navigates via + /// Next/PrevSibling within its owning list, and never leaks into the owner's child space. + /// + [TestFixture] + public class TriviaTests + { + static (SyntaxTree tree, ExpressionStatement stmt) MakeTreeWithStatement() + { + var stmt = new ExpressionStatement(new IdentifierExpression("x")); + var tree = new SyntaxTree(); + tree.Members.Add(stmt); + return (tree, stmt); + } + + [Test] + public void Attached_Trivia_Navigates_Within_Its_List() + { + var (_, stmt) = MakeTreeWithStatement(); + var a = new Comment(" a"); + var b = new Comment(" b"); + var c = new Comment(" c"); + stmt.AddLeadingTrivia(a); + stmt.AddLeadingTrivia(b); + stmt.AddLeadingTrivia(c); + + Assert.That(a.Parent, Is.SameAs(stmt)); + Assert.That(a.NextSibling, Is.SameAs(b)); + Assert.That(b.NextSibling, Is.SameAs(c)); + Assert.That(c.NextSibling, Is.Null); + Assert.That(c.PrevSibling, Is.SameAs(b)); + Assert.That(a.PrevSibling, Is.Null); + Assert.That(a.Slot, Is.Null); + } + + [Test] + public void Prepend_Reindexes_Existing_Trivia() + { + var (_, stmt) = MakeTreeWithStatement(); + var first = new Comment(" first"); + var second = new Comment(" second"); + stmt.AddLeadingTrivia(second); + stmt.PrependLeadingTrivia(first); + + Assert.That(stmt.LeadingTrivia.ToArray(), Is.EqualTo(new[] { first, second })); + Assert.That(first.NextSibling, Is.SameAs(second)); + Assert.That(second.PrevSibling, Is.SameAs(first)); + Assert.That(second.NextSibling, Is.Null); + } + + [Test] + public void Remove_Detaches_And_Reindexes_Remaining_Trivia() + { + var (_, stmt) = MakeTreeWithStatement(); + var a = new Comment(" a"); + var b = new Comment(" b"); + var c = new Comment(" c"); + stmt.AddLeadingTrivia(a); + stmt.AddLeadingTrivia(b); + stmt.AddLeadingTrivia(c); + + b.Remove(); + + Assert.That(b.Parent, Is.Null); + Assert.That(stmt.LeadingTrivia.ToArray(), Is.EqualTo(new[] { a, c })); + Assert.That(a.NextSibling, Is.SameAs(c)); + Assert.That(c.PrevSibling, Is.SameAs(a)); + } + + [Test] + public void Removed_Trivia_Can_Be_Reattached() + { + var (_, stmt) = MakeTreeWithStatement(); + var comment = new Comment(" migrating"); + stmt.AddLeadingTrivia(comment); + comment.Remove(); + + var other = new ExpressionStatement(new IdentifierExpression("y")); + other.AddTrailingTrivia(comment); + + Assert.That(comment.Parent, Is.SameAs(other)); + Assert.That(other.TrailingTrivia.Single(), Is.SameAs(comment)); + } + + [Test] + public void Adding_Attached_Trivia_Elsewhere_Throws() + { + var (_, stmt) = MakeTreeWithStatement(); + var comment = new Comment(" taken"); + stmt.AddLeadingTrivia(comment); + + var other = new ExpressionStatement(new IdentifierExpression("y")); + Assert.Throws(() => other.AddLeadingTrivia(comment)); + } + + [Test] + public void ReplaceWith_On_Attached_Trivia_Throws() + { + var (_, stmt) = MakeTreeWithStatement(); + var comment = new Comment(" doc", CommentType.Documentation); + stmt.AddLeadingTrivia(comment); + + Assert.Throws(() => comment.ReplaceWith(new Comment(" other"))); + // The failed replacement must not have touched the owning statement's real children. + Assert.That(stmt.Expression, Is.InstanceOf()); + Assert.That(stmt.LeadingTrivia.Single(), Is.SameAs(comment)); + } + + [Test] + public void ReplaceWith_Function_On_Attached_Trivia_Throws() + { + var (_, stmt) = MakeTreeWithStatement(); + var comment = new Comment(" doc", CommentType.Documentation); + stmt.AddLeadingTrivia(comment); + + Assert.Throws(() => comment.ReplaceWith(_ => new Comment(" other"))); + Assert.That(stmt.LeadingTrivia.Single(), Is.SameAs(comment)); + } + + [Test] + public void GetNextNode_Stays_Within_Trivia_Space() + { + var (tree, stmt) = MakeTreeWithStatement(); + var second = new ExpressionStatement(new IdentifierExpression("y")); + tree.Members.Add(second); + var a = new Comment(" a"); + var b = new Comment(" b"); + stmt.AddLeadingTrivia(a); + stmt.AddLeadingTrivia(b); + + Assert.That(a.GetNextNode(), Is.SameAs(b)); + // Leading trivia precedes its owner, so continuing past the end of the trivia list into + // the owner's sibling space would skip the owner's whole subtree. + Assert.That(b.GetNextNode(), Is.Null); + Assert.That(a.GetPrevNode(), Is.Null); + } + + [Test] + public void Clone_Of_Owner_Reparents_Cloned_Trivia() + { + var (_, stmt) = MakeTreeWithStatement(); + var a = new Comment(" a"); + var b = new Comment(" b"); + stmt.AddLeadingTrivia(a); + stmt.AddLeadingTrivia(b); + + var copy = (ExpressionStatement)stmt.Clone(); + var copiedTrivia = copy.LeadingTrivia.ToArray(); + + Assert.That(copiedTrivia, Has.Length.EqualTo(2)); + Assert.That(copiedTrivia[0], Is.Not.SameAs(a)); + Assert.That(copiedTrivia[0].Parent, Is.SameAs(copy)); + Assert.That(copiedTrivia[0].NextSibling, Is.SameAs(copiedTrivia[1])); + // The original's trivia must be untouched. + Assert.That(a.Parent, Is.SameAs(stmt)); + Assert.That(a.NextSibling, Is.SameAs(b)); + } + + [Test] + public void Clone_Of_Attached_Trivia_Is_Fully_Detached() + { + var (_, stmt) = MakeTreeWithStatement(); + var a = new Comment(" a"); + var b = new Comment(" b"); + stmt.AddLeadingTrivia(a); + stmt.AddLeadingTrivia(b); + + var clone = (Comment)a.Clone(); + + Assert.That(clone.Parent, Is.Null); + Assert.That(clone.triviaSiblings, Is.Null); + + // A detached clone attached as a regular child must navigate in child space, not in the + // source node's trivia list. + var host = new SyntaxTree(); + host.Members.Add(clone); + Assert.That(clone.NextSibling, Is.Null); + Assert.That(clone.PrevSibling, Is.Null); + clone.Remove(); + Assert.That(stmt.LeadingTrivia.ToArray(), Is.EqualTo(new[] { a, b })); + } + + [Test] + public void CopyAnnotationsFrom_Clones_Trivia_Instead_Of_Sharing() + { + var source = new ExpressionStatement(new IdentifierExpression("x")); + var sourceComment = new Comment(" note"); + source.AddTrailingTrivia(sourceComment); + + var target = new ExpressionStatement(new IdentifierExpression("y")); + target.CopyAnnotationsFrom(source); + + var targetComment = target.TrailingTrivia.Single(); + Assert.That(targetComment, Is.Not.SameAs(sourceComment)); + Assert.That(targetComment.Parent, Is.SameAs(target)); + Assert.That(sourceComment.Parent, Is.SameAs(source)); + + // The lists must be independent: mutating one node's trivia must not affect the other. + target.AddTrailingTrivia(new Comment(" extra")); + Assert.That(source.TrailingTrivia.Count(), Is.EqualTo(1)); + targetComment.Remove(); + Assert.That(source.TrailingTrivia.Single(), Is.SameAs(sourceComment)); + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/Annotations.cs b/ICSharpCode.Decompiler/CSharp/Annotations.cs index c46274e2d..55039ac66 100644 --- a/ICSharpCode.Decompiler/CSharp/Annotations.cs +++ b/ICSharpCode.Decompiler/CSharp/Annotations.cs @@ -216,8 +216,13 @@ namespace ICSharpCode.Decompiler.CSharp { foreach (object annotation in other.Annotations) { + // The trivia holder must not be shared between nodes: each trivia's Parent points at + // its single owning node. Trivia is deep-copied onto the target instead. + if (annotation is AstNode.NodeTrivia) + continue; node.AddAnnotation(annotation); } + node.CopyTriviaFrom(other); return node; } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs index a141ec29f..5fe61fd49 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs @@ -79,7 +79,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax // Comments and preprocessor directives attached to this node, kept off the child-index space. // The holder lives in the annotation channel, so a node without trivia (the overwhelmingly // common case) costs nothing extra, and CloneAnnotations copies it for free. - sealed class NodeTrivia : ICloneable + internal sealed class NodeTrivia : ICloneable { public List? Leading; public List? Trailing; @@ -105,10 +105,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax public void AddLeadingTrivia(Trivia trivia) { - if (trivia == null) - throw new ArgumentNullException(nameof(trivia)); NodeTrivia holder = GetOrCreateTrivia(); - (holder.Leading ??= new List()).Add(trivia); + var list = holder.Leading ??= new List(); + InsertTrivia(list, list.Count, trivia); } /// @@ -117,18 +116,73 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax /// public void PrependLeadingTrivia(Trivia trivia) { - if (trivia == null) - throw new ArgumentNullException(nameof(trivia)); NodeTrivia holder = GetOrCreateTrivia(); - (holder.Leading ??= new List()).Insert(0, trivia); + InsertTrivia(holder.Leading ??= new List(), 0, trivia); } public void AddTrailingTrivia(Trivia trivia) + { + NodeTrivia holder = GetOrCreateTrivia(); + var list = holder.Trailing ??= new List(); + InsertTrivia(list, list.Count, trivia); + } + + // The single mutation path for attaching trivia: validate, insert, then reindex from the + // insertion point so every trivia's parent/list/index state is consistent for any position. + void InsertTrivia(List list, int index, Trivia trivia) + { + ValidateNewTrivia(trivia); + list.Insert(index, trivia); + ReindexTrivia(this, list, index); + } + + void ValidateNewTrivia(Trivia trivia) { if (trivia == null) throw new ArgumentNullException(nameof(trivia)); - NodeTrivia holder = GetOrCreateTrivia(); - (holder.Trailing ??= new List()).Add(trivia); + if (trivia == this) + throw new ArgumentException("Cannot add a node to itself as trivia.", nameof(trivia)); + if (trivia.parent != null) + throw new ArgumentException("Node is already used in another tree.", nameof(trivia)); + } + + static void ReindexTrivia(AstNode parent, List list, int start) + { + for (int i = start; i < list.Count; i++) + list[i].SetTriviaParent(parent, list, i); + } + + // Points the trivia held in this node's annotation back at this node. Clone needs this: + // the cloned NodeTrivia's deep-copied trivia still carry the source owner's parent state. + void ReparentTrivia() + { + var holder = Annotation(); + if (holder == null) + return; + if (holder.Leading != null) + ReindexTrivia(this, holder.Leading, 0); + if (holder.Trailing != null) + ReindexTrivia(this, holder.Trailing, 0); + } + + // Deep-copies another node's trivia onto this node, appending to any trivia already present. + // The NodeTrivia holder must never be shared between nodes: each trivia's parent points at + // its single owning node, so an annotation-copying operation clones the trivia instead. + internal void CopyTriviaFrom(AstNode other) + { + var otherHolder = other.Annotation(); + if (otherHolder == null) + return; + if (otherHolder.Leading != null) + { + foreach (var trivia in otherHolder.Leading) + AddLeadingTrivia((Trivia)trivia.Clone()); + } + if (otherHolder.Trailing != null) + { + foreach (var trivia in otherHolder.Trailing) + AddTrailingTrivia((Trivia)trivia.Clone()); + } } NodeTrivia GetOrCreateTrivia() @@ -151,6 +205,8 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax get { if (parent == null) return null; + if (this is Trivia { triviaSiblings: { } siblings }) + return childIndex + 1 < siblings.Count ? siblings[childIndex + 1] : null; // Inline the validity check: sibling navigation is one of the hottest operations, and the // indices are almost always already current, so skip the (non-inlinable) call when valid. if (!parent.childIndicesValid) @@ -170,6 +226,8 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax get { if (parent == null) return null; + if (this is Trivia { triviaSiblings: { } siblings }) + return childIndex > 0 ? siblings[childIndex - 1] : null; if (!parent.childIndicesValid) parent.EnsureChildIndices(); for (int i = childIndex - 1; i >= 0; i--) @@ -440,6 +498,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax get { if (parent == null) return null; + // Trivia occupies no child slot; its childIndex indexes its owning trivia list instead. + if (this is Trivia { triviaSiblings: not null }) + return null; parent.EnsureChildIndices(); return parent.GetChildSlotInfo(childIndex); } @@ -524,6 +585,28 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax Debug.Assert(slot.ChildType.IsInstanceOfType(child), "child's type must be valid in its slot"); child.CheckInvariant(); } + var trivia = Annotation(); + if (trivia != null) + { + CheckTriviaInvariant(trivia.Leading); + CheckTriviaInvariant(trivia.Trailing); + } + } + + // Trivia lives outside the slot space but carries the same parent/index invariants, scoped + // to the Leading/Trailing list that holds it. + [System.Diagnostics.Conditional("DEBUG")] + void CheckTriviaInvariant(List? list) + { + if (list == null) + return; + for (int i = 0; i < list.Count; i++) + { + Trivia t = list[i]; + Debug.Assert(t.parent == this, "trivia's Parent must point back to its owning node"); + Debug.Assert(t.triviaSiblings == list, "trivia's sibling list must be the list that holds it"); + Debug.Assert(t.childIndex == i, "trivia's index must match its position in the trivia list"); + } } // Self-reference and two-tree guards shared by the single-slot setters; lifts a node out of the @@ -589,6 +672,8 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax internal void ClearParentAndIndex() { parent = null; + if (this is Trivia trivia) + trivia.triviaSiblings = null; childIndex = -1; } #endregion @@ -668,6 +753,15 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { if (parent == null) return; + if (this is Trivia { triviaSiblings: { } siblings }) + { + var oldParent = parent; + int oldIndex = childIndex; + siblings.RemoveAt(oldIndex); + ClearParentAndIndex(); + ReindexTrivia(oldParent, siblings, oldIndex); + return; + } parent.EnsureChildIndices(); CSharpSlotInfo kind = parent.GetChildSlotInfo(childIndex).Kind!; AstNodeCollection? collection = parent.GetCollectionByKind(kind); @@ -693,6 +787,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { throw new InvalidOperationException("Cannot replace the root node"); } + ThrowIfTrivia(); parent.EnsureChildIndices(); CSharpSlotInfo slot = parent.GetChildSlotInfo(childIndex); // Because this method doesn't statically check the new node's type with the slot, @@ -718,6 +813,15 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax parent.SetChild(childIndex, newNode); } + // Attached trivia has no child slot to substitute into: its childIndex indexes the owning + // trivia list, which the slot arithmetic in ReplaceWith must never touch. Trivia is replaced + // by removing it and attaching new trivia to the owning node. + void ThrowIfTrivia() + { + if (this is Trivia { triviaSiblings: not null }) + throw new InvalidOperationException("Cannot replace trivia; remove it and attach the replacement to the owning node instead."); + } + public AstNode? ReplaceWith(Func replaceFunction) { if (replaceFunction == null) @@ -726,6 +830,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { throw new InvalidOperationException("Cannot replace the root node"); } + ThrowIfTrivia(); AstNode oldParent = parent; AstNode? oldSuccessor = NextSibling; CSharpSlotInfo? oldSlot = this.Slot; @@ -760,11 +865,15 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax AstNode copy = (AstNode)MemberwiseClone(); copy.parent = null; copy.childIndex = -1; + // MemberwiseClone copied the source's trivia-list reference; the copy is detached. + if (copy is Trivia copiedTrivia) + copiedTrivia.triviaSiblings = null; // Deep-copy the children (CloneChildrenInto first drops the shallow field copies that // MemberwiseClone left pointing at this node's children). CloneChildrenInto(copy); // Finally, clone the annotation, if necessary copy.CloneAnnotations(); + copy.ReparentTrivia(); return copy; } @@ -818,6 +927,10 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { if (NextSibling != null) return NextSibling; + // Trivia navigation ends at its owning list: leading trivia precedes its owner in document + // order, so continuing into the owner's sibling space would skip the owner's whole subtree. + if (this is Trivia) + return null; if (Parent != null) return Parent.GetNextNode(); return null; @@ -840,6 +953,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { if (PrevSibling != null) return PrevSibling; + // Trivia navigation ends at its owning list; see GetNextNode. + if (this is Trivia) + return null; if (Parent != null) return Parent.GetPrevNode(); return null; diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/GeneralScope/Trivia.cs b/ICSharpCode.Decompiler/CSharp/Syntax/GeneralScope/Trivia.cs index dbd8ee9d4..b8010929f 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/GeneralScope/Trivia.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/GeneralScope/Trivia.cs @@ -18,6 +18,8 @@ #nullable enable +using System.Collections.Generic; + namespace ICSharpCode.Decompiler.CSharp.Syntax { /// @@ -29,6 +31,11 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax /// public abstract class Trivia : AstNode { + // The Leading or Trailing list of the owning node that currently holds this trivia (null while + // detached). Lives here rather than on AstNode so that plain nodes, which can never be trivia, + // do not pay for the field; AstNode's sibling navigation branches on it via a Trivia type test. + internal List? triviaSiblings; + TextLocation startLocation; TextLocation endLocation; @@ -59,5 +66,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { this.endLocation = value; } + + internal void SetTriviaParent(AstNode newParent, List siblings, int index) + { + SetParent(newParent); + triviaSiblings = siblings; + childIndex = index; + } } } diff --git a/ICSharpCode.Decompiler/DecompilerSettings.cs b/ICSharpCode.Decompiler/DecompilerSettings.cs index 40021b79d..955003893 100644 --- a/ICSharpCode.Decompiler/DecompilerSettings.cs +++ b/ICSharpCode.Decompiler/DecompilerSettings.cs @@ -1351,6 +1351,20 @@ namespace ICSharpCode.Decompiler } } + bool expandXmlDocumentationComments = false; + + [Browsable(false)] + public bool ExpandXmlDocumentationComments { + get { return expandXmlDocumentationComments; } + set { + if (expandXmlDocumentationComments != value) + { + expandXmlDocumentationComments = value; + OnPropertyChanged(); + } + } + } + bool expandMemberDefinitions = false; [Browsable(false)] diff --git a/ICSharpCode.Decompiler/Output/TextTokenWriter.cs b/ICSharpCode.Decompiler/Output/TextTokenWriter.cs index 776551224..7e6007145 100644 --- a/ICSharpCode.Decompiler/Output/TextTokenWriter.cs +++ b/ICSharpCode.Decompiler/Output/TextTokenWriter.cs @@ -355,11 +355,13 @@ namespace ICSharpCode.Decompiler output.Write("*/"); break; case CommentType.Documentation: - bool isLastLine = !(nodeStack.Peek().NextSibling is Comment); + // Only a following documentation comment continues the fold: a regular comment in + // the same trivia list must not keep the fold open (it never calls MarkFoldEnd). + bool isLastLine = nodeStack.Peek().NextSibling is not Comment { CommentType: CommentType.Documentation }; if (!inDocumentationComment && !isLastLine) { inDocumentationComment = true; - output.MarkFoldStart("///" + content, true); + output.MarkFoldStart("///" + content, defaultCollapsed: !settings.ExpandXmlDocumentationComments); } output.Write("///"); output.Write(content); diff --git a/ILSpy.Tests/TextView/DisplaySettingsBridgeTests.cs b/ILSpy.Tests/TextView/DisplaySettingsBridgeTests.cs index db295e12d..93655a52c 100644 --- a/ILSpy.Tests/TextView/DisplaySettingsBridgeTests.cs +++ b/ILSpy.Tests/TextView/DisplaySettingsBridgeTests.cs @@ -21,7 +21,6 @@ using AwesomeAssertions; using ICSharpCode.Decompiler; using ICSharpCode.ILSpy.Options; -using ICSharpCode.ILSpy.TextView; using NUnit.Framework; @@ -73,11 +72,20 @@ public class DisplaySettingsBridgeTests [Test] public void Expand_Flags_Still_Bridged() { - var display = new DisplaySettings { ExpandUsingDeclarations = true, ExpandMemberDefinitions = true }; - var settings = new DecompilerSettings { ExpandUsingDeclarations = false, ExpandMemberDefinitions = false }; + var display = new DisplaySettings { + ExpandXmlDocumentationComments = true, + ExpandUsingDeclarations = true, + ExpandMemberDefinitions = true + }; + var settings = new DecompilerSettings { + ExpandXmlDocumentationComments = false, + ExpandUsingDeclarations = false, + ExpandMemberDefinitions = false + }; SettingsService.ApplyDisplaySettings(settings, display); + settings.ExpandXmlDocumentationComments.Should().BeTrue(); settings.ExpandUsingDeclarations.Should().BeTrue(); settings.ExpandMemberDefinitions.Should().BeTrue(); } diff --git a/ILSpy/Bookmarks/BookmarkViewState.cs b/ILSpy/Bookmarks/BookmarkViewState.cs index 65ccec116..26a43d455 100644 --- a/ILSpy/Bookmarks/BookmarkViewState.cs +++ b/ILSpy/Bookmarks/BookmarkViewState.cs @@ -30,6 +30,7 @@ namespace ICSharpCode.ILSpy.Bookmarks /// Display settings that affect the rendered C# text layout. public sealed record BookmarkRenderedLayoutSettings( bool FoldBraces, + bool ExpandXmlDocumentationComments, bool ExpandMemberDefinitions, bool ExpandUsingDeclarations, bool ShowDebugInfo, @@ -40,6 +41,7 @@ namespace ICSharpCode.ILSpy.Bookmarks public static BookmarkRenderedLayoutSettings From(DisplaySettings display) => new( display.FoldBraces, + display.ExpandXmlDocumentationComments, display.ExpandMemberDefinitions, display.ExpandUsingDeclarations, display.ShowDebugInfo, @@ -50,6 +52,7 @@ namespace ICSharpCode.ILSpy.Bookmarks public void ApplyTo(DisplaySettings display) { display.FoldBraces = FoldBraces; + display.ExpandXmlDocumentationComments = ExpandXmlDocumentationComments; display.ExpandMemberDefinitions = ExpandMemberDefinitions; display.ExpandUsingDeclarations = ExpandUsingDeclarations; display.ShowDebugInfo = ShowDebugInfo; diff --git a/ILSpy/Options/DisplaySettingReactions.cs b/ILSpy/Options/DisplaySettingReactions.cs index 422aaf4d2..5d730bae5 100644 --- a/ILSpy/Options/DisplaySettingReactions.cs +++ b/ILSpy/Options/DisplaySettingReactions.cs @@ -64,6 +64,7 @@ namespace ICSharpCode.ILSpy.Options // Decompiler / disassembler output (see SettingsService.ApplyDisplaySettings + // GetIndentationString, and CSharpILMixedLanguage / ILLanguage for the IL-detail ones). [nameof(DisplaySettings.FoldBraces)] = DisplaySettingReaction.Redecompile, + [nameof(DisplaySettings.ExpandXmlDocumentationComments)] = DisplaySettingReaction.Redecompile, [nameof(DisplaySettings.ExpandMemberDefinitions)] = DisplaySettingReaction.Redecompile, [nameof(DisplaySettings.ExpandUsingDeclarations)] = DisplaySettingReaction.Redecompile, [nameof(DisplaySettings.ShowDebugInfo)] = DisplaySettingReaction.Redecompile, diff --git a/ILSpy/Options/DisplaySettings.cs b/ILSpy/Options/DisplaySettings.cs index bfd7503d4..99515b0eb 100644 --- a/ILSpy/Options/DisplaySettings.cs +++ b/ILSpy/Options/DisplaySettings.cs @@ -57,6 +57,9 @@ namespace ICSharpCode.ILSpy.Options [ObservableProperty] bool foldBraces; + [ObservableProperty] + bool expandXmlDocumentationComments; + [ObservableProperty] bool expandMemberDefinitions; @@ -112,6 +115,7 @@ namespace ICSharpCode.ILSpy.Options EnableWordWrap = (bool?)section.Attribute(nameof(EnableWordWrap)) ?? false; SortResults = (bool?)section.Attribute(nameof(SortResults)) ?? true; FoldBraces = (bool?)section.Attribute(nameof(FoldBraces)) ?? false; + ExpandXmlDocumentationComments = (bool?)section.Attribute(nameof(ExpandXmlDocumentationComments)) ?? false; ExpandMemberDefinitions = (bool?)section.Attribute(nameof(ExpandMemberDefinitions)) ?? false; ExpandUsingDeclarations = (bool?)section.Attribute(nameof(ExpandUsingDeclarations)) ?? false; IndentationUseTabs = (bool?)section.Attribute(nameof(IndentationUseTabs)) ?? true; @@ -139,6 +143,7 @@ namespace ICSharpCode.ILSpy.Options section.SetAttributeValue(nameof(EnableWordWrap), EnableWordWrap); section.SetAttributeValue(nameof(SortResults), SortResults); section.SetAttributeValue(nameof(FoldBraces), FoldBraces); + section.SetAttributeValue(nameof(ExpandXmlDocumentationComments), ExpandXmlDocumentationComments); section.SetAttributeValue(nameof(ExpandMemberDefinitions), ExpandMemberDefinitions); section.SetAttributeValue(nameof(ExpandUsingDeclarations), ExpandUsingDeclarations); section.SetAttributeValue(nameof(IndentationUseTabs), IndentationUseTabs); diff --git a/ILSpy/Options/DisplaySettingsPanel.axaml b/ILSpy/Options/DisplaySettingsPanel.axaml index 9d9190a60..5606e3bd7 100644 --- a/ILSpy/Options/DisplaySettingsPanel.axaml +++ b/ILSpy/Options/DisplaySettingsPanel.axaml @@ -71,6 +71,8 @@ Content="{x:Static res:Resources.HighlightMatchingBraces}" /> + + /// Looks up a localized string similar to Expand XML documentation comments after decompilation. + /// + public static string ExpandXmlDocumentationCommentsAfterDecompilation { + get { + return ResourceManager.GetString("ExpandXmlDocumentationCommentsAfterDecompilation", resourceCulture); + } + } /// /// Looks up a localized string similar to Extract all package entries. diff --git a/ILSpy/Properties/Resources.resx b/ILSpy/Properties/Resources.resx index 423ba5286..7e3470336 100644 --- a/ILSpy/Properties/Resources.resx +++ b/ILSpy/Properties/Resources.resx @@ -711,6 +711,9 @@ Are you sure you want to continue? Expand using declarations after decompilation + + Expand XML documentation comments after decompilation + Export Project/Solution... diff --git a/ILSpy/SettingsService.cs b/ILSpy/SettingsService.cs index f084a8371..fe902bef6 100644 --- a/ILSpy/SettingsService.cs +++ b/ILSpy/SettingsService.cs @@ -86,6 +86,7 @@ namespace ICSharpCode.ILSpy /// internal static void ApplyDisplaySettings(Decompiler.DecompilerSettings settings, DisplaySettings display) { + settings.ExpandXmlDocumentationComments = display.ExpandXmlDocumentationComments; settings.ExpandUsingDeclarations = display.ExpandUsingDeclarations; settings.ExpandMemberDefinitions = display.ExpandMemberDefinitions; settings.FoldBraces = display.FoldBraces;