[Xamarin.Android.Build.Tasks] Add `$(AndroidErrorOnCustomJavaObject)` (#797)

Fixes:  https://bugzilla.xamarin.com/show_bug.cgi?id=56819

Bumps to Java.Interop/master/ab3c2b26.

Nothing *prevents* "anybody" from custom implementing `IJavaObject`:

	class MyBadClass : Android.Runtime.IJavaObject {
	    public IntPtr Handle {
	        get {return IntPtr.Zero;}
	    }

	    public void Dispose () {}
	}

The problem is that the above doesn't actually work: the
`IJavaObject.Handle` value contains the JNI Object Reference to pass
into JNI. The above code will thus result in always passing `null`
into Java code, which could result in a `NullPointerException`, but
will always result in *not* doing what was intended.

While it is *theoretically* possible to manually implement
`IJavaObject`, it's not something *I* would want to contemplate, nor
is it for the faint of heart, so long ago we decided to emit a warning
if we encounter a type which:

 1. Implements `Android.Runtime.IJavaObject`, and
 2. Does *not* also inherit `Java.Lang.Object` or
    `Java.Lang.Throwable`.

As such, the above `MyBadClass` elicits the warning:

	Type 'MyBadClass' implements Android.Runtime.IJavaObject but does not inherit from Java.Lang.Object. It is not supported.

This is all well and good, but (effectively) *nobody* reads warnings,
*especially* since our toolchain emits so many warnings that enabling
`/warnaserror` is for the truly foolhardy, so *lots* of warnings is,
unfortunately, normal.

Which brings us to Bug #56819: Could we make this an error?

Answer: Of course we can. That said, I have no idea how much existing
code this will *break* if we turned it into an error. That might be
good and useful, but if there's no way to *disable* the error,
existing apps which (appear to) work will no longer build.

That's not desirable.

Add a new `$(AndroidErrorOnCustomJavaObject)` MSBuild property to
control what happens when such custom `IJavaObject` implementations
are found. When True, the default, emit a new XA4212 *error*.
The error can be turned back into a warning by setting
`$(AndroidErrorOnCustomJavaObject)` within the `App.csproj`:

	<!-- Within App.csproj -->
	<PropertyGroup>
	  <AndroidErrorOnCustomJavaObject>False</AndroidErrorOnCustomJavaObject>
	</PropertyGroup>

Documentation/build_process.md

@@ -217,6 +217,27 @@ when packaing Release applications.
 
     This property is `False` by default.
 
+-   **AndroidErrorOnCustomJavaObject** – A boolean property that
+    determines whether types may implement `Android.Runtime.IJavaObject`
+    *without* also inheriting from `Java.Lang.Object` or `Java.Lang.Throwable`:
+
+        class BadType : IJavaObject {
+            public IntPtr Handle {
+                get {return IntPtr.Zero;}
+            }
+        
+            public void Dispose()
+            {
+            }
+        }
+
+    When True, such types will generate an XA4212 error, otherwise a
+    XA4212 warning will be generated.
+
+    Support for this property was added in Xamarin.Android 7.6.
+
+    This property is `True` by default.
+
 -   **AndroidFastDeploymentType** – A `:` (colon)-separated list
     of values to control what types can be deployed to the
     [Fast Deployment directory](#Fast_Deployment) on the target device

external/Java.Interop

@@ -1 +1 @@
-Subproject commit 33436346553b7ad1b2e3b922f04f1703b9a9ec64
+Subproject commit 16be390ab82a5b1dda34aa2cb3237c9634a37266

src/Xamarin.Android.Build.Tasks/Tasks/AsyncTask.cs

@@ -78,27 +78,27 @@ namespace Xamarin.Android.Tasks
 				LogDebugMessage ("    {0}", item.ItemSpec);
 		}
 
-		protected void LogMessage (string message)
+		public void LogMessage (string message)
 		{
 			LogMessage (message, importance: MessageImportance.Normal);
 		}
 
-		protected void LogMessage (string message, params object[] messageArgs)
+		public void LogMessage (string message, params object[] messageArgs)
 		{
 			LogMessage (string.Format (message, messageArgs));
 		}
 
-		protected void LogDebugMessage (string message)
+		public void LogDebugMessage (string message)
 		{
 			LogMessage (message , importance: MessageImportance.Low);
 		}
 
-		protected void LogDebugMessage (string message, params object[] messageArgs)
+		public void LogDebugMessage (string message, params object[] messageArgs)
 		{
 			LogMessage (string.Format (message, messageArgs), importance: MessageImportance.Low);
 		}
 
-		protected void LogMessage (string message, MessageImportance importance = MessageImportance.Normal)
+		public void LogMessage (string message, MessageImportance importance = MessageImportance.Normal)
 		{
 			if (UIThreadId == Thread.CurrentThread.ManagedThreadId) {
 				#pragma warning disable 618
@@ -121,27 +121,27 @@ namespace Xamarin.Android.Tasks
 			}
 		}
 
-		protected void LogError (string message)
+		public void LogError (string message)
 		{
 			LogError (code: null, message: message, file: null, lineNumber: 0);
 		}
 
-		protected void LogError (string message, params object[] messageArgs)
+		public void LogError (string message, params object[] messageArgs)
 		{
 			LogError (code: null, message: string.Format (message, messageArgs));
 		}
 
-		protected void LogCodedError (string code, string message)
+		public void LogCodedError (string code, string message)
 		{
 			LogError (code: code, message: message, file: null, lineNumber: 0);
 		}
 
-		protected void LogCodedError (string code, string message, params object[] messageArgs)
+		public void LogCodedError (string code, string message, params object[] messageArgs)
 		{
 			LogError (code: code, message: string.Format (message, messageArgs), file: null, lineNumber: 0);
 		}
 
-		protected void LogError (string code, string message, string file = null, int lineNumber = 0)
+		public void LogError (string code, string message, string file = null, int lineNumber = 0)
 		{
 			if (UIThreadId == Thread.CurrentThread.ManagedThreadId) {
 				#pragma warning disable 618
@@ -180,12 +180,12 @@ namespace Xamarin.Android.Tasks
 			}
 		}
 
-		protected void LogWarning (string message, params object[] messageArgs)
+		public void LogWarning (string message, params object[] messageArgs)
 		{
 			LogWarning (string.Format (message, messageArgs));
 		}
 
-		protected void LogWarning (string message)
+		public void LogWarning (string message)
 		{
 			if (UIThreadId == Thread.CurrentThread.ManagedThreadId) {
 				#pragma warning disable 618

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs

@@ -522,7 +522,7 @@ namespace Xamarin.Android.Tasks
 		{
 			int count = 0;
 			var abis = supportedAbis.Split (new char[] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries);
-			using (var res = new DirectoryAssemblyResolver (Console.WriteLine, loadDebugSymbols: false)) {
+			using (var res = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
 			foreach (var assembly in EmbeddedNativeLibraryAssemblies)
 				res.Load (assembly.ItemSpec);
 				foreach (var assemblyPath in EmbeddedNativeLibraryAssemblies) {

src/Xamarin.Android.Build.Tasks/Tasks/CheckTargetFrameworks.cs

@@ -50,7 +50,7 @@ namespace Xamarin.Android.Tasks
 			Log.LogDebugMessage ("  ProjectFile: {0}", ProjectFile);
 			Log.LogDebugTaskItems ("  ResolvedUserAssemblies: {0}", ResolvedAssemblies);
 
-			using (var res = new DirectoryAssemblyResolver (Log.LogWarning, loadDebugSymbols: false)) {
+			using (var res = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
 				foreach (var assembly in ResolvedAssemblies) {
 					res.Load (Path.GetFullPath (assembly.ItemSpec));
 					var apiLevel = ExtractApiLevel (res, assembly);

src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using Microsoft.Build.Framework;
@@ -51,6 +52,8 @@ namespace Xamarin.Android.Tasks
 
 		public bool UseSharedRuntime { get; set; }
 
+		public bool ErrorOnCustomJavaObject { get; set; }
+
 		[Required]
 		public string ResourceDirectory { get; set; }
 
@@ -69,6 +72,7 @@ namespace Xamarin.Android.Tasks
 			Log.LogDebugMessage ("  PackageName: {0}", PackageName);
 			Log.LogDebugMessage ("  AndroidSdkDir: {0}", AndroidSdkDir);
 			Log.LogDebugMessage ("  AndroidSdkPlatform: {0}", AndroidSdkPlatform);
+			Log.LogDebugMessage ($"  {nameof (ErrorOnCustomJavaObject)}: {ErrorOnCustomJavaObject}");
 			Log.LogDebugMessage ("  OutputDirectory: {0}", OutputDirectory);
 			Log.LogDebugMessage ("  MergedAndroidManifestOutput: {0}", MergedAndroidManifestOutput);
 			Log.LogDebugMessage ("  UseSharedRuntime: {0}", UseSharedRuntime);
@@ -83,7 +87,7 @@ namespace Xamarin.Android.Tasks
 			try {
 				// We're going to do 3 steps here instead of separate tasks so
 				// we can share the list of JLO TypeDefinitions between them
-				using (var res = new DirectoryAssemblyResolver (Log.LogWarning, loadDebugSymbols: true)) {
+				using (var res = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: true)) {
 					Run (res);
 				}
 			}
@@ -117,9 +121,12 @@ namespace Xamarin.Android.Tasks
 			var fxAdditions = MonoAndroidHelper.GetFrameworkAssembliesToTreatAsUserAssemblies (ResolvedAssemblies)
 				.Where (a => assemblies.All (x => Path.GetFileName (x) != Path.GetFileName (a)));
 			assemblies = assemblies.Concat (fxAdditions).ToList ();
-			
+
 			// Step 1 - Find all the JLO types
-			var all_java_types = JavaTypeScanner.GetJavaTypes (assemblies, res, Log.LogWarning);
+			var scanner = new JavaTypeScanner (this.CreateTaskLogger ()) {
+				ErrorOnCustomJavaObject     = ErrorOnCustomJavaObject,
+			};
+			var all_java_types = scanner.GetJavaTypes (assemblies, res);
 
 			WriteTypeMappings (all_java_types);
 

src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesigner.cs

@@ -106,7 +106,7 @@ namespace Xamarin.Android.Tasks
 			var assemblyNames = new List<string> ();
 			if (IsApplication && References != null && References.Any ()) {
 				// FIXME: should this be unified to some better code with ResolveLibraryProjectImports?
-				using (var resolver = new DirectoryAssemblyResolver (Log.LogWarning, loadDebugSymbols: false)) {
+				using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
 					foreach (var assemblyName in References) {
 						var suffix = assemblyName.ItemSpec.EndsWith (".dll") ? String.Empty : ".dll";
 						string hintPath = assemblyName.GetMetadata ("HintPath").Replace (Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar);

src/Xamarin.Android.Build.Tasks/Tasks/GetAdditionalResourcesFromAssemblies.cs

@@ -394,7 +394,7 @@ namespace Xamarin.Android.Tasks {
 				? CachePath
 				: Path.Combine (Environment.GetFolderPath (Environment.SpecialFolder.LocalApplicationData), CacheBaseDir);
 
-				using (var resolver = new DirectoryAssemblyResolver (LogWarning, loadDebugSymbols: false)) {
+				using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
 					foreach (var assemblyItem in Assemblies) {
 						string fullPath = Path.GetFullPath (assemblyItem.ItemSpec);
 						if (assemblies.Contains (fullPath)) {

src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs

@@ -85,7 +85,7 @@ namespace Xamarin.Android.Tasks
 			var rp = new ReaderParameters {
 				InMemory    = true,
 			};
-			using (var res = new DirectoryAssemblyResolver (Log.LogWarning, loadDebugSymbols: false, loadReaderParameters: rp)) {
+			using (var res = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false, loadReaderParameters: rp)) {
 				return Execute (res);
 			}
 		}

src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs

@@ -45,7 +45,7 @@ namespace Xamarin.Android.Tasks
 
 		public override bool Execute ()
 		{
-			using (var resolver = new DirectoryAssemblyResolver (Log.LogWarning, loadDebugSymbols: false)) {
+			using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
 				return Execute (resolver);
 			}
 		}

src/Xamarin.Android.Build.Tasks/Tasks/ResolveLibraryProjectImports.cs

@@ -78,7 +78,7 @@ namespace Xamarin.Android.Tasks
 
 			assemblyMap.Load (AssemblyIdentityMapFile);
 
-			using (var resolver = new DirectoryAssemblyResolver (Log.LogWarning, loadDebugSymbols: false)) {
+			using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
 				Extract (resolver, jars, resolvedResourceDirectories, resolvedAssetDirectories, resolvedEnvironmentFiles);
 			}
 

src/Xamarin.Android.Build.Tasks/Tasks/StripEmbeddedLibraries.cs

@@ -28,7 +28,7 @@ namespace Xamarin.Android.Tasks
 			Log.LogDebugMessage ("StripEmbeddedLibraries Task");
 			Log.LogDebugTaskItems ("  Assemblies: ", Assemblies);
 
-			using (var res = new DirectoryAssemblyResolver (Log.LogWarning, true, new ReaderParameters { ReadWrite = true } )) {
+			using (var res = new DirectoryAssemblyResolver (this.CreateTaskLogger (), true, new ReaderParameters { ReadWrite = true } )) {
 				return Execute (res);
 			}
 		}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs

@@ -1993,6 +1993,36 @@ namespace "+ libName + @" {
 			Assert.IsTrue(sb.BuildProject(app1, "SignAndroidPackage"), "Build of project should have succeeded");
 			sb.Dispose();
 		}
+
+		[Test]
+		public void XA4212 ()
+		{
+			var proj = new XamarinAndroidApplicationProject () {
+			};
+			proj.Sources.Add (new BuildItem ("Compile", "MyBadJavaObject.cs") { TextContent = () => @"
+using System;
+using Android.Runtime;
+namespace UnnamedProject {
+    public class MyBadJavaObject : IJavaObject
+    {
+        public IntPtr Handle {
+			get {return IntPtr.Zero;}
+        }
+
+        public void Dispose ()
+        {
+        }
+    }
+}" });
+			using (var builder = CreateApkBuilder (Path.Combine ("temp", TestContext.CurrentContext.Test.Name))) {
+				builder.ThrowOnBuildFailure = false;
+				Assert.IsFalse (builder.Build (proj), "Build should have failed with XA4212.");
+				StringAssert.Contains ($"error XA4", builder.LastBuildOutput, "Error should be XA4212");
+				StringAssert.Contains ($"Type `UnnamedProject.MyBadJavaObject` implements `Android.Runtime.IJavaObject`", builder.LastBuildOutput, "Error should mention MyBadJavaObject");
+				Assert.IsTrue (builder.Build (proj, parameters: new [] { "AndroidErrorOnCustomJavaObject=False" }), "Build should have succeeded.");
+				StringAssert.Contains ($"warning XA4", builder.LastBuildOutput, "warning XA4212");
+			}
+		}
 	}
 }
 

src/Xamarin.Android.Build.Tasks/Utilities/MSBuildExtensions.cs

@@ -131,6 +131,43 @@ namespace Xamarin.Android.Tasks
 					messageArgs:        messageArgs);
 		}
 
+		public static Action<TraceLevel, string> CreateTaskLogger (this Task task)
+		{
+			Action<TraceLevel, string> logger = (level, value) => {
+				switch (level) {
+				case TraceLevel.Error:
+					task.Log.LogError ("{0}", value);
+					break;
+				case TraceLevel.Warning:
+					task.Log.LogWarning ("{0}", value);
+					break;
+				default:
+					task.Log.LogDebugMessage ("{0}", value);
+					break;
+				}
+			};
+			return logger;
+		}
+
+		public static Action<TraceLevel, string> CreateTaskLogger (this AsyncTask task)
+		{
+			Action<TraceLevel, string> logger = (level, value) => {
+				switch (level) {
+				case TraceLevel.Error:
+					task.LogError (value);
+					break;
+				case TraceLevel.Warning:
+					task.LogWarning (value);
+					break;
+				default:
+					task.LogDebugMessage (value);
+					break;
+				}
+			};
+			return logger;
+		}
+
+
 		public static IEnumerable<ITaskItem> Concat (params ITaskItem[][] values)
 		{
 			if (values == null)

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

@@ -182,6 +182,8 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
 	<AndroidSkipJavacVersionCheck Condition="'$(AndroidSkipJavacVersionCheck)' == ''">False</AndroidSkipJavacVersionCheck>
 	<AndroidBuildApplicationPackage Condition=" '$(AndroidBuildApplicationPackage)' == ''">False</AndroidBuildApplicationPackage>
 
+	<AndroidErrorOnCustomJavaObject Condition=" '$(AndroidErrorOnCustomJavaObject)' == '' ">True</AndroidErrorOnCustomJavaObject>
+
 	<!-- Ahead-of-time compilation properties -->
 	<AndroidAotMode Condition=" '$(AndroidAotMode)' == '' And '$(AotAssemblies)' == 'True' ">Normal</AndroidAotMode>
 	<AotAssemblies Condition=" '$(AndroidAotMode)' != '' ">True</AotAssemblies>
@@ -1733,6 +1735,7 @@ because xbuild doesn't support framework reference assemblies.
   <GenerateJavaStubs
     ResolvedAssemblies="@(_ResolvedAssemblies)"
     ResolvedUserAssemblies="@(_ResolvedUserAssemblies)"
+    ErrorOnCustomJavaObject="$(AndroidErrorOnCustomJavaObject)"
 	ManifestTemplate="$(_AndroidManifestAbs)"
 	MergedManifestDocuments="@(ExtractedManifestDocuments)"
 	Debug="$(AndroidIncludeDebugSymbols)"