From cf0d4d93b4b2b224ea3aac5bdccd5f8bf0635587 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 25 Apr 2025 18:20:18 +0200 Subject: [PATCH 1/5] Extensions: xml docs --- .../BinderFactory.BinderFactoryVisitor.cs | 29 +- .../Compiler/DocumentationCommentCompiler.cs | 23 + ...cumentationCommentIDVisitor.PartVisitor.cs | 2 +- ...urceExtensionImplementationMethodSymbol.cs | 9 + .../Test/Emit3/Semantics/ExtensionTests.cs | 660 ++++++++++++++++++ 5 files changed, 717 insertions(+), 6 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/BinderFactory.BinderFactoryVisitor.cs b/src/Compilers/CSharp/Portable/Binder/BinderFactory.BinderFactoryVisitor.cs index 2e82ab6e0a8b5..16acc28f6909d 100644 --- a/src/Compilers/CSharp/Portable/Binder/BinderFactory.BinderFactoryVisitor.cs +++ b/src/Compilers/CSharp/Portable/Binder/BinderFactory.BinderFactoryVisitor.cs @@ -1213,7 +1213,7 @@ public override Binder VisitXmlNameAttribute(XmlNameAttributeSyntax parent) { case XmlNameAttributeElementKind.Parameter: case XmlNameAttributeElementKind.ParameterReference: - result = GetParameterNameAttributeValueBinder(memberSyntax, result); + result = GetParameterNameAttributeValueBinder(memberSyntax, isParamRef: elementKind == XmlNameAttributeElementKind.ParameterReference, nextBinder: result); break; case XmlNameAttributeElementKind.TypeParameter: result = GetTypeParameterNameAttributeValueBinder(memberSyntax, includeContainingSymbols: false, nextBinder: result); @@ -1234,16 +1234,28 @@ public override Binder VisitXmlNameAttribute(XmlNameAttributeSyntax parent) /// We're in a <param> or <paramref> element, so we want a binder that can see /// the parameters of the associated member and nothing else. /// - private Binder GetParameterNameAttributeValueBinder(MemberDeclarationSyntax memberSyntax, Binder nextBinder) + private Binder GetParameterNameAttributeValueBinder(MemberDeclarationSyntax memberSyntax, bool isParamRef, Binder nextBinder) { if (memberSyntax is BaseMethodDeclarationSyntax { ParameterList: { ParameterCount: > 0 } } baseMethodDeclSyntax) { Binder outerBinder = VisitCore(memberSyntax.Parent); MethodSymbol method = GetMethodSymbol(baseMethodDeclSyntax, outerBinder); + + if (isParamRef && method.TryGetInstanceExtensionParameter(out var _)) + { + nextBinder = new WithExtensionParameterBinder(method.ContainingType, nextBinder); + } + return new WithParametersBinder(method.Parameters, nextBinder); } - - if (memberSyntax is TypeDeclarationSyntax { ParameterList: { ParameterCount: > 0 } } typeDeclaration) + else if (memberSyntax is ExtensionDeclarationSyntax extensionDeclaration) + { + Binder outerBinder = VisitCore(memberSyntax); + SourceNamedTypeSymbol type = ((NamespaceOrTypeSymbol)outerBinder.ContainingMemberOrLambda).GetSourceTypeMember((TypeDeclarationSyntax)memberSyntax); + Debug.Assert(type.IsExtension); + return new WithExtensionParameterBinder(type, nextBinder); + } + else if (memberSyntax is TypeDeclarationSyntax { ParameterList: { ParameterCount: > 0 } } typeDeclaration) { _ = typeDeclaration.ParameterList; Binder outerBinder = VisitCore(memberSyntax); @@ -1268,6 +1280,11 @@ private Binder GetParameterNameAttributeValueBinder(MemberDeclarationSyntax memb ImmutableArray parameters = property.Parameters; + if (isParamRef && property.TryGetInstanceExtensionParameter(out var _)) + { + nextBinder = new WithExtensionParameterBinder(property.ContainingType, nextBinder); + } + // BREAK: Dev11 also allows "value" for readonly properties, but that doesn't // make sense and we don't have a symbol. if ((object)property.SetMethod != null) @@ -1278,8 +1295,10 @@ private Binder GetParameterNameAttributeValueBinder(MemberDeclarationSyntax memb if (parameters.Any()) { - return new WithParametersBinder(parameters, nextBinder); + nextBinder = new WithParametersBinder(parameters, nextBinder); } + + return nextBinder; } else if (memberKind == SyntaxKind.DelegateDeclaration) { diff --git a/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs index ec6661e4541b2..cb9f2952a15eb 100644 --- a/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs @@ -237,6 +237,29 @@ public override void VisitNamedType(NamedTypeSymbol symbol) } #nullable enable + public override void VisitMethod(MethodSymbol symbol) + { + if (symbol is SourceExtensionImplementationMethodSymbol implementation) + { + var symbolForDocComment = implementation.UnderlyingMethod is SourcePropertyAccessorSymbol accessorSymbol + ? accessorSymbol.AssociatedSymbol + : implementation.UnderlyingMethod; + + if (symbolForDocComment.GetDocumentationCommentXml(preferredCulture: null, _processIncludes, _cancellationToken).IsEmpty()) + { + return; + } + + WriteLine("", symbol.GetDocumentationCommentId()); + Indent(); + WriteLine("", symbolForDocComment.GetDocumentationCommentId()); + Unindent(); + WriteLine(""); + return; + } + + DefaultVisit(symbol); + } /// /// Compile documentation comments on the symbol and write them to the stream if one is provided. diff --git a/src/Compilers/CSharp/Portable/DocumentationComments/DocumentationCommentIDVisitor.PartVisitor.cs b/src/Compilers/CSharp/Portable/DocumentationComments/DocumentationCommentIDVisitor.PartVisitor.cs index 4f725ebe8d2f7..d40824ac80acb 100644 --- a/src/Compilers/CSharp/Portable/DocumentationComments/DocumentationCommentIDVisitor.PartVisitor.cs +++ b/src/Compilers/CSharp/Portable/DocumentationComments/DocumentationCommentIDVisitor.PartVisitor.cs @@ -181,7 +181,7 @@ public override object VisitNamedType(NamedTypeSymbol symbol, StringBuilder buil builder.Append('.'); } - builder.Append(symbol.Name); + builder.Append(symbol.IsExtension ? symbol.ExtensionName : symbol.Name); if (symbol.Arity != 0) { diff --git a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs index 1201586ed0ee4..cad71ab21d477 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs @@ -4,6 +4,8 @@ using System.Collections.Immutable; using System.Diagnostics; +using System.Globalization; +using System.Threading; using Microsoft.CodeAnalysis.CSharp.Emit; using Microsoft.CodeAnalysis.PooledObjects; @@ -11,6 +13,8 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols { internal sealed class SourceExtensionImplementationMethodSymbol : RewrittenMethodSymbol // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Do we need to implement ISynthesizedMethodBodyImplementationSymbol? { + private string? lazyDocComment; + public SourceExtensionImplementationMethodSymbol(MethodSymbol sourceMethod) : base(sourceMethod, TypeMap.Empty, sourceMethod.ContainingType.TypeParameters.Concat(sourceMethod.TypeParameters)) { @@ -131,6 +135,11 @@ internal override int TryGetOverloadResolutionPriority() return UnderlyingMethod.TryGetOverloadResolutionPriority(); } + public override string GetDocumentationCommentXml(CultureInfo? preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default) + { + return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment); + } + private sealed class ExtensionMetadataMethodParameterSymbol : RewrittenMethodParameterSymbol { public ExtensionMetadataMethodParameterSymbol(SourceExtensionImplementationMethodSymbol containingMethod, ParameterSymbol sourceParameter) : diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs index af139c7a228fc..649536c41eece 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs @@ -36601,4 +36601,664 @@ static unsafe class E Assert.False(verifier.HasLocalsInit("E.M")); Assert.True(verifier.HasLocalsInit("E.M2")); } + + [Fact] + public void XmlDoc_01() + { + // Instance members + var src = """ +/// Summary for E +static class E +{ + /// Summary for extension block + /// Description for T + /// Description for t + extension(T t) + { + /// Summary for M + /// Description for U + /// Description for u + public void M(U u) => throw null!; + + /// Summary for P + public int P => 0; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments, assemblyName: "Test"); + comp.VerifyEmitDiagnostics(); + + var e = comp.GetMember("E"); + AssertEx.Equal(""" + + Summary for E + + +""", e.GetDocumentationCommentXml()); + + var extension = e.GetTypeMembers().Single(); + Assert.Equal("T:E.<>E__0`1", extension.GetDocumentationCommentId()); + AssertEx.Equal(""" + + Summary for extension block + Description for T + Description for t + + +""", extension.GetDocumentationCommentXml()); + + var mSkeleton = extension.GetMember("M"); + AssertEx.Equal(""" + + Summary for M + Description for U + Description for u + + +""", mSkeleton.GetDocumentationCommentXml()); + + var mImplementation = e.GetMember("M"); + AssertEx.Equal(""" + + + + +""", mImplementation.GetDocumentationCommentXml()); + + var p = extension.GetMember("P"); + AssertEx.Equal(""" + + Summary for P + + +""", p.GetDocumentationCommentXml()); + + var pGetImplementation = e.GetMember("get_P"); + AssertEx.Equal(""" + + + + +""", pGetImplementation.GetDocumentationCommentXml()); + + var expected = """ + + + + Test + + + + Summary for E + + + Summary for extension block + Description for T + Description for t + + + Summary for M + Description for U + Description for u + + + Summary for P + + + + + + + + + +"""; + AssertEx.Equal(expected, GetDocumentationCommentText(comp)); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + Assert.Equal(["(T, T)", "(t, T t)", "(U, U)", "(u, U u)"], PrintXmlNameSymbols(tree, model)); + } + + private static IEnumerable PrintXmlNameSymbols(SyntaxTree tree, SemanticModel model) + { + var docComments = tree.GetCompilationUnitRoot().DescendantTrivia().Select(trivia => trivia.GetStructure()).OfType(); + var xmlNames = docComments.SelectMany(doc => doc.DescendantNodes().OfType()); + var result = xmlNames.Select(name => print(name)); + return result; + + string print(XmlNameAttributeSyntax name) + { + IdentifierNameSyntax identifier = name.Identifier; + var symbol = model.GetSymbolInfo(identifier).Symbol; + var symbolDisplay = symbol is null ? "null" : symbol.ToTestDisplayString(); + return (identifier, symbolDisplay).ToString(); + } + } + + [Fact] + public void XmlDoc_02() + { + // Static members + var src = """ +/// Summary for E +static class E +{ + /// Summary for extension block + /// Description for T + /// Description for t + extension(T t) + { + /// Summary for M + /// Description for U + /// Description for u + public static void M(U u) => throw null!; + + /// Summary for P + public static int P => 0; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics(); + + var e = comp.GetMember("E"); + Assert.Equal(""" + + Summary for E + + +""", e.GetDocumentationCommentXml()); + + var extension = e.GetTypeMembers().Single(); + AssertEx.Equal(""" + + Summary for extension block + Description for T + Description for t + + +""", extension.GetDocumentationCommentXml()); + + var mSkeleton = extension.GetMember("M"); + AssertEx.Equal(""" + + Summary for M + Description for U + Description for u + + +""", mSkeleton.GetDocumentationCommentXml()); + + var mImplementation = e.GetMember("M"); + AssertEx.Equal(""" + + + + +""", mImplementation.GetDocumentationCommentXml()); + + var p = extension.GetMember("P"); + AssertEx.Equal(""" + + Summary for P + + +""", p.GetDocumentationCommentXml()); + + var pGetImplementation = e.GetMember("get_P"); + AssertEx.Equal(""" + + + + +""", pGetImplementation.GetDocumentationCommentXml()); + } + + [Fact] + public void XmlDoc_03() + { + // No docs + var src = """ +static class E +{ + extension(T t) + { + public static void M(U u) => throw null!; + public static int P => 0; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics(); + + var e = comp.GetMember("E"); + Assert.Empty(e.GetDocumentationCommentXml()); + + var extension = e.GetTypeMembers().Single(); + Assert.Empty(extension.GetDocumentationCommentXml()); + + var mSkeleton = extension.GetMember("M"); + Assert.Empty(mSkeleton.GetDocumentationCommentXml()); + + var mImplementation = e.GetMember("M"); + Assert.Empty(mImplementation.GetDocumentationCommentXml()); + + var p = extension.GetMember("P"); + Assert.Empty(p.GetDocumentationCommentXml()); + + var pGetImplementation = e.GetMember("get_P"); + Assert.Empty(pGetImplementation.GetDocumentationCommentXml()); + } + + [Fact] + public void XmlDoc_Param_01() + { + // Unnamed extension parameter, with an attempted corresponding param + var src = """ +/// Summary for E +static class E +{ + /// Summary for extension block + /// Description for extension parameter + extension(object) + { + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (5,22): warning CS1572: XML comment has a param tag for '', but there is no parameter by that name + // /// Description for extension parameter + Diagnostic(ErrorCode.WRN_UnmatchedParamTag, "").WithArguments("").WithLocation(5, 22)); + } + + [Fact] + public void XmlDoc_Param_02() + { + // Unnamed and undocumented extension parameter + var src = """ +static class E +{ + /// Summary for extension block + extension(object) + { + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics(); + } + + [Fact] + public void XmlDoc_Param_03() + { + // param on member instead of extension block + var src = """ +/// Summary for E +static class E +{ + /// Summary for extension block + extension(object o) + { + /// Summary for M + /// Description for o + public void M(object o2) => throw null!; + } +} +"""; + // Tracked by https://github.com/dotnet/roslyn/issues/76130 : consider warning (WRN_MissingParamTag) about missing documentation for extension parameter + // since one of the instance members has a tag + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (8,26): warning CS1572: XML comment has a param tag for 'o', but there is no parameter by that name + // /// Description for o + Diagnostic(ErrorCode.WRN_UnmatchedParamTag, "o").WithArguments("o").WithLocation(8, 26), + // (9,30): warning CS1573: Parameter 'o2' has no matching param tag in the XML comment for 'E.extension(object).M(object)' (but other parameters do) + // public void M(object o2) => throw null!; + Diagnostic(ErrorCode.WRN_MissingParamTag, "o2").WithArguments("o2", "E.extension(object).M(object)").WithLocation(9, 30)); + } + + [Fact] + public void XmlDoc_Param_04() + { + var src = """ +/// Summary for E +static class E +{ + /// Summary for extension block + /// Description for o + extension(object o) + { + /// Summary for M + public void M(object o2) => throw null!; + } +} +"""; + // Tracked by https://github.com/dotnet/roslyn/issues/76130 : consider warning (WRN_MissingParamTag) about missing documentation for member parameter + // since the extension parameter is documented + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics(); + } + + [Fact] + public void XmlDoc_Param_05() + { + // multiple parameters in extension block, one is covered by param + var src = """ +static class E +{ + /// Summary for extension block + /// Description for o + extension(object o, object o2) + { + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (5,25): error CS9285: An extension container can have only one receiver parameter + // extension(object o, object o2) + Diagnostic(ErrorCode.ERR_ReceiverParameterOnlyOne, "object o2").WithLocation(5, 25)); + } + + [Fact] + public void XmlDoc_Param_06() + { + var src = """ +static class E +{ + /// Summary for extension block + /// First description for o + /// Second description for o + extension(object o) + { + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (5,16): warning CS1571: XML comment has a duplicate param tag for 'o' + // /// Second description for o + Diagnostic(ErrorCode.WRN_DuplicateParamTag, @"name=""o""").WithArguments("o").WithLocation(5, 16)); + } + + [Fact] + public void XmlDoc_Param_07() + { + var src = """ +static class E +{ + /// Summary for extension block + /// Description for other + extension(object o) + { + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (4,22): warning CS1572: XML comment has a param tag for 'other', but there is no parameter by that name + // /// Description for other + Diagnostic(ErrorCode.WRN_UnmatchedParamTag, "other").WithArguments("other").WithLocation(4, 22)); + } + + [Fact] + public void XmlDoc_TypeParam_01() + { + var src = """ +static class E +{ + /// Summary for extension block + /// First description for T + /// Second description for T + extension(T t) + { + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (5,20): warning CS1710: XML comment has a duplicate typeparam tag for 'T' + // /// Second description for T + Diagnostic(ErrorCode.WRN_DuplicateTypeParamTag, @"name=""T""").WithArguments("T").WithLocation(5, 20)); + } + + [Fact] + public void XmlDoc_TypeParam_02() + { + // only one type parameter documented + var src = """ +static class E +{ + /// Summary for extension block + /// Description for T + extension(C c) + { + } +} +class C { } +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (5,18): warning CS1712: Type parameter 'U' has no matching typeparam tag in the XML comment on 'E.extension(C)' (but other type parameters do) + // extension(C c) + Diagnostic(ErrorCode.WRN_MissingTypeParamTag, "U").WithArguments("U", "E.extension(C)").WithLocation(5, 18)); + } + + [Fact] + public void XmlDoc_TypeParam_03() + { + // typeparam on member instead of extension block + var src = """ +/// Summary for E +static class E +{ + /// Summary for extension block + extension(T t) + { + /// Summary for M + /// Description for T + public static void M(U u) => throw null!; + } +} +"""; + // Tracked by https://github.com/dotnet/roslyn/issues/76130 : consider warning (WRN_MissingTypeParamTag) about missing documentation for extension type parameter + // since one of the members has a tag + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (8,30): warning CS1711: XML comment has a typeparam tag for 'T', but there is no type parameter by that name + // /// Description for T + Diagnostic(ErrorCode.WRN_UnmatchedTypeParamTag, "T").WithArguments("T").WithLocation(8, 30), + // (9,30): warning CS1712: Type parameter 'U' has no matching typeparam tag in the XML comment on 'E.extension(T).M(U)' (but other type parameters do) + // public static void M(U u) => throw null!; + Diagnostic(ErrorCode.WRN_MissingTypeParamTag, "U").WithArguments("U", "E.extension(T).M(U)").WithLocation(9, 30)); + } + + [Fact] + public void XmlDoc_TypeParam_04() + { + var src = """ +/// Summary for E +static class E +{ + /// Summary for extension block + /// Description for T + extension(T t) + { + /// Summary for M + public static void M(U u) => throw null!; + } +} +"""; + // Tracked by https://github.com/dotnet/roslyn/issues/76130 : consider warning (WRN_MissingTypeParamTag) about missing documentation for member type parameter + // since the extension type parameter is documented + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics(); + } + + [Fact] + public void XmlDoc_TypeParam_05() + { + var src = """ +static class E +{ + /// Summary for extension block + /// Description for TOther + extension(T t) + { + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (4,26): warning CS1711: XML comment has a typeparam tag for 'TOther', but there is no type parameter by that name + // /// Description for TOther + Diagnostic(ErrorCode.WRN_UnmatchedTypeParamTag, "TOther").WithArguments("TOther").WithLocation(4, 26), + // (5,15): warning CS1712: Type parameter 'T' has no matching typeparam tag in the XML comment on 'E.extension(T)' (but other type parameters do) + // extension(T t) + Diagnostic(ErrorCode.WRN_MissingTypeParamTag, "T").WithArguments("T", "E.extension(T)").WithLocation(5, 15)); + } + + [Fact] + public void XmlDoc_ParamRef_01() + { + // paramref to extension parameter + var src = """ +/// Summary for E +static class E +{ + /// Good paramref + extension(object o) + { + /// Good paramref + public void M(object o2) => throw null!; + + /// Error paramref + public static void M2(object o2) => throw null!; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (10,53): warning CS1734: XML comment on 'E.extension(object).M2(object)' has a paramref tag for 'o', but there is no parameter by that name + // /// Error paramref + Diagnostic(ErrorCode.WRN_UnmatchedParamRefTag, "o").WithArguments("o", "E.extension(object).M2(object)").WithLocation(10, 53)); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + Assert.Equal(["(o, System.Object o)", "(o, System.Object o)", "(o, null)"], PrintXmlNameSymbols(tree, model)); + } + + [Fact] + public void XmlDoc_ParamRef_02() + { + // paramref to extension parameter on properties + var src = """ +static class E +{ + extension(object o) + { + /// Good paramref + public int P => 42; + + /// Error paramref + public static int P2 => 42; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (8,53): warning CS1734: XML comment on 'E.extension(object).P2' has a paramref tag for 'o', but there is no parameter by that name + // /// Error paramref + Diagnostic(ErrorCode.WRN_UnmatchedParamRefTag, "o").WithArguments("o", "E.extension(object).P2").WithLocation(8, 53)); + } + + [Fact] + public void XmlDoc_ParamRef_03() + { + var src = """ +static class E +{ + extension(object o) + { + /// Error paramref + public int P => 42; + + /// Error paramref + public static int P2 => 42; + + /// Good paramref + public int P3 { set { } } + + /// Good paramref + public static int P4 { set { } } + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (5,53): warning CS1734: XML comment on 'E.extension(object).P' has a paramref tag for 'value', but there is no parameter by that name + // /// Error paramref + Diagnostic(ErrorCode.WRN_UnmatchedParamRefTag, "value").WithArguments("value", "E.extension(object).P").WithLocation(5, 53), + // (8,53): warning CS1734: XML comment on 'E.extension(object).P2' has a paramref tag for 'value', but there is no parameter by that name + // /// Error paramref + Diagnostic(ErrorCode.WRN_UnmatchedParamRefTag, "value").WithArguments("value", "E.extension(object).P2").WithLocation(8, 53)); + } + + [Fact] + public void XmlDoc_TypeParamRef_01() + { + var src = """ +static class E +{ + /// Summary for extension block with typeparamref + extension(T t) + { + /// Summary for M with typeparamref + public void M() => throw null!; + + /// Summary for M with typeparamref + public static void M2() => throw null!; + + /// Summary for M with typeparamref + public int P => 42; + + /// Summary for M with typeparamref + public static int P2 => 42; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics(); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + Assert.Equal(["(T, T)", "(T, T)", "(T, T)", "(T, T)", "(T, T)"], PrintXmlNameSymbols(tree, model)); + } + + [Fact] + public void XmlDoc_TypeParamRef_02() + { + var src = """ +static class E +{ + /// Error typeparamref + extension(int) + { + /// Good typeparamref + public static void M(T t) => throw null!; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (3,57): warning CS1735: XML comment on 'E.extension(int)' has a typeparamref tag for 'T', but there is no type parameter by that name + // /// Error typeparamref + Diagnostic(ErrorCode.WRN_UnmatchedTypeParamRefTag, "T").WithArguments("T", "E.extension(int)").WithLocation(3, 57)); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + Assert.Equal(["(T, null)", "(T, T)"], PrintXmlNameSymbols(tree, model)); + } } From f82ebbf67a655476cf926b5709106c8b2393d917 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 29 Apr 2025 17:13:13 +0200 Subject: [PATCH 2/5] Address feedback --- .../Compiler/DocumentationCommentCompiler.cs | 28 +++++++-- .../Test/Emit3/Semantics/ExtensionTests.cs | 61 +++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs index cb9f2952a15eb..826201c5177fa 100644 --- a/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs @@ -241,11 +241,12 @@ public override void VisitMethod(MethodSymbol symbol) { if (symbol is SourceExtensionImplementationMethodSymbol implementation) { - var symbolForDocComment = implementation.UnderlyingMethod is SourcePropertyAccessorSymbol accessorSymbol - ? accessorSymbol.AssociatedSymbol - : implementation.UnderlyingMethod; + MethodSymbol underlyingMethod = implementation.UnderlyingMethod; + Symbol symbolForDocComment = underlyingMethod.IsAccessor() + ? underlyingMethod.AssociatedSymbol + : underlyingMethod; - if (symbolForDocComment.GetDocumentationCommentXml(preferredCulture: null, _processIncludes, _cancellationToken).IsEmpty()) + if (!hasDocumentationTrivia(symbolForDocComment)) { return; } @@ -258,7 +259,24 @@ public override void VisitMethod(MethodSymbol symbol) return; } - DefaultVisit(symbol); + base.VisitMethod(symbol); + return; + + static bool hasDocumentationTrivia(Symbol symbol) + { + foreach (SyntaxReference reference in symbol.DeclaringSyntaxReferences) + { + foreach (var trivia in reference.GetSyntax().GetLeadingTrivia()) + { + if (trivia.IsDocumentationCommentTrivia) + { + return true; + } + } + } + + return false; + } } /// diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs index 649536c41eece..b253e34caf7fa 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs @@ -36824,7 +36824,10 @@ static class E { extension(T t) { + // comment public static void M(U u) => throw null!; + + // comment public static int P => 0; } } @@ -36851,6 +36854,64 @@ static class E Assert.Empty(pGetImplementation.GetDocumentationCommentXml()); } + [Fact] + public void XmlDoc_04() + { + // Error docs + var src = """ +static class E +{ + extension(T t) + { + /// + public static void M(U u) => throw null!; + + /// + public static int P => 0; + } +} +"""; + var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreviewWithDocumentationComments); + comp.VerifyEmitDiagnostics( + // (5,24): warning CS1570: XML comment has badly formed XML -- 'End tag 'error' does not match the start tag 'summary'.' + // /// + Diagnostic(ErrorCode.WRN_XMLParseError, "error").WithArguments("error", "summary").WithLocation(5, 24), + // (8,24): warning CS1570: XML comment has badly formed XML -- 'End tag 'error' does not match the start tag 'summary'.' + // /// + Diagnostic(ErrorCode.WRN_XMLParseError, "error").WithArguments("error", "summary").WithLocation(8, 24)); + + var e = comp.GetMember("E"); + var extension = e.GetTypeMembers().Single(); + + var mSkeleton = extension.GetMember("M"); + AssertEx.Equal(""" + + +""", mSkeleton.GetDocumentationCommentXml()); + + var mImplementation = e.GetMember("M"); + AssertEx.Equal(""" + + + + +""", mImplementation.GetDocumentationCommentXml()); + + var p = extension.GetMember("P"); + AssertEx.Equal(""" + + +""", p.GetDocumentationCommentXml()); + + var pGetImplementation = e.GetMember("get_P"); + AssertEx.Equal(""" + + + + +""", pGetImplementation.GetDocumentationCommentXml()); + } + [Fact] public void XmlDoc_Param_01() { From 3cc3d488ecf3dd886fb6450a4e9c35d07a511b44 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 29 Apr 2025 17:37:30 +0200 Subject: [PATCH 3/5] Add assertion --- .../SourceExtensionImplementationMethodSymbol.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs index cad71ab21d477..f90199f73b21d 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs @@ -137,7 +137,15 @@ internal override int TryGetOverloadResolutionPriority() public override string GetDocumentationCommentXml(CultureInfo? preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default) { - return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment); + // Neither the culture nor the expandIncludes affect the XML for extension implementation methods. + string result = SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes: false, ref lazyDocComment); + +#if DEBUG + string withIncludes = DocumentationCommentCompiler.GetDocumentationCommentXml(this, processIncludes: true, default); + Debug.Assert(string.Equals(result, withIncludes, System.StringComparison.Ordinal)); +#endif + + return result; } private sealed class ExtensionMetadataMethodParameterSymbol : RewrittenMethodParameterSymbol From 1c1e2a485a1da70ee259adf608091f9df8e2b533 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Wed, 30 Apr 2025 12:37:57 +0200 Subject: [PATCH 4/5] Comment --- .../Extensions/SourceExtensionImplementationMethodSymbol.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs index f90199f73b21d..fdf94297bed85 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs @@ -61,8 +61,6 @@ internal override int ParameterCount public sealed override DllImportData? GetDllImportData() => null; internal sealed override bool IsExternal => false; - // Tracked by https://github.com/dotnet/roslyn/issues/76130 : How doc comments are supposed to work? GetDocumentationCommentXml - internal sealed override bool IsDeclaredReadOnly => false; public sealed override Symbol ContainingSymbol => _originalMethod.ContainingType.ContainingSymbol; From 0125e7401e41630abb0165f03d5eb72c7d000561 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Wed, 30 Apr 2025 17:37:09 +0200 Subject: [PATCH 5/5] Tweak DEBUG check --- .../Extensions/SourceExtensionImplementationMethodSymbol.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs index fdf94297bed85..86a317cea970c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs @@ -139,7 +139,8 @@ public override string GetDocumentationCommentXml(CultureInfo? preferredCulture string result = SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes: false, ref lazyDocComment); #if DEBUG - string withIncludes = DocumentationCommentCompiler.GetDocumentationCommentXml(this, processIncludes: true, default); + string? ignored = null; + string withIncludes = SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes: true, lazyXmlText: ref ignored); Debug.Assert(string.Equals(result, withIncludes, System.StringComparison.Ordinal)); #endif