[generator] Emit new when hiding Handle, Message (#49)

The internal Xamarin.Android CI system is reporting an error when
running the `generator` unit tests, but it's a false positive.
It's *not* a bug in `generator`. Rather, it's a bug in Mono 4.2, in
which [`CSharpCodeCompiler` treats multi-line warnings as errors][0].
Since the `generator` unit tests emit such multi-line warnings when
hiding `Java.Lang.Object.Handle` (via
`Xamarin.Test.SomeObject.Handle()`) and `System.Exception.Message`
(via `Java.Lang.Throwable.Message`), and Mono 4.2 treats the
multi-line warnings as errors, the tests fail.

There are three plausible solutions so that we don't get erroneous
reports from the `generator` tests when running on Mono 4.2:

1. Remove those tests. (Uh...no.)
2. Upgrade Mono on the CI machine to Mono 4.4, which has the fix.
3. Fix `generator` so that the warning isn't emitted.

(2) would require an unknown timeframe with unknown repercussions.

Thus, the chosen fix: improve `generator` so that the warning isn't
generated.

[0]: mono/mono#2248

Author: jonpryor

tools/generator/GenBase.cs

@@ -178,12 +178,30 @@ public string MetadataXPathReference {
 		public string GetObjectHandleProperty (string variable)
 		{
 			var handleType  = "Java.Lang.Object";
-			if (FullName == "Java.Lang.Throwable" || Ancestors ().Any (a => a.FullName == "Java.Lang.Throwable"))
+			if (IsThrowable ())
 				handleType  = "Java.Lang.Throwable";
 
 			return $"((global::{handleType}) {variable}).Handle";
 		}
 
+		bool IsThrowable ()
+		{
+			if (FullName == "Java.Lang.Throwable" || Ancestors ().Any (a => a.FullName == "Java.Lang.Throwable"))
+				return true;
+			return false;
+		}
+
+		public bool RequiresNew (string memberName)
+		{
+			if (Array.BinarySearch (ObjectRequiresNew, memberName, StringComparer.OrdinalIgnoreCase) >= 0) {
+				return true;
+			}
+			if (IsThrowable () && Array.BinarySearch (ThrowableRequiresNew, memberName, StringComparer.OrdinalIgnoreCase) >= 0) {
+				return true;
+			}
+			return false;
+		}
+
 		protected IEnumerable<InterfaceGen> GetAllImplementedInterfaces ()
 		{
 			var set = new HashSet<InterfaceGen> ();
@@ -508,15 +526,19 @@ bool CanMethodBeIsStyleSetter (Method m)
 		}
 
 		// Keep sorted alphabetically
-		static readonly string[] blacklist = new string[]{
+		static readonly string[]    ObjectRequiresNew       = new string[]{
 			"Handle",
 		};
 
+		static readonly string[]    ThrowableRequiresNew    = new string []{
+			"Message",
+		};
+
 		bool IsInfrastructural (string name)
 		{
 			// some names are reserved for use by us, e.g. we don't want another
 			// Handle property, as that conflicts with Java.Lang.Object.Handle.
-			return Array.BinarySearch (blacklist, name) >= 0;
+			return Array.BinarySearch (ObjectRequiresNew, name) >= 0;
 		}
 
 		protected void GenCharSequenceEnumerator (StreamWriter sw, string indent, CodeGenerationOptions opt)

tools/generator/Method.cs

@@ -799,6 +799,9 @@ public void Generate (StreamWriter sw, string indent, CodeGenerationOptions opt,
 
 			string static_arg = IsStatic ? " static" : String.Empty;
 			string virt_ov = IsOverride ? " override" : IsVirtual ? " virtual" : String.Empty;
+			if ((string.IsNullOrEmpty (virt_ov) || virt_ov == " virtual") && type.RequiresNew (AdjustedName)) {
+				virt_ov = " new" + virt_ov;
+			}
 			string seal = IsOverride && IsFinal ? " sealed" : null;
 			string ret = opt.GetOutputName (RetVal.FullName);
 			GenerateIdField (sw, indent, opt);

tools/generator/Property.cs

@@ -65,13 +65,17 @@ public void GenerateAbstractDeclaration (StreamWriter sw, string indent, CodeGen
 				overrides = true;
 			}
 
+			bool requiresNew     = false;
 			string abstract_name = AdjustedName;
 			string visibility = Getter.RetVal.IsGeneric ? "protected" : Getter.Visibility;
-			if (!overrides)
+			if (!overrides) {
+				requiresNew      = gen.RequiresNew (abstract_name);
 				GenerateCallbacks (sw, indent, opt, gen, abstract_name);
-			sw.WriteLine ("{0}{1} abstract{2} {3} {4} {{",
+			}
+			sw.WriteLine ("{0}{1}{2} abstract{3} {4} {5} {{",
 					indent,
 					visibility,
+					requiresNew ? " new" : "",
 					overrides ? " override" : "",
 					opt.GetOutputName (Getter.ReturnType),
 					abstract_name);
@@ -199,22 +203,23 @@ public void Generate (GenBase gen, StreamWriter sw, string indent, CodeGeneratio
 				force_override = true;
 
 			string decl_name = AdjustedName;
+			string needNew          = gen.RequiresNew (decl_name) ? " new" : "";
 			string virtual_override = String.Empty;
 			bool is_virtual = Getter.IsVirtual && (Setter == null || Setter.IsVirtual);
 			if (with_callbacks && is_virtual) {
-				virtual_override = " virtual";
+				virtual_override = needNew + " virtual";
 				Getter.GenerateCallback (sw, indent, opt, gen, AdjustedName);
 			}
 			if (with_callbacks && is_virtual && Setter != null) {
-				virtual_override = " virtual";
+				virtual_override = needNew + " virtual";
 				Setter.GenerateCallback (sw, indent, opt, gen, AdjustedName);
 			}
 			virtual_override = force_override ? " override" : virtual_override;
 			if ((Getter ?? Setter).IsStatic)
 				virtual_override = " static";
 			// It should be using AdjustedName instead of Name, but ICharSequence ("Formatted") properties are not caught by this...
 			else if (gen.BaseSymbol != null && gen.BaseSymbol.GetPropertyByName (Name, true) != null)
-				virtual_override = " override";
+				virtual_override = needNew + " override";
 
 			Getter.GenerateIdField (sw, indent, opt);
 			if (Setter != null)

tools/generator/Tests/BaseGeneratorTest.cs

@@ -57,7 +57,10 @@ protected void CompareOutputs (string sourceDir, string destinationDir)
 				if (!File.Exists (dest)) {
 					Assert.Fail (string.Format ("Expected {0} but it was not generated.", dest));
 				} else if (!FileCompare (file, dest)) {
-					Assert.Fail (string.Format ("The Files {0} and {1} do not match", file, dest));
+					var fullSource  = Path.GetFullPath (file);
+					var fullDest    = Path.GetFullPath (dest);
+					string message  = $"File contents differ; run: diff -u {fullSource} \\{Environment.NewLine}\t{fullDest}";
+					Assert.Fail (message);
 				}
 			}
 		}

tools/generator/Tests/expected.ji/NormalMethods/Xamarin.Test.SomeObject.cs

@@ -71,7 +71,7 @@ static int n_Handle_Ljava_lang_Object_Ljava_lang_Throwable_ (IntPtr jnienv, IntP
 
 		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='handle' and count(parameter)=2 and parameter[1][@type='java.lang.Object'] and parameter[2][@type='java.lang.Throwable']]"
 		[Register ("handle", "(Ljava/lang/Object;Ljava/lang/Throwable;)I", "GetHandle_Ljava_lang_Object_Ljava_lang_Throwable_Handler")]
-		public virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
+		public new virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
 		{
 			const string __id = "handle.(Ljava/lang/Object;Ljava/lang/Throwable;)I";
 			try {

tools/generator/Tests/expected.ji/Streams/Java.Lang.Throwable.cs

@@ -32,7 +32,7 @@ static IntPtr n_GetMessage (IntPtr jnienv, IntPtr native__this)
 		}
 #pragma warning restore 0169
 
-		public virtual unsafe string Message {
+		public new virtual unsafe string Message {
 			// Metadata.xml XPath method reference: path="/api/package[@name='java.lang']/class[@name='Throwable']/method[@name='getMessage' and count(parameter)=0]"
 			[Register ("getMessage", "()Ljava/lang/String;", "GetGetMessageHandler")]
 			get {

tools/generator/Tests/expected/NormalMethods/Xamarin.Test.SomeObject.cs

@@ -77,7 +77,7 @@ static int n_Handle_Ljava_lang_Object_Ljava_lang_Throwable_ (IntPtr jnienv, IntP
 		static IntPtr id_handle_Ljava_lang_Object_Ljava_lang_Throwable_;
 		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='handle' and count(parameter)=2 and parameter[1][@type='java.lang.Object'] and parameter[2][@type='java.lang.Throwable']]"
 		[Register ("handle", "(Ljava/lang/Object;Ljava/lang/Throwable;)I", "GetHandle_Ljava_lang_Object_Ljava_lang_Throwable_Handler")]
-		public virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
+		public new virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
 		{
 			if (id_handle_Ljava_lang_Object_Ljava_lang_Throwable_ == IntPtr.Zero)
 				id_handle_Ljava_lang_Object_Ljava_lang_Throwable_ = JNIEnv.GetMethodID (class_ref, "handle", "(Ljava/lang/Object;Ljava/lang/Throwable;)I");

tools/generator/Tests/expected/Streams/Java.Lang.Throwable.cs

@@ -32,7 +32,7 @@ static IntPtr n_GetMessage (IntPtr jnienv, IntPtr native__this)
 #pragma warning restore 0169
 
 		static IntPtr id_getMessage;
-		public virtual unsafe string Message {
+		public new virtual unsafe string Message {
 			// Metadata.xml XPath method reference: path="/api/package[@name='java.lang']/class[@name='Throwable']/method[@name='getMessage' and count(parameter)=0]"
 			[Register ("getMessage", "()Ljava/lang/String;", "GetGetMessageHandler")]
 			get {