Skip to content

[Java.Interop.Tools.JavaSource] Improve <code> parsing #1125

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

Merged
jonpryor merged 6 commits into from
Jul 12, 2023
Merged

[Java.Interop.Tools.JavaSource] Improve <code> parsing #1125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d442c15
[Java.Interop.Tools.JavaSource] Fix up common HTML tags
pjcollins Jun 13, 2023
945535d
Fix test
pjcollins Jun 14, 2023
a232127
Apply feedback
pjcollins Jul 11, 2023
c8ef42d
Add test for trailing open code tag
pjcollins Jul 12, 2023
0d41cc8
Merge remote-tracking branch 'origin/main' into dev/pjc/api33-doc-fix
pjcollins Jul 12, 2023
00896fb
Fix merge
pjcollins Jul 12, 2023
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
18 changes: 16 additions & 2 deletions ...ols.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.HtmlBnfTerms.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,17 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
}
};

CodeElementDeclaration.Rule = CreateStartElement ("code", grammar) + InlineDeclarations + CreateEndElement ("code", grammar);
CodeElementDeclaration.Rule = CodeElementContentTerm;
CodeElementDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
var target = parseNode.ChildNodes [1].AstNode;
// Parse the entire <code> element captured in the token
var codeElementText = parseNode.ChildNodes [0].Token.Text;
int startIndex = codeElementText.IndexOf ('>');
int stopIndex = codeElementText.LastIndexOf ('<');
if (startIndex == -1 || stopIndex == -1) {
parseNode.AstNode = new XText (codeElementText);
return;
}
var target = codeElementText.Substring (startIndex + 1, stopIndex - startIndex - 1);
parseNode.AstNode = new XElement ("c", target);
};
}
Expand Down Expand Up @@ -232,6 +240,12 @@ static string GetChildNodesAsString (ParseTreeNode parseNode)
public readonly NonTerminal InlineHyperLinkDeclaration = new NonTerminal (nameof (InlineHyperLinkDeclaration), ConcatChildNodes);
public readonly NonTerminal CodeElementDeclaration = new NonTerminal (nameof (CodeElementDeclaration), ConcatChildNodes);

public readonly Terminal CodeElementContentTerm = new RegexBasedTerminal ("<code>", $@"(?i)<code\s*[^>]*>(.|\s)*?(<\/code>|<\/null>|<code>)") {
AstConfig = new AstNodeConfig {
NodeCreator = (context, parseNode) => parseNode.AstNode = "",
},
};

public readonly Terminal InlineHyperLinkOpenTerm = new RegexBasedTerminal ("<a attr=", @"(?i)<a\s*.*=") {
AstConfig = new AstNodeConfig {
NodeCreator = (context, parseNode) => parseNode.AstNode = "",
Expand Down
33 changes: 32 additions & 1 deletion tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocGrammar.HtmlBnfTermsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,38 @@ public void CodeElementDeclaration ()
var r = p.Parse ("<code>input.position()</code>");
Assert.IsFalse (r.HasErrors (), DumpMessages (r, p));
Assert.AreEqual ("<c>input.position()</c>", r.Root.AstNode.ToString ());
}

r = p.Parse ("<code>null</null>");
Assert.IsFalse (r.HasErrors (), DumpMessages (r, p));
Assert.AreEqual ("<c>null</c>", r.Root.AstNode.ToString ());

r = p.Parse ("<code>android:label=\"@string/resolve_title\"</code>");
Assert.IsFalse (r.HasErrors (), DumpMessages (r, p));
Assert.AreEqual ("<c>android:label=\"@string/resolve_title\"</c>", r.Root.AstNode.ToString ());

r = p.Parse ("<code>Activity.RESULT_OK<code>");
Copy link
Contributor

@jonpryor jonpryor Jul 12, 2023

Choose a reason for hiding this comment

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

I would like to extend this test to see what happens with the content after the <code>, e.g. from android/telephony/gsm/SmsManager.java:

     *  The result code will be <code>Activity.RESULT_OK<code> for success,
     *  or one of these errors:
     *  <code>RESULT_ERROR_GENERIC_FAILURE</code>

or shorter: <code>should be code<code> but what about this <code>and this?</code>. Yes, <code>should be code<code> results in <c>should be code</c>, but what about the rest? Would we get:

<c>should be code</c><c> but what about this </c><c>and this?</c>

which is entirely defensible and reasonable, but is worth "calling out" in the unit tests.

Copy link
Member Author

@pjcollins pjcollins Jul 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We don't appear to create an additional code element for the intermediate content, presumably because we capture the incorrectly placed open <code> tag while parsing the previous code element. I've added another test for this.

Assert.IsFalse (r.HasErrors (), DumpMessages (r, p));
Assert.AreEqual ("<c>Activity.RESULT_OK</c>", r.Root.AstNode.ToString ());

r = p.Parse ("<code class=prettyprint>format.setString(MediaFormat.KEY_FRAME_RATE, null)</code>");
Assert.IsFalse (r.HasErrors (), DumpMessages (r, p));
Assert.AreEqual ("<c>format.setString(MediaFormat.KEY_FRAME_RATE, null)</c>", r.Root.AstNode.ToString ());

r = p.Parse (@"<code>
<p> [ 0, 0, 0, 0, 0 ]
<p> [ 0, 0, 0, 0, 0 ]
<p> [ 0, 0, 1, 0, 0 ]
<p> [ 0, 0, 0, 0, 0 ]
<p> [ 0, 0, 0, 0, 0 ]
</code>");
Assert.IsFalse (r.HasErrors (), DumpMessages (r, p));
Assert.AreEqual (@"<c>
&lt;p&gt; [ 0, 0, 0, 0, 0 ]
&lt;p&gt; [ 0, 0, 0, 0, 0 ]
&lt;p&gt; [ 0, 0, 1, 0, 0 ]
&lt;p&gt; [ 0, 0, 0, 0, 0 ]
&lt;p&gt; [ 0, 0, 0, 0, 0 ]
</c>", r.Root.AstNode.ToString ());
}
}
}
28 changes: 24 additions & 4 deletions tests/Java.Interop.Tools.JavaSource-Tests/SourceJavadocToXmldocParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,16 +144,36 @@ more description here.</para>
</member>",
},
new ParseResult {
Javadoc = "Something {@link #method}: description, \"<code>declaration</code>\" or \"<code>another declaration</code>\".\n\n@apiSince 1\n",
Javadoc = "Something {@link #method}: description, \"<code>declaration</code>\" or <code><pre><p>some content</code></pre></p>.\n\n@apiSince 1\n",
FullXml = @"<member>
<summary>Something <c>#method</c>: description, ""<c>declaration</c>"" or ""<c>another declaration</c>"".</summary>
<summary>Something <c>#method</c>: description, ""<c>declaration</c>"" or <c>&lt;pre&gt;&lt;p&gt;some content</c>&lt;/pre&gt;&lt;/p&gt;.</summary>
<remarks>
<para>Something <c>#method</c>: description, ""<c>declaration</c>"" or ""<c>another declaration</c>"".</para>
<para>Something <c>#method</c>: description, ""<c>declaration</c>"" or <c>&lt;pre&gt;&lt;p&gt;some content</c>&lt;/pre&gt;&lt;/p&gt;.</para>
<para>Added in API level 1.</para>
</remarks>
</member>",
IntelliSenseXml = @"<member>
<summary>Something <c>#method</c>: description, ""<c>declaration</c>"" or ""<c>another declaration</c>"".</summary>
<summary>Something <c>#method</c>: description, ""<c>declaration</c>"" or <c>&lt;pre&gt;&lt;p&gt;some content</c>&lt;/pre&gt;&lt;/p&gt;.</summary>
</member>",
},
new ParseResult {
Javadoc = @"The result code will be <code>Activity.RESULT_OK<code> for success,
or one of these errors:
<code>RESULT_ERROR_GENERIC_FAILURE</code>",
FullXml = @"<member>
<summary>The result code will be <c>Activity.RESULT_OK</c> for success,
or one of these errors:
<c>RESULT_ERROR_GENERIC_FAILURE</c></summary>
<remarks>
<para>The result code will be <c>Activity.RESULT_OK</c> for success,
or one of these errors:
<c>RESULT_ERROR_GENERIC_FAILURE</c></para>
</remarks>
</member>",
IntelliSenseXml = @"<member>
<summary>The result code will be <c>Activity.RESULT_OK</c> for success,
or one of these errors:
<c>RESULT_ERROR_GENERIC_FAILURE</c></summary>
</member>",
},
new ParseResult {
Expand Down