Skip to content

Commit 59a90c4

Browse files
Redthjonpryor
Redth
authored and
jonpryor
committed
[Xamarin.Android.Tools.Bytecode] Fix api.xml detection (#202)
Context: dotnet/android#978 (comment) Commit 4d6c5a9 had a minor problem: it updated `ClassPath.GetDocletType()` to check to see if the `path` parameter was an `api.xml`-compatible XML file, but *earlier in the method* it was *required* that `path` be a *directory*. These semantics can't be intermixed, and resulted in an error from `Xamarin.Android.Tools.BytecodeTests.ParameterFixupTests.XmlDeclaration_FixedUpFromDocumentation()` during a xamarin-android integration PR: An unexpected exception was thrown : System.UnauthorizedAccessException: Access to the path '/Users/builder/android-toolchain/sdk/docs/reference' is denied. Additionally, further code review demonstrated suggested that the `ClassPath.DocletType` property was fundamentally *broken*: `ClassPath.DocumentationPaths` can contain *multiple* paths, of possibly *different* types, but `ClassPath.DocletType` -- if set -- was used for *all* `ClassPath.DocumentationPaths` values. This doesn't make sense, in any way. Remove `ClassPath.DocletType`, and update `class-parse --docstype` documentation to note that it's obsolete. Move `ClassPath.GetDocletType()` to `AndroidDocScraper.GetDocletType()`, and make it public, so that it can now be unit tested. Fix the semantics of `AndroidDocScraper.GetDocletType(string path)` so that `path` can be a directory or a file, as appropriate, without generating an error in `XmlDeclaration_FixedUpFromDocumentation()`. Cleanup the unit tests.
1 parent 56482df commit 59a90c4

File tree

5 files changed

+95
-77
lines changed

5 files changed

+95
-77
lines changed

‎src/Xamarin.Android.Tools.Bytecode/ClassPath.cs‎

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ public class ClassPath {
2626

2727
public string ApiSource { get; set; }
2828

29-
public JavaDocletType? DocletType { get; set; }
30-
3129
public IEnumerable<string> DocumentationPaths { get; set; }
3230

3331
public string AndroidFrameworkPlatform { get; set; }
@@ -233,38 +231,9 @@ void FixupParametersFromDocs (XElement api)
233231
}
234232
}
235233

236-
JavaDocletType GetDocletType (string path)
237-
{
238-
var kind = JavaDocletType.DroidDoc;
239-
char [] buf = new char [500];
240-
string packagesHtml = Path.Combine (path, "packages.html");
241-
if (File.Exists (packagesHtml) && File.ReadAllText (packagesHtml).Contains ("<body class=\"gc-documentation develop reference api "))
242-
kind = JavaDocletType.DroidDoc2;
243-
using (var reader = File.OpenText (Path.Combine (path, "index.html")))
244-
reader.ReadBlock (buf, 0, buf.Length);
245-
string rawHTML = new string (buf);
246-
if (rawHTML.Contains ("Generated by javadoc (build 1.6"))
247-
kind = JavaDocletType.Java6;
248-
else if (rawHTML.Contains ("Generated by javadoc (version 1.7"))
249-
kind = JavaDocletType.Java7;
250-
else if (rawHTML.Contains ("Generated by javadoc (1.8"))
251-
kind = JavaDocletType.Java8;
252-
253-
// Check to see if it's an api.xml formatted doc
254-
string rawXML = null;
255-
using (var reader = File.OpenText (path)) {
256-
int len = reader.ReadBlock (buf, 0, buf.Length);
257-
rawXML = new string (buf, 0, len);
258-
}
259-
if (rawXML.Contains ("<api>") && rawXML.Contains ("<package"))
260-
kind = JavaDocletType._ApiXml;
261-
262-
return kind;
263-
}
264-
265234
IAndroidDocScraper CreateDocScraper (string src)
266235
{
267-
switch (DocletType ?? GetDocletType (src)) {
236+
switch (AndroidDocScraper.GetDocletType (src)) {
268237
default: return new DroidDoc2Scraper (src);
269238
case JavaDocletType.DroidDoc: return new DroidDocScraper (src);
270239
case JavaDocletType.Java6: return new JavaDocScraper (src);

‎src/Xamarin.Android.Tools.Bytecode/JavaDocumentScraper.cs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,42 @@ public static void LoadXml (String filename)
290290
Log.Error ("Annotations parser error: " + ex);
291291
}
292292
}
293+
294+
public static JavaDocletType GetDocletType (string path)
295+
{
296+
var kind = JavaDocletType.DroidDoc;
297+
char[] buf = new char[500];
298+
299+
string packagesHtml = Path.Combine (path, "packages.html");
300+
if (File.Exists (packagesHtml) && File.ReadAllText (packagesHtml).Contains ("<body class=\"gc-documentation develop reference api "))
301+
kind = JavaDocletType.DroidDoc2;
302+
303+
string indexHtml = Path.Combine (path, "index.html");
304+
if (File.Exists (indexHtml)) {
305+
using (var reader = File.OpenText (indexHtml))
306+
reader.ReadBlock (buf, 0, buf.Length);
307+
string rawHTML = new string (buf);
308+
if (rawHTML.Contains ("Generated by javadoc (build 1.6"))
309+
kind = JavaDocletType.Java6;
310+
else if (rawHTML.Contains ("Generated by javadoc (version 1.7"))
311+
kind = JavaDocletType.Java7;
312+
else if (rawHTML.Contains ("Generated by javadoc (1.8"))
313+
kind = JavaDocletType.Java8;
314+
}
315+
316+
// Check to see if it's an api.xml formatted doc
317+
if (File.Exists (path)) {
318+
string rawXML = null;
319+
using (var reader = File.OpenText (path)) {
320+
int len = reader.ReadBlock (buf, 0, buf.Length);
321+
rawXML = new string (buf, 0, len);
322+
}
323+
if (rawXML.Contains ("<api>") && rawXML.Contains ("<package"))
324+
kind = JavaDocletType._ApiXml;
325+
}
326+
327+
return kind;
328+
}
293329
}
294330

295331
public interface IAndroidDocScraper

‎src/Xamarin.Android.Tools.Bytecode/Tests/ClassFileFixture.cs‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,29 @@ protected static string LoadString (string resource)
2525
return r.ReadToEnd ();
2626
}
2727

28-
protected static void AssertXmlDeclaration (string classResource, string xmlResource, string documentationPath = null, JavaDocletType? javaDocletType = null)
28+
protected static string LoadToTempFile (string resource)
29+
{
30+
var tempFilePath = Path.GetTempFileName ();
31+
32+
using (var w = File.Create (tempFilePath))
33+
using (var s = Assembly.GetExecutingAssembly ().GetManifestResourceStream (resource))
34+
s.CopyTo (w);
35+
36+
return tempFilePath;
37+
}
38+
39+
protected static void AssertXmlDeclaration (string classResource, string xmlResource, string documentationPath = null)
2940
{
3041
var classPathBuilder = new ClassPath () {
3142
ApiSource = "class-parse",
3243
DocumentationPaths = new string[] {
3344
documentationPath,
3445
},
3546
};
36-
if (javaDocletType.HasValue)
37-
classPathBuilder.DocletType = javaDocletType.Value;
3847
classPathBuilder.Add (LoadClassFile (classResource));
3948

4049
var actual = new StringWriter ();
4150
classPathBuilder.ApiSource = "class-parse";
42-
if (javaDocletType.HasValue)
43-
classPathBuilder.DocletType = javaDocletType.Value;
4451
classPathBuilder.SaveXmlDescription (actual);
4552

4653
var expected = LoadString (xmlResource);

‎src/Xamarin.Android.Tools.Bytecode/Tests/ParameterFixupTests.cs‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.IO;
33
using NUnit.Framework;
4+
using Xamarin.Android.Tools.Bytecode;
45

56
namespace Xamarin.Android.Tools.BytecodeTests
67
{
@@ -30,27 +31,17 @@ public void XmlDeclaration_FixedUpFromDocumentation()
3031
}
3132

3233
[Test]
33-
public void XmlDeclaration_FixedUpFromApiXmlDocumentation()
34+
public void XmlDeclaration_FixedUpFromApiXmlDocumentation ()
3435
{
3536
string tempFile = null;
3637

37-
try
38-
{
39-
tempFile = Path.GetTempFileName();
40-
File.WriteAllText(tempFile, LoadString("ParameterFixupApiXmlDocs.xml"));
41-
42-
AssertXmlDeclaration("Collection.class", "ParameterFixupFromDocs.xml", tempFile, Bytecode.JavaDocletType._ApiXml);
43-
}
44-
catch (Exception ex)
45-
{
46-
try
47-
{
48-
if (File.Exists(tempFile))
49-
File.Delete(tempFile);
50-
}
51-
catch { }
38+
try {
39+
tempFile = LoadToTempFile ("ParameterFixupApiXmlDocs.xml");
5240

53-
Assert.Fail("An unexpected exception was thrown : {0}", ex);
41+
AssertXmlDeclaration ("Collection.class", "ParameterFixupFromDocs.xml", tempFile);
42+
} finally {
43+
if (File.Exists (tempFile))
44+
File.Delete (tempFile);
5445
}
5546
}
5647

@@ -63,6 +54,39 @@ public void XmlDeclaration_DoesNotThrowAnExceptionIfDocumentationNotFound ()
6354
Assert.Fail ("An unexpected exception was thrown : {0}", ex);
6455
}
6556
}
57+
58+
[Test]
59+
public void DocletType_ShouldDetectApiXml ()
60+
{
61+
string tempFile = null;
62+
63+
try {
64+
tempFile = LoadToTempFile ("ParameterFixupApiXmlDocs.xml");
65+
66+
Assert.AreEqual (JavaDocletType._ApiXml, AndroidDocScraper.GetDocletType (tempFile));
67+
} finally {
68+
if (File.Exists (tempFile))
69+
File.Delete (tempFile);
70+
}
71+
}
72+
73+
[Test]
74+
public void DocletType_ShouldDetectDroidDocs ()
75+
{
76+
var androidSdkPath = Environment.GetEnvironmentVariable ("ANDROID_SDK_PATH");
77+
if (string.IsNullOrEmpty (androidSdkPath)) {
78+
Assert.Ignore("The `ANDROID_SDK_PATH` environment variable isn't set; " +
79+
"cannot test importing parameter names from HTML. Skipping...");
80+
return;
81+
}
82+
83+
var droidDocsPath = Path.Combine (androidSdkPath, "docs", "reference");
84+
85+
if (!Directory.Exists (droidDocsPath))
86+
Assert.Fail("The Android SDK Documentation path `{0}` was not found.", droidDocsPath);
87+
88+
Assert.AreEqual(JavaDocletType.DroidDoc2, AndroidDocScraper.GetDocletType(droidDocsPath));
89+
}
6690
}
6791
}
6892

‎tools/class-parse/Program.cs‎

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,9 @@ class App {
1515

1616
public static void Main (string[] args)
1717
{
18-
JavaDocletType docsType = 0;
19-
2018
bool dump = false;
2119
bool help = false;
20+
bool docsType = false;
2221
int verbosity = 0;
2322
bool autorename = false;
2423
var outputFile = (string) null;
@@ -40,9 +39,8 @@ public static void Main (string[] args)
4039
"Documentation {PATH} for parameter fixup",
4140
doc => docsPaths.Add (doc) },
4241
{ "docstype=",
43-
"{TYPE} of the docs within --docspath. Values:\n " +
44-
string.Join ("\n ", JavaDocletTypeMapping.Keys.OrderBy (s => s)),
45-
t => docsType = GetJavaDocletType (t) },
42+
"OBSOLETE: Previously used to specify a doc type (now auto detected).",
43+
t => docsType = t != null },
4644
{ "v|verbose:",
4745
"See stack traces on error.",
4846
(int? v) => verbosity = v.HasValue ? v.Value : verbosity + 1 },
@@ -61,6 +59,8 @@ public static void Main (string[] args)
6159
p.WriteOptionDescriptions (Console.Out);
6260
return;
6361
}
62+
if (docsType)
63+
Console.WriteLine ("class-parse: --docstype is obsolete and no longer a valid option.");
6464
var output = outputFile == null
6565
? Console.Out
6666
: (TextWriter) new StreamWriter (outputFile, append: false, encoding: new UTF8Encoding (encoderShouldEmitUTF8Identifier: false));
@@ -71,7 +71,6 @@ public static void Main (string[] args)
7171
ApiSource = "class-parse",
7272
AndroidFrameworkPlatform = platform,
7373
DocumentationPaths = docsPaths.Count == 0 ? null : docsPaths,
74-
DocletType = docsType,
7574
AutoRename = autorename
7675
};
7776
foreach (var file in files) {
@@ -93,23 +92,6 @@ public static void Main (string[] args)
9392
output.Close ();
9493
}
9594

96-
static Dictionary<string, JavaDocletType> JavaDocletTypeMapping = new Dictionary<string, JavaDocletType> {
97-
{ "droiddoc", JavaDocletType.DroidDoc },
98-
{ "droiddoc2", JavaDocletType.DroidDoc2 },
99-
{ "java6", JavaDocletType.Java6 },
100-
{ "java7", JavaDocletType.Java7 },
101-
{ "java8", JavaDocletType.Java8 },
102-
{ "apixml", JavaDocletType._ApiXml },
103-
};
104-
105-
static JavaDocletType GetJavaDocletType (string value)
106-
{
107-
JavaDocletType type;
108-
if (value != null && JavaDocletTypeMapping.TryGetValue (value.ToLowerInvariant (), out type))
109-
return type;
110-
return JavaDocletType.DroidDoc;
111-
}
112-
11395
static void DumpFileToXml (ClassPath jar, string file)
11496
{
11597
using (var s = File.OpenRead (file)) {

0 commit comments

Comments
 (0)