Display an error if an ITaskFactory does not set the TaskType property.

If TaskType is null it can cause a NullReferenceException in our code.  This change ensures that ITaskFactory implementations return a value for this property.

Fixes dotnet#1508

src/Build.UnitTests/BackEnd/TaskRegistry_Tests.cs

@@ -23,6 +23,7 @@
 using InvalidProjectFileException = Microsoft.Build.Exceptions.InvalidProjectFileException;
 using Microsoft.Build.Utilities;
 using Microsoft.CodeAnalysis.BuildTasks;
+using Shouldly;
 using Xunit;
 
 namespace Microsoft.Build.UnitTests.BackEnd
@@ -1221,6 +1222,23 @@ public void NoChildrenElements()
             Assert.Equal(0, registeredTaskRecords[0].ParameterGroupAndTaskBody.UsingTaskParameters.Count);
             Assert.Null(registeredTaskRecords[0].ParameterGroupAndTaskBody.InlineTaskXmlBody);
         }
+
+        [Fact]
+        public void TaskFactoryWithNullTaskTypeLogsError()
+        {
+            List<ProjectUsingTaskElement> elementList = new List<ProjectUsingTaskElement>();
+            ProjectRootElement project = ProjectRootElement.Create();
+
+            ProjectUsingTaskElement element = project.AddUsingTask("Foo", Assembly.GetExecutingAssembly().Location, null);
+            element.TaskFactory = typeof(TaskRegistry_Tests.NullTaskTypeTaskFactory).FullName;
+            elementList.Add(element);
+
+            TaskRegistry registry = CreateTaskRegistryAndRegisterTasks(elementList);
+
+            InvalidProjectFileException exception = Should.Throw<InvalidProjectFileException>(() => registry.GetRegisteredTask("Foo", "none", null, false, new TargetLoggingContext(_loggingService, new BuildEventContext(1, 1, BuildEventContext.InvalidProjectContextId, 1)), ElementLocation.Create("none", 1, 2)));
+
+            exception.ErrorCode.ShouldBe("MSB4239");
+        }
         #endregion
 
         #region ParameterGroupTests
@@ -2310,5 +2328,23 @@ public IDictionary CloneCustomMetadata()
         }
 
         #endregion
+
+        /// <summary>
+        /// A task factory that returns null for the TaskType property.
+        /// </summary>
+        public class NullTaskTypeTaskFactory : ITaskFactory
+        {
+            public string FactoryName => nameof(NullTaskTypeTaskFactory);
+
+            public Type TaskType => null;
+
+            public bool Initialize(string taskName, IDictionary<string, TaskPropertyInfo> parameterGroup, string taskBody, IBuildEngine taskFactoryLoggingHost) => true;
+
+            public TaskPropertyInfo[] GetTaskParameters() => null;
+
+            public ITask CreateTask(IBuildEngine taskFactoryLoggingHost) => null;
+
+            public void CleanupTask(ITask task) { }
+        }
     }
 }

src/Build/Instance/TaskRegistry.cs

@@ -1411,6 +1411,12 @@ private bool GetTaskFactory(TargetLoggingContext targetLoggingContext, ElementLo
                                                 RegisteredName);
                                         }
                                     }
+
+                                    // Throw an error if the ITaskFactory did not set the TaskType property.  If the property is null, it can cause NullReferenceExceptions in our code
+                                    if (factory.TaskType == null)
+                                    {
+                                        ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "TaskFactoryTaskTypeIsNotSet", factory.FactoryName);
+                                    }
                                 }
                                 finally
                                 {
@@ -1425,6 +1431,10 @@ private bool GetTaskFactory(TargetLoggingContext targetLoggingContext, ElementLo
                                     return false;
                                 }
                             }
+                            catch(InvalidProjectFileException)
+                            {
+                                throw;
+                            }
                             catch (InvalidCastException e)
                             {
                                 string message = String.Empty;

src/Build/Resources/Strings.resx

@@ -1085,6 +1085,12 @@
       LOCALIZATION: The prefix "MSBUILD : error MSBxxxx:" should not be localized.
     </comment>
   </data>
+  <data name="TaskFactoryTaskTypeIsNotSet" UESanitized="false" Visibility="Public">
+    <value>MSB4239: The task factory "{0}" must return a value for the "TaskType" property.</value>
+    <comment>{StrBegin="MSB4239: "}
+      LOCALIZATION: The prefix "MSBUILD : error MSBxxxx:" should not be localized.
+    </comment>
+  </data>
   <data name="TaskLoadFailure" UESanitized="false" Visibility="Public">
     <value>MSB4062: The "{0}" task could not be loaded from the assembly {1}. {2} Confirm that the &lt;UsingTask&gt; declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask.</value>
     <comment>{StrBegin="MSB4062: "}UE: This message is shown when a task cannot be loaded from its assembly for various reasons e.g. corrupt assembly,
@@ -1628,7 +1634,7 @@ Utilization:          {0} Average Utilization: {1:###.0}</value>
         MSB4128 is being used in FileLogger.cs (can't be added here yet as strings are currently frozen)
         MSB4129 is used by Shared\XmlUtilities.cs (can't be added here yet as strings are currently frozen)
 
-        Next message code should be MSB4239.
+        Next message code should be MSB4240.
               
         Some unused codes which can also be reused (because their messages were deleted, and UE hasn't indexed the codes yet):
             <none>

src/Framework/ITaskFactory.cs

@@ -23,7 +23,7 @@ public interface ITaskFactory
         string FactoryName { get; }
 
         /// <summary>
-        /// Gets the type of the task this factory will instantiate.
+        /// Gets the type of the task this factory will instantiate.  Implementations must return a value for this property.
         /// </summary>
         Type TaskType { get; }