[Java.Interop.Tools.JavaSource] Merge @return block values (#836)

pjcollins · web-flow · commit 0e01fb5d1db5 · 2021-05-12T18:28:14.000-04:00
Context: dotnet/android#5485 In an attempt to generate updated documentation for API-30 I noticed a minor issue in generator Javadoc import (7574f16): when a `<javadoc/>` element contains multiple `@return` values: <class name="Example" …> <method name="getP" …> <javadoc> <![CDATA[ … @return some docs @return additional docs ]]> </javadoc> </method> </class> We would generate multiple `<returns/>` elements, one for each `@return`: partial class Example { /// <returns>some docs</returns> /// <returns>additional docs</returns> public int P { [Register("getP", …)] get => … } } This is "fine", as far as the C# compiler is concerned, which results in a Documentation XML file containing both `<returns/>`: <doc> <members> <member name="P:Example.P"> <returns>some docs</returns> <returns>additional docs</returns> </member> </members> </doc> The problem is when we later try to *import* this documentation via `mdoc update --import`, as dotnet/android#5485 does; if it's a *method* which contains multiple `<returns/>` elements and mdoc 5.7.4.9 is used (included with macOS Mono 6.12): $ mdoc update -o docs assembly.dll --import assembly.xml Then the resulting imported [**mdoc**(5)][0] documentation only contains the *first* element: <Docs> <summary>To be added.</summary> <returns>some docs</returns> <remarks>To be added.</remarks> </Docs> Using [mdoc 5.8.0][1] will preserve both `<returns/>` elements, but "higher level" tooling such as HTML rendering or IntelliSense may not properly support multiple `<returns/>` elements. *However*, if it's a *property* which contains multiple `<returns/>` elements, then `mdoc update` will convert each `<returns/>` element into a `<value/>` element: $ mdoc update -o docs assembly.dll --import assembly.xml … <Docs> <summary>To be added.</summary> <value>some docs</value> <value>additional docs</value> <remarks>To be added.</remarks> </Docs> This is "odd", but it gets worse: on subsequent `mdoc update` invocations, *additional* `<value/>` elements are created! $ mdoc update -o docs assembly.dll --import assembly.xml … <Docs> <summary>To be added.</summary> <value>additional docs</value> <value>some docs</value> <value>additional docs</value> <remarks>To be added.</remarks> </Docs> $ mdoc update -o docs assembly.dll --import assembly.xml … <Docs> <summary>To be added.</summary> <value>some docs</value> <value>additional docs</value> <value>some docs</value> <value>additional docs</value> <remarks>To be added.</remarks> </Docs> While an `mdoc` bug, we can prevent this from happening by updating the Javadoc importer so that multiple `@returns` blocks are *merged* into a single `<returns/>` element: partial class Example { /// <returns>some docs additional docs</returns> public int P {…} } [0]: http://docs.go-mono.com/?link=man%3amdoc(5) [1]: https://www.nuget.org/packages/mdoc/5.8.0
diff --git a/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.BlockTagsBnfTerms.cs b/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.BlockTagsBnfTerms.cs
@@ -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) {
+							r.Add (" ", AstNodeToXmlContent (parseNode.ChildNodes [1]));
+							parseNode.AstNode = r;
+						}
+					}
 				};
 
 				SeeDeclaration.Rule = "@see" + BlockValues;
diff --git a/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocParserTests.cs b/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocParserTests.cs
@@ -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)
@@ -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>
@@ -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}.",
+				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 {
@@ -166,7 +175,7 @@ more description here.</para>
 			},
 		};
 
-		class ParseResult {
+		public class ParseResult {
 			public  string  Javadoc;
 			public  string  FullXml;
 			public  string  IntelliSenseXml;
