Skip to content
This repository was archived by the owner on Jul 6, 2026. It is now read-only.

[generator] Handle multiple @return javadoc values#836

Merged
jonpryor merged 2 commits into from
May 12, 2021
Merged

[generator] Handle multiple @return javadoc values #836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fbd7b2
[generator] Handle multiple @return javadoc values
pjcollins May 11, 2021
b21e665
Apply feedback
pjcollins May 12, 2021
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
16 changes: 13 additions & 3 deletions ...avaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.BlockTagsBnfTerms.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -114,10 +114,20 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
if (!grammar.ShouldImport (ImportJavadoc.ReturnTag)) {
return;
}
var r = new XElement ("returns",
// When encountering multiple @return keys in a line, append subsequent @return key content to the original <returns> element.
var jdi = FinishParse (context, parseNode);
if (jdi.Returns.Count == 0) {
var r = new XElement ("returns",
AstNodeToXmlContent (parseNode.ChildNodes [1]));
FinishParse (context, parseNode).Returns.Add (r);
parseNode.AstNode = r;
FinishParse (context, parseNode).Returns.Add (r);
parseNode.AstNode = r;
} else {
var r = jdi.Returns.First () as XElement;
if (r != null) {

@jonpryorjonpryorMay 11, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can't be null; .First()requires that an element be returned, and unless we have a bug and we do .Returns.Add(null), there should only be non-null elements within jdi.Returns.

If we want to allownull, use .FirstOrDefault().

Actually, on review this entire block doesn't quite make sense to me: ritself shouldn't change from r.Add(), and thus this should be a no-op (unless jdi.Returns somehow has > 1 element in it? But how could that happen?)

FinishParse(context,parseNode).Returns.Clear();FinishParse(context,parseNode).Returns.Add(r);

Neither of the above statements should be needed.

@pjcollinspjcollinsMay 12, 2021•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The null check was intended to handle the possibility of jdi.Returns.First () not being an XElement, though I think we only ever add XElements to this ICollection<XNode> in the section above and so the check probably shouldn't be needed.

The FinishParse (context, parseNode).Returns collection will have more than zero elements in it when the <javadoc> entry that is being parsed has multiple @return values in it, as shown in the test addition below. The Returns.Clear and Returns.Add additions do look to be redundant here though, I will clean that up.

r.Add (" ", AstNodeToXmlContent (parseNode.ChildNodes [1]));
parseNode.AstNode = r;
}
}
};

SeeDeclaration.Rule = "@see" + BlockValues;
Expand Down
39 changes: 24 additions & 15 deletions tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocParserTests.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,19 @@ namespace Java.Interop.Tools.JavaSource.Tests
[TestFixture]
public class SourceJavadocToXmldocParserTests : SourceJavadocToXmldocGrammarFixture {

[Test]
public void TryParse ()
[Test, TestCaseSource (nameof (TryParse_Success))]
public void TryParse (ParseResult parseResult)
{
foreach (var values in TryParse_Success) {
ParseTree parseTree;
var p = new SourceJavadocToXmldocParser (XmldocStyle.Full);
var n = p.TryParse (values.Javadoc, null, out parseTree);
Assert.IsFalse (parseTree.HasErrors (), DumpMessages (parseTree, p));
Assert.AreEqual (values.FullXml, GetMemberXml (n), $"while parsing input: ```{values.Javadoc}```");
ParseTree parseTree;
var p = new SourceJavadocToXmldocParser (XmldocStyle.Full);
var n = p.TryParse (parseResult.Javadoc, null, out parseTree);
Assert.IsFalse (parseTree.HasErrors (), DumpMessages (parseTree, p));
Assert.AreEqual (parseResult.FullXml, GetMemberXml (n), $"while parsing input: ```{parseResult.Javadoc}```");

p = new SourceJavadocToXmldocParser (XmldocStyle.IntelliSense);
n = p.TryParse (values.Javadoc, null, out parseTree);
Assert.IsFalse (parseTree.HasErrors (), DumpMessages (parseTree, p));
Assert.AreEqual (values.IntelliSenseXml, GetMemberXml (n), $"while parsing input: ```{values.Javadoc}```");
}
p = new SourceJavadocToXmldocParser (XmldocStyle.IntelliSense);
n = p.TryParse (parseResult.Javadoc, null, out parseTree);
Assert.IsFalse (parseTree.HasErrors (), DumpMessages (parseTree, p));
Assert.AreEqual (parseResult.IntelliSenseXml, GetMemberXml (n), $"while parsing input: ```{parseResult.Javadoc}```");
}

static string GetMemberXml (IEnumerable<XNode> members)
Expand All@@ -40,7 +38,7 @@ static string GetMemberXml (IEnumerable<XNode> members)
return e.ToString ();
}

static readonly ParseResult[] TryParse_Success = new ParseResult[]{
public static readonly ParseResult[] TryParse_Success = new ParseResult[]{
new ParseResult {
Javadoc = "Summary.\n\nP2.\n\n<p>Hello!</p>",
FullXml = @"<member>
Expand DownExpand Up@@ -78,6 +76,17 @@ static string GetMemberXml (IEnumerable<XNode> members)
<returns>
<c>true</c> if something
or other; otherwise <c>false</c>.</returns>
</member>",
},
new ParseResult {
Javadoc = "@return {@code true} if something else @return {@code false}.",

@jonpryorjonpryorMay 11, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm half surprised that @tag parsing isn't requiring newlines in there, and that we don't need\n@return ……

FullXml = @"<member>
<returns>
<c>true</c> if something else <c>false</c>.</returns>
</member>",
IntelliSenseXml = @"<member>
<returns>
<c>true</c> if something else <c>false</c>.</returns>
</member>",
},
new ParseResult {
Expand DownExpand Up@@ -166,7 +175,7 @@ more description here.</para>
},
};

class ParseResult {
public class ParseResult {
public string Javadoc;
public string FullXml;
public string IntelliSenseXml;
Expand Down