[msbuild] Make sure all the Base/Core tasks are abstract. (#8888)

Also add a cecil test ensure our leaf MSBuild tasks don't have code.

msbuild/Xamarin.Mac.Tasks.Core/Tasks/ALToolUploadTaskBase.cs

@@ -2,7 +2,7 @@ using System.Text;
 
 namespace Xamarin.Mac.Tasks
 {
-	public class ALToolUploadTaskBase : MacDev.Tasks.ALToolTaskBase
+	public abstract class ALToolUploadTaskBase : MacDev.Tasks.ALToolTaskBase
 	{
 		protected override string GenerateCommandLineCommands ()
 		{

msbuild/Xamarin.Mac.Tasks.Core/Tasks/ALToolValidateTaskBase.cs

@@ -2,7 +2,7 @@ using System.Text;
 
 namespace Xamarin.Mac.Tasks
 {
-	public class ALToolValidateTaskBase : MacDev.Tasks.ALToolTaskBase
+	public abstract class ALToolValidateTaskBase : MacDev.Tasks.ALToolTaskBase
 	{
 		protected override string GenerateCommandLineCommands ()
 		{

msbuild/Xamarin.Mac.Tasks.Core/Tasks/EmbedProvisionProfileTaskBase.cs

@@ -11,7 +11,7 @@ using Xamarin.Localization.MSBuild;
 
 namespace Xamarin.Mac.Tasks
 {
-	public class EmbedProvisionProfileTaskBase : XamarinTask
+	public abstract class EmbedProvisionProfileTaskBase : XamarinTask
 	{
 		#region Inputs
 

msbuild/Xamarin.Mac.Tasks.Core/Tasks/MmpTaskBase.cs

@@ -20,7 +20,7 @@ using Xamarin.Localization.MSBuild;
 
 namespace Xamarin.Mac.Tasks
 {
-	public class MmpTaskBase : BundlerToolTaskBase
+	public abstract class MmpTaskBase : BundlerToolTaskBase
 	{
 		protected override string ToolName {
 			get { return "mmp"; }

msbuild/Xamarin.MacDev.Tasks.Core/Tasks/CollectFrameworksBase.cs

@@ -9,7 +9,7 @@ using Xamarin.Localization.MSBuild;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class CollectFrameworksBase : XamarinTask
+	public abstract class CollectFrameworksBase : XamarinTask
 	{
 		#region Inputs
 

msbuild/Xamarin.MacDev.Tasks.Core/Tasks/CreateInstallerPackageTaskBase.cs

@@ -13,7 +13,7 @@ using Xamarin.Localization.MSBuild;
 
 namespace Xamarin.MacDev.Tasks
 {
-	public class CreateInstallerPackageTaskBase : XamarinToolTask
+	public abstract class CreateInstallerPackageTaskBase : XamarinToolTask
 	{
 		#region Inputs
 		[Required]

msbuild/Xamarin.iOS.Tasks.Core/Tasks/ALToolUploadTaskBase.cs

@@ -2,7 +2,7 @@ using System.Text;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class ALToolUploadTaskBase : MacDev.Tasks.ALToolTaskBase
+	public abstract class ALToolUploadTaskBase : MacDev.Tasks.ALToolTaskBase
 	{
 		protected override string GenerateCommandLineCommands ()
 		{

msbuild/Xamarin.iOS.Tasks.Core/Tasks/ALToolValidateTaskBase.cs

@@ -2,7 +2,7 @@ using System.Text;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class ALToolValidateTaskBase : MacDev.Tasks.ALToolTaskBase
+	public abstract class ALToolValidateTaskBase : MacDev.Tasks.ALToolTaskBase
 	{
 		protected override string GenerateCommandLineCommands ()
 		{

msbuild/Xamarin.iOS.Tasks.Core/Tasks/ArchiveTaskBase.cs

@@ -10,7 +10,7 @@ using Xamarin.MacDev.Tasks;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class ArchiveTaskBase : Xamarin.MacDev.Tasks.ArchiveTaskBase
+	public abstract class ArchiveTaskBase : Xamarin.MacDev.Tasks.ArchiveTaskBase
 	{
 		[Required]
 		public ITaskItem[] ITunesSourceFiles { get; set; }

msbuild/Xamarin.iOS.Tasks.Core/Tasks/CollectAssetPacksTaskBase.cs

@@ -9,7 +9,7 @@ using Xamarin.MacDev.Tasks;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class CollectAssetPacksTaskBase : XamarinTask
+	public abstract class CollectAssetPacksTaskBase : XamarinTask
 	{
 		#region Inputs
 

msbuild/Xamarin.iOS.Tasks.Core/Tasks/CollectITunesSourceFilesTaskBase.cs

@@ -9,7 +9,7 @@ using Xamarin.MacDev.Tasks;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class CollectITunesSourceFilesTaskBase : XamarinTask
+	public abstract class CollectITunesSourceFilesTaskBase : XamarinTask
 	{
 		static readonly string[] iTunesFileNames = { "iTunesMetadata.plist", "iTunesArtwork@2x", "iTunesArtwork" };
 

msbuild/Xamarin.iOS.Tasks.Core/Tasks/CreateAssetPackTaskBase.cs

@@ -7,7 +7,7 @@ using Xamarin.MacDev.Tasks;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class CreateAssetPackTaskBase : XamarinToolTask
+	public abstract class CreateAssetPackTaskBase : XamarinToolTask
 	{
 		#region Inputs
 

msbuild/Xamarin.iOS.Tasks.Core/Tasks/WriteAssetPackManifestTaskBase.cs

@@ -7,7 +7,7 @@ using Xamarin.MacDev.Tasks;
 
 namespace Xamarin.iOS.Tasks
 {
-	public class WriteAssetPackManifestTaskBase : XamarinTask
+	public abstract class WriteAssetPackManifestTaskBase : XamarinTask
 	{
 		#region Inputs
 

tests/cecil-tests/Helper.cs

@@ -18,12 +18,16 @@ namespace Cecil.Tests {
 		static Dictionary<string, AssemblyDefinition> cache = new Dictionary<string, AssemblyDefinition> ();
 
 		// make sure we load assemblies only once into memory
-		public static AssemblyDefinition? GetAssembly (string assembly)
+		public static AssemblyDefinition? GetAssembly (string assembly, ReaderParameters? parameters = null)
 		{
 			if (!File.Exists (assembly))
 				return null;
 			if (!cache.TryGetValue (assembly, out var ad)) {
+				if (parameters == null) {
 					ad = AssemblyDefinition.ReadAssembly (assembly);
+				} else {
+					ad = AssemblyDefinition.ReadAssembly (assembly, parameters);
+				}
 				cache.Add (assembly, ad);
 			}
 			return ad;
@@ -72,5 +76,19 @@ namespace Cecil.Tests {
 				yield return new TestCaseData (Configuration.XamarinMacFullDll);
 			}
 		}
+
+		public static IEnumerable TaskAssemblies {
+			get {
+				yield return CreateTestFixtureDataFromPath (Path.Combine (Configuration.SdkRootXI, "lib", "msbuild", "iOS", "Xamarin.iOS.Tasks.dll"));
+				yield return CreateTestFixtureDataFromPath (Path.Combine (Configuration.SdkRootXM, "lib", "msbuild",  "Xamarin.Mac.Tasks.dll"));
+			}
+		}
+
+		static TestFixtureData CreateTestFixtureDataFromPath (string path)
+		{
+			var rv = new TestFixtureData (path);
+			rv.SetArgDisplayNames (Path.GetFileName (path));
+			return rv;
+		}
 	}
 }

tests/cecil-tests/TaskAssemblyTests.cs

@@ -0,0 +1,169 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+
+using NUnit.Framework;
+
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+
+#nullable enable
+
+namespace Cecil.Tests {
+	[TestFixture]
+	[TestFixtureSource (typeof (Helper), "TaskAssemblies")]
+	public class TaskAssemblyTests {
+		string assembly;
+
+		public TaskAssemblyTests (string assembly)
+		{
+			this.assembly = assembly;
+		}
+
+		bool IsTaskType (TypeDefinition? td)
+		{
+			if (td == null)
+				return false;
+
+			if (td.HasInterfaces) {
+				foreach (var iface in td.Interfaces) {
+					if (iface.InterfaceType.Namespace == "Microsoft.Build.Framework" && iface.InterfaceType.Name == "ITask")
+						return true;
+				}
+			}
+
+			return IsTaskType (td.BaseType?.Resolve ());
+		}
+
+		[Test]
+		public void EnsureOnlyCodeInBaseTasks ()
+		{
+			var parameters = new ReaderParameters (ReadingMode.Deferred);
+			var resolver = new DefaultAssemblyResolver ();
+			resolver.AddSearchDirectory ("/Library/Frameworks/Mono.framework/Versions/Current/lib/mono/msbuild/Current/bin");
+			parameters.AssemblyResolver = resolver;
+
+			var asm = Helper.GetAssembly (assembly, parameters)!;
+			var checking_types = new List<TypeDefinition> ();
+			var should_be_abstract = new List<string> ();
+			foreach (var type in asm.MainModule.Types.OrderBy (v => v.FullName)) {
+				if (!IsTaskType (type)) {
+					// Console.WriteLine ($"ℹ️ {type.FullName} does not implement ITask");
+					continue;
+				}
+
+				if (type.IsAbstract)
+					continue;
+
+				// All the Base and Core classes should be abstract
+				if (type.Name.EndsWith ("Base", StringComparison.Ordinal) || type.Name.EndsWith ("Core", StringComparison.Ordinal)) {
+					should_be_abstract.Add (type.FullName);
+					continue;
+				}
+
+				checking_types.Add (type);
+			}
+
+			var failures = new List<string> ();
+			foreach (var type in checking_types) {
+				// We don't care about default constructors
+				var methods = type.Methods.Where (v => {
+					if (v.IsConstructor && v.Parameters.Count == 0 && v.HasBody) {
+						var skipNop = new Func<Instruction, Instruction?> (v => {
+							if (v == null)
+								return null;
+							while (v.OpCode.Code == Code.Nop)
+								v = v.Next;
+							return v;
+						});
+						// There should be this sequence of instructions, otherwise the default constructor has user code, and shouldn't be ignored:
+						var ins = skipNop (v.Body.Instructions [0]);
+						if (ins?.OpCode.Code != Code.Ldarg_0)
+							return true;
+						ins = skipNop (ins.Next);
+						if (ins?.OpCode.Code != Code.Call)
+							return true;
+						ins = skipNop (ins.Next);
+						if (ins?.OpCode.Code != Code.Ret)
+							return true;
+						ins = skipNop (ins.Next);
+						if (ins != null)
+							return true;
+						return false;
+					}
+					return true;
+				});
+
+				var failed = methods.Any () || type.HasProperties || type.HasFields;
+				var known_failure = IsKnownFailure (type.FullName);
+
+				if (!failed) {
+					if (known_failure) {
+						failures.Add ($"{type.FullName} is marked as a known failure when it didn't fail. Maybe the list of known failures need to be updated?");
+						continue;
+					}
+					continue; // We successfully succeeded!
+				}
+
+				if (failed && known_failure) {
+					Console.WriteLine ($"⚠️  {type.FullName} is known failure");
+					continue;
+				}
+
+				// We failed, and this type is not a known failure
+				var sb = new StringBuilder ();
+				sb.AppendLine ($"{type.FullName} is not an empty type:");
+				if (methods.Any ()) {
+					sb.AppendLine ($"    It has {methods.Count ()} methods:");
+					foreach (var method in methods)
+						sb.AppendLine ($"        {method.FullName}");
+				}
+				if (type.HasProperties) {
+					sb.AppendLine ($"    It has {type.Properties.Count} properties:");
+					foreach (var property in type.Properties)
+						sb.AppendLine ($"        {property.Name}");
+				}
+				if (type.HasProperties) {
+					sb.AppendLine ($"    It has {type.Fields.Count} fields:");
+					foreach (var field in type.Fields)
+						sb.AppendLine ($"        {field.Name}");
+				}
+				failures.Add (sb.ToString ());
+				Console.WriteLine ($"❌ {sb}");
+			}
+
+			Assert.That (failures, Is.Empty, "Types with code");
+			Assert.That (checking_types.Count, Is.AtLeast (50), "Checked types"); // Make sure the initial type filtering doesn't filter away too much by mistake
+			Assert.That (should_be_abstract, Is.Empty, "Classes that should be abstract");
+		}
+
+		bool IsKnownFailure (string typename)
+		{
+			switch (typename) {
+			case "Xamarin.MacDev.Tasks.UnpackLibraryResources":
+				return true;
+			}
+
+			switch (Path.GetFileNameWithoutExtension (assembly)) {
+			case "Xamarin.iOS.Tasks":
+				// No other known failures
+				return false;
+			case "Xamarin.Mac.Tasks":
+				switch (typename) {
+				case "Xamarin.Mac.Tasks.CodesignVerify":
+				case "Xamarin.Mac.Tasks.CompileEntitlements":
+				case "Xamarin.Mac.Tasks.IBTool":
+				case "Xamarin.Mac.Tasks.Metal":
+				case "Xamarin.Mac.Tasks.MetalLib":
+					return true;
+				}
+				return false;
+			default:
+				throw new NotImplementedException ($"Unknown assembly: {assembly}");
+			}
+		}
+	}
+}
+