Skip to content

Commit

Permalink
[Java.Interop.Tools.JavaSource] Fix tag parsing errors (#997)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pjcollins authored Jun 30, 2022
1 parent 99c68a8 commit 265ad76
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
| DeprecatedDeclaration
| DeprecatedSinceDeclaration
| ExceptionDeclaration
| InheritDocDeclaration
| HideDeclaration
| ParamDeclaration
| ReturnDeclaration
| SeeDeclaration
Expand Down Expand Up @@ -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,
},
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
| LinkDeclaration
| LinkplainDeclaration
| LiteralDeclaration
| SeeDeclaration
| ValueDeclaration
;

Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ internal enum ImportJavadoc {
SinceTag = 1 << 9,
VersionTag = 1 << 10,
ExtraRemarks = 1 << 11,
InheritDocTag = 1 << 12,
}

[Flags]
Expand All @@ -43,6 +44,7 @@ public enum XmldocStyle {
| ImportJavadoc.SinceTag
| ImportJavadoc.VersionTag
| ImportJavadoc.ExtraRemarks
| ImportJavadoc.InheritDocTag
,
IntelliSense = ImportJavadoc.Summary
| ImportJavadoc.ExceptionTag
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>",
},
Expand Down Expand Up @@ -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>",
},
};
Expand Down

0 comments on commit 265ad76

Please sign in to comment.