-
Notifications
You must be signed in to change notification settings - Fork 418
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.
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
Using StructuredDocumentation for Signature Help and appropriate documentation for Parameter symbols #1085
Using StructuredDocumentation for Signature Help and appropriate documentation for Parameter symbols #1085
Changes from 11 commits
d9da163
c3ca7b9
82621ab
b045909
e1fc79f
021226a
d936717
9a56685
2eeaefa
2b948c2
54e7066
0239199
5b402b2
43202ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,20 +18,31 @@ public class DocumentationComment | |
public string ValueText { get; } | ||
public DocumentationItem[] Exception { get; } | ||
|
||
private DocumentationComment(string summaryText, DocumentationItem[] typeParamElements, DocumentationItem[] paramElements, string returnsText, string remarksText, string exampleText, string valueText, DocumentationItem[ ] exception) | ||
public DocumentationComment | ||
(string summaryText = "", | ||
DocumentationItem[] typeParamElements = null, | ||
DocumentationItem[] paramElements = null, | ||
string returnsText = "", | ||
string remarksText = "", | ||
string exampleText = "", | ||
string valueText = "", | ||
DocumentationItem[] exception = null) | ||
{ | ||
SummaryText = summaryText; | ||
TypeParamElements = typeParamElements; | ||
ParamElements = paramElements; | ||
TypeParamElements = typeParamElements ?? new DocumentationItem[0]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Array.Empty |
||
ParamElements = paramElements ?? new DocumentationItem[0]; | ||
ReturnsText = returnsText; | ||
RemarksText = remarksText; | ||
ExampleText = exampleText; | ||
ValueText = valueText; | ||
Exception = exception; | ||
Exception = exception ?? new DocumentationItem[0]; | ||
} | ||
|
||
public static DocumentationComment From(string xmlDocumentation, string lineEnding) | ||
{ | ||
if (string.IsNullOrEmpty(xmlDocumentation)) | ||
return Empty; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline. |
||
|
||
var reader = new StringReader("<docroot>" + xmlDocumentation + "</docroot>"); | ||
StringBuilder summaryText = new StringBuilder(); | ||
List<DocumentationItemBuilder> typeParamElements = new List<DocumentationItemBuilder>(); | ||
|
@@ -175,6 +186,15 @@ private static string TrimStartRetainingSingleLeadingSpace(string input) | |
return input; | ||
return $" {input.TrimStart()}"; | ||
} | ||
|
||
//Gets the parameter documentation from this object | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this a doc comment or remove? |
||
public string GetParameterText(string name) | ||
=> Array.Find(ParamElements, parameter => parameter.Name == name)?.Documentation ?? string.Empty; | ||
|
||
public string GetTypeParameterText(string name) | ||
=> Array.Find(TypeParamElements, typeParam => typeParam.Name == name)?.Documentation ?? string.Empty; | ||
|
||
public static readonly DocumentationComment Empty = new DocumentationComment(); | ||
} | ||
|
||
class DocumentationItemBuilder | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
using Microsoft.CodeAnalysis; | ||
using OmniSharp.Roslyn.CSharp.Services.Documentation; | ||
|
||
namespace OmniSharp.Roslyn.CSharp.Services | ||
{ | ||
public static class DocumentationHelper | ||
{ | ||
public static string GetParameterDocumentation(IParameterSymbol parameter, string lineEnding = "\n") | ||
{ | ||
var contaningSymbolDef = parameter.ContainingSymbol.OriginalDefinition; | ||
return DocumentationConverter.GetStructuredDocumentation(contaningSymbolDef.GetDocumentationCommentXml(), lineEnding).GetParameterText(parameter.Name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would wrap a line of this length. |
||
} | ||
|
||
public static string GetTypeParameterDocumentation(ITypeParameterSymbol typeParam, string lineEnding = "\n") | ||
{ | ||
var contaningSymbol = typeParam.ContainingSymbol; | ||
return DocumentationConverter.GetStructuredDocumentation(contaningSymbol.GetDocumentationCommentXml(), lineEnding).GetTypeParameterText(typeParam.Name); | ||
} | ||
|
||
public static string GetAliasDocumentation(IAliasSymbol alias, string lineEnding = "\n") | ||
{ | ||
var target = alias.Target; | ||
return DocumentationConverter.GetStructuredDocumentation(target.GetDocumentationCommentXml(), lineEnding).SummaryText; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
using OmniSharp.Mef; | ||
using OmniSharp.Models; | ||
using OmniSharp.Models.SignatureHelp; | ||
using OmniSharp.Roslyn.CSharp.Services.Documentation; | ||
|
||
namespace OmniSharp.Roslyn.CSharp.Services.Signatures | ||
{ | ||
|
@@ -168,19 +169,19 @@ private static SignatureHelpItem BuildSignature(IMethodSymbol symbol) | |
signature.Documentation = symbol.GetDocumentationCommentXml(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rchande @DustinCampbell Do we want to return the Parsed xml for other editors as well. Currently GetDocumentationCommentXml() gives the xml with the tags. |
||
signature.Name = symbol.MethodKind == MethodKind.Constructor ? symbol.ContainingType.Name : symbol.Name; | ||
signature.Label = symbol.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); | ||
signature.StructuredDocumentation = DocumentationConverter.GetStructuredDocumentation(symbol); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this code is calling symbol.GetDocumentationCommentXml once for the method itself and once for every parameter in the method. Is there a way we can just parse the XML once? |
||
|
||
signature.Parameters = symbol.Parameters.Select(parameter => | ||
{ | ||
return new SignatureHelpParameter() | ||
{ | ||
Name = parameter.Name, | ||
Label = parameter.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat), | ||
Documentation = parameter.GetDocumentationCommentXml() | ||
Documentation = signature.StructuredDocumentation.GetParameterText(parameter.Name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, now there's only one caller for each of those methods in DocumentationHelper. Can they move inside StructuredDocumentation? |
||
}; | ||
}); | ||
|
||
return signature; | ||
} | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -758,6 +758,90 @@ bool LocalFunction(int i) | |
Assert.Equal("int i", signature.Parameters.ElementAt(0).Label); | ||
} | ||
|
||
[Fact] | ||
public async Task ReturnsDocumentationForParameters() | ||
{ | ||
const string source = | ||
@"class Program | ||
{ | ||
public static void Main() | ||
{ | ||
var flag = Compare($$); | ||
} | ||
///<summary>Checks if object is tagged with the tag.</summary> | ||
/// <param name=""gameObject"">The game object.</param> | ||
/// <param name=""tagName"">Name of the tag.</param> | ||
/// <returns>Returns <c> true</c> if object is tagged with tag.</returns> | ||
public static bool Compare(GameObject gameObject, string tagName) | ||
{ | ||
return true; | ||
} | ||
}"; | ||
var actual = await GetSignatureHelp(source); | ||
Assert.Single(actual.Signatures); | ||
|
||
var signature = actual.Signatures.ElementAt(0); | ||
Assert.Equal(2, signature.Parameters.Count()); | ||
Assert.Equal("gameObject", signature.Parameters.ElementAt(0).Name); | ||
Assert.Equal("The game object.", signature.Parameters.ElementAt(0).Documentation); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if there are additional tags inside the parameter documentation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The StructuredDocumentation removes all the tags and appends the text to the appropriate section hence any additional tags inside the parameter documentation will be processed accordingly. |
||
Assert.Equal("tagName", signature.Parameters.ElementAt(1).Name); | ||
Assert.Equal("Name of the tag.", signature.Parameters.ElementAt(1).Documentation); | ||
} | ||
|
||
[Fact] | ||
public async Task ReturnsDocumentationForParametersNestedTags() | ||
{ | ||
const string source = | ||
@"class Program | ||
{ | ||
public static void Main() | ||
{ | ||
var flag = Compare($$); | ||
} | ||
///<summary>Checks if object is tagged with the tag.</summary> | ||
/// <param name=""gameObject"">The game object.</param> | ||
/// <param name=""tagName"">Name of the tag.It has the default value as <c>null</c></param> | ||
/// <returns>Returns <c> true</c> if object is tagged with tag.</returns> | ||
public static bool Compare(GameObject gameObject, string tagName = null) | ||
{ | ||
return true; | ||
} | ||
}"; | ||
var actual = await GetSignatureHelp(source); | ||
Assert.Single(actual.Signatures); | ||
|
||
var signature = actual.Signatures.ElementAt(0); | ||
Assert.Equal(2, signature.Parameters.Count()); | ||
Assert.Equal("Name of the tag.It has the default value as null", signature.Parameters.ElementAt(1).Documentation); | ||
} | ||
|
||
[Fact] | ||
public async Task ReturnsStructuredDocumentation() | ||
{ | ||
const string source = | ||
@"class Program | ||
{ | ||
public static void Main() | ||
{ | ||
var flag = Compare($$); | ||
} | ||
///<summary>Checks if object is tagged with the tag.</summary> | ||
/// <param name=""gameObject"">The game object.</param> | ||
/// <param name=""tagName"">Name of the tag.</param> | ||
/// <returns>Returns <c>true</c> if object is tagged with tag.</returns> | ||
public static bool Compare(GameObject gameObject, string tagName) | ||
{ | ||
return true; | ||
} | ||
}"; | ||
var actual = await GetSignatureHelp(source); | ||
Assert.Single(actual.Signatures); | ||
|
||
var signature = actual.Signatures.ElementAt(0); | ||
Assert.Equal("Checks if object is tagged with the tag.", signature.StructuredDocumentation.SummaryText); | ||
Assert.Equal("Returns true if object is tagged with tag.", signature.StructuredDocumentation.ReturnsText); | ||
} | ||
|
||
[Fact] | ||
public async Task SkipReceiverOfExtensionMethods() | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: paren should be on the same line as DocumentationComment