Skip to content

Commit e033ffd

Browse files
jpobstjpobst
committed
Address more feedback.
1 parent 565c125 commit e033ffd

21 files changed

+136
-139
lines changed

‎src/Java.Interop.Tools.JavaTypeSystem/Adapters/JavaXmlApiExporter.cs‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ static void SaveTypeParameters (JavaTypeParameters parameters, XmlWriter writer)
158158
}
159159

160160
static void SaveConstructor (JavaConstructorModel ctor, XmlWriter writer)
161-
=> SaveMember (ctor, writer, "constructor", null, null, null, null, null, ctor.ParentType.FullName, null, null, null, ctor.Parameters, ctor.IsBridge, null, ctor.IsSynthetic, null);
161+
=> SaveMember (ctor, writer, "constructor", null, null, null, null, null, ctor.DeclaringType.FullName, null, null, null, ctor.Parameters, ctor.IsBridge, null, ctor.IsSynthetic, null);
162162

163163
static void SaveField (JavaFieldModel field, XmlWriter writer)
164164
{
@@ -182,9 +182,9 @@ static void SaveField (JavaFieldModel field, XmlWriter writer)
182182

183183
static void SaveMethod (JavaMethodModel method, XmlWriter writer)
184184
{
185-
bool check (JavaMethodModel _) => _.BaseMethod?.ParentType?.Visibility == "public" &&
186-
!method.IsStatic &&
187-
method.Parameters.All (p => p.InstantiatedGenericArgumentName == null);
185+
bool check (JavaMethodModel _) => _.BaseMethod?.DeclaringType?.Visibility == "public" &&
186+
!method.IsStatic &&
187+
method.Parameters.All (p => p.InstantiatedGenericArgumentName == null);
188188

189189
//// skip synthetic methods, that's what jar2xml does.
190190
//// However, jar2xml is based on Java reflection and it generates synthetic methods
@@ -206,29 +206,29 @@ bool check (JavaMethodModel _) => _.BaseMethod?.ParentType?.Visibility == "publi
206206
//// - none of the arguments are type parameters.
207207
//// - finally, it is the synthetic method already checked above.
208208
if (method.BaseMethod != null &&
209-
!method.BaseMethod.IsAbstract &&
210-
method.BaseMethod.Visibility == method.Visibility &&
211-
method.BaseMethod.IsAbstract == method.IsAbstract &&
212-
method.BaseMethod.IsFinal == method.IsFinal &&
213-
!method.IsSynthetic &&
214-
check (method))
209+
!method.BaseMethod.IsAbstract &&
210+
method.BaseMethod.Visibility == method.Visibility &&
211+
method.BaseMethod.IsAbstract == method.IsAbstract &&
212+
method.BaseMethod.IsFinal == method.IsFinal &&
213+
!method.IsSynthetic &&
214+
check (method))
215215
return;
216216

217-
SaveMember (method, writer, "method",
218-
XmlConvert.ToString (method.IsAbstract),
219-
XmlConvert.ToString (method.IsNative),
220-
GetVisibleReturnTypeString (method),
221-
XmlConvert.ToString (method.IsSynchronized),
222-
null,
223-
null,
224-
null,
225-
null,
226-
null,
227-
method.Parameters,
228-
method.IsBridge,
229-
method.ReturnJni,
230-
method.IsSynthetic,
231-
method.ReturnNotNull);
217+
SaveMember (m: method, writer: writer, elementName: "method",
218+
abs: XmlConvert.ToString (method.IsAbstract),
219+
native: XmlConvert.ToString (method.IsNative),
220+
ret: GetVisibleReturnTypeString (method),
221+
sync: XmlConvert.ToString (method.IsSynchronized),
222+
transient: null,
223+
type: null,
224+
typeGeneric: null,
225+
value: null,
226+
volat: null,
227+
parameters: method.Parameters,
228+
extBridge: method.IsBridge,
229+
jniReturn: method.ReturnJni,
230+
extSynthetic: method.IsSynthetic,
231+
notNull: method.ReturnNotNull);
232232
}
233233

234234
static string GetVisibleReturnTypeString (JavaMethodModel method)
@@ -241,7 +241,7 @@ static string GetVisibleReturnTypeString (JavaMethodModel method)
241241

242242
public static string? GetVisibleParamterTypeName (this JavaParameterModel parameter)
243243
{
244-
if (GetVisibleNonSpecialType (parameter.ParentMethod, parameter.TypeModel) is JavaTypeReference jtr)
244+
if (GetVisibleNonSpecialType (parameter.DeclaringMethod, parameter.TypeModel) is JavaTypeReference jtr)
245245
return jtr.ToString ();
246246

247247
return parameter.GenericType;
@@ -252,7 +252,7 @@ static string GetVisibleReturnTypeString (JavaMethodModel method)
252252
if (r == null || r.SpecialName != null || r.ReferencedTypeParameter != null || r.ArrayPart != null)
253253
return null;
254254

255-
var requiredVisibility = method?.Visibility == "public" && method.ParentType?.Visibility == "public" ? "public" : method?.Visibility;
255+
var requiredVisibility = method?.Visibility == "public" && method.DeclaringType?.Visibility == "public" ? "public" : method?.Visibility;
256256

257257
for (var t = r; t != null; t = (t.ReferencedType as JavaClassModel)?.BaseTypeReference) {
258258
if (t.ReferencedType == null)
@@ -273,11 +273,11 @@ static bool IsAcceptableVisibility (string? required, string? actual)
273273
}
274274

275275
static void SaveMember (JavaMemberModel m, XmlWriter writer, string elementName,
276-
string? abs, string? native, string? ret, string? sync,
277-
string? transient, string? type, string? typeGeneric,
278-
string? value, string? volat,
279-
IEnumerable<JavaParameterModel>? parameters,
280-
bool? extBridge, string? jniReturn, bool? extSynthetic, bool? notNull)
276+
string? abs, string? native, string? ret, string? sync,
277+
string? transient, string? type, string? typeGeneric,
278+
string? value, string? volat,
279+
IEnumerable<JavaParameterModel>? parameters,
280+
bool? extBridge, string? jniReturn, bool? extSynthetic, bool? notNull)
281281
{
282282
// If any of the parameters contain reference to non-public type, it cannot be generated.
283283
// TODO

‎src/Java.Interop.Tools.JavaTypeSystem/Adapters/JavaXmlApiImporter.cs‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ static JavaTypeCollection Parse (XDocument doc, JavaTypeCollection? collection =
3737
var root = doc.Root;
3838

3939
if (root is null)
40-
throw new Exception ("Invalid XML file");
40+
throw new ArgumentException ("Invalid XML file doesn't contain a root node");
4141

4242
collection.ApiSource = root.XGetAttributeOrNull ("api-source");
4343
collection.Platform = root.XGetAttributeOrNull ("platform");
@@ -62,7 +62,7 @@ static JavaTypeCollection Parse (XDocument doc, JavaTypeCollection? collection =
6262
foreach (var type in packages.SelectMany (p => p.Types).Where (t => t.NestedName.Contains ('.')).OrderBy (t => t.FullName.Count (c => c == '.')).ToArray ()) {
6363
collection.AddType (type);
6464

65-
// Remove nested types from Package
65+
// Remove nested types from Package. In this model, Package only contains top-level types, which then contain nested types.
6666
type.Package.Types.Remove (type);
6767
}
6868

@@ -83,13 +83,13 @@ public static JavaPackage ParsePackage (XElement package, JavaTypeCollection col
8383
foreach (var elem in package.Elements ()) {
8484
switch (elem.Name.LocalName) {
8585
case "class":
86-
if (package.XGetAttributeAsBool ("obfuscated"))
86+
if (elem.XGetAttributeAsBool ("obfuscated"))
8787
continue;
8888

8989
pkg.Types.Add (ParseClass (pkg, elem));
9090
break;
9191
case "interface":
92-
if (package.XGetAttributeAsBool ("obfuscated"))
92+
if (elem.XGetAttributeAsBool ("obfuscated"))
9393
continue;
9494

9595
pkg.Types.Add (ParseInterface (pkg, elem));
@@ -189,7 +189,7 @@ public static JavaMethodModel ParseMethod (JavaTypeModel type, XElement element)
189189
javaFinal: element.XGetAttributeAsBool ("final"),
190190
javaStatic: element.XGetAttributeAsBool ("static"),
191191
javaReturn: element.XGetAttribute ("return"),
192-
javaParentType: type,
192+
javaDeclaringType: type,
193193
deprecated: element.XGetAttribute ("deprecated"),
194194
jniSignature: element.XGetAttribute ("jni-signature"),
195195
isSynthetic: element.XGetAttributeAsBool ("synthetic"),
@@ -221,18 +221,14 @@ public static JavaConstructorModel ParseConstructor (JavaTypeModel type, XElemen
221221
var method = new JavaConstructorModel (
222222
javaName: element.XGetAttribute ("name"),
223223
javaVisibility: element.XGetAttribute ("visibility"),
224-
javaAbstract: element.XGetAttributeAsBool ("abstract"),
225-
javaFinal: element.XGetAttributeAsBool ("final"),
226224
javaStatic: element.XGetAttributeAsBool ("static"),
227-
javaParentType: type,
225+
javaDeclaringType: type,
228226
deprecated: element.XGetAttribute ("deprecated"),
229227
jniSignature: element.XGetAttribute ("jni-signature"),
230228
isSynthetic: element.XGetAttributeAsBool ("synthetic"),
231229
isBridge: element.XGetAttributeAsBool ("bridge")
232230
);
233231

234-
if (element.Element ("typeParameters") is XElement tp)
235-
ParseTypeParameters (method.TypeParameters, tp);
236232
foreach (var child in element.Elements ("exception"))
237233
method.Exceptions.Add (ParseException (child));
238234

@@ -256,7 +252,7 @@ public static JavaFieldModel ParseField (JavaTypeModel type, XElement element)
256252
typeGeneric: element.XGetAttribute ("type-generic-aware"),
257253
isStatic: element.XGetAttributeAsBool ("static"),
258254
value: element.Attribute ("value")?.Value,
259-
parent: type,
255+
declaringType: type,
260256
isFinal: element.XGetAttributeAsBool ("final"),
261257
deprecated: element.XGetAttribute ("deprecated"),
262258
jniSignature: element.XGetAttribute ("jni-signature"),
@@ -298,7 +294,7 @@ public static JavaExceptionModel ParseException (XElement element)
298294
public static JavaParameterModel ParseParameter (JavaMethodModel method, XElement element)
299295
{
300296
var parameter = new JavaParameterModel (
301-
parent: method,
297+
declaringMethod: method,
302298
javaName: element.XGetAttribute ("name"),
303299
javaType: element.XGetAttribute ("type"),
304300
jniType: element.XGetAttribute ("jni-type"),

‎src/Java.Interop.Tools.JavaTypeSystem/Adapters/ManagedApiImporter.cs‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,29 +53,29 @@ public static JavaTypeCollection Parse (AssemblyDefinition assembly, JavaTypeCol
5353
static bool ShouldImport (TypeDefinition td)
5454
{
5555
// We want to exclude "IBlahInvoker" and "IBlahImplementor" and "BlahConsts" types
56-
if (td.Name.EndsWith ("Invoker")) {
56+
if (td.Name.EndsWith ("Invoker", StringComparison.Ordinal)) {
5757
var n = td.FullName;
58-
n = n.Substring (0, n.Length - 7);
58+
n = n.Substring (0, n.Length - "Invoker".Length);
5959

6060
var types = td.DeclaringType != null ? td.DeclaringType.Resolve ().NestedTypes : td.Module.Types;
6161

6262
if (types.Any (t => t.FullName == n))
6363
return false;
6464
}
6565

66-
if (td.Name.EndsWith ("Implementor")) {
66+
if (td.Name.EndsWith ("Implementor", StringComparison.Ordinal)) {
6767
var n = td.FullName;
68-
n = n.Substring (0, n.Length - 11);
68+
n = n.Substring (0, n.Length - "Implementor".Length);
6969

7070
var types = td.DeclaringType != null ? td.DeclaringType.Resolve ().NestedTypes : td.Module.Types;
7171

7272
if (types.Any (t => t.FullName == n))
7373
return false;
7474
}
7575

76-
if (td.Name.EndsWith ("Consts")) {
76+
if (td.Name.EndsWith ("Consts", StringComparison.Ordinal)) {
7777
var n = td.FullName;
78-
n = n.Substring (0, n.Length - 6);
78+
n = n.Substring (0, n.Length - "Consts".Length);
7979

8080
var types = td.DeclaringType != null ? td.DeclaringType.Resolve ().NestedTypes : td.Module.Types;
8181

@@ -132,7 +132,7 @@ static bool ShouldImport (TypeDefinition td)
132132
if (reg_attr is null)
133133
return null;
134134

135-
var encoded_fullname = ((string) reg_attr.ConstructorArguments [0].Value).Replace ('/', '.');
135+
var encoded_fullname = ((string) reg_attr.ConstructorArguments [0].Value);
136136
var (package, nested_name) = DecodeRegisterJavaFullName (encoded_fullname);
137137

138138
var model = new JavaInterfaceModel (
@@ -153,7 +153,7 @@ static bool ShouldImport (TypeDefinition td)
153153
return model;
154154
}
155155

156-
public static JavaMethodModel? ParseMethod (MethodDefinition method, JavaTypeModel parent)
156+
public static JavaMethodModel? ParseMethod (MethodDefinition method, JavaTypeModel declaringType)
157157
{
158158
if (method.IsPrivate || method.IsAssembly)
159159
return null;
@@ -174,7 +174,7 @@ static bool ShouldImport (TypeDefinition td)
174174
javaFinal: method.IsFinal,
175175
javaStatic: method.IsStatic,
176176
javaReturn: jni_signature.Return.Type,
177-
javaParentType: parent,
177+
javaDeclaringType: declaringType,
178178
deprecated: deprecated,
179179
jniSignature: jni_signature.ToString (),
180180
isSynthetic: false,
@@ -191,7 +191,7 @@ static bool ShouldImport (TypeDefinition td)
191191
return model;
192192
}
193193

194-
static JavaParameterModel ParseParameterModel (JavaMethodModel parent, JniTypeName jniParameter, ParameterDefinition managedParameter)
194+
static JavaParameterModel ParseParameterModel (JavaMethodModel declaringMethod, JniTypeName jniParameter, ParameterDefinition managedParameter)
195195
{
196196
var raw_type = jniParameter.Type;
197197

@@ -208,7 +208,7 @@ static JavaParameterModel ParseParameterModel (JavaMethodModel parent, JniTypeNa
208208
// if (TypeReferenceToJavaType (managedParameter.ParameterType) is string s)
209209
// raw_type = s;
210210

211-
return new JavaParameterModel (parent, managedParameter.Name, raw_type, jniParameter.Jni, false);
211+
return new JavaParameterModel (declaringMethod, managedParameter.Name, raw_type, jniParameter.Jni, false);
212212
}
213213

214214
static void AddReferenceTypeRecursive (JavaTypeModel type, JavaTypeCollection collection)
@@ -351,10 +351,11 @@ static string GetBaseTypeJni (TypeDefinition type)
351351
static string FullNameCorrected (this TypeReference t) => t.FullName.Replace ('/', '.');
352352

353353
public static string Visibility (this MethodDefinition m) =>
354-
m.IsPublic ? "public" : m.IsFamilyOrAssembly ? "protected internal" : m.IsFamily ? "protected" : m.IsAssembly ? "internal" : "private";
354+
m.IsPublic ? "public" : m.IsFamilyOrAssembly ? "protected internal" : m.IsFamily ? "protected" : m.IsAssembly ? "internal" : "private";
355355

356356
static (string package, string nestedName) DecodeRegisterJavaFullName (string value)
357357
{
358+
value = value.Replace ('/', '.');
358359
var idx = value.LastIndexOf ('.');
359360

360361
var package = idx >= 0 ? value.Substring (0, idx) : string.Empty;
@@ -366,9 +367,9 @@ public static string Visibility (this MethodDefinition m) =>
366367
static string FormatJniSignature (string package, string nestedName)
367368
{
368369
if (package.HasValue ())
369-
return $"L{package.Replace ('.', '/')}/{nestedName.Replace ('$', '/')};";
370+
return $"L{package.Replace ('.', '/')}/{nestedName};";
370371

371-
return "L" + nestedName.Replace ('$', '/') + ";";
372+
return "L" + nestedName + ";";
372373
}
373374

374375
static JavaPackage GetOrCreatePackage (JavaTypeCollection collection, string package, string managedName)

‎src/Java.Interop.Tools.JavaTypeSystem/Extensions/CollectionExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace Java.Interop.Tools.JavaTypeSystem
66
{
7-
public static class CollectionExtensions
7+
static class CollectionExtensions
88
{
99
public static bool ContainsAny<T> (this ICollection<T> collection, params T[] values)
1010
{

‎src/Java.Interop.Tools.JavaTypeSystem/Extensions/StringExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
namespace Java.Interop.Tools.JavaTypeSystem
55
{
6-
public static class StringExtensions
6+
static class StringExtensions
77
{
88
/// <summary>
99
/// Shortcut for !string.IsNullOrWhiteSpace (s)

‎src/Java.Interop.Tools.JavaTypeSystem/Extensions/XmlExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace Java.Interop.Tools.JavaTypeSystem
66
{
7-
public static class XmlExtensions
7+
static class XmlExtensions
88
{
99
public static string XGetAttribute (this XElement element, string name)
1010
{

‎src/Java.Interop.Tools.JavaTypeSystem/JavaModels/IJavaResolvable.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ namespace Java.Interop.Tools.JavaTypeSystem.Models
55
{
66
public interface IJavaResolvable
77
{
8-
void Resolve (JavaTypeCollection types, List<JavaUnresolvableModel> unresolvables);
8+
void Resolve (JavaTypeCollection types, ICollection<JavaUnresolvableModel> unresolvables);
99
}
1010
}

‎src/Java.Interop.Tools.JavaTypeSystem/JavaModels/JavaBuiltInType.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ public class JavaBuiltInType : JavaTypeModel
88
{
99
public JavaBuiltInType (string name) : base (new JavaPackage ("", "", null), name, "public", false, true, "not deprecated", false, "") { }
1010

11-
public override void Resolve (JavaTypeCollection types, List<JavaUnresolvableModel> unresolvables)
11+
public override void Resolve (JavaTypeCollection types, ICollection<JavaUnresolvableModel> unresolvables)
1212
{
1313
throw new NotImplementedException ();
1414
}

‎src/Java.Interop.Tools.JavaTypeSystem/JavaModels/JavaClassModel.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public JavaClassModel (JavaPackage javaPackage, string javaNestedName, string ja
2323
BaseTypeJni = baseTypeJni;
2424
}
2525

26-
public override void Resolve (JavaTypeCollection types, List<JavaUnresolvableModel> unresolvables)
26+
public override void Resolve (JavaTypeCollection types, ICollection<JavaUnresolvableModel> unresolvables)
2727
{
2828
var type_parameters = GetApplicableTypeParameters ().ToArray ();
2929

@@ -103,7 +103,7 @@ void PrepareGenericInheritanceMapping ()
103103
// NRT - This is checked above but compiler can't figure it out
104104
if (BaseTypeReference.ReferencedType!.TypeParameters.Count != BaseTypeReference.TypeParameters.Count)
105105
throw new Exception (string.Format ("On {0}.{1}, referenced generic arguments count do not match the base type parameters definition",
106-
ParentType?.Name, Name));
106+
DeclaringType?.Name, Name));
107107

108108
generic_inheritance_mapping = new Dictionary<JavaTypeReference, JavaTypeReference> ();
109109
foreach (var kvp in BaseTypeReference.ReferencedType.TypeParameters.Zip (

‎src/Java.Interop.Tools.JavaTypeSystem/JavaModels/JavaConstructorModel.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ namespace Java.Interop.Tools.JavaTypeSystem.Models
44
{
55
public class JavaConstructorModel : JavaMethodModel
66
{
7-
public JavaConstructorModel (string javaName, string javaVisibility, bool javaAbstract, bool javaFinal, bool javaStatic, JavaTypeModel javaParentType, string deprecated, string jniSignature, bool isSynthetic, bool isBridge)
8-
: base (javaName, javaVisibility, javaAbstract, javaFinal, javaStatic, "void", javaParentType, deprecated, jniSignature, isSynthetic, isBridge, string.Empty, false, false, false)
7+
public JavaConstructorModel (string javaName, string javaVisibility, bool javaStatic, JavaTypeModel javaDeclaringType, string deprecated, string jniSignature, bool isSynthetic, bool isBridge)
8+
: base (javaName, javaVisibility, false, false, javaStatic, "void", javaDeclaringType, deprecated, jniSignature, isSynthetic, isBridge, string.Empty, false, false, false)
99
{
1010
}
1111

12-
public override string ToString () => $"Constructor: {ParentType.FullName}.{Name}";
12+
public override string ToString () => $"Constructor: {DeclaringType.FullName}.{Name}";
1313
}
1414
}

0 commit comments

Comments
 (0)