From c971c695f43e61f31a899cf7e4ae1a48a1a8e36a 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 e9db64ecf9..dff12179a9 100644 --- a/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs +++ b/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs @@ -23,6 +23,7 @@ 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 @@ public class TextTokenWriterTests 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 @@ public void WriteReference(IMember member, string text, bool isDefinition = fals } 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 @@ public void VirtualAndAbstractModifiersAreNotReferences() 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 0000000000..1b0d3509e5 --- /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 c46274e2d2..55039ac66a 100644 --- a/ICSharpCode.Decompiler/CSharp/Annotations.cs +++ b/ICSharpCode.Decompiler/CSharp/Annotations.cs @@ -216,8 +216,13 @@ public static T CopyAnnotationsFrom(this T node, AstNode other) where T : Ast { 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 a141ec29f3..5fe61fd49d 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs @@ -79,7 +79,7 @@ internal void StorePrintEnd(TextLocation value) // 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 @@ public IEnumerable TrailingTrivia { 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 @@ public void AddLeadingTrivia(Trivia trivia) /// 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 @@ public AstNode? NextSibling { 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 @@ public AstNode? PrevSibling { 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 @@ public CSharpSlotInfo? Slot { 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 @@ internal virtual void CheckInvariant() 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 @@ internal void SetParent(AstNode newParent) internal void ClearParentAndIndex() { parent = null; + if (this is Trivia trivia) + trivia.triviaSiblings = null; childIndex = -1; } #endregion @@ -668,6 +753,15 @@ public void Remove() { 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 @@ public void ReplaceWith(AstNode? newNode) { 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 @@ public void ReplaceWith(AstNode? newNode) 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 @@ public void ReplaceWith(AstNode? newNode) { throw new InvalidOperationException("Cannot replace the root node"); } + ThrowIfTrivia(); AstNode oldParent = parent; AstNode? oldSuccessor = NextSibling; CSharpSlotInfo? oldSlot = this.Slot; @@ -760,11 +865,15 @@ public AstNode Clone() 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 @@ bool PatternMatching.INode.DoMatchCollection(System.Collections.Generic.IReadOnl { 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 @@ bool PatternMatching.INode.DoMatchCollection(System.Collections.Generic.IReadOnl { 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 dbd8ee9d4c..b8010929f6 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 @@ internal void SetEndLocation(TextLocation value) { 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 40021b79da..955003893e 100644 --- a/ICSharpCode.Decompiler/DecompilerSettings.cs +++ b/ICSharpCode.Decompiler/DecompilerSettings.cs @@ -1351,6 +1351,20 @@ public bool FoldBraces { } } + 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 7765512241..7e60071455 100644 --- a/ICSharpCode.Decompiler/Output/TextTokenWriter.cs +++ b/ICSharpCode.Decompiler/Output/TextTokenWriter.cs @@ -355,11 +355,13 @@ public override void WriteComment(CommentType commentType, string content) 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 db295e12d3..93655a52ca 100644 --- a/ILSpy.Tests/TextView/DisplaySettingsBridgeTests.cs +++ b/ILSpy.Tests/TextView/DisplaySettingsBridgeTests.cs @@ -21,7 +21,6 @@ using ICSharpCode.Decompiler; using ICSharpCode.ILSpy.Options; -using ICSharpCode.ILSpy.TextView; using NUnit.Framework; @@ -73,11 +72,20 @@ public void Indentation_Uses_Tabs_When_Enabled() [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 65ccec1162..26a43d4552 100644 --- a/ILSpy/Bookmarks/BookmarkViewState.cs +++ b/ILSpy/Bookmarks/BookmarkViewState.cs @@ -30,6 +30,7 @@ public sealed record BookmarkFoldingRange(int Start, int End); /// 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 @@ public sealed record BookmarkRenderedLayoutSettings( public static BookmarkRenderedLayoutSettings From(DisplaySettings display) => new( display.FoldBraces, + display.ExpandXmlDocumentationComments, display.ExpandMemberDefinitions, display.ExpandUsingDeclarations, display.ShowDebugInfo, @@ -50,6 +52,7 @@ public static BookmarkRenderedLayoutSettings From(DisplaySettings display) 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 422aaf4d2e..5d730bae51 100644 --- a/ILSpy/Options/DisplaySettingReactions.cs +++ b/ILSpy/Options/DisplaySettingReactions.cs @@ -64,6 +64,7 @@ public static class DisplaySettingReactions // 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 bfd7503d4c..99515b0eb9 100644 --- a/ILSpy/Options/DisplaySettings.cs +++ b/ILSpy/Options/DisplaySettings.cs @@ -57,6 +57,9 @@ public sealed partial class DisplaySettings : ObservableObject, ISettingsSection [ObservableProperty] bool foldBraces; + [ObservableProperty] + bool expandXmlDocumentationComments; + [ObservableProperty] bool expandMemberDefinitions; @@ -112,6 +115,7 @@ public void LoadFromXml(XElement section) 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 @@ public XElement SaveToXml() 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 9d9190a600..5606e3bd7c 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 423ba5286f..7e34703366 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 f084a8371b..fe902bef6d 100644 --- a/ILSpy/SettingsService.cs +++ b/ILSpy/SettingsService.cs @@ -86,6 +86,7 @@ public Decompiler.DecompilerSettings CreateEffectiveDecompilerSettings() /// 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;