From b6dc7e1f6ad4e72acc63f79460308c644a45343b Mon Sep 17 00:00:00 2001
From: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
Date: Thu, 26 Apr 2018 00:04:45 -0400
Subject: [PATCH] Support loading parameters from properties for Type=Class.

When authoring a Task in an assembly, task parameters are declared
simply by creating public properties. CodeTaskFactory instead requires
the user to manually declare all properties in a <ParameterGroup/>
element. This is tedious when using Type="Class" because the
parameters must be listed both as property declarations and in
<ParametersGroup/>. Also, this requirement increases maintenance when
using inheritence to extend or intercept an existing Task from another
assembly.

This change causes simple reflection to be used to calculate the parameters
for all Type="Class" tasks. A warning is emitted if <ParameterGroup/> is
specified and it is ignored.

Fixes #33.
---
 Samples/Directory.Build.targets               | 27 +++++++++++++++++
 .../CodeTaskFactoryTests.cs                   | 21 +++++++++++++-
 src/RoslynCodeTaskFactory/CodeTaskFactory.cs  | 29 +++++++++++++++++++
 .../Properties/Strings.Designer.cs            |  9 ++++++
 .../Properties/Strings.resx                   |  3 ++
 5 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/Samples/Directory.Build.targets b/Samples/Directory.Build.targets
index dd7fa89..ed1082d 100644
--- a/Samples/Directory.Build.targets
+++ b/Samples/Directory.Build.targets
@@ -64,6 +64,31 @@
     </Task>
   </UsingTask>
 
+  <!-- A simple in-line class task which inherits parameters from its base class -->
+  <UsingTask TaskName="InterceptingCopy"
+             TaskFactory="CodeTaskFactory"
+             AssemblyFile="$(RoslynCodeTaskFactory)"
+             Condition=" '$(RoslynCodeTaskFactory)' != '' ">
+    <Task>
+      <Reference Include="Microsoft.Build.Tasks.Core"/>
+      <Code Type="Class" Language="cs">
+        <![CDATA[
+          using Microsoft.Build.Framework;
+          using System;
+
+          public class InterceptingCopy : Microsoft.Build.Tasks.Copy
+          {
+            public override bool Execute()
+            {
+              Log.LogMessage(MessageImportance.High, "Copying '{0}' to '{1}'", String.Join(", ", (object[])SourceFiles), String.Join(", ", (object[])DestinationFiles));
+              return base.Execute();
+            }
+          }
+        ]]>
+      </Code>
+    </Task>
+  </UsingTask>
+
   <Target Name="RunSampleTask" AfterTargets="Build">
     <!-- Call the compiled task -->
     <MySample Parameter1="A value for parameter 1" Parameter2="A value for parameter 2">
@@ -84,6 +109,8 @@
     </PathGetFileName>
 
     <Message Text="File name: '$(MyFileName)'" Importance="High" />
+
+    <InterceptingCopy SourceFiles="Class1.cs" DestinationFiles="$(OutputPath)CopyOfClass1.cs" />
   </Target>
 
 </Project>
\ No newline at end of file
diff --git a/src/RoslynCodeTaskFactory.UnitTests/CodeTaskFactoryTests.cs b/src/RoslynCodeTaskFactory.UnitTests/CodeTaskFactoryTests.cs
index f371815..29c824b 100644
--- a/src/RoslynCodeTaskFactory.UnitTests/CodeTaskFactoryTests.cs
+++ b/src/RoslynCodeTaskFactory.UnitTests/CodeTaskFactoryTests.cs
@@ -152,6 +152,21 @@ public void CodeLanguageFromTaskBody()
             TryLoadTaskBodyAndExpectSuccess("<Code Language=\"ViSuAl BaSic\">code</Code>", expectedCodeLanguage: "VB");
         }
 
+        [Test]
+        public void CodeTypeClassIgnoresParameterGroupWarning()
+        {
+            TryLoadTaskBodyAndExpectSuccess(
+                taskBody: "<Code Type=\"Class\">code</Code>",
+                parameters: new[]
+                {
+                    new TaskPropertyInfo("P", typeof(string), false, false),
+                },
+                expectedWarningMessages: new[]
+                {
+                    "Parameters are discovered through reflection for Type=\"Class\". Values specified in <ParameterGroup/> will be ignored.",
+                });
+        }
+
         [Test]
         public void CodeTypeFromTaskBody()
         {
@@ -466,7 +481,8 @@ private void TryLoadTaskBodyAndExpectSuccess(
             ISet<string> expectedNamespaces = null,
             string expectedCodeLanguage = null,
             CodeTaskFactoryCodeType? expectedCodeType = null,
-            string expectedSourceCode = null)
+            string expectedSourceCode = null,
+            IReadOnlyList<string> expectedWarningMessages = null)
         {
             MockBuildEngine buildEngine = new MockBuildEngine();
 
@@ -478,6 +494,7 @@ private void TryLoadTaskBodyAndExpectSuccess(
             bool success = CodeTaskFactory.TryLoadTaskBody(log, TaskName, taskBody, parameters ?? new List<TaskPropertyInfo>(), out TaskInfo taskInfo);
 
             buildEngine.Errors.ShouldBe(new string[0]);
+            buildEngine.Warnings.ShouldBe(expectedWarningMessages ?? new string[0]);
 
             Assert.True(success);
 
@@ -523,6 +540,8 @@ private sealed class MockBuildEngine : IBuildEngine
 
             public string ProjectFileOfTaskNode { get; } = String.Empty;
 
+            public IEnumerable<string> Warnings => Events.OfType<BuildWarningEventArgs>().Select(i => i.Message);
+
             public bool BuildProjectFile(string projectFileName, string[] targetNames, IDictionary globalProperties, IDictionary targetOutputs)
             {
                 throw new NotSupportedException();
diff --git a/src/RoslynCodeTaskFactory/CodeTaskFactory.cs b/src/RoslynCodeTaskFactory/CodeTaskFactory.cs
index 499df24..76f6021 100644
--- a/src/RoslynCodeTaskFactory/CodeTaskFactory.cs
+++ b/src/RoslynCodeTaskFactory/CodeTaskFactory.cs
@@ -187,6 +187,28 @@ public bool Initialize(string taskName, IDictionary<string, TaskPropertyInfo> pa
                 TaskType = assembly.GetExportedTypes().FirstOrDefault(type => type.Name.Equals(taskName));
             }
 
+            if (TaskType != null)
+            {
+                // Perform automatic parameter detection if the user supplied a class.
+                // This reduces the burden of the developer by not requiring them to
+                // manually specify <ParameterGroup/>.
+                //
+                if (taskInfo.CodeType == CodeTaskFactoryCodeType.Class)
+                {
+                    PropertyInfo[] properties = TaskType.GetProperties(BindingFlags.Instance | BindingFlags.Public);
+                    _parameters = new TaskPropertyInfo[properties.Length];
+                    for (int i = 0; i < properties.Length; i++)
+                    {
+                        PropertyInfo property = properties[i];
+                        _parameters[i] = new TaskPropertyInfo(
+                            property.Name,
+                            property.PropertyType,
+                            property.GetCustomAttribute<OutputAttribute>() != null,
+                            property.GetCustomAttribute<RequiredAttribute>() != null);
+                    }
+                }
+            }
+
             AppDomain.CurrentDomain.AssemblyResolve += AppDomain_AssemblyResolve;
 
             // Initialization succeeded if we found a type matching the task name from the compiled assembly
@@ -395,6 +417,13 @@ internal static bool TryLoadTaskBody(TaskLoggingHelper log, string taskName, str
                 taskInfo.CodeType = codeType;
             }
 
+            // Warn that <ParameterGroup/> is ignored if any parameters are supplied when Type="Class".
+            //
+            if (taskInfo.CodeType == CodeTaskFactoryCodeType.Class && parameters.Any())
+            {
+                log.LogWarningWithCodeFromResources("CodeTaskFactory_ParameterGroupIgnoredForCodeTypeClass");
+            }
+
             if (languageAttribute != null)
             {
                 if (String.IsNullOrWhiteSpace(languageAttribute.Value))
diff --git a/src/RoslynCodeTaskFactory/Properties/Strings.Designer.cs b/src/RoslynCodeTaskFactory/Properties/Strings.Designer.cs
index 828f2a8..9675260 100644
--- a/src/RoslynCodeTaskFactory/Properties/Strings.Designer.cs
+++ b/src/RoslynCodeTaskFactory/Properties/Strings.Designer.cs
@@ -159,6 +159,15 @@ internal static string CodeTaskFactory_NoSourceCode {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Parameters are discovered through reflection for Type="Class". Values specified in <ParameterGroup/> will be ignored..
+        /// </summary>
+        internal static string CodeTaskFactory_ParameterGroupIgnoredForCodeTypeClass {
+            get {
+                return ResourceManager.GetString("CodeTaskFactory_ParameterGroupIgnoredForCodeTypeClass", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Waiting for debugger to attach ({0} PID {1}).  Press enter to continue....
         /// </summary>
diff --git a/src/RoslynCodeTaskFactory/Properties/Strings.resx b/src/RoslynCodeTaskFactory/Properties/Strings.resx
index f9e148b..2346bd6 100644
--- a/src/RoslynCodeTaskFactory/Properties/Strings.resx
+++ b/src/RoslynCodeTaskFactory/Properties/Strings.resx
@@ -159,6 +159,9 @@
     <value>MSB3428: You must specify source code within the Code element or a path to a file containing source code.</value>
     <comment>{StrBegin="MSB3428: "}</comment>
   </data>
+  <data name="CodeTaskFactory_ParameterGroupIgnoredForCodeTypeClass" xml:space="preserve">
+    <value>Parameters are discovered through reflection for Type="Class". Values specified in &lt;ParameterGroup/&gt; will be ignored.</value>
+  </data>
   <data name="CodeTaskFactory_WaitingForDebugger" xml:space="preserve">
     <value>Waiting for debugger to attach ({0} PID {1}).  Press enter to continue...</value>
   </data>