Fix C# CA1063 warning by implementing the recommended Dispose methods.

Previously just the Dispose() method was generated.
Now the Dispose() and Dispose(bool disposing) methods are generated.
Changes are required if custom "csfinalize", "csdestruct" or "csdestruct_derived"
typemaps are being used. Details in #421 on Github. SWIG will error out if one of
the "csfinalize, "csdestruct" or "csdestruct_derived" typemaps are found. Example
error message:

  foo.h:60: Error: A deprecated csfinalize typemap was found for Foo, please remove
  it and replace all csdestruct, csdestruct_derived and csfinalize typemaps by the
  csdispose, csdispose_derived, csdisposing and csdisposing_derived typemaps.

Closes #421

CHANGES.current

@@ -7,6 +7,21 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
 Version 4.0.0 (in progress)
 ===========================
 
+2019-03-30: wsfulton
+            [C#] #421 Fix CA1063 warning by implementing the recommended Dispose methods for
+            the IDisposable interface. Previously just the Dispose() method was generated.
+            Now the Dispose() and Dispose(bool disposing) methods are generated.
+            Changes are required if custom "csfinalize", "csdestruct" or "csdestruct_derived"
+            typemaps are being used. Details in #421 on Github. SWIG will error out if one of
+            the "csfinalize, "csdestruct" or "csdestruct_derived" typemaps are found. Example
+            error message:
+
+              foo.h:60: Error: A deprecated csfinalize typemap was found for Foo, please remove
+              it and replace all csdestruct, csdestruct_derived and csfinalize typemaps by the
+              csdispose, csdispose_derived, csdisposing and csdisposing_derived typemaps.
+
+            *** POTENTIAL INCOMPATIBILITY ***
+
 2019-03-25: Liryna
             [C#] #1143 Add std_list.i for std::list support.
             The C# std::list<T> wrappers are made to look and feel like a C# 

Doc/Manual/CSharp.html

@@ -232,8 +232,8 @@ javacode                    -> cscode
 javaimports                 -> csimports
 javabody                    -> csbody
 javafinalize                -> csfinalize
-javadestruct                -> csdestruct
+javadestruct                -> csdisposing and csdispose
-javadestruct_derived        -> csdestruct_derived
+javadestruct_derived        -> csdisposing_derived and csdispose_derived
 javainterfacecode           -> csinterfacecode
 </pre></div>
 

Examples/test-suite/allprotected.i

@@ -79,5 +79,9 @@ protected:
 };
 int ProtectedBase::staticMemberVariable = 10;
 
+class ProtectedDerived : public ProtectedBase {
+public:
+  ProtectedDerived(const char *s) : ProtectedBase(s) {}
+};
 %}
 

Examples/test-suite/csharp/allprotected_runme.cs

@@ -13,6 +13,20 @@ public class runme
   {
     MyProtectedBase mpb = new MyProtectedBase("MyProtectedBase");
     mpb.accessProtected();
+    try {
+      // C++ destructor is protected
+      mpb.Dispose();
+      throw new Exception("failed to catch MethodAccessException");
+    } catch (MethodAccessException) {
+      // Exception message: C++ destructor does not have public access
+    }
+    ProtectedDerived pd = new ProtectedDerived("ProtectedDerived");
+    // Destroying via the ProtectedDerived's destructor should work
+    pd.Dispose();
+
+    ProtectedBase pb = new ProtectedDerived("ProtectedDerived");
+    // ProtectedDerived's destructor should be called via the Dispose(disposing) virtual call
+    pb.Dispose();
   }
 }
 

Examples/test-suite/minherit2.i

@@ -17,8 +17,8 @@
 #if defined(SWIGCSHARP)
 #define javaclassmodifiers   csclassmodifiers
 #define javabody             csbody
-#define javafinalize         csfinalize
+#define javafinalize         csdispose
-#define javadestruct         csdestruct
+#define javadestruct         csdisposing
 #define javaout              csout
 #define javainterfaces       csinterfaces
 #define javabase             csbase

Lib/csharp/boost_intrusive_ptr.i

@@ -314,7 +314,7 @@
   }
 %}
 
-%typemap(csdestruct, methodname="Dispose", methodmodifiers="public") TYPE {
+%typemap(csdisposing, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") TYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwnBase) {
@@ -323,11 +323,10 @@
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
     }
   }
 
-%typemap(csdestruct_derived, methodname="Dispose", methodmodifiers="public") TYPE {
+%typemap(csdisposing_derived, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") TYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwnDerived) {
@@ -336,8 +335,7 @@
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
+      base.Dispose(disposing);
-      base.Dispose();
     }
   }
 
@@ -473,7 +471,7 @@
   }
 %}
 
-%typemap(csdestruct, methodname="Dispose", methodmodifiers="public") TYPE {
+%typemap(csdisposing, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") TYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwnBase) {
@@ -482,11 +480,10 @@
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
     }
   }
 
-%typemap(csdestruct_derived, methodname="Dispose", methodmodifiers="public") TYPE {
+%typemap(csdisposing_derived, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") TYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwnDerived) {
@@ -495,8 +492,7 @@
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
+      base.Dispose(disposing);
-      base.Dispose();
     }
   }
 

Lib/csharp/boost_shared_ptr.i

@@ -282,7 +282,7 @@
   }
 %}
 
-%typemap(csdestruct, methodname="Dispose", methodmodifiers="public") TYPE {
+%typemap(csdisposing, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") TYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwnBase) {
@@ -291,11 +291,10 @@
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
     }
   }
 
-%typemap(csdestruct_derived, methodname="Dispose", methodmodifiers="public") TYPE {
+%typemap(csdisposing_derived, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") TYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwnDerived) {
@@ -304,8 +303,7 @@
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
+      base.Dispose(disposing);
-      base.Dispose();
     }
   }
 

Lib/csharp/csharp.swg

@@ -959,17 +959,24 @@ SWIGINTERN const char * SWIG_UnpackData(const char *c, void *ptr, size_t sz) {
 SWIG_CSBODY_PROXY(internal, internal, SWIGTYPE)
 SWIG_CSBODY_TYPEWRAPPER(internal, protected, internal, SWIGTYPE)
 
-%typemap(csfinalize) SWIGTYPE %{
+%typemap(csdispose) SWIGTYPE %{
   ~$csclassname() {
-    Dispose();
+    Dispose(false);
+  }
+
+  public void Dispose() {
+    Dispose(true);
+    global::System.GC.SuppressFinalize(this);
   }
 %}
 
+%typemap(csdispose_derived) SWIGTYPE ""
+
 %typemap(csconstruct, excode=SWIGEXCODE,directorconnect="\n    SwigDirectorConnect();") SWIGTYPE %{: this($imcall, true) {$excode$directorconnect
   }
 %}
 
-%typemap(csdestruct, methodname="Dispose", methodmodifiers="public") SWIGTYPE {
+%typemap(csdisposing, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") SWIGTYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwn) {
@@ -978,11 +985,10 @@ SWIG_CSBODY_TYPEWRAPPER(internal, protected, internal, SWIGTYPE)
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
     }
   }
 
-%typemap(csdestruct_derived, methodname="Dispose", methodmodifiers="public") SWIGTYPE {
+%typemap(csdisposing_derived, methodname="Dispose", methodmodifiers="protected", parameters="bool disposing") SWIGTYPE {
     lock(this) {
       if (swigCPtr.Handle != global::System.IntPtr.Zero) {
         if (swigCMemOwn) {
@@ -991,8 +997,7 @@ SWIG_CSBODY_TYPEWRAPPER(internal, protected, internal, SWIGTYPE)
         }
         swigCPtr = new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero);
       }
-      global::System.GC.SuppressFinalize(this);
+      base.Dispose(disposing);
-      base.Dispose();
     }
   }
 

Source/Modules/csharp.cxx

@@ -1873,38 +1873,56 @@ public:
 	   typemapLookup(n, "csbody", typemap_lookup_type, WARN_CSHARP_TYPEMAP_CSBODY_UNDEF),	// main body of class
 	   NIL);
 
-    // C++ destructor is wrapped by the Dispose method
+    // C++ destructor is wrapped by the Finalize and Dispose methods
-    // Note that the method name is specified in a typemap attribute called methodname
+
+    const char *tmap_method = derived ? "csdestruct_derived" : "csdestruct";
+    const String *tm = typemapExists(n, tmap_method, typemap_lookup_type);
+    if (tm) {
+      Swig_error(Getfile(tm), Getline(tm),
+	  "A deprecated %s typemap was found for %s, please remove it and replace all csdestruct, csdestruct_derived and csfinalize typemaps by the csdispose, csdispose_derived, csdisposing and csdisposing_derived typemaps.\n",
+	  tmap_method, proxy_class_name);
+    }
+    tmap_method = "csfinalize";
+    tm = typemapExists(n, tmap_method, typemap_lookup_type);
+    if (tm) {
+      Swig_error(Getfile(tm), Getline(tm),
+	  "A deprecated %s typemap was found for %s, please remove it and replace all csdestruct, csdestruct_derived and csfinalize typemaps by the csdispose, csdispose_derived, csdisposing and csdisposing_derived typemaps.\n",
+	  tmap_method, proxy_class_name);
+    }
+
+    tmap_method = derived ? "csdisposing_derived" : "csdisposing";
     String *destruct = NewString("");
-    const String *tm = NULL;
     attributes = NewHash();
-    String *destruct_methodname = NULL;
+    const String *destruct_methodname = NULL;
-    String *destruct_methodmodifiers = NULL;
+    const String *destruct_methodmodifiers = NULL;
+    const String *destruct_parameters = NULL;
     if (derived) {
-      tm = typemapLookup(n, "csdestruct_derived", typemap_lookup_type, WARN_NONE, attributes);
+      tm = typemapLookup(n, "csdisposing_derived", typemap_lookup_type, WARN_NONE, attributes);
-      destruct_methodname = Getattr(attributes, "tmap:csdestruct_derived:methodname");
+      destruct_methodname = Getattr(attributes, "tmap:csdisposing_derived:methodname");
-      destruct_methodmodifiers = Getattr(attributes, "tmap:csdestruct_derived:methodmodifiers");
+      destruct_methodmodifiers = Getattr(attributes, "tmap:csdisposing_derived:methodmodifiers");
+      destruct_parameters = Getattr(attributes, "tmap:csdisposing_derived:parameters");
     } else {
-      tm = typemapLookup(n, "csdestruct", typemap_lookup_type, WARN_NONE, attributes);
+      tm = typemapLookup(n, "csdisposing", typemap_lookup_type, WARN_NONE, attributes);
-      destruct_methodname = Getattr(attributes, "tmap:csdestruct:methodname");
+      destruct_methodname = Getattr(attributes, "tmap:csdisposing:methodname");
-      destruct_methodmodifiers = Getattr(attributes, "tmap:csdestruct:methodmodifiers");
+      destruct_methodmodifiers = Getattr(attributes, "tmap:csdisposing:methodmodifiers");
+      destruct_parameters = Getattr(attributes, "tmap:csdisposing:parameters");
     }
     if (tm && *Char(tm)) {
       if (!destruct_methodname) {
-	Swig_error(Getfile(n), Getline(n), "No methodname attribute defined in csdestruct%s typemap for %s\n", (derived ? "_derived" : ""), proxy_class_name);
+	Swig_error(Getfile(n), Getline(n), "No methodname attribute defined in %s typemap for %s\n", tmap_method, proxy_class_name);
       }
       if (!destruct_methodmodifiers) {
 	Swig_error(Getfile(n), Getline(n),
-		   "No methodmodifiers attribute defined in csdestruct%s typemap for %s.\n", (derived ? "_derived" : ""), proxy_class_name);
+		   "No methodmodifiers attribute defined in %s typemap for %s.\n", tmap_method, proxy_class_name);
       }
+      if (!destruct_parameters)
+	destruct_parameters = empty_string;
     }
     // Emit the Finalize and Dispose methods
     if (tm) {
-      // Finalize method
+      // Finalize and Dispose methods
-      if (*Char(destructor_call)) {
+      Printv(proxy_class_def, typemapLookup(n, derived ? "csdispose_derived" : "csdispose", typemap_lookup_type, WARN_NONE), NIL);
-	Printv(proxy_class_def, typemapLookup(n, "csfinalize", typemap_lookup_type, WARN_NONE), NIL);
+      // Dispose(bool disposing) method
-      }
-      // Dispose method
       Printv(destruct, tm, NIL);
       if (*Char(destructor_call))
 	Replaceall(destruct, "$imcall", destructor_call);
@@ -1917,7 +1935,7 @@ public:
 	  Printv(proxy_class_def, methodmods, NIL);
 	else
 	  Printv(proxy_class_def, destruct_methodmodifiers, " ", derived ? "override" : "virtual", NIL);
-	Printv(proxy_class_def, " void ", destruct_methodname, "() ", destruct, "\n", NIL);
+	Printv(proxy_class_def, " void ", destruct_methodname, "(", destruct_parameters, ") ", destruct, "\n", NIL);
       }
     }
     if (*Char(interface_upcasts))
@@ -3552,6 +3570,24 @@ public:
     return tm;
   }
 
+  /* -----------------------------------------------------------------------------
+   * typemapExists()
+   * n - for input only and must contain info for Getfile(n) and Getline(n) to work
+   * tmap_method - typemap method name
+   * type - typemap type to lookup
+   * returns found typemap or NULL if not found
+   * ----------------------------------------------------------------------------- */
+
+  const String *typemapExists(Node *n, const_String_or_char_ptr tmap_method, SwigType *type) {
+    Node *node = NewHash();
+    Setattr(node, "type", type);
+    Setfile(node, Getfile(n));
+    Setline(node, Getline(n));
+    const String *tm = Swig_typemap_lookup(tmap_method, node, "", 0);
+    Delete(node);
+    return tm;
+  }
+
   /* -----------------------------------------------------------------------------
    * canThrow()
    * Determine whether the code in the typemap can throw a C# exception.