Skip to content

Commit 61c70ec

Browse files
radekdoulikjonpryor
authored andcommitted
[Java.Interop] Gendarme-related cleaning (#205)
* `Gendarme.Rules.Performance.AvoidRepetitiveCastsRule`: `M:Java.Interop.JniValueMarshaler`1<T> Java.Interop.JniRuntime/JniValueManager::GetValueMarshaler()` Refactored to cast only once and made shorter. * `Gendarme.Rules.Performance.AvoidUnsealedUninheritedInternalTypeRule`: Sealed the classes * `Gendarme.Rules.Design.AvoidVisibleFieldsRule`: Mentioned ``T:Java.Interop.JavaArray`1``. This field is only used from `internal` methods, thus changed the field itself to `internal`. * `Gendarme.Rules.Correctness.CheckParametersNullityInVisibleMethodsRule` `M:System.Linq.Expressions.Expression Java.Interop.JniValueMarshaler::ReturnObjectReferenceToJni(Java.Interop.Expressions.JniValueMarshalerContext,System.String,System.Linq.Expressions.Expression)` Check for `context` being `null`. After these we are down to 122 remaining Gendarme defects.
1 parent 00ad8d6 commit 61c70ec

File tree

13 files changed

+45
-37
lines changed

13 files changed

+45
-37
lines changed

gendarme-ignore.txt

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ M: System.Linq.Expressions.Expression`1<System.Func`4<System.Reflection.Construc
198198

199199
R: Gendarme.Rules.Exceptions.DoNotSwallowErrorsCatchingNonSpecificExceptionsRule
200200
M: System.Reflection.Assembly Java.Interop.JniRuntime/JniTypeManager::TryLoadAssembly(System.Reflection.AssemblyName)
201+
# The exception is transitioned to Jni
202+
M:System.Void Java.Interop.ManagedPeer::Construct(System.IntPtr,System.IntPtr,System.IntPtr,System.IntPtr,System.IntPtr,System.IntPtr)
201203

202204
R: Gendarme.Rules.Exceptions.ExceptionShouldBeVisibleRule
203205
T: Java.Interop.JniLocationException
@@ -525,7 +527,6 @@ R: Gendarme.Rules.Performance.AvoidRepetitiveCastsRule
525527
# The "repetitive" cast is in a Debug.Assert() call, which should be "stand-alone"
526528
M: Java.Interop.JniObjectReference Java.Interop.JniEnvironment/Strings::NewString(System.Object)
527529

528-
529530
R: Gendarme.Rules.Performance.AvoidUncalledPrivateCodeRule
530531
# These are `public` for "completeness" reasons.
531532
M: Java.Interop.JniObjectReferenceType Java.Interop.JniEnvironment/References::GetObjectRefType(Java.Interop.JniObjectReference)
@@ -554,7 +555,6 @@ R: Gendarme.Rules.Performance.AvoidUnusedParametersRule
554555
# This method is virtual; *overrides* need the parameter, not the default implementation.
555556
M: System.Collections.Generic.IEnumerable`1<System.Type> Java.Interop.JniRuntime/JniTypeManager::CreateGetTypesForSimpleReferenceEnumerator(System.String)
556557

557-
558558
R: Gendarme.Rules.Performance.UseTypeEmptyTypesRule
559559
# The PCL profile we're using doen't *have* Type.EmptyTypes!
560560
M: System.Void Java.Interop.JniRuntime/JniTypeManager::.cctor()
@@ -568,3 +568,12 @@ M: System.Void Java.Interop.JniEnvironment/Exceptions::Throw(System.Exception)
568568
R: Gendarme.Rules.Correctness.DisposableFieldsShouldBeDisposedRule
569569
# We call Dispose on marshalMemberBuilder field in the JniRuntime::Dispose method. Looks like gendarme bug, it doesn't handle well the `?.` operator.
570570
T: Java.Interop.JniRuntime
571+
572+
R: Gendarme.Rules.Performance.AvoidUnusedPrivateFieldsRule
573+
# The weak_handle and refs_added fields are used from C code, in java-interop-gc-bridge-mono.c, and thus invisible to Gendarme
574+
T: Java.Interop.JavaException
575+
T: Java.Interop.JavaObject
576+
577+
R: Gendarme.Rules.Design.Generic.DoNotExposeGenericListsRule
578+
# We don't care here as we don't hold the list and create it on request. So to avoid performace penalty, we keep the list as return type
579+
M: System.Collections.Generic.List`1<Java.Interop.JniSurfacedPeerInfo> Java.Interop.JniRuntime/JniValueManager::GetSurfacedPeers()

src/Java.Interop/Java.Interop/JavaArray.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ internal delegate TArray ArrayCreator<TArray> (ref JniObjectReference reference,
1414

1515
// Value was created via CreateMarshalCollection, and thus can
1616
// be disposed of with impunity when no longer needed.
17-
protected bool forMarshalCollection;
17+
internal bool forMarshalCollection;
1818

1919
internal JavaArray (ref JniObjectReference handle, JniObjectReferenceOptions transfer)
2020
: base (ref handle, transfer)

src/Java.Interop/Java.Interop/JavaObjectArray.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ internal override bool TargetTypeIsCurrentType (Type targetType)
134134
targetType == typeof (JavaObjectArray<T>);
135135
}
136136

137-
internal class ValueMarshaler : JniValueMarshaler<IList<T>> {
137+
internal sealed class ValueMarshaler : JniValueMarshaler<IList<T>> {
138138

139139
public override IList<T> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type targetType)
140140
{

src/Java.Interop/Java.Interop/JavaProxyThrowable.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
namespace Java.Interop {
44

55
[JniTypeSignature (JniTypeName)]
6-
class JavaProxyThrowable : JavaException
6+
sealed class JavaProxyThrowable : JavaException
77
{
88
new internal const string JniTypeName = "com/xamarin/java_interop/internal/JavaProxyThrowable";
99

src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ internal static Boolean GetValueFromJni (ref JniObjectReference self, JniObjectR
8686
}
8787
}
8888

89-
class JniBooleanValueMarshaler : JniValueMarshaler<Boolean> {
89+
sealed class JniBooleanValueMarshaler : JniValueMarshaler<Boolean> {
9090

9191
internal static readonly JniBooleanValueMarshaler Instance = new JniBooleanValueMarshaler ();
9292

@@ -154,7 +154,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
154154
}
155155
}
156156

157-
class JniNullableBooleanValueMarshaler : JniValueMarshaler<Boolean?> {
157+
sealed class JniNullableBooleanValueMarshaler : JniValueMarshaler<Boolean?> {
158158

159159
internal static readonly JniNullableBooleanValueMarshaler Instance = new JniNullableBooleanValueMarshaler ();
160160

@@ -213,7 +213,7 @@ internal static SByte GetValueFromJni (ref JniObjectReference self, JniObjectRef
213213
}
214214
}
215215

216-
class JniSByteValueMarshaler : JniValueMarshaler<SByte> {
216+
sealed class JniSByteValueMarshaler : JniValueMarshaler<SByte> {
217217

218218
internal static readonly JniSByteValueMarshaler Instance = new JniSByteValueMarshaler ();
219219

@@ -281,7 +281,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
281281
}
282282
}
283283

284-
class JniNullableSByteValueMarshaler : JniValueMarshaler<SByte?> {
284+
sealed class JniNullableSByteValueMarshaler : JniValueMarshaler<SByte?> {
285285

286286
internal static readonly JniNullableSByteValueMarshaler Instance = new JniNullableSByteValueMarshaler ();
287287

@@ -340,7 +340,7 @@ internal static Char GetValueFromJni (ref JniObjectReference self, JniObjectRefe
340340
}
341341
}
342342

343-
class JniCharValueMarshaler : JniValueMarshaler<Char> {
343+
sealed class JniCharValueMarshaler : JniValueMarshaler<Char> {
344344

345345
internal static readonly JniCharValueMarshaler Instance = new JniCharValueMarshaler ();
346346

@@ -408,7 +408,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
408408
}
409409
}
410410

411-
class JniNullableCharValueMarshaler : JniValueMarshaler<Char?> {
411+
sealed class JniNullableCharValueMarshaler : JniValueMarshaler<Char?> {
412412

413413
internal static readonly JniNullableCharValueMarshaler Instance = new JniNullableCharValueMarshaler ();
414414

@@ -467,7 +467,7 @@ internal static Int16 GetValueFromJni (ref JniObjectReference self, JniObjectRef
467467
}
468468
}
469469

470-
class JniInt16ValueMarshaler : JniValueMarshaler<Int16> {
470+
sealed class JniInt16ValueMarshaler : JniValueMarshaler<Int16> {
471471

472472
internal static readonly JniInt16ValueMarshaler Instance = new JniInt16ValueMarshaler ();
473473

@@ -535,7 +535,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
535535
}
536536
}
537537

538-
class JniNullableInt16ValueMarshaler : JniValueMarshaler<Int16?> {
538+
sealed class JniNullableInt16ValueMarshaler : JniValueMarshaler<Int16?> {
539539

540540
internal static readonly JniNullableInt16ValueMarshaler Instance = new JniNullableInt16ValueMarshaler ();
541541

@@ -594,7 +594,7 @@ internal static Int32 GetValueFromJni (ref JniObjectReference self, JniObjectRef
594594
}
595595
}
596596

597-
class JniInt32ValueMarshaler : JniValueMarshaler<Int32> {
597+
sealed class JniInt32ValueMarshaler : JniValueMarshaler<Int32> {
598598

599599
internal static readonly JniInt32ValueMarshaler Instance = new JniInt32ValueMarshaler ();
600600

@@ -662,7 +662,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
662662
}
663663
}
664664

665-
class JniNullableInt32ValueMarshaler : JniValueMarshaler<Int32?> {
665+
sealed class JniNullableInt32ValueMarshaler : JniValueMarshaler<Int32?> {
666666

667667
internal static readonly JniNullableInt32ValueMarshaler Instance = new JniNullableInt32ValueMarshaler ();
668668

@@ -721,7 +721,7 @@ internal static Int64 GetValueFromJni (ref JniObjectReference self, JniObjectRef
721721
}
722722
}
723723

724-
class JniInt64ValueMarshaler : JniValueMarshaler<Int64> {
724+
sealed class JniInt64ValueMarshaler : JniValueMarshaler<Int64> {
725725

726726
internal static readonly JniInt64ValueMarshaler Instance = new JniInt64ValueMarshaler ();
727727

@@ -789,7 +789,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
789789
}
790790
}
791791

792-
class JniNullableInt64ValueMarshaler : JniValueMarshaler<Int64?> {
792+
sealed class JniNullableInt64ValueMarshaler : JniValueMarshaler<Int64?> {
793793

794794
internal static readonly JniNullableInt64ValueMarshaler Instance = new JniNullableInt64ValueMarshaler ();
795795

@@ -848,7 +848,7 @@ internal static Single GetValueFromJni (ref JniObjectReference self, JniObjectRe
848848
}
849849
}
850850

851-
class JniSingleValueMarshaler : JniValueMarshaler<Single> {
851+
sealed class JniSingleValueMarshaler : JniValueMarshaler<Single> {
852852

853853
internal static readonly JniSingleValueMarshaler Instance = new JniSingleValueMarshaler ();
854854

@@ -916,7 +916,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
916916
}
917917
}
918918

919-
class JniNullableSingleValueMarshaler : JniValueMarshaler<Single?> {
919+
sealed class JniNullableSingleValueMarshaler : JniValueMarshaler<Single?> {
920920

921921
internal static readonly JniNullableSingleValueMarshaler Instance = new JniNullableSingleValueMarshaler ();
922922

@@ -975,7 +975,7 @@ internal static Double GetValueFromJni (ref JniObjectReference self, JniObjectRe
975975
}
976976
}
977977

978-
class JniDoubleValueMarshaler : JniValueMarshaler<Double> {
978+
sealed class JniDoubleValueMarshaler : JniValueMarshaler<Double> {
979979

980980
internal static readonly JniDoubleValueMarshaler Instance = new JniDoubleValueMarshaler ();
981981

@@ -1043,7 +1043,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
10431043
}
10441044
}
10451045

1046-
class JniNullableDoubleValueMarshaler : JniValueMarshaler<Double?> {
1046+
sealed class JniNullableDoubleValueMarshaler : JniValueMarshaler<Double?> {
10471047

10481048
internal static readonly JniNullableDoubleValueMarshaler Instance = new JniNullableDoubleValueMarshaler ();
10491049

src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ namespace Java.Interop {
9090
}
9191
}
9292

93-
class Jni<#= type.Type #>ValueMarshaler : JniValueMarshaler<<#= type.Type #>> {
93+
sealed class Jni<#= type.Type #>ValueMarshaler : JniValueMarshaler<<#= type.Type #>> {
9494

9595
internal static readonly Jni<#= type.Type #>ValueMarshaler Instance = new Jni<#= type.Type #>ValueMarshaler ();
9696

@@ -158,7 +158,7 @@ namespace Java.Interop {
158158
}
159159
}
160160

161-
class JniNullable<#= type.Type #>ValueMarshaler : JniValueMarshaler<<#= type.Type #>?> {
161+
sealed class JniNullable<#= type.Type #>ValueMarshaler : JniValueMarshaler<<#= type.Type #>?> {
162162

163163
internal static readonly JniNullable<#= type.Type #>ValueMarshaler Instance = new JniNullable<#= type.Type #>ValueMarshaler ();
164164

src/Java.Interop/Java.Interop/JniRuntime.JniMarshalMemberBuilder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public bool IsDirectMethod (ParameterInfo[] methodParameters)
143143
}
144144
}
145145

146-
class IntPtrValueMarshaler : JniValueMarshaler<IntPtr> {
146+
sealed class IntPtrValueMarshaler : JniValueMarshaler<IntPtr> {
147147
internal static IntPtrValueMarshaler Instance = new IntPtrValueMarshaler ();
148148

149149
public override Expression CreateParameterFromManagedExpression (Java.Interop.Expressions.JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize)

src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -441,9 +441,8 @@ public JniValueMarshaler<T> GetValueMarshaler<T>()
441441
return r;
442442
lock (Marshalers) {
443443
JniValueMarshaler d;
444-
if (Marshalers.TryGetValue (typeof (T), out d))
445-
return (JniValueMarshaler<T>) d;
446-
Marshalers.Add (typeof (T), d = new DelegatingValueMarshaler<T> (m));
444+
if (!Marshalers.TryGetValue (typeof (T), out d))
445+
Marshalers.Add (typeof (T), d = new DelegatingValueMarshaler<T> (m));
447446
return (JniValueMarshaler<T>) d;
448447
}
449448
}
@@ -503,7 +502,7 @@ protected virtual JniValueMarshaler GetValueMarshalerCore (Type type)
503502
}
504503
}
505504

506-
class VoidValueMarshaler : JniValueMarshaler {
505+
sealed class VoidValueMarshaler : JniValueMarshaler {
507506

508507
internal static VoidValueMarshaler Instance = new VoidValueMarshaler ();
509508

@@ -527,7 +526,7 @@ public override void DestroyArgumentState (object value, ref JniValueMarshalerSt
527526
}
528527
}
529528

530-
class JavaPeerableValueMarshaler : JniValueMarshaler<IJavaPeerable> {
529+
sealed class JavaPeerableValueMarshaler : JniValueMarshaler<IJavaPeerable> {
531530

532531
internal static JavaPeerableValueMarshaler Instance = new JavaPeerableValueMarshaler ();
533532

@@ -594,7 +593,7 @@ public override Expression CreateParameterToManagedExpression (JniValueMarshaler
594593
}
595594
}
596595

597-
class DelegatingValueMarshaler<T> : JniValueMarshaler<T> {
596+
sealed class DelegatingValueMarshaler<T> : JniValueMarshaler<T> {
598597

599598
JniValueMarshaler ValueMarshaler;
600599

@@ -634,7 +633,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
634633
}
635634
}
636635

637-
class ProxyValueMarshaler : JniValueMarshaler<object> {
636+
sealed class ProxyValueMarshaler : JniValueMarshaler<object> {
638637

639638
internal static ProxyValueMarshaler Instance = new ProxyValueMarshaler ();
640639

src/Java.Interop/Java.Interop/JniStringValueMarshaler.cs

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

88
namespace Java.Interop {
99

10-
class JniStringValueMarshaler : JniValueMarshaler<string> {
10+
sealed class JniStringValueMarshaler : JniValueMarshaler<string> {
1111

1212
internal static readonly JniStringValueMarshaler Instance = new JniStringValueMarshaler ();
1313

src/Java.Interop/Java.Interop/JniValueMarshaler.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
namespace Java.Interop.Expressions {
1010

11-
class VariableCollection : KeyedCollection<string, ParameterExpression> {
11+
sealed class VariableCollection : KeyedCollection<string, ParameterExpression> {
1212

1313
protected override string GetKeyForItem (ParameterExpression item)
1414
{
@@ -162,6 +162,8 @@ protected Expression ReturnObjectReferenceToJni (JniValueMarshalerContext contex
162162
{
163163
Func<JniObjectReference, IntPtr> m = JniEnvironment.References.NewReturnToJniRef;
164164
var r = Expression.Variable (MarshalType, namePrefix + "_rtn");
165+
if (context == null)
166+
throw new ArgumentNullException (nameof (context));
165167
context.LocalVariables.Add (r);
166168
context.CreationStatements.Add (
167169
Expression.Assign (r,

0 commit comments

Comments
 (0)