Skip to content

Escape cref value in generated <see> doc comments#4566

Closed
pepone wants to merge 3 commits intoicerpc:mainfrom
pepone:fix/doc-comment-cref-escape
Closed

Escape cref value in generated <see> doc comments#4566
pepone wants to merge 3 commits intoicerpc:mainfrom
pepone:fix/doc-comment-cref-escape

Conversation

@pepone
Copy link
Copy Markdown
Member

@pepone pepone commented Apr 29, 2026

Fix #4494

cs::type can contain XML-special characters (e.g. generics like
List<int>), which would produce malformed XML doc comments when
spliced raw into a cref attribute.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Slice-to-C# doc comment formatter to properly XML-escape generated <see cref="..."/> attribute values, preventing malformed XML when entity names/namespaces contain XML-special characters (Fix #4494).

Changes:

  • XML-escapes the cref attribute value when emitting inline <see ... /> links for resolved comment links.

Copy link
Copy Markdown
Member

@InsertCreativityHere InsertCreativityHere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct.

The string that we generate here is fully contained within double quotes:
<see cref="..." /> So where's the need to escape XML characters?
The only way to break the XML is to put a literal \" character in their cs::type or cs::identifier, which means they've gone insane. Not something we should protect against :)

Assuming sanity, there's no reason for &, <, > or " to ever appear in cs::identifier.
I don't think that & or " can ever validly be part of a type-name in C#, can they?
< and > can appear in cs::type for a generic type, but according to the docs, the correct behavior here isn't to XML escape them, it's to replace them with { and }:

@externl
Copy link
Copy Markdown
Member

externl commented Apr 29, 2026

I don't think this is correct.

The string that we generate here is fully contained within double quotes: <see cref="..." /> So where's the need to escape XML characters? The only way to break the XML is to put a literal \" character in their cs::type or cs::identifier, which means they've gone insane. Not something we should protect against :)

Assuming sanity, there's no reason for &, <, > or " to ever appear in cs::identifier. I don't think that & or " can ever validly be part of a type-name in C#, can they? < and > can appear in cs::type for a generic type, but according to the docs, the correct behavior here isn't to XML escape them, it's to replace them with { and }:

Seems XML is even more complicated than this. For an attribute value, < is bad, > is fine.

@InsertCreativityHere
Copy link
Copy Markdown
Member

InsertCreativityHere commented Apr 29, 2026

If you search for Dictionary{ on this page:
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/recommended-tags#cref-attribute

So, instead of XML escaping, we should be ignoring " and &, and we should replace < and > with { and }.
The {} syntax is probably because DocFx wants to avoid this specific headache with XML : v)

@pepone
Copy link
Copy Markdown
Member Author

pepone commented Apr 29, 2026

Seems XML is even more complicated than this. For an attribute value, < is bad, > is fine.

I guess this is not a general XML rule, but a C# documentation one, isn't it?

@externl
Copy link
Copy Markdown
Member

externl commented Apr 29, 2026

See also: https://learn.microsoft.com/en-us/dotnet/desktop/xaml-services/xml-character-entities

pepone added 2 commits April 30, 2026 12:46
For inline @link to a TypeAlias, emit <see cref="MappedType"/> when the
mapped type is non-generic, or <c>MappedType</c> (XML-escaped) when it
is generic — closed generics like IList<string> have no valid C# cref
form, so a clickable link isn't possible. For @see, emit <seealso/>
only for non-generic mapped types and skip generic ones.

Add typealias coverage to DocumentationTests.slice for non-generic,
sequence, dictionary, and result mappings.
@pepone pepone closed this Apr 30, 2026
@pepone
Copy link
Copy Markdown
Member Author

pepone commented Apr 30, 2026

Replaced by #4573

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Audit-Low] DocCommentFormatter.FormatEntityCref doesn't escape name/…

4 participants