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

Typelookup clean up and add objects for parameters and similar symbols #1062

Merged
merged 9 commits into from
Dec 14, 2017
Merged

Typelookup clean up and add objects for parameters and similar symbols #1062

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
941e653
Removed the Summary tag in documentation
akshita31 Dec 12, 2017
734b475
Removed Unnecessary Tags
akshita31 Dec 13, 2017
c52d1b0
Removed Unnecessary Tags
akshita31 Dec 13, 2017
c843e42
Added object for parameters and modified tests
akshita31 Dec 13, 2017
f12ecb3
Merge branch 'typelookup_v2' of https://github.com/akshita31/omnishar…
akshita31 Dec 13, 2017
81cf69b
Merge branch 'master' into typelookup_v2
akshita31 Dec 13, 2017
35cca90
Rename DocumentationItem and Code Clean Up
akshita31 Dec 13, 2017
6ca471d
Merge branch 'typelookup_v2' of https://github.com/akshita31/omnishar…
akshita31 Dec 13, 2017
516971f
Merge branch 'master' into typelookup_v2
DustinCampbell Dec 13, 2017
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
66 changes: 40 additions & 26 deletions src/OmniSharp.Abstractions/Models/v1/TypeLookup/DocumentationComment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ namespace OmniSharp.Models.TypeLookup
public class DocumentationComment
{
public string SummaryText { get; }
public string[] TypeParamElements { get; }
public string[] ParamElements { get; }
public DocumentedObject[] TypeParamElements { get; }
Copy link

Choose a reason for hiding this comment

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

This could just be Array<KeyValuePair<string, string>> or Array<Tuple<string, string>>

Copy link

Choose a reason for hiding this comment

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

Thoughts @DustinCampbell ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what Newtownsoft.Json supports here. If we go with a custom type, I might call it DocumentationItem.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akshita31 : Note that Array is not a thing in C#. @rchande meant KeyValuePair<string, string>>[] and Tuple<string, string>[]. I would lean away from tuple because it would mean that the protocol would have weird names like Item1 and Item2 in it, which isn't what we want.

public DocumentedObject[] ParamElements { get; }
public string ReturnsText { get; }
public string RemarksText { get; }
public string ExampleText { get; }
public string ValueText { get; }
public string[ ] Exception { get; }
public DocumentedObject[ ] Exception { get; }
Copy link

Choose a reason for hiding this comment

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

Extra space


private DocumentationComment(string summaryText, string[] typeParamElements, string[] paramElements, string returnsText, string remarksText, string exampleText, string valueText, string [ ] exception)
private DocumentationComment(string summaryText, DocumentedObject[] typeParamElements, DocumentedObject[] paramElements, string returnsText, string remarksText, string exampleText, string valueText, DocumentedObject[ ] exception)
{
SummaryText = summaryText;
TypeParamElements = typeParamElements;
Expand All @@ -34,13 +34,13 @@ public static DocumentationComment From(string xmlDocumentation, string lineEndi
{
var reader = new StringReader("<docroot>" + xmlDocumentation + "</docroot>");
StringBuilder summaryText = new StringBuilder();
List<StringBuilder> typeParamElements = new List<StringBuilder>();
List<StringBuilder> paramElements = new List<StringBuilder>();
List<DocumentedObjectHelper> typeParamElements = new List<DocumentedObjectHelper>();
List<DocumentedObjectHelper> paramElements = new List<DocumentedObjectHelper>();
StringBuilder returnsText = new StringBuilder();
StringBuilder remarksText = new StringBuilder();
StringBuilder exampleText = new StringBuilder();
StringBuilder valueText = new StringBuilder();
List<StringBuilder> exception = new List<StringBuilder>();
List<DocumentedObjectHelper> exception = new List<DocumentedObjectHelper>();

using (var xml = XmlReader.Create(reader))
{
Expand All @@ -60,26 +60,21 @@ public static DocumentationComment From(string xmlDocumentation, string lineEndi
xml.Skip();
break;
case "remarks":
remarksText.Append("Remarks: ");
currentSectionBuilder = remarksText;
break;
case "example":
exampleText.Append("Example: ");
currentSectionBuilder = exampleText;
break;
case "exception":
StringBuilder ExceptionInstance = new StringBuilder();
ExceptionInstance.Append(GetCref(xml["cref"]).TrimEnd());
ExceptionInstance.Append(": ");
currentSectionBuilder = ExceptionInstance;
exception.Add(ExceptionInstance);
DocumentedObjectHelper exceptionInstance = new DocumentedObjectHelper();
exceptionInstance.Name = GetCref(xml["cref"]).TrimEnd();
currentSectionBuilder = exceptionInstance.Documentation;
exception.Add(exceptionInstance);
break;
case "returns":
returnsText.Append("Returns: ");
currentSectionBuilder = returnsText;
break;
case "summary":
summaryText.Append("Summary: ");
currentSectionBuilder = summaryText;
break;
case "see":
Expand All @@ -95,25 +90,22 @@ public static DocumentationComment From(string xmlDocumentation, string lineEndi
currentSectionBuilder.Append(" ");
break;
case "param":
StringBuilder paramInstance = new StringBuilder();
paramInstance.Append(TrimMultiLineString(xml["name"], lineEnding));
paramInstance.Append(": ");
currentSectionBuilder = paramInstance;
DocumentedObjectHelper paramInstance = new DocumentedObjectHelper();
paramInstance.Name = TrimMultiLineString(xml["name"], lineEnding);
currentSectionBuilder = paramInstance.Documentation;
paramElements.Add(paramInstance);
break;
case "typeparamref":
currentSectionBuilder.Append(xml["name"]);
currentSectionBuilder.Append(" ");
break;
case "typeparam":
StringBuilder typeParamInstance = new StringBuilder();
typeParamInstance.Append(TrimMultiLineString(xml["name"], lineEnding));
typeParamInstance.Append(": ");
currentSectionBuilder = typeParamInstance;
DocumentedObjectHelper typeParamInstance = new DocumentedObjectHelper();
typeParamInstance.Name = TrimMultiLineString(xml["name"], lineEnding);
currentSectionBuilder = typeParamInstance.Documentation;
typeParamElements.Add(typeParamInstance);
break;
case "value":
valueText.Append("Value: ");
currentSectionBuilder = valueText;
break;
case "br":
Expand All @@ -140,7 +132,8 @@ public static DocumentationComment From(string xmlDocumentation, string lineEndi
return null;
}
}
return new DocumentationComment(summaryText.ToString(), typeParamElements.Select(s => s.ToString()).ToArray(), paramElements.Select(s => s.ToString()).ToArray(), returnsText.ToString(), remarksText.ToString(), exampleText.ToString(), valueText.ToString(), exception.Select(s => s.ToString()).ToArray());

return new DocumentationComment(summaryText.ToString(), typeParamElements.Select(s => s.ConvertToDocumentedObject()).ToArray(), paramElements.Select(s => s.ConvertToDocumentedObject()).ToArray(), returnsText.ToString(), remarksText.ToString(), exampleText.ToString(), valueText.ToString(), exception.Select(s => s.ConvertToDocumentedObject()).ToArray());
Copy link

Choose a reason for hiding this comment

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

Put each argument on its own line.

}

private static string TrimMultiLineString(string input, string lineEnding)
Expand All @@ -166,4 +159,25 @@ private static string GetCref(string cref)
return cref + " ";
}
}

class DocumentedObjectHelper
Copy link

Choose a reason for hiding this comment

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

DocumentedObjectBuilder?

{
public string Name { get; set; }
public StringBuilder Documentation { get; set; }

public DocumentedObjectHelper()
Copy link

Choose a reason for hiding this comment

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

extra spaces

{
Documentation = new StringBuilder();
}

public DocumentedObject ConvertToDocumentedObject()
Copy link

Choose a reason for hiding this comment

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

extra spaces

{
var documentedObject = new DocumentedObject
{
Name = Name,
Documentation = Documentation.ToString()
};
return documentedObject;
Copy link

Choose a reason for hiding this comment

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

Newline after close brace of block when followed by non-brace.

}
}
}
15 changes: 15 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/TypeLookup/NamedDocumentedObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace OmniSharp.Models.TypeLookup
{
public class DocumentedObject
{
public string Name { get; set; }
Copy link

Choose a reason for hiding this comment

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

Can we get rid of the public setters? This type should be immutable.

public string Documentation { get; set; }
public DocumentedObject() { }
}
}
89 changes: 39 additions & 50 deletions tests/OmniSharp.Roslyn.CSharp.Tests/TypeLookupFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ class testissue
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Remarks: You may have some additional information about this class here.";
@"You may have some additional information about this class here.";
Assert.Equal(expected, response.StructuredDocumentation.RemarksText);
}

Expand All @@ -310,7 +310,7 @@ class testissue
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Summary: Checks if object is tagged with the tag.";
@"Checks if object is tagged with the tag.";
Assert.Equal(expected, response.StructuredDocumentation.SummaryText);
}

Expand All @@ -327,7 +327,7 @@ class testissue
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Returns: Returns true if object is tagged with tag.";
@"Returns true if object is tagged with tag.";
Assert.Equal(expected, response.StructuredDocumentation.ReturnsText);
}

Expand All @@ -344,7 +344,7 @@ class testissue
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Example: Checks if object is tagged with the tag.";
@"Checks if object is tagged with the tag.";
Assert.Equal(expected, response.StructuredDocumentation.ExampleText);
}

Expand All @@ -363,12 +363,10 @@ class testissue
var response = await GetTypeLookUpResponse(content);
Assert.Equal(2, response.StructuredDocumentation.Exception.Count());

var expectedException0 =
@"A: A description";
Assert.Equal(expectedException0, response.StructuredDocumentation.Exception[0]);
var expectedException1 =
@"B: B description";
Assert.Equal(expectedException1, response.StructuredDocumentation.Exception[1]);
Assert.Equal("A", response.StructuredDocumentation.Exception[0].Name);
Assert.Equal("A description", response.StructuredDocumentation.Exception[0].Documentation);
Assert.Equal("B", response.StructuredDocumentation.Exception[1].Name);
Assert.Equal("B description", response.StructuredDocumentation.Exception[1].Documentation);
}

[Fact]
Expand All @@ -386,12 +384,10 @@ class testissue
var response = await GetTypeLookUpResponse(content);
Assert.Equal(2, response.StructuredDocumentation.ParamElements.Length);

var expectedParam0 =
@"gameObject: The game object.";
Assert.Equal(expectedParam0, response.StructuredDocumentation.ParamElements[0]);
var expectedParam1 =
@"tagName: Name of the tag.";
Assert.Equal(expectedParam1, response.StructuredDocumentation.ParamElements[1]);
Assert.Equal("gameObject", response.StructuredDocumentation.ParamElements[0].Name);
Assert.Equal("The game object.", response.StructuredDocumentation.ParamElements[0].Documentation);
Assert.Equal("tagName", response.StructuredDocumentation.ParamElements[1].Name);
Assert.Equal("Name of the tag.", response.StructuredDocumentation.ParamElements[1].Documentation);
}

[Fact]
Expand All @@ -413,12 +409,10 @@ public class TestClass
var response = await GetTypeLookUpResponse(content);
Assert.Equal(2, response.StructuredDocumentation.TypeParamElements.Count());

var expected0 =
@"T: The element type of the array";
Assert.Equal(expected0, response.StructuredDocumentation.TypeParamElements[0]);
var expected1 =
@"X: The element type of the list";
Assert.Equal(expected1, response.StructuredDocumentation.TypeParamElements[1]);
Assert.Equal("T", response.StructuredDocumentation.TypeParamElements[0].Name);
Assert.Equal("The element type of the array", response.StructuredDocumentation.TypeParamElements[0].Documentation);
Assert.Equal("X", response.StructuredDocumentation.TypeParamElements[1].Name);
Assert.Equal("The element type of the list", response.StructuredDocumentation.TypeParamElements[1].Documentation);
}

[Fact]
Expand All @@ -438,10 +432,10 @@ public string Na$$me
";
var response = await GetTypeLookUpResponse(content);
var expectedValue =
@"Value: The Name property gets/sets the value of the string field, _name.";
@"The Name property gets/sets the value of the string field, _name.";
Assert.Equal(expectedValue, response.StructuredDocumentation.ValueText);
var expectedSummary =
@"Summary: The Name property represents the employee's name.";
@"The Name property represents the employee's name.";
Assert.Equal(expectedSummary, response.StructuredDocumentation.SummaryText);
}

Expand All @@ -458,7 +452,7 @@ public class TestClass
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Summary: DoWork is a method in the TestClass class. System.Console.WriteLine(System.String) for information about output statements.";
@"DoWork is a method in the TestClass class. System.Console.WriteLine(System.String) for information about output statements.";
Assert.Equal(expected, response.StructuredDocumentation.SummaryText);
}

Expand All @@ -477,7 +471,7 @@ public class TestClass
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Summary: Creates a new array of arbitrary type T ";
@"Creates a new array of arbitrary type T ";
Assert.Equal(expected, response.StructuredDocumentation.SummaryText);
}

Expand Down Expand Up @@ -505,7 +499,7 @@ public class TestClass
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Example: This sample shows how to call the TestClass.GetZero method.
@"This sample shows how to call the TestClass.GetZero method.

class TestClass
{
Expand Down Expand Up @@ -534,7 +528,7 @@ public class TestClass
";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Summary: DoWork is a method in the TestClass class.
@"DoWork is a method in the TestClass class.

Here's how you could make a second paragraph in a description.";
Assert.Equal(expected.Replace("\r", ""), response.StructuredDocumentation.SummaryText);
Expand All @@ -559,7 +553,7 @@ static void Main()
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Summary: DoWork is a method in the TestClass class.
@"DoWork is a method in the TestClass class.
See also: TestClass.Main ";
Assert.Equal(expected.Replace("\r", ""), response.StructuredDocumentation.SummaryText);
}
Expand All @@ -579,16 +573,14 @@ class testissue
}";
var response = await GetTypeLookUpResponse(content);
var expected =
@"Summary: Checks if object is tagged with the tag.";
@"Checks if object is tagged with the tag.";
Assert.Equal(expected, response.StructuredDocumentation.SummaryText);

Assert.Equal(2, response.StructuredDocumentation.ParamElements.Length);
var expectedParam0 =
@"gameObject: The game object.";
Assert.Equal(expectedParam0, response.StructuredDocumentation.ParamElements[0]);
var expectedParam1 =
@"tagName: Name of the tag.";
Assert.Equal(expectedParam1, response.StructuredDocumentation.ParamElements[1]);
Assert.Equal("gameObject", response.StructuredDocumentation.ParamElements[0].Name);
Assert.Equal("The game object.", response.StructuredDocumentation.ParamElements[0].Documentation);
Assert.Equal("tagName", response.StructuredDocumentation.ParamElements[1].Name);
Assert.Equal("Name of the tag.", response.StructuredDocumentation.ParamElements[1].Documentation);
}

[Fact]
Expand All @@ -610,34 +602,31 @@ class testissue
}";
var response = await GetTypeLookUpResponse(content);
var expectedSummary =
@"Summary: Checks if object is tagged with the tag.";
@"Checks if object is tagged with the tag.";
Assert.Equal(expectedSummary, response.StructuredDocumentation.SummaryText);

Assert.Single(response.StructuredDocumentation.ParamElements);
var expectedParam =
@"gameObject: The game object.";
Assert.Equal(expectedParam, response.StructuredDocumentation.ParamElements[0]);

Assert.Equal("gameObject", response.StructuredDocumentation.ParamElements[0].Name);
Assert.Equal("The game object.", response.StructuredDocumentation.ParamElements[0].Documentation);

var expectedExample =
@"Example: Invoke using A.Compare(5) where A is an instance of the class testissue.";
@"Invoke using A.Compare(5) where A is an instance of the class testissue.";
Assert.Equal(expectedExample, response.StructuredDocumentation.ExampleText);

Assert.Single(response.StructuredDocumentation.TypeParamElements);
var expectedTypeParam =
@"T: The element type of the array";
Assert.Equal(expectedTypeParam, response.StructuredDocumentation.TypeParamElements[0]);
Assert.Equal("T", response.StructuredDocumentation.TypeParamElements[0].Name);
Assert.Equal("The element type of the array", response.StructuredDocumentation.TypeParamElements[0].Documentation);

Assert.Single(response.StructuredDocumentation.Exception);
var expectedException =
@"System.Exception: Thrown when something goes wrong";
Assert.Equal(expectedException, response.StructuredDocumentation.Exception[0]);
Assert.Equal("System.Exception", response.StructuredDocumentation.Exception[0].Name);
Assert.Equal("Thrown when something goes wrong", response.StructuredDocumentation.Exception[0].Documentation);

var expectedRemarks =
@"Remarks: You may have some additional information about this class here.";
@"You may have some additional information about this class here.";
Assert.Equal(expectedRemarks, response.StructuredDocumentation.RemarksText);

var expectedReturns =
@"Returns: Returns an array of type T .";
@"Returns an array of type T .";
Assert.Equal(expectedReturns, response.StructuredDocumentation.ReturnsText);
}
}
Expand Down