[Java.Interop.Tools.JavaSource] Fix tag parsing errors (#997)

pjcollins · web-flow · commit 265ad762c390 · 2022-06-30T14:45:51.000-04:00
Improve tag parsing by adding grammar rules for `@hide`, `@inheritDoc`, and `{@see}`. The new grammar rules are not complete, but these new grammar rules, along with improvements to existing rules, reduces the number of `## Unable to translate remarks for …` errors from 740 to 391, nearly in half. The rules for `@param`, `@return`, `@throws`, and `@unknown` have been updated to handle more variations of these tags. In some cases, these tags are only followed by a single word or no content at all. New line character filters and tab have been added to `nonSpaceTerm` to fix instances where an "empty" tag would be merged with the tag on the following line. See [`Android.Database.DatabaseUtils.GetCollationKey(String)`][0] as an example. Before `nonSpaceTerm` change: <param name="name">To be added.</param> <param name="name&#xA;@return&#xA;@return">the collation key</param> <summary>return the collation key</summary> <returns>To be added.</returns> After `nonSpaceTerm` change: <param name="name">name</param> <summary>return the collation key</summary> <returns>the collation key</returns> [0]: https://github.com/xamarin/android-api-docs/blob/a7e914965a0e80c9454149399551d87a18997b2e/docs/Mono.Android/en/Android.Database/DatabaseUtils.xml#L1505-L1508
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
@@ -25,6 +25,8 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					| DeprecatedDeclaration
 					| DeprecatedSinceDeclaration
 					| ExceptionDeclaration
+					| InheritDocDeclaration
+					| HideDeclaration
 					| ParamDeclaration
 					| ReturnDeclaration
 					| SeeDeclaration
@@ -78,7 +80,7 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					parseNode.AstNode   = p;
 				};
 
-				var nonSpaceTerm = new RegexBasedTerminal ("[^ ]", "[^ ]+") {
+				var nonSpaceTerm = new RegexBasedTerminal ("[^ \n\r\t]", "[^ \n\r\t]+") {
 					AstConfig = new AstNodeConfig {
 						NodeCreator = (context, parseNode) => parseNode.AstNode = parseNode.Token.Value,
 					},
@@ -99,19 +101,42 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					FinishParse (context, parseNode);
 				};
 
-				ParamDeclaration.Rule = "@param" + nonSpaceTerm + BlockValues;
+				// Ignore @hide tags
+				HideDeclaration.Rule = "@hide";
+				HideDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
+					FinishParse (context, parseNode);
+				};
+
+				InheritDocDeclaration.Rule = "@inheritDoc";
+				InheritDocDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
+					if (!grammar.ShouldImport (ImportJavadoc.InheritDocTag)) {
+						return;
+					}
+					// TODO: Iterate through parents for corresponding javadoc element.
+					FinishParse (context, parseNode);
+				};
+
+				ParamDeclaration.Rule = "@param" + nonSpaceTerm
+					| "@param" + nonSpaceTerm + BlockValues
+					;
 				ParamDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
 					if (!grammar.ShouldImport (ImportJavadoc.ParamTag)) {
 						return;
 					}
-					var p = new XElement ("param",
-							new XAttribute ("name", string.Join ("", AstNodeToXmlContent (parseNode.ChildNodes [1]))),
-							AstNodeToXmlContent (parseNode.ChildNodes [2]));
+					var paramName = string.Join ("", AstNodeToXmlContent (parseNode.ChildNodes [1]));
+					var p = new XElement ("param", new XAttribute ("name", paramName));
+					if (parseNode.ChildNodes.Count >= 3) {
+						p.Add (AstNodeToXmlContent (parseNode.ChildNodes [2]));
+					} else {
+						p.Add (paramName);
+					}
 					FinishParse (context, parseNode).Parameters.Add (p);
 					parseNode.AstNode   = p;
 				};
 
-				ReturnDeclaration.Rule = "@return" + BlockValues;
+				ReturnDeclaration.Rule = "@return"
+					| "@return" + BlockValues
+					;
 				ReturnDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
 					if (!grammar.ShouldImport (ImportJavadoc.ReturnTag)) {
 						return;
@@ -158,7 +183,9 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					parseNode.AstNode   = p;
 				};
 
-				ThrowsDeclaration.Rule = "@throws" + nonSpaceTerm + BlockValues;
+				ThrowsDeclaration.Rule = "@throws" + nonSpaceTerm
+					| "@throws" + nonSpaceTerm + BlockValues
+					;
 				ThrowsDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
 					if (!grammar.ShouldImport (ImportJavadoc.ExceptionTag)) {
 						return;
@@ -205,7 +232,9 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					parseNode.AstNode = parseNode.Token.Value.ToString ();
 
 
-				UnknownTagDeclaration.Rule = unknownTagTerminal + BlockValues;
+				UnknownTagDeclaration.Rule = unknownTagTerminal
+					| unknownTagTerminal + BlockValues
+					;
 				UnknownTagDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
 					if (!grammar.ShouldImport (ImportJavadoc.Remarks)) {
 						return;
@@ -242,6 +271,8 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 			public  readonly    NonTerminal DeprecatedDeclaration      = new NonTerminal (nameof (DeprecatedDeclaration));
 			public  readonly    NonTerminal DeprecatedSinceDeclaration = new NonTerminal (nameof (DeprecatedSinceDeclaration));
 			public  readonly    NonTerminal ExceptionDeclaration       = new NonTerminal (nameof (ExceptionDeclaration));
+			public  readonly    NonTerminal HideDeclaration            = new NonTerminal (nameof (HideDeclaration));
+			public  readonly    NonTerminal InheritDocDeclaration      = new NonTerminal (nameof (InheritDocDeclaration));
 			public  readonly    NonTerminal ParamDeclaration           = new NonTerminal (nameof (ParamDeclaration));
 			public  readonly    NonTerminal ReturnDeclaration          = new NonTerminal (nameof (ReturnDeclaration));
 			public  readonly    NonTerminal SeeDeclaration             = new NonTerminal (nameof (SeeDeclaration));
diff --git a/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.InlineTagsBnfTerms.cs b/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.InlineTagsBnfTerms.cs
@@ -26,6 +26,7 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					| LinkDeclaration
 					| LinkplainDeclaration
 					| LiteralDeclaration
+					| SeeDeclaration
 					| ValueDeclaration
 					;
 
@@ -75,6 +76,14 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					parseNode.AstNode = new XText (content);
 				};
 
+				SeeDeclaration.Rule = grammar.ToTerm ("{@see") + InlineValue + "}";
+				SeeDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
+					// TODO: @see supports multiple forms; see: https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#see
+					// Also need to convert to appropriate CREF value, ignore for now
+					var target = parseNode.ChildNodes [1].AstNode;
+					parseNode.AstNode = new XElement ("c", target);
+				};
+
 				ValueDeclaration.Rule = grammar.ToTerm ("{@value}")
 					| grammar.ToTerm ("{@value") + InlineValue + "}";
 				ValueDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
@@ -114,10 +123,12 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 			public  readonly    NonTerminal LinkplainDeclaration        = new NonTerminal (nameof (LinkplainDeclaration));
 
 			// https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#literal
-			public  readonly    NonTerminal LiteralDeclaration         = new NonTerminal (nameof (LinkplainDeclaration));
+			public  readonly    NonTerminal LiteralDeclaration          = new NonTerminal (nameof (LiteralDeclaration));
+
+			public  readonly    NonTerminal SeeDeclaration              = new NonTerminal (nameof (SeeDeclaration));
 
 			// https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#value
-			public  readonly    NonTerminal ValueDeclaration           = new NonTerminal (nameof (ValueDeclaration));
+			public  readonly    NonTerminal ValueDeclaration            = new NonTerminal (nameof (ValueDeclaration));
 		}
 	}
 }
diff --git a/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocParser.cs b/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocParser.cs
@@ -26,6 +26,7 @@ internal enum ImportJavadoc {
 		SinceTag            = 1 << 9,
 		VersionTag          = 1 << 10,
 		ExtraRemarks        = 1 << 11,
+		InheritDocTag       = 1 << 12,
 	}
 
 	[Flags]
@@ -43,6 +44,7 @@ public enum XmldocStyle {
 			| ImportJavadoc.SinceTag
 			| ImportJavadoc.VersionTag
 			| ImportJavadoc.ExtraRemarks
+			| ImportJavadoc.InheritDocTag
 			,
 		IntelliSense = ImportJavadoc.Summary
 			| ImportJavadoc.ExceptionTag
diff --git a/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocGrammar.BlockTagsBnfTermsTests.cs b/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocGrammar.BlockTagsBnfTermsTests.cs
@@ -56,14 +56,39 @@ public void ExceptionDeclaration ()
 			//Assert.AreEqual ("<exception cref=\"Throwable\">Just Because.</exception>", r.Root.AstNode.ToString ());
 		}
 
+		[Test]
+		public void InheritDocDeclaration ()
+		{
+			var p = CreateParser (g => g.BlockTagsTerms.InheritDocDeclaration);
+
+			var r = p.Parse ("@inheritDoc");
+			Assert.IsFalse (r.HasErrors (), "@inheritDoc: " + DumpMessages (r, p));
+			// TODO: Enable after adding support for @inheritDoc
+			Assert.IsNull (r.Root.AstNode, "@inheritDoc should be ignored, but node was not null.");
+		}
+
+		[Test]
+		public void HideDeclaration ()
+		{
+			var p = CreateParser (g => g.BlockTagsTerms.HideDeclaration);
+
+			var r = p.Parse ("@hide");
+			Assert.IsFalse (r.HasErrors (), "@hide: " + DumpMessages (r, p));
+			Assert.IsNull (r.Root.AstNode, "@hide should be ignored, but node was not null.");
+		}
+
 		[Test]
 		public void ParamDeclaration ()
 		{
 			var p = CreateParser (g => g.BlockTagsTerms.ParamDeclaration);
 
-			var r = p.Parse ("@param a Insert description here\n");
+			var r = p.Parse ("@param a Insert description here\nand here.");
 			Assert.IsFalse (r.HasErrors (), "@param: " + DumpMessages (r, p));
-			Assert.AreEqual ("<param name=\"a\">Insert description here</param>", r.Root.AstNode.ToString ());
+			Assert.AreEqual ($"<param name=\"a\">Insert description here{Environment.NewLine}and here.</param>", r.Root.AstNode.ToString ());
+
+			r = p.Parse ("@param b");
+			Assert.IsFalse (r.HasErrors (), "name only @param: " + DumpMessages (r, p));
+			Assert.AreEqual ("<param name=\"b\">b</param>", r.Root.AstNode.ToString ());
 		}
 
 		[Test]
@@ -120,16 +145,21 @@ public void ThrowsDeclaration ()
 			var p = CreateParser (g => g.BlockTagsTerms.ThrowsDeclaration);
 
 			var r = p.Parse ("@throws Throwable the {@code Exception} raised by this method");
-			Assert.IsFalse (r.HasErrors (), "@throws: " + DumpMessages (r, p));
+			Assert.IsFalse (r.HasErrors (), "{@code} @throws: " + DumpMessages (r, p));
 			Assert.IsNull (r.Root.AstNode, "@throws should be ignored, but node with code block was not null.");
 			// TODO: Re-enable when we can generate valid crefs
 			//Assert.AreEqual ("<exception cref=\"Throwable\">the <c>Exception</c> raised by this method</exception>", r.Root.AstNode.ToString ());
 
 			r = p.Parse ("@throws Throwable something <i>or other</i>!");
-			Assert.IsFalse (r.HasErrors (), "@throws: " + DumpMessages (r, p));
-			Assert.IsNull (r.Root.AstNode, "@throws should be ignored, but node was not null.");
+			Assert.IsFalse (r.HasErrors (), "<i> @throws: " + DumpMessages (r, p));
+			Assert.IsNull (r.Root.AstNode, "@throws should be ignored, but node with <i> was not null.");
 			// TODO: Re-enable when we can generate valid crefs
 			//Assert.AreEqual ("<exception cref=\"Throwable\">something <i>or other</i>!</exception>", r.Root.AstNode.ToString ());
+
+			r = p.Parse ("@throws android.content.ActivityNotFoundException");
+			Assert.IsFalse (r.HasErrors (), "name only @throws: " + DumpMessages (r, p));
+			// TODO: Re-enable when we can generate valid crefs
+			Assert.IsNull (r.Root.AstNode, "@throws should be ignored, but name only node was not null.");
 		}
 
 		[Test]
@@ -140,6 +170,10 @@ public void UnknownTagDeclaration ()
 			var r = p.Parse ("@this-is-not-supported something {@code foo} else.");
 			Assert.IsFalse (r.HasErrors (), "@this-is-not-supported: " + DumpMessages (r, p));
 			Assert.AreEqual (null, r.Root.AstNode);
+
+			r = p.Parse ("@standalonetag");
+			Assert.IsFalse (r.HasErrors (), "@standalonetag: " + DumpMessages (r, p));
+			Assert.AreEqual (null, r.Root.AstNode);
 		}
 	}
 }
diff --git a/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocGrammar.InlineTagsBnfTermsTests.cs b/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocGrammar.InlineTagsBnfTermsTests.cs
@@ -75,6 +75,16 @@ public void LiteralDeclaration ()
 			Assert.AreEqual ("A&lt;B&gt;C", r.Root.AstNode.ToString ());
 		}
 
+		[Test]
+		public void SeeDeclaration ()
+		{
+			var p = CreateParser (g => g.InlineTagsTerms.SeeDeclaration);
+
+			var r = p.Parse ("{@see #cancelNotification(String, String, int)}");
+			Assert.IsFalse (r.HasErrors (), DumpMessages (r, p));
+			Assert.AreEqual ("<c>#cancelNotification(String, String, int)</c>", r.Root.AstNode.ToString ());
+		}
+
 		[Test]
 		public void ValueDeclaration ()
 		{
diff --git a/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocParserTests.cs b/tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocParserTests.cs
@@ -104,11 +104,15 @@ What about soft paragraphs?
 <p>What about <i>hard</i> paragraphs?
 
 @param a something
+@param b
+@param c
 @see #method()
 @apiSince 1
 ",
 				FullXml = @"<member>
   <param name=""a"">something</param>
+  <param name=""b"">b</param>
+  <param name=""c"">c</param>
   <summary>This is the summary sentence.</summary>
   <remarks>
     <para>This is the summary sentence.  Insert
@@ -120,6 +124,8 @@ more description here.</para>
 </member>",
 				IntelliSenseXml = @"<member>
   <param name=""a"">something</param>
+  <param name=""b"">b</param>
+  <param name=""c"">c</param>
   <summary>This is the summary sentence.</summary>
 </member>",
 			},
@@ -183,19 +189,25 @@ more description here.
 @param manifest The value of the <a
 href=""{@docRoot}guide/topics/manifest/manifest-element.html#vcode"">{@code
 android:versionCode}</a> manifest attribute.
+@param empty
+@return the return value
 ",
 				FullXml = $@"<member>
   <param name=""manifest"">The value of the <see href=""{DocRootPrefixExpected}guide/topics/manifest/manifest-element.html#vcode""><c>android:versionCode</c></see> manifest attribute.</param>
+  <param name=""empty"">empty</param>
   <summary>See <see href=""http://man7.org/linux/man-pages/man2/accept.2.html"">accept(2)</see>.</summary>
   <remarks>
     <para>See <see href=""http://man7.org/linux/man-pages/man2/accept.2.html"">accept(2)</see>.  Insert
 more description here.
 How about another link <see href=""http://man7.org/linux/man-pages/man2/accept.2.html"">accept(2)</see></para>
   </remarks>
+  <returns>the return value</returns>
 </member>",
 				IntelliSenseXml = $@"<member>
   <param name=""manifest"">The value of the <see href=""{DocRootPrefixExpected}guide/topics/manifest/manifest-element.html#vcode""><c>android:versionCode</c></see> manifest attribute.</param>
+  <param name=""empty"">empty</param>
   <summary>See <see href=""http://man7.org/linux/man-pages/man2/accept.2.html"">accept(2)</see>.</summary>
+  <returns>the return value</returns>
 </member>",
 			},
 		};

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ internal enum ImportJavadoc {`
`26`	`26`	`SinceTag = 1 << 9,`
`27`	`27`	`VersionTag = 1 << 10,`
`28`	`28`	`ExtraRemarks = 1 << 11,`
	`29`	`+ InheritDocTag = 1 << 12,`
`29`	`30`	`}`
`30`	`31`
`31`	`32`	`[Flags]`
`@@ -43,6 +44,7 @@ public enum XmldocStyle {`
`43`	`44`	`\| ImportJavadoc.SinceTag`
`44`	`45`	`\| ImportJavadoc.VersionTag`
`45`	`46`	`\| ImportJavadoc.ExtraRemarks`
	`47`	`+ \| ImportJavadoc.InheritDocTag`
`46`	`48`	`,`
`47`	`49`	`IntelliSense = ImportJavadoc.Summary`
`48`	`50`	`\| ImportJavadoc.ExceptionTag`