Skip to content

Commit 56482df

Browse files
radekdoulikjonpryor
authored andcommitted
[Java.Interop] Review Critical and High Gendarme reports (#196)
Checked and handled the Gendarme reports labeled with **Severity: Critical** and **Severity: High** in `make fxcop` output. In one case refactored the `JniRuntime.RaisePendingException()` method to reuse code by calling `JniEnvironment.Exceptions.Throw()`.
1 parent 5cf61f5 commit 56482df

File tree

3 files changed

+15
-8
lines changed

3 files changed

+15
-8
lines changed

gendarme-ignore.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,8 @@ M: Java.Interop.JniObjectReferenceType Java.Interop.JniEnvironment/References::G
532532
M: Java.Interop.JniObjectReference Java.Interop.JniEnvironment/Reflection::ToReflectedMethod(Java.Interop.JniObjectReference,Java.Interop.JniMethodInfo,System.Boolean)
533533
M: Java.Interop.JniObjectReference Java.Interop.JniEnvironment/Reflection::ToReflectedField(Java.Interop.JniObjectReference,Java.Interop.JniFieldInfo,System.Boolean)
534534
M: Java.Interop.JniObjectReferenceFlags Java.Interop.JniObjectReference::get_Flags()
535+
# This method is used in Java.Interop.GenericMarshaler.dll, the internals are visible to this assembly. Looks like gendarme doesn't know InternalsVisibleTo attribute.
536+
M: Java.Interop.JniObjectReference Java.Interop.JniPeerMembers/JniInstanceMethods::AllocObject(System.Type)
535537

536538
# I think Gendarme is buggy here; `JavaArray<T>.CheckLength(IList<T>)` *is* used.
537539
M: System.Int32 Java.Interop.JavaArray`1::CheckLength(System.Collections.Generic.IList`1<T>)
@@ -557,3 +559,12 @@ R: Gendarme.Rules.Performance.UseTypeEmptyTypesRule
557559
# The PCL profile we're using doen't *have* Type.EmptyTypes!
558560
M: System.Void Java.Interop.JniRuntime/JniTypeManager::.cctor()
559561

562+
R: Gendarme.Rules.BadPractice.CheckNewExceptionWithoutThrowingRule
563+
# This method constructs JavaException and calls ToString () on it. We only care about Java stack trace here, so we don't need to throw the exception to get managed StackTrace.
564+
M: System.Exception Java.Interop.ManagedPeer::CreateJniLocationException()
565+
# This method uses a JavaException and passes its PeerReference property along. We don't want to throw that property.
566+
M: System.Void Java.Interop.JniEnvironment/Exceptions::Throw(System.Exception)
567+
568+
R: Gendarme.Rules.Correctness.DisposableFieldsShouldBeDisposedRule
569+
# We call Dispose on marshalMemberBuilder field in the JniRuntime::Dispose method. Looks like gendarme bug, it doesn't handle well the `?.` operator.
570+
T: Java.Interop.JniRuntime

src/Java.Interop/Java.Interop/JniEnvironment.Errors.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public static void ThrowNew (JniObjectReference klass, string message)
3232
public static void Throw (Exception e)
3333
{
3434
if (e == null)
35-
throw new ArgumentNullException ("e");
35+
throw new ArgumentNullException (nameof (e));
3636
var je = e as JavaException;
3737
if (je == null) {
3838
je = new JavaProxyThrowable (e);

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -414,16 +414,12 @@ partial class JniRuntime {
414414

415415
public virtual void RaisePendingException (Exception pendingException)
416416
{
417+
#if XA_INTEGRATION
417418
if (pendingException == null)
418419
throw new ArgumentNullException (nameof (pendingException));
419-
#if XA_INTEGRATION
420420
throw new NotSupportedException ("Do not know how to marshal System.Exception instances.");
421-
#else // XA_INTEGRATION
422-
var je = pendingException as JavaException;
423-
if (je == null) {
424-
je = new JavaProxyThrowable (pendingException);
425-
}
426-
JniEnvironment.Exceptions.Throw (je.PeerReference);
421+
#else // !XA_INTEGRATION
422+
JniEnvironment.Exceptions.Throw (pendingException);
427423
#endif // !XA_INTEGRATION
428424
}
429425
}

0 commit comments

Comments
 (0)