Map TypeAlias doc links to the underlying C# type#4573
Map TypeAlias doc links to the underlying C# type#4573pepone wants to merge 1 commit intoicerpc:mainfrom
Conversation
Fix icerpc#4494 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.
There was a problem hiding this comment.
Pull request overview
Updates the Slice-to-C# documentation generator so doc-comment links that target typealias entities resolve (when possible) to the aliased underlying C# type, and expands the documentation Slice test inputs to exercise these cases.
Changes:
- Map inline
{@link TypeAlias}to the underlying mapped C# type, emitting<see cref="..."/>for non-generic mappings and<c>...</c>for generic mappings. - Map
@see TypeAliasto<seealso/>only for non-generic underlying mappings; skip generic mappings. - Extend
DocumentationTests.slicewith additional typealias/link scenarios (string, sequence, dictionary, result).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/ZeroC.Slice.Generator/DocCommentFormatter.cs |
Adjusts how @link/@see are formatted for TypeAlias targets by mapping them to the underlying C# type. |
tests/IceRpc.Slice.Generator.Tests/DocumentationTests.slice |
Adds Slice definitions and doc comments that reference type aliases to validate the new link formatting behavior. |
| ResolvedCommentLink { Entity: TypeAlias alias } => FormatTypeAliasInline(alias, currentNamespace), | ||
| ResolvedCommentLink r => $"""<see cref="{FormatEntityCref(r.Entity, currentNamespace)}" />""", | ||
| UnresolvedCommentLink u => $"<c>{CommentTag.XmlEscape(u.Identifier)}</c>", |
There was a problem hiding this comment.
The <see cref="..." /> emitted for resolved inline links interpolates FormatEntityCref(...) directly into an XML attribute value without XML-escaping. Since FormatEntityCref can incorporate cs::type (and namespace strings), this can produce malformed XML doc comments when the type/namespace contains characters like &, <, > or ". Escape the cref value (e.g., via CommentTag.XmlEscape(...)) before splicing it into the <see/> tag, or switch to generating this tag through CommentTag to ensure consistent escaping.
| string mapped = alias.UnderlyingType.Type.ToTypeString(currentNamespace); | ||
| return mapped.Contains('<', StringComparison.Ordinal) | ||
| ? $"<c>{CommentTag.XmlEscape(mapped)}</c>" | ||
| : $"""<see cref="{mapped}" />"""; |
There was a problem hiding this comment.
For non-generic type-alias mappings, this emits <see cref="{mapped}" /> without XML-escaping mapped. ToTypeString can return strings influenced by cs::type, so special XML characters could break the doc comment. Escape mapped before inserting it into the cref attribute (the generic branch already escapes via XmlEscape).
| : $"""<see cref="{mapped}" />"""; | |
| : $"""<see cref="{CommentTag.XmlEscape(mapped)}" />"""; |
InsertCreativityHere
left a comment
InsertCreativityHere
left a comment
There was a problem hiding this comment.
Looks good to me!
| /// Returns the {@link UserName} for the active user. | ||
| getName() -> UserName | ||
|
|
||
| /// Returns a {@link UserList} of active user names. | ||
| getNames() -> UserList | ||
|
|
||
| /// Returns a {@link UserMap} keyed by user name. | ||
| getSessions() -> UserMap | ||
|
|
||
| /// Returns a {@link UserResult} containing the session or an error message. | ||
| getResult() -> UserResult |
There was a problem hiding this comment.
Feel free to keep the separate methods if you want. But, you could also accomplish the exact same testing with a single operation
/// Returns the {@link UserName} for the active user.
/// This does not return a {@link UserList}, or a {@link UserMap}, or even a {@link UserResult}.
getName() -> UserName
Unless there's some oddity in the C# code-generator I'm unaware of, there really shouldn't be any difference between these things.
Fix #4494