From b2a8e1e6d112eeb438991f4339b5ee38155e6530 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 21 Aug 2026 12:22:07 -0700 Subject: [PATCH] Fixes link formatting and generation. Related to #1205 --- .../Asp.Versioning.OpenApi.csproj | 2 +- .../Asp.Versioning.OpenApi/ReleaseNotes.txt | 2 +- .../Transformers/XmlComments.cs | 56 ++++-- .../Simulators/Documented.cs | 47 +++++ .../Simulators/MinimalApi.cs | 21 +++ .../Transformers/XmlCommentsStructureTest.cs | 176 +++++++++++++++++- 6 files changed, 285 insertions(+), 19 deletions(-) diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Asp.Versioning.OpenApi.csproj b/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Asp.Versioning.OpenApi.csproj index af1bb6d2..ab33ba13 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Asp.Versioning.OpenApi.csproj +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Asp.Versioning.OpenApi.csproj @@ -1,7 +1,7 @@  - 10.2.2 + 10.2.3 10.2.0.0 $(DefaultTargetFramework) Asp.Versioning.OpenApi diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/ReleaseNotes.txt b/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/ReleaseNotes.txt index 114fecc8..b123f687 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/ReleaseNotes.txt +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/ReleaseNotes.txt @@ -1 +1 @@ -Fixed XML Comment whitespace handling [Issue #1205](https://github.com/dotnet/aspnet-api-versioning/issues/1205) \ No newline at end of file +Fixed XML Comment list and link handling [Issue #1205](https://github.com/dotnet/aspnet-api-versioning/issues/1205) \ No newline at end of file diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Transformers/XmlComments.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Transformers/XmlComments.cs index 2996d93c..4bfb79c2 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Transformers/XmlComments.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.OpenApi/Transformers/XmlComments.cs @@ -3,12 +3,12 @@ namespace Asp.Versioning.OpenApi.Transformers; using System.Collections.Concurrent; -using System.Globalization; using System.IO; using System.Linq; using System.Reflection; using System.Text; using System.Xml.Linq; +using static System.Globalization.CultureInfo; using static System.Reflection.BindingFlags; /// @@ -18,6 +18,10 @@ namespace Asp.Versioning.OpenApi.Transformers; public class XmlComments { private const int MaxInheritDocDepth = 8; + private static readonly CompositeFormat ItemOfFormat = CompositeFormat.Parse( "**{0}**: {1}" ); + private static readonly CompositeFormat LinkFormat = CompositeFormat.Parse( "[{0}]({1})" ); + private static readonly CompositeFormat BlockFormat = CompositeFormat.Parse( "\n{0}\n" ); + private static readonly CompositeFormat FenceBlockFormat = CompositeFormat.Parse( "\n{0}\n{1}\n{0}\n" ); private readonly ConcurrentDictionary members = new(); /// @@ -176,7 +180,7 @@ public virtual bool IsParameterDeprecated( MemberInfo member, string name ) /// tutorial /// for more information. public virtual string GetResponseDescription( MemberInfo member, int statusCode ) - => GetResponseDescription( member, statusCode.ToString( CultureInfo.InvariantCulture ) ); + => GetResponseDescription( member, statusCode.ToString( InvariantCulture ) ); /// /// Gets the response description from the specified member, if any. @@ -454,8 +458,12 @@ private static void ResolveListTags( XElement element ) _ => BulletedList.Bullets(), }; var items = list.Elements( "item" ).Select( item => bullets.Next() + ItemOf( item ) ); + var block = string.Format( InvariantCulture, BlockFormat, string.Join( '\n', items ) ); - list.ReplaceWith( new XText( string.Join( "\n", items ) ) ); + // a list is a block, so like a table it is surrounded by blank lines. the line that follows the last + // item would otherwise be a lazy continuation of it and the text written after the list would be + // pulled into it; the line that precedes the first item would be the paragraph the list interrupts. + list.ReplaceWith( new XText( block ) ); } } @@ -483,7 +491,7 @@ private static string ItemOf( XElement item ) return text; } - return text.Length == 0 ? name : "**" + name + "**: " + text; + return text.Length == 0 ? name : string.Format( InvariantCulture, ItemOfFormat, name, text ); } // a table is the one list type with a markdown equivalent that is not a list. it only resolves to a table when @@ -554,7 +562,14 @@ private static void AppendRow( StringBuilder table, List cells, int colu { var cell = i < cells.Count ? cells[i] : string.Empty; - table.Append( cell.Length == 0 ? " " : " " + cell + " " ).Append( '|' ); + table.Append( ' ' ); + + if ( cell.Length > 0 ) + { + table.Append( cell ).Append( ' ' ); + } + + table.Append( '|' ); } table.Append( '\n' ); @@ -570,8 +585,9 @@ private static void ResolveCodeBlocks( XElement element ) { var content = TrimEachLine( code.Value ); var fence = FenceFor( content ); + var block = new XText( string.Format( InvariantCulture, FenceBlockFormat, fence, content ) ); - code.ReplaceWith( new XText( "\n" + fence + "\n" + content + "\n" + fence + "\n" ) ); + code.ReplaceWith( block ); } } @@ -602,7 +618,8 @@ private static void ResolveInlineCode( XElement element ) } // , , and are the html tags a documentation comment carries inline, and each has a direct markdown - // equivalent. rewriting them keeps the meaning that reading the text of the enclosing element would drop. + // equivalent. and join them in the one form that is a link rather than a reference to a + // code element. rewriting them keeps the meaning that reading the text of the enclosing element would drop. // the tags are visited from the inside out so that one nested in another is rewritten before it is absorbed. // // emphasis is delimited by an asterisk rather than an underscore. the two are interchangeable on their own, @@ -618,6 +635,7 @@ private static void ResolveInlineTags( XElement element ) "b" => Delimit( inline.Value, "**" ), "i" => Delimit( inline.Value, "*" ), "a" => LinkOf( inline ), + "see" or "seealso" => SeeOf( inline ), _ => default, }; @@ -628,8 +646,19 @@ private static void ResolveInlineTags( XElement element ) } } - // a link with no text renders as its own address, which is all there is to show. a link with no address is - // not a link at all, so only the text it wrapped is kept. + // and are links written with the tags a documentation comment already uses + // for a reference; they are the form the documentation generators accept as an alternative to , so they + // resolve the same way. both are inline where they are written, so the address belongs in the sentence around + // it; only the tooling that renders a separate "see also" section treats as a block of its own. + // + // every other form names a code element instead of an address - and have + // no markdown of their own - and is left in the tree for the text it wraps, if any, to be read in place. + private static string? SeeOf( XElement see ) => see.Attribute( "href" ) is null ? default : LinkOf( see ); + + // a link with no text has nothing to show but its own address, so the address becomes the text. writing the + // address on its own would rely on the renderer turning it into a link, which is an extension that not every + // renderer implements; Scalar does and Swagger UI does not. a link with no address is not a link at all, so + // only the text it wrapped is kept. private static string LinkOf( XElement anchor ) { var text = Flatten( anchor.Value ); @@ -640,7 +669,12 @@ private static string LinkOf( XElement anchor ) return text; } - return text.Length == 0 ? href : "[" + text + "](" + href + ")"; + if ( string.IsNullOrWhiteSpace( text ) ) + { + text = href; + } + + return string.Format( InvariantCulture, LinkFormat, text, href ); } // a span occupies a single line and its delimiters cannot be padded by whitespace, so the content is @@ -817,7 +851,7 @@ private sealed class BulletedList private BulletedList( Func generate ) => this.generate = generate; - private static string Increment( int number ) => number.ToString( CultureInfo.InvariantCulture ) + ". "; + private static string Increment( int number ) => number.ToString( InvariantCulture ) + ". "; private static string Bullet( int number ) => "* "; diff --git a/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/Documented.cs b/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/Documented.cs index b5b52d89..0dbe9bf6 100644 --- a/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/Documented.cs +++ b/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/Documented.cs @@ -154,4 +154,51 @@ public class Documented /// Gets or sets the site, which is . /// public string Site { get; set; } + + /// + /// Gets or sets the spec, which is described by . + /// + public string Spec { get; set; } + + /// + /// Gets or sets the anonymous, which is described by the specification. + /// + public string Anonymous { get; set; } + + /// + /// Gets or sets the manual, which is described by the + /// specification. + /// + public string Manual { get; set; } + + /// + /// Gets or sets the guide, which is described by . + /// + public string Guide { get; set; } + + /// + /// Gets or sets the cited. + /// + public string Cited { get; set; } + + /// + /// Gets or sets the related, which is described by the related + /// specification. + /// + public string Related { get; set; } + + /// + /// Gets or sets the referred. + /// + public string Referred { get; set; } + + /// + /// Gets or sets the outline. + /// + /// First step + /// Second step + /// + /// Text after list + /// + public string Outline { get; set; } } \ No newline at end of file diff --git a/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/MinimalApi.cs b/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/MinimalApi.cs index ca8d95ff..8266437d 100644 --- a/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/MinimalApi.cs +++ b/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Simulators/MinimalApi.cs @@ -64,6 +64,27 @@ public static class MinimalApi /// The outlined answer. public static int Outlined() => 42; + /// Stepped + /// + /// Text before list + /// + /// First step + /// Second step + /// + /// Text after list + /// + /// The stepped answer. + public static int Stepped() => 42; + + /// Linked + /// + /// + /// + /// + /// + /// The linked answer. + public static int Linked() => 42; + /// /// Echo /// diff --git a/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Transformers/XmlCommentsStructureTest.cs b/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Transformers/XmlCommentsStructureTest.cs index 7261cd15..1305edc5 100644 --- a/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Transformers/XmlCommentsStructureTest.cs +++ b/src/AspNetCore/WebApi/test/Asp.Versioning.OpenApi.Tests/Transformers/XmlCommentsStructureTest.cs @@ -107,7 +107,7 @@ public void bulleted_list_should_be_resolved_into_bullets() var summary = comments.GetSummary( property ); // assert - summary.Should().Be( "Gets or sets the tags.\n* First\n* Second" ); + summary.Should().Be( "Gets or sets the tags.\n\n* First\n* Second" ); } [Fact] @@ -121,7 +121,7 @@ public void numbered_list_should_be_resolved_into_bullets() var summary = comments.GetSummary( property ); // assert - summary.Should().Be( "Gets or sets the steps.\n1. First\n2. Second" ); + summary.Should().Be( "Gets or sets the steps.\n\n1. First\n2. Second" ); } [Fact] @@ -137,6 +137,7 @@ public void list_item_should_be_resolved_from_a_term_and_description() // assert summary.Should().Be( "Gets or sets the definitions.\n" + + "\n" + "* **Foo**: Does foo\n" + "* Bar\n" + "* Just a description\n" + @@ -193,7 +194,7 @@ public void table_of_one_column_should_be_resolved_into_bullets() var summary = comments.GetSummary( property ); // assert - summary.Should().Be( "Gets or sets the narrow.\n* Only\n* One" ); + summary.Should().Be( "Gets or sets the narrow.\n\n* Only\n* One" ); } [Fact] @@ -348,7 +349,7 @@ public void anchor_should_be_resolved_into_a_link() } [Fact] - public void anchor_without_text_should_be_resolved_into_its_address() + public void self_closing_anchor_should_be_resolved_into_a_link_labeled_by_its_address() { // arrange var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); @@ -358,7 +359,130 @@ public void anchor_without_text_should_be_resolved_into_its_address() var summary = comments.GetSummary( property ); // assert - summary.Should().Be( "Gets or sets the site, which is https://example.com." ); + summary.Should().Be( + "Gets or sets the site, which is [https://example.com](https://example.com)." ); + } + + [Fact] + public void empty_anchor_should_be_resolved_into_a_link_labeled_by_its_address() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Spec ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( + "Gets or sets the spec, which is described by " + + "[https://example.com/spec](https://example.com/spec)." ); + } + + [Fact] + public void anchor_without_an_address_should_be_resolved_into_its_text() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Anonymous ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( "Gets or sets the anonymous, which is described by the specification." ); + } + + [Fact] + public void see_with_an_address_should_be_resolved_into_a_link() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Manual ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( + "Gets or sets the manual, which is described by [the specification](https://example.com)." ); + } + + [Fact] + public void see_with_an_address_and_without_text_should_be_labeled_by_its_address() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Guide ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( + "Gets or sets the guide, which is described by " + + "[https://example.com/guide](https://example.com/guide)." ); + } + + [Fact] + public void see_with_a_reference_should_not_be_resolved_into_a_link() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Cited ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( "Gets or sets the cited." ); + } + + [Fact] + public void seealso_with_an_address_should_be_resolved_into_a_link() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Related ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( + "Gets or sets the related, which is described by " + + "[the related specification](https://example.com/related)." ); + } + + [Fact] + public void seealso_with_a_reference_should_not_be_resolved_into_a_link() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Referred ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( "Gets or sets the referred." ); + } + + [Fact] + public void anchors_without_text_should_be_resolved_into_links() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var method = typeof( MinimalApi ).GetMethod( nameof( MinimalApi.Linked ) ); + + // act + var remarks = comments.GetRemarks( method ); + + // assert + remarks.Should().Be( + "[https://example.org/spec](https://example.org/spec)\n" + + "\n" + + "[https://example.org/spec](https://example.org/spec)" ); } [Fact] @@ -463,9 +587,49 @@ public void text_around_a_list_should_not_be_indented() // assert remarks.Should().Be( "Text before list\n" + - "\n" + + "\n\n" + "* First\n" + "* Second\n" + + "\n\n" + + "Text after list" ); + } + + [Fact] + public void text_after_a_list_should_not_be_absorbed_by_the_last_item() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var method = typeof( MinimalApi ).GetMethod( nameof( MinimalApi.Stepped ) ); + + // act + var remarks = comments.GetRemarks( method ); + + // assert + remarks.Should().Be( + "Text before list\n" + + "\n" + + "1. First step\n" + + "2. Second step\n" + + "\n" + + "Text after list" ); + } + + [Fact] + public void text_after_a_list_should_not_be_absorbed_within_a_summary() + { + // arrange + var comments = XmlComments.FromFile( FilePath.XmlCommentFile ); + var property = typeof( Documented ).GetProperty( nameof( Documented.Outline ) ); + + // act + var summary = comments.GetSummary( property ); + + // assert + summary.Should().Be( + "Gets or sets the outline.\n" + + "\n" + + "1. First step\n" + + "2. Second step\n" + "\n" + "Text after list" ); }