Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions src/ZeroC.Slice.Generator/DocCommentFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,22 @@ internal static IEnumerable<CommentTag> FormatSeeAlsoTags(Comment? comment, stri

foreach (CommentLink link in seeTags)
{
if (link is ResolvedCommentLink r)
if (link is not ResolvedCommentLink r)
{
continue;
}

if (r.Entity is TypeAlias alias)
{
// Type aliases don't generate a C# type; map to the underlying C# type. We can't reliably
// produce a seealso for a generic mapped type without C# parsing, so skip those.
string mapped = alias.UnderlyingType.Type.ToTypeString(currentNamespace);
if (!mapped.Contains('<', StringComparison.Ordinal))
{
yield return new CommentTag("seealso", "cref", mapped, "");
}
}
else
{
yield return new CommentTag("seealso", "cref", FormatEntityCref(r.Entity, currentNamespace), "");
}
Expand All @@ -45,13 +60,23 @@ internal static IEnumerable<CommentTag> FormatSeeAlsoTags(Comment? comment, stri

private static string FormatInlineLink(CommentLink link, string currentNamespace) => link switch
{
// Type aliases don't generate C# types, so output the identifier as plain text.
ResolvedCommentLink { Entity: TypeAlias } r => $"<c>{CommentTag.XmlEscape(r.Entity.Identifier)}</c>",
ResolvedCommentLink { Entity: TypeAlias alias } => FormatTypeAliasInline(alias, currentNamespace),
ResolvedCommentLink r => $"""<see cref="{FormatEntityCref(r.Entity, currentNamespace)}" />""",
UnresolvedCommentLink u => $"<c>{CommentTag.XmlEscape(u.Identifier)}</c>",
Comment on lines +63 to 65
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
_ => ""
};

private static string FormatTypeAliasInline(TypeAlias alias, string currentNamespace)
{
// Type aliases don't generate a C# type; link to the underlying C# type instead. For generic mapped
// types we emit the type name as inline code (XML-escaped) to avoid constructing cref-friendly forms
// like IList{T} or IDictionary{T, U}.
string mapped = alias.UnderlyingType.Type.ToTypeString(currentNamespace);
return mapped.Contains('<', StringComparison.Ordinal)
? $"<c>{CommentTag.XmlEscape(mapped)}</c>"
: $"""<see cref="{mapped}" />""";
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
: $"""<see cref="{mapped}" />""";
: $"""<see cref="{CommentTag.XmlEscape(mapped)}" />""";

Copilot uses AI. Check for mistakes.
}

private static string FormatEntityCref(Entity entity, string currentNamespace)
{
string name = entity switch
Expand Down
28 changes: 28 additions & 0 deletions tests/IceRpc.Slice.Generator.Tests/DocumentationTests.slice
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,35 @@ module IceRpc::Slice::Generator::Tests

// This file contains Slice definitions for testing the mapping of doc comments

// Type aliases used to test how doc comment links to a typealias are mapped to its underlying C# type:
// - UserName maps to a non-generic C# type (string), so links resolve to <see cref="string" />.
// - UserList, UserMap, UserResult map to generic C# types, so inline @link is emitted as <c>...</c>
// (XML-escaped) and @see is skipped.
typealias UserName = string
typealias UserList = Sequence<string>
typealias UserMap = Dictionary<string, MySession>
typealias UserResult = Result<MySession, string>

/// Tests how doc comment links to type aliases are mapped to their underlying C# types.
/// The {@link UserName} alias maps to a non-generic type while {@link UserList},
/// {@link UserMap}, and {@link UserResult} map to generic types.
/// @see UserName
/// @see UserList
/// @see UserMap
/// @see UserResult
interface MyTypealiasDocs {
/// 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
Comment on lines +24 to +34
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

}

/// This is the session manager interface summary. The caller must use the {@link MyAuthorizationManager::createAuthToken}
/// operation to create an authentication token before creating a session.
Expand Down
Loading