Skip to content

[api-xml-adjuster] method overrides cannot expose non-public types. #94

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.

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 11 additions & 2 deletions src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,12 @@ public override string ToString ()

public partial class JavaParameter
{
public JavaParameter (JavaMethodBase parent)
{
Parent = parent;
}

public JavaMethodBase Parent { get; private set; }
public string Name { get; set; }
public string Type { get; set; }

Expand Down Expand Up @@ -309,7 +315,7 @@ public JavaTypeParameter (JavaTypeParameters parent)

public override string ToString ()
{
return Name + (GenericConstraints == null ? null : GenericConstraints.ToString ());
return Name;
Copy link
Member

Choose a reason for hiding this comment

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

Why the removal of the constraints from JavaTypeParameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically we have right to choose what to output in this ToString() overrides.

There are places that should output generic constraints and that should NOT. When writing generic type parameter definitions they should be there. When writing USE of generic type parameters they should NOT. And this generic constraints part were always written.

An exceptional case that it SHOULD be written for USE of generics is that the name is "?" which is handled specially.

}
}

Expand All @@ -326,7 +332,10 @@ public JavaGenericConstraints ()

public override string ToString ()
{
return " " + (BoundsType ?? "extends") + " " + string.Join (" & ", GenericConstraints);
string csts = string.Join (" & ", GenericConstraints);
if (csts == "java.lang.Object")
return string.Empty;
return " " + (BoundsType ?? "extends") + " " + csts;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using System;
using System.Linq;

namespace Xamarin.Android.Tools.ApiXmlAdjuster
{
public static class JavaApiFixVisibilityExtensions
{
public static string GetVisibleTypeName (this JavaParameter parameter)
{
var r = GetVisibleNonSpecialType (parameter);
return r != null ? r.ToString () : parameter.Type;
}

public static string GetVisibleReturnTypeName (this JavaMethod method)
{
var r = GetVisibleNonSpecialReturnType (method);
return r != null ? r.ToString () : method.Return;
}

public static JavaTypeReference GetVisibleNonSpecialType (this JavaParameter parameter)
{
return GetVisibleNonSpecialType (parameter.Parent, parameter.ResolvedType);
}

public static JavaTypeReference GetVisibleNonSpecialReturnType (this JavaMethod method)
{
return GetVisibleNonSpecialType (method, method.ResolvedReturnType);
}

static JavaTypeReference GetVisibleNonSpecialType (this JavaMethodBase method, JavaTypeReference r)
{
if (r == null || r.SpecialName != null || r.ReferencedTypeParameter != null || r.ArrayPart != null)
return null;
var requiredVisibility = method.Visibility == "public" && method.Parent.Visibility == "public" ? "public" : method.Visibility;
for (var t = r; t != null; t = (t.ReferencedType as JavaClass)?.ResolvedExtends) {
if (t.ReferencedType == null)
break;
if (IsAcceptableVisibility (required: requiredVisibility, actual: t.ReferencedType.Visibility))
return t;
}
return null;
}

static bool IsAcceptableVisibility (string required, string actual)
{
if (required == "public")
return actual == "public";
else
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ static JavaTypeReference JavaTypeNameToReference (this JavaApi api, JavaTypeName
var tp = contextTypeParameters.Where (tps => tps != null).SelectMany (tps => tps.TypeParameters).FirstOrDefault (_ => _.Name == tn.FullNameNonGeneric);
if (tp != null)
return new JavaTypeReference (tp, tn.ArrayPart);
if (tn.FullNameNonGeneric == JavaTypeReference.GenericWildcard.SpecialName)
return new JavaTypeReference (tn.BoundsType, tn.GenericConstraints?.Select (gc => JavaTypeNameToReference (api, gc, contextTypeParameters)), tn.ArrayPart);
var primitive = JavaTypeReference.GetSpecialType (tn.FullNameNonGeneric);
if (primitive != null)
return tn.ArrayPart == null ? primitive : new JavaTypeReference (primitive, tn.ArrayPart);
return tn.ArrayPart == null && tn.GenericConstraints == null ? primitive : new JavaTypeReference (primitive, tn.ArrayPart, tn.BoundsType, tn.GenericConstraints?.Select (gc => JavaTypeNameToReference (api, gc, contextTypeParameters)));
var type = api.FindNonGenericType (tn.FullNameNonGeneric);
return new JavaTypeReference (type,
tn.GenericArguments != null ? tn.GenericArguments.Select (_ => api.JavaTypeNameToReference (_, contextTypeParameters)).ToArray () : null,
Expand Down Expand Up @@ -111,7 +113,7 @@ static void ResolveMethodBase (this JavaMethodBase m, JavaTypeParameters methodT
public static void Resolve (this JavaMethod m)
{
if (m.TypeParameters != null)
m.TypeParameters.Resolve (m.GetApi (), m.Parent.TypeParameters);
m.TypeParameters.Resolve (m.GetApi (), m.TypeParameters);
m.ResolveMethodBase (m.TypeParameters);
m.ResolvedReturnType = m.GetApi ().Parse (m.Return, m.Parent.TypeParameters, m.TypeParameters);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ static void Save (this JavaMethod method, XmlWriter writer)
SaveCommon (method, writer, "method",
XmlConvert.ToString (method.Abstract),
XmlConvert.ToString (method.Native),
method.Return,
method.GetVisibleReturnTypeName (),
XmlConvert.ToString (method.Synchronized),
null,
null,
Expand Down Expand Up @@ -232,8 +232,8 @@ static void SaveCommon (this JavaMember m, XmlWriter writer, string elementName,
foreach (var p in parameters) {
writer.WriteStartElement ("parameter");
writer.WriteAttributeString ("name", p.Name);
writer.WriteAttributeString ("type", p.Type);
writer.WriteString ("\n ");
writer.WriteAttributeString ("type", p.GetVisibleTypeName ());
writer.WriteString ("\n ");
writer.WriteFullEndElement ();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ static void LoadMethodBase (this JavaMethodBase methodBase, string elementName,
tp.Load (reader);
method.TypeParameters = tp;
} else if (reader.LocalName == "parameter") {
var p = new JavaParameter ();
var p = new JavaParameter (methodBase);
p.Load (reader);
methodBase.Parameters.Add (p);
} else if (reader.LocalName == "exception") {
Expand Down
17 changes: 10 additions & 7 deletions src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaTypeName.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ namespace Xamarin.Android.Tools.ApiXmlAdjuster
//
// A generic type parameter, or a primitive type, is also represented by this too.
//
internal class JavaTypeName
public class JavaTypeName
{
const string extendsLabel = " extends ";
const string superLabel = " super ";

static readonly string [] genericConstraintsLabels = { extendsLabel, superLabel };

private JavaTypeName ()
JavaTypeName ()
{
}

Expand All @@ -40,6 +40,7 @@ public static JavaTypeName Parse (string dottedFullName)
int gccidx = gcidx < 0 ? -1 : dottedFullName.IndexOf (',', 0, gcidx);
if (gcidx > 0 && gcgidx < 0 && gccidx < 0) {
string args = dottedFullName.Substring (gcidx + label.Length).Trim ();
ret.BoundsType = label;
ret.GenericConstraints = ParseCommaSeparatedTypeNames (args).Select (s => Parse (s)).ToArray ();
dottedFullName = dottedFullName.Substring (0, gcidx).Trim ();
}
Expand Down Expand Up @@ -69,7 +70,7 @@ public static JavaTypeName Parse (string dottedFullName)

return ret;
}

static IEnumerable<string> ParseCommaSeparatedTypeNames (string args)
{
int comma = args.IndexOf (',');
Expand All @@ -79,7 +80,7 @@ static IEnumerable<string> ParseCommaSeparatedTypeNames (string args)
int open = args.IndexOf ('<', 0, comma);
if (open > 0) {
int openCount = 1;
int i = open;
int i = open + 1;
while (i < args.Length) {
if (args [i] == '<')
openCount++;
Expand All @@ -90,10 +91,11 @@ static IEnumerable<string> ParseCommaSeparatedTypeNames (string args)
break;
}
yield return args.Substring (0, i);
if (i > args.Length) {
if (i < args.Length) {
comma = args.IndexOf (',', i);
foreach (var s in ParseCommaSeparatedTypeNames (args.Substring (comma + 1)))
yield return s;
if (comma > 0)
foreach (var s in ParseCommaSeparatedTypeNames (args.Substring (comma + 1)))
yield return s;
}
} else {
yield return args.Substring (0, comma);
Expand All @@ -104,6 +106,7 @@ static IEnumerable<string> ParseCommaSeparatedTypeNames (string args)
}

public string FullNameNonGeneric { get; set; }
public string BoundsType { get; set; } // " extends " / " super "
public IList<JavaTypeName> GenericConstraints { get; private set; }
public IList<JavaTypeName> GenericArguments { get; private set; }
public string ArrayPart { get; set; }
Expand Down
27 changes: 22 additions & 5 deletions src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaTypeReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,28 @@ static JavaTypeReference ()
GenericWildcard = new JavaTypeReference ("?");
}

public JavaTypeReference (string specialName)
JavaTypeReference (string specialName)
{
SpecialName = specialName;
}

public JavaTypeReference (JavaTypeReference referencedType, string arrayPart)
public JavaTypeReference (string constraintLabel, IEnumerable<JavaTypeReference> wildcardConstraints, string arrayPart)
{
SpecialName = GenericWildcard.SpecialName;
ArrayPart = arrayPart;
WildcardBoundsType = constraintLabel;
WildcardConstraints = wildcardConstraints != null && wildcardConstraints.Any () ? wildcardConstraints.ToList () : null;
}

public JavaTypeReference (JavaTypeReference referencedType, string arrayPart, string wildcardBoundsType, IEnumerable<JavaTypeReference> wildcardConstraints)
{
if (referencedType == null)
throw new ArgumentNullException ("referencedType");
SpecialName = referencedType.SpecialName;
WildcardBoundsType = wildcardBoundsType;
WildcardConstraints = wildcardConstraints?.ToList ();
ReferencedType = referencedType.ReferencedType;
ReferencedTypeParameter = referencedType.ReferencedTypeParameter;
TypeParameters = referencedType.TypeParameters;
ArrayPart = arrayPart;
}
Expand All @@ -81,17 +92,23 @@ public JavaTypeReference (JavaType referencedType, IList<JavaTypeReference> type
}

public string SpecialName { get; private set; }

public string WildcardBoundsType { get; private set; }
public IList<JavaTypeReference> WildcardConstraints { get; private set; }

public JavaType ReferencedType { get; private set; }
public JavaTypeParameter ReferencedTypeParameter { get; private set; }
public IList<JavaTypeReference> TypeParameters { get; private set; }
public string ArrayPart { get; private set; }

public override string ToString ()
{
if (SpecialName != null)
return SpecialName;
if (SpecialName == GenericWildcard.SpecialName && WildcardConstraints != null)
return SpecialName + WildcardBoundsType + string.Join (" & ", WildcardConstraints);
else if (SpecialName != null)
return SpecialName + ArrayPart;
else if (ReferencedTypeParameter != null)
return ReferencedTypeParameter.Name;
return ReferencedTypeParameter.ToString () + ArrayPart;
else
return string.Format ("{0}{1}{2}{3}{4}",
ReferencedType.Parent.Name,
Expand Down
18 changes: 18 additions & 0 deletions src/Xamarin.Android.Tools.ApiXmlAdjuster/Tests/JavaApiTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,24 @@ public void TestToString ()
var kls = pkg.Types.First (t => t.FullName == "android.database.ContentObservable");
Assert.AreEqual ("[Class] android.database.ContentObservable", kls.ToString ());
}

[Test]
public void ParseName ()
{
var tn = JavaTypeName.Parse ("java.util.Function<java.util.Map.Entry<K, V>, ? extends U>");
Assert.AreEqual ("java.util.Function", tn.FullNameNonGeneric, "top failed to parse name");
Assert.AreEqual (2, tn.GenericArguments.Count, "top incorrect number of parsed generic arguments");
var ga1 = tn.GenericArguments [0];
Assert.AreEqual ("java.util.Map.Entry", ga1.FullNameNonGeneric, "genarg#0 name mismatch");
Assert.AreEqual (2, ga1.GenericArguments.Count, "genarg#0 incorrect number of parsed generic arguments");
Assert.AreEqual ("K", ga1.GenericArguments [0].FullNameNonGeneric, "genarg#0.1 name mismatch");
Assert.AreEqual ("V", ga1.GenericArguments [1].FullNameNonGeneric, "genarg#0.2 name mismatch");
var ga2 = tn.GenericArguments [1];
Assert.AreEqual ("?", ga2.FullNameNonGeneric, "genarg#1 name mismatch");
Assert.AreEqual (" extends ", ga2.BoundsType, "genarg#1 incorrect bounds type");
Copy link
Member

@jonpryor jonpryor Oct 13, 2016

Choose a reason for hiding this comment

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

This is a bit "odd", in that it means that ga2.BoundsType contains leading and trailing spaces, e.g. this is " extends ", not "extends".

Should the BoundsType property not contain spaces, while the spaces are included in e.g. ga2.ToString()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course I thought about that, but I didn't find very reason to perform extra string stripping and cause more memory consumption.

Assert.AreEqual (1, ga2.GenericConstraints.Count, "genarg#1 incorrect number of parsed generic constraints");
Assert.AreEqual ("U", ga2.GenericConstraints [0].FullNameNonGeneric, "genarg#1.1 constraint name mismatch");
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class JavaApiTestHelper
"src",
"Xamarin.Android.Tools.ApiXmlAdjuster",
"Tests",
"api-10.xml.in");
"api-24.xml.in");

public static JavaApi GetLoadedApi ()
{
Expand Down
Loading