Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement support for overloads in documentation comments #18168

Closed
wants to merge 1 commit into from
Closed

Implement support for overloads in documentation comments #18168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 8 additions & 39 deletions src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,42 +579,6 @@ private ImmutableArray<Symbol> ProcessParameterlessCrefMemberLookupResults(
out Symbol ambiguityWinner,
DiagnosticBag diagnostics)
{
// If the syntax indicates arity zero, then we match methods of any arity.
// However, if there are both generic and non-generic methods, then the
// generic methods should be ignored.
if (symbols.Length > 1 && arity == 0)
{
bool hasNonGenericMethod = false;
bool hasGenericMethod = false;
foreach (Symbol s in symbols)
{
if (s.Kind != SymbolKind.Method)
{
continue;
}

if (((MethodSymbol)s).Arity == 0)
{
hasNonGenericMethod = true;
}
else
{
hasGenericMethod = true;
}

if (hasGenericMethod && hasNonGenericMethod)
{
break; //Nothing else to be learned.
}
}

if (hasNonGenericMethod && hasGenericMethod)
{
symbols = symbols.WhereAsArray(s =>
s.Kind != SymbolKind.Method || ((MethodSymbol)s).Arity == 0);
}
}

Debug.Assert(!symbols.IsEmpty);

Symbol symbol = symbols[0];
Expand Down Expand Up @@ -654,9 +618,14 @@ private ImmutableArray<Symbol> ProcessParameterlessCrefMemberLookupResults(
}
else if (secondBest.IsFromCompilation == best.IsFromCompilation)
{
CrefSyntax crefSyntax = GetRootCrefSyntax(memberSyntax);
int otherIndex = symbolIndex == 0 ? 1 : 0;
diagnostics.Add(ErrorCode.WRN_AmbiguousXMLReference, crefSyntax.Location, crefSyntax.ToString(), symbol, symbols[otherIndex]);
// Overloads are supported, but only when resolving method symbols, and only when no parameters or
// type parameters are specified.
if (arity != 0 || !symbols.All(SymbolKind.Method))
{
CrefSyntax crefSyntax = GetRootCrefSyntax(memberSyntax);
int otherIndex = symbolIndex == 0 ? 1 : 0;
diagnostics.Add(ErrorCode.WRN_AmbiguousXMLReference, crefSyntax.Location, crefSyntax.ToString(), symbol, symbols[otherIndex]);
}

ambiguityWinner = ConstructWithCrefTypeParameters(arity, typeArgumentListSyntax, symbol);
return symbols.SelectAsArray(sym => ConstructWithCrefTypeParameters(arity, typeArgumentListSyntax, sym));
Expand Down
26 changes: 26 additions & 0 deletions src/Compilers/CSharp/Portable/Compiler/DocumentationCommentCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,32 @@ private static string GetDocumentationCommentId(CrefSyntax crefSyntax, Binder bi
default:
symbol = ambiguityWinner;
Debug.Assert((object)symbol != null);

// If all of the symbols are methods and no type arguments or parameters were specified, there is no
// need to disambiguate.
MemberCrefSyntax memberCrefSyntax = crefSyntax as MemberCrefSyntax
?? (crefSyntax as QualifiedCrefSyntax)?.Member;
if (memberCrefSyntax is NameMemberCrefSyntax nameMemberCrefSyntax
&& nameMemberCrefSyntax.Parameters is null
&& nameMemberCrefSyntax.Name is SimpleNameSyntax simpleNameSyntax
&& simpleNameSyntax.Arity == 0)
{
if (symbols.All(SymbolKind.Method))
{
var pool = PooledStringBuilder.GetInstance();
try
{
StringBuilder builder = pool.Builder;
DocumentationCommentIDVisitor.OverloadInstance.Visit(ambiguityWinner, builder);
return builder.Length == 0 ? null : builder.ToString();
}
finally
{
pool.Free();
}
}
}

break;
}

Expand Down
22 changes: 18 additions & 4 deletions ...pilers/CSharp/Portable/DocumentationComments/DocumentationCommentIDVisitor.PartVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,25 @@ internal sealed partial class DocumentationCommentIDVisitor
/// </summary>
private sealed class PartVisitor : CSharpSymbolVisitor<StringBuilder, object>
{
// Everyone outside this type uses this one.
internal static readonly PartVisitor Instance = new PartVisitor(inParameterOrReturnType: false);
// Everyone outside this type uses this one except for the case of overloaded methods.
internal static readonly PartVisitor Instance = new PartVisitor(inParameterOrReturnType: false, overloadedMethods: false);

/// <summary>
/// This one is used when generating an ID for a method symbol using
/// <see cref="DocumentationCommentIDVisitor.OverloadInstance"/>.
/// </summary>
internal static readonly PartVisitor OverloadInstance = new PartVisitor(inParameterOrReturnType: false, overloadedMethods: true);

// Select callers within this type use this one.
private static readonly PartVisitor s_parameterOrReturnTypeInstance = new PartVisitor(inParameterOrReturnType: true);
private static readonly PartVisitor s_parameterOrReturnTypeInstance = new PartVisitor(inParameterOrReturnType: true, overloadedMethods: false);

private readonly bool _inParameterOrReturnType;
private readonly bool _overloadedMethods;

private PartVisitor(bool inParameterOrReturnType)
private PartVisitor(bool inParameterOrReturnType, bool overloadedMethods)
{
_inParameterOrReturnType = inParameterOrReturnType;
_overloadedMethods = overloadedMethods;
}

public override object VisitArrayType(ArrayTypeSymbol symbol, StringBuilder builder)
Expand Down Expand Up @@ -97,6 +105,12 @@ public override object VisitMethod(MethodSymbol symbol, StringBuilder builder)
builder.Append('.');
builder.Append(GetEscapedMetadataName(symbol));

if (_overloadedMethods)
{
// No need to add arity, parameters, or suffix for overloaded methods
return null;
}

if (symbol.Arity != 0)
{
builder.Append("``");
Expand Down
28 changes: 24 additions & 4 deletions src/Compilers/CSharp/Portable/DocumentationComments/DocumentationCommentIDVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,22 @@ namespace Microsoft.CodeAnalysis.CSharp
{
internal sealed partial class DocumentationCommentIDVisitor : CSharpSymbolVisitor<StringBuilder, object>
{
public static readonly DocumentationCommentIDVisitor Instance = new DocumentationCommentIDVisitor();
/// <summary>
/// Generates a documentation comment ID for a single symbol.
/// </summary>
public static readonly DocumentationCommentIDVisitor Instance = new DocumentationCommentIDVisitor(overloadedMethods: false);

private DocumentationCommentIDVisitor()
/// <summary>
/// Generates documentation comment IDs for methods which resolve to the overload instead of a single specific
/// method.
/// </summary>
public static readonly DocumentationCommentIDVisitor OverloadInstance = new DocumentationCommentIDVisitor(overloadedMethods: true);

private readonly bool _overloadedMethods;

private DocumentationCommentIDVisitor(bool overloadedMethods)
{
_overloadedMethods = overloadedMethods;
}

public override object DefaultVisit(Symbol symbol, StringBuilder builder)
Expand All @@ -32,8 +44,16 @@ public override object VisitNamespace(NamespaceSymbol symbol, StringBuilder buil

public override object VisitMethod(MethodSymbol symbol, StringBuilder builder)
{
builder.Append("M:");
PartVisitor.Instance.Visit(symbol, builder);
if (_overloadedMethods)
{
builder.Append("O:");
PartVisitor.OverloadInstance.Visit(symbol, builder);
}
else
{
builder.Append("M:");
PartVisitor.Instance.Visit(symbol, builder);
}

return null;
}
Expand Down
15 changes: 15 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@ public static bool IsNestedType(this Symbol symbol)
return symbol is NamedTypeSymbol && (object)symbol.ContainingType != null;
}

public static bool All<T>(this ImmutableArray<T> array, SymbolKind kind)
where T : Symbol
{
for (int i = 0, n = array.Length; i < n; i++)
{
var item = array[i];
if (item?.Kind != kind)
{
return false;
}
}

return true;
}

public static bool Any<T>(this ImmutableArray<T> array, SymbolKind kind)
where T : Symbol
{
Expand Down
3 changes: 0 additions & 3 deletions src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18855,9 +18855,6 @@ public static void Main ()
// (7,14): warning CS1591: Missing XML comment for publicly visible type or member 'MyClass'
// public class MyClass
Diagnostic(ErrorCode.WRN_MissingXMLComment, "MyClass").WithArguments("MyClass"),
// (9,19): warning CS0419: Ambiguous reference in cref attribute: 'I.F'. Assuming 'I.F()', but could have also matched other overloads including 'I.F(int)'.
// /// <see cref="I.F"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "I.F").WithArguments("I.F", "I.F()", "I.F(int)"),
// (13,23): warning CS1591: Missing XML comment for publicly visible type or member 'MyClass.Main()'
// public static void Main ()
Diagnostic(ErrorCode.WRN_MissingXMLComment, "Main").WithArguments("MyClass.Main()"));
Expand Down
5 changes: 1 addition & 4 deletions src/Compilers/CSharp/Test/Symbol/DocumentationComments/CrefTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,10 +1201,7 @@ void M(int x) { }
var expectedWinner = expectedCandidates.Single(m => m.ParameterCount == 0);

Symbol actualWinner;
var actualCandidates = GetReferencedSymbols(crefSyntax, compilation, out actualWinner,
// (3,20): warning CS0419: Ambiguous reference in cref attribute: 'M'. Assuming 'C.M()', but could have also matched other overloads including 'C.M(int)'.
// /// See <see cref="M"/>.
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "M").WithArguments("M", "C.M()", "C.M(int)"));
var actualCandidates = GetReferencedSymbols(crefSyntax, compilation, out actualWinner);

Assert.Equal(expectedWinner, actualWinner);
AssertEx.SetEqual(expectedCandidates.AsEnumerable(), actualCandidates.ToArray());
Expand Down
37 changes: 9 additions & 28 deletions src/Compilers/CSharp/Test/Symbol/DocumentationComments/DocumentationCommentCompilerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4622,15 +4622,9 @@ void M<T>(string x) { }
var comp = CreateCompilationWithMscorlibAndDocumentationComments(source);

var actual = GetDocumentationCommentText(comp,
// (2,16): warning CS0419: Ambiguous reference in cref attribute: 'M'. Assuming 'C.M(int)', but could have also matched other overloads including 'C.M(string)'.
// /// <see cref="M"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "M").WithArguments("M", "C.M(int)", "C.M(string)"),
// (4,16): warning CS0419: Ambiguous reference in cref attribute: 'M{T}'. Assuming 'C.M<T>(int)', but could have also matched other overloads including 'C.M<T>(string)'.
// /// <see cref="M{T}"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "M{T}").WithArguments("M{T}", "C.M<T>(int)", "C.M<T>(string)"),
// (7,16): warning CS0419: Ambiguous reference in cref attribute: 'C.M'. Assuming 'C.M(int)', but could have also matched other overloads including 'C.M(string)'.
// /// <see cref="C.M"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "C.M").WithArguments("C.M", "C.M(int)", "C.M(string)"),
// (9,16): warning CS0419: Ambiguous reference in cref attribute: 'C.M{T}'. Assuming 'C.M<T>(int)', but could have also matched other overloads including 'C.M<T>(string)'.
// /// <see cref="C.M{T}"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "C.M{T}").WithArguments("C.M{T}", "C.M<T>(int)", "C.M<T>(string)"));
Expand All @@ -4643,12 +4637,12 @@ void M<T>(string x) { }
</assembly>
<members>
<member name=""T:C"">
<see cref=""M:C.M(System.Int32)""/>
<see cref=""O:C.M""/>
<see cref=""M:C.M(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>

<see cref=""M:C.M(System.Int32)""/>
<see cref=""O:C.M""/>
<see cref=""M:C.M(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>
Expand Down Expand Up @@ -4685,18 +4679,12 @@ void M<T, U>(string x) { }
var comp = CreateCompilationWithMscorlibAndDocumentationComments(source);

var actual = GetDocumentationCommentText(comp,
// (2,16): warning CS0419: Ambiguous reference in cref attribute: 'M'. Assuming 'C.M<T>(int)', but could have also matched other overloads including 'C.M<T>(string)'.
// /// <see cref="M"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "M").WithArguments("M", "C.M<T>(int)", "C.M<T>(string)"),
// (3,16): warning CS0419: Ambiguous reference in cref attribute: 'M(int)'. Assuming 'C.M<T>(int)', but could have also matched other overloads including 'C.M<T, U>(int)'.
// /// <see cref="M(int)"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "M(int)").WithArguments("M(int)", "C.M<T>(int)", "C.M<T, U>(int)"),
// (4,16): warning CS0419: Ambiguous reference in cref attribute: 'M{T}'. Assuming 'C.M<T>(int)', but could have also matched other overloads including 'C.M<T>(string)'.
// /// <see cref="M{T}"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "M{T}").WithArguments("M{T}", "C.M<T>(int)", "C.M<T>(string)"),
// (7,16): warning CS0419: Ambiguous reference in cref attribute: 'C.M'. Assuming 'C.M<T>(int)', but could have also matched other overloads including 'C.M<T>(string)'.
// /// <see cref="C.M"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "C.M").WithArguments("C.M", "C.M<T>(int)", "C.M<T>(string)"),
// (8,16): warning CS0419: Ambiguous reference in cref attribute: 'C.M(int)'. Assuming 'C.M<T>(int)', but could have also matched other overloads including 'C.M<T, U>(int)'.
// /// <see cref="C.M(int)"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "C.M(int)").WithArguments("C.M(int)", "C.M<T>(int)", "C.M<T, U>(int)"),
Expand All @@ -4712,12 +4700,12 @@ void M<T, U>(string x) { }
</assembly>
<members>
<member name=""T:C"">
<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""O:C.M""/>
<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>

<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""O:C.M""/>
<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>
<see cref=""M:C.M``1(System.Int32)""/>
Expand Down Expand Up @@ -4751,10 +4739,7 @@ void N(string x) { }
";
var comp = CreateCompilationWithMscorlibAndDocumentationComments(source);

var actual = GetDocumentationCommentText(comp,
// (5,16): warning CS0419: Ambiguous reference in cref attribute: 'N'. Assuming 'C.N(int)', but could have also matched other overloads including 'C.N(string)'.
// /// <see cref="N"/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "N").WithArguments("N", "C.N(int)", "C.N(string)"));
var actual = GetDocumentationCommentText(comp);

var expected = @"
<?xml version=""1.0""?>
Expand All @@ -4764,10 +4749,10 @@ void N(string x) { }
</assembly>
<members>
<member name=""T:C"">
<see cref=""M:C.M(System.Int32)""/>
<see cref=""O:C.M""/>
<see cref=""M:C.M(System.Int32)""/>

<see cref=""M:C.N(System.Int32)""/>
<see cref=""O:C.N""/>
<see cref=""M:C.N(System.Int32)""/>
</member>
</members>
Expand Down Expand Up @@ -6053,7 +6038,6 @@ static void Main() {{ }}
[Fact]
public void Dev10_785160()
{
// Someone suggested preferring the more public member in case of ambiguity, but it was not implemented.
var source = @"
/// <see cref='M'/>
class C
Expand All @@ -6063,10 +6047,7 @@ public void M(int x) { }
}
";
var comp = CreateCompilationWithMscorlibAndDocumentationComments(source);
var actual = GetDocumentationCommentText(comp,
// (2,16): warning CS0419: Ambiguous reference in cref attribute: 'M'. Assuming 'C.M(char)', but could have also matched other overloads including 'C.M(int)'.
// /// <see cref='M'/>
Diagnostic(ErrorCode.WRN_AmbiguousXMLReference, "M").WithArguments("M", "C.M(char)", "C.M(int)"));
var actual = GetDocumentationCommentText(comp);
var expected = @"
<?xml version=""1.0""?>
<doc>
Expand All @@ -6075,7 +6056,7 @@ public void M(int x) { }
</assembly>
<members>
<member name=""T:C"">
<see cref='M:C.M(System.Char)'/>
<see cref='O:C.M'/>
</member>
</members>
</doc>
Expand Down
2 changes: 1 addition & 1 deletion src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4230,7 +4230,7 @@ public static T GetGenericValue<T>(T para)
public static void GetGenericValue()
{
}
}", Documentation("This sample shows how to specify the TestClass.GetGenericValue() method as a cref attribute."));
}", Documentation("This sample shows how to specify the TestClass.GetGenericValue method as a cref attribute."));
}

[WorkItem(813350, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/813350")]
Expand Down