Skip to content

[class-parse] support updated DroidDoc. #96

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 1 commit into from
Oct 20, 2016
Merged

[class-parse] support updated DroidDoc. #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/Xamarin.Android.Tools.Bytecode/ClassPath.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace Xamarin.Android.Tools.Bytecode {

public enum JavaDocletType {
DroidDoc,
DroidDoc2,
Java6,
Java7,
Java8,
Expand Down Expand Up @@ -225,7 +226,8 @@ void FixupParametersFromDocs (XElement api)
AndroidDocScraper CreateDocScraper (string dir)
{
switch (DocletType) {
default: return new DroidDocScraper (dir);
default: return new DroidDoc2Scraper (dir);
case JavaDocletType.DroidDoc: return new DroidDocScraper (dir);
Copy link
Contributor

@jonpryor jonpryor Oct 20, 2016

Choose a reason for hiding this comment

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

Shouldn't there also be a case JavaDocletType.DroidDoc2: entry, so that this can be explicitly selected?

Copy link
Contributor Author

@atsushieno atsushieno Oct 20, 2016

Choose a reason for hiding this comment

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

? Why? What is the point of "explicitly selected" ?

Copy link
Contributor

@jonpryor jonpryor Oct 20, 2016

Choose a reason for hiding this comment

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

The point is that the doc scrapers are specific, in that you can't reliably use the DroidDoc2Scraper with DroidDocScraper input, right?

So if someone has:

mono class-parse.exe --docspath=path/to/droiddoc1 foo.jar

Previously, they'll be using the DroidDocScraper. After this change, they'll be using the DroidDoc2Scraper, which won't support the previous docs, which may "break" things.

Consequently, if you can always explicitly specify the type -- and do so -- your binding won't break when we change the default scraper in the future:

mono class-parse.exe --docspath=path/to/droiddoc1 --docstype=droiddoc foo.jar

That said..., the <ClassParse/> MSBuild task has no way of specifying the ClassParse.DocletType property, which means the MSBuild task will always attempt to use the "first" entry, which is DroidDoc, despite the fact that ClassParse.DocumentationPaths can contain ~anything, as it's the concatenation of $(JavaDocPaths), $(Java7DocPaths), $(Java8DocPaths), and $(DroidDocPaths).

What presumably would be "best" is dropping the entire concept of ClassParse.DocletType, and instead have ClassParse.CreateDocScraper() "probe" the type of docs contained within dir so that we automatically use the appropriate scraper type.

However, we've mentioned this before and I believe that this "auto-probe" idea was considered to be difficult/impossible/flakey/inconsistent/etc., so I'm not entirely sure what's the best path forward for integration within Xamarin.Android.

Copy link
Contributor Author

@atsushieno atsushieno Oct 20, 2016

Choose a reason for hiding this comment

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

It has changed the default doc scraper from the old droiddoc to the new droiddoc. Who's using DroidDoc? Only ourselves. Unless someone is really enthusiastic and take sources from AOSP to generate documentation using droiddoc from there, no one has chance to use it in their own project. Thus we can change the default behavior with no hesitate.

ClassParse task not being able to differentiate doclet type is another issue that I believe we can fix, just like javadoc-to-mdoc alraedy implements. But it is much easier to make that build task to take the right doclet type and parse docs as expected. That can be done in much less cost and keeps compatibility with former behavior.

Copy link
Contributor

@jonpryor jonpryor Oct 20, 2016

Choose a reason for hiding this comment

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

That can be done in much less cost and keeps compatibility with former behavior.

Agreed.

case JavaDocletType.Java6: return new JavaDocScraper (dir);
case JavaDocletType.Java7: return new Java7DocScraper (dir);
case JavaDocletType.Java8: return new Java8DocScraper (dir);
Expand Down
135 changes: 88 additions & 47 deletions src/Xamarin.Android.Tools.Bytecode/JavaDocumentScraper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,60 @@ namespace Xamarin.Android.Tools.Bytecode
{
enum JavaDocKind {
DroidDoc,
DroidDoc2,
Java6,
Java7
Java7,
Java8
}

class DroidDocScraper : AndroidDocScraper
{
const String pattern_head_droiddoc = "<span class=\"sympad\"><a href=\".*";

public DroidDocScraper (string dir)
: base (dir, pattern_head_droiddoc, null, " ", false)
{
}
}

class DroidDoc2Scraper : AndroidDocScraper
{
const String pattern_head_droiddoc = "<tr class=\"api .+\".*>.*<code>.*<a href=\".*";
const String reset_pattern_head = "<p>";

public DroidDoc2Scraper (string dir)
: base (dir, pattern_head_droiddoc, reset_pattern_head, " ", true, "\\(", ", ", "\\)", "\\s*</code>")
{
}

string prev_path;
string [] prev_contents;

protected override IEnumerable<string> GetContentLines (string path)
{
if (prev_path == path)
return prev_contents;
else {
prev_path = path;
string all = File.ReadAllText (path).Replace ('\r', ' ').Replace ('\n', ' ');
int start = all.IndexOf ("<!-- ======== START OF CLASS DATA ======== -->", StringComparison.Ordinal);
all = start < 0 ? all : all.Substring (start);
int end = all.IndexOf ("<!-- ========= END OF CLASS DATA ========= -->", StringComparison.Ordinal);
all = end < 0 ? all : all.Substring (0, end);
// <tr>...</tr> is the basic structure so </tr> is used as the end of member, but we also use <p> here.
// Sometimes another </code> can appear after "</code>" (for the end of context member) and that messes regex match.
// So, with any <p>, we interrupt consecutive matches.
prev_contents = all.Split (new string [] { "<p>", "</tr>" }, StringSplitOptions.RemoveEmptyEntries);
return prev_contents;
}
}

protected override bool ShouldResetMatchBuffer (string text)
{
return true;
}
}

class JavaDocScraper : AndroidDocScraper
{
const String pattern_head_javadoc = "<TD><CODE><B><A HREF=\"[./]*"; // I'm not sure how path could be specified... (./ , ../ , or even /)
Expand Down Expand Up @@ -80,7 +120,7 @@ class Java8DocScraper : AndroidDocScraper
const String parameter_pair_splitter_javadoc = "&nbsp;";

public Java8DocScraper (string dir)
: base (dir, pattern_head_javadoc, reset_pattern_head_javadoc, parameter_pair_splitter_javadoc, true, "-", "-", "-")
: base (dir, pattern_head_javadoc, reset_pattern_head_javadoc, parameter_pair_splitter_javadoc, true, "-", "-", "-", null)
{
}
}
Expand All @@ -94,14 +134,15 @@ public abstract class AndroidDocScraper
readonly String open_method;
readonly String param_sep;
readonly String close_method;
readonly String post_close_method_parens;
string root;

protected AndroidDocScraper (string dir, String patternHead, String resetPatternHead, String parameterPairSplitter, bool continuousParamLines)
: this (dir, patternHead, resetPatternHead, parameterPairSplitter, continuousParamLines, "\\(", ", ", "\\)")
: this (dir, patternHead, resetPatternHead, parameterPairSplitter, continuousParamLines, "\\(", ", ", "\\)", null)
{
}

protected AndroidDocScraper (string dir, String patternHead, String resetPatternHead, String parameterPairSplitter, bool continuousParamLines, string openMethod, string paramSep, string closeMethod)
protected AndroidDocScraper (string dir, String patternHead, String resetPatternHead, String parameterPairSplitter, bool continuousParamLines, string openMethod, string paramSep, string closeMethod, string postCloseMethodParens)
{
if (dir == null)
throw new ArgumentNullException ("dir");
Expand All @@ -112,7 +153,8 @@ protected AndroidDocScraper (string dir, String patternHead, String resetPattern
continuous_param_lines = continuousParamLines;
open_method = openMethod;
param_sep = paramSep;
close_method = closeMethod;
close_method = closeMethod;
post_close_method_parens = postCloseMethodParens ?? string.Empty;
if (!Directory.Exists (dir))
throw new Exception ("Directory '" + dir + "' does not exist");

Expand All @@ -124,18 +166,22 @@ protected AndroidDocScraper (string dir, String patternHead, String resetPattern
//foreach (var f in Directory.GetFiles (dir, "*.html", SearchOption.AllDirectories))
// LoadDocument (f.Substring (dir.Length + 1), f);
}
void LoadDocument (string packageDir, string file)

protected virtual IEnumerable<string> GetContentLines (string path)
{
string pkgName = packageDir.Replace ('/', '.');
string className = Path.GetFileNameWithoutExtension (file).Replace ('$', '.');

string html = File.ReadAllText (file);
return File.ReadAllText (path).Split ('\n');
}

protected virtual bool ShouldResetMatchBuffer (string text)
{
// sometimes we get incomplete tag, so cache it until it gets complete or matched.
// I *know* this is a hack.
return reset_pattern_head == null || text.EndsWith (">", StringComparison.Ordinal) || !continuous_param_lines && !text.StartsWith (reset_pattern_head, StringComparison.Ordinal);
}

public String[] GetParameterNames (string package, string type, string method, string[] ptypes, bool isVarArgs)
public virtual String[] GetParameterNames (string package, string type, string method, string[] ptypes, bool isVarArgs)
{
String path = package.Replace ('.', '/') + '/' + type.Replace ('$', '.') + ".html";
string path = package.Replace ('.', '/') + '/' + type.Replace ('$', '.') + ".html";
string file = Path.Combine (root, path);
if (!File.Exists (file)) {
Log.Warning (1,"Warning: no document found : " + file);
Expand All @@ -153,44 +199,39 @@ public String[] GetParameterNames (string package, string type, string method, s
buffer.Append (param_sep);
buffer.Append (ptypes [i].Replace ('$', '.'));
}
buffer.Replace ("[", "\\[").Replace ("]", "\\]");
buffer.Append (close_method);
buffer.Append ("\".*\\((.*)\\)");
buffer.Replace ("[", "\\[").Replace ("]", "\\]").Replace ("?", "\\?");
buffer.Append ("\".*\\(([^(]*)\\)");
buffer.Append (post_close_method_parens);
buffer.Replace ("?", "\\?");
Regex pattern = new Regex (buffer.ToString (), RegexOptions.Multiline);

try {
var reader = File.OpenText (file);
try {
String text = "";
String prev = null;
while ((text = reader.ReadLine ()) != null) {
if (prev != null)
prev = text = prev + text;
var matcher = pattern.Match (text);
if (matcher.Success) {
var plist = matcher.Groups [1];
String[] parms = plist.Value.Split (new string [] {", "}, StringSplitOptions.RemoveEmptyEntries);
if (parms.Length != ptypes.Length) {
Log.Warning (1, "failed matching {0} (expected {1} params, got {2} params)", buffer, ptypes.Length, parms.Length);
return null;
}
String[] result = new String [ptypes.Length];
for (int i = 0; i < ptypes.Length; i++) {
String[] toks = parms [i].Split (parameter_pair_splitter);
result [i] = toks [toks.Length - 1];
}
reader.Close ();
return result;
String text = "";
String prev = null;
foreach (var _text in GetContentLines (file)) {
text = _text.TrimEnd ('\r');
if (prev != null)
prev = text = prev + text;
var matcher = pattern.Match (text);
if (matcher.Success) {
var plist = matcher.Groups [1];
String[] parms = plist.Value.Split (new string [] {", "}, StringSplitOptions.RemoveEmptyEntries);
if (parms.Length != ptypes.Length) {
Log.Warning (1, "failed matching {0} (expected {1} params, got {2} params)", buffer, ptypes.Length, parms.Length);
return null;
}
String[] result = new String [ptypes.Length];
for (int i = 0; i < ptypes.Length; i++) {
String[] toks = parms [i].Split (parameter_pair_splitter);
result [i] = toks [toks.Length - 1];
}
// sometimes we get incomplete tag, so cache it until it gets complete or matched.
// I *know* this is a hack.
if (reset_pattern_head == null || text.EndsWith (">", StringComparison.Ordinal) || !continuous_param_lines && !text.StartsWith (reset_pattern_head, StringComparison.Ordinal))
prev = null;
else
prev = text;
return result;
}
} finally {
reader.Close ();
if (ShouldResetMatchBuffer (text))
prev = null;
else
prev = text;
}
} catch (Exception e) {
Log.Error ("ERROR in {0}.{1}: {2}", type, method, e);
Expand Down
1 change: 1 addition & 0 deletions tools/class-parse/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public static void Main (string[] args)

static Dictionary<string, JavaDocletType> JavaDocletTypeMapping = new Dictionary<string, JavaDocletType> {
{ "droiddoc", JavaDocletType.DroidDoc },
{ "droiddoc2", JavaDocletType.DroidDoc2 },
{ "java6", JavaDocletType.Java6 },
{ "java7", JavaDocletType.Java7 },
{ "java8", JavaDocletType.Java8 },
Expand Down