Fix #252

* Create BasicSimplifyTransformationSet * Add more tracing to transformations and topological sort * Fix wizard logic to use correct transformation sets
hvanbakel · Jul 19, 2019 · df8d833 · df8d833
1 parent 20e717a
commit df8d833
Show file tree

Hide file tree

Showing 11 changed files with 167 additions and 70 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -1,3 +1,5 @@
+root = true
+
 [*]
 indent_style = tab
 tab_width = 4

diff --git a/Directory.Build.props b/Directory.Build.props
@@ -10,7 +10,7 @@
 		<Copyright>Copyright Hans van Bakel</Copyright>
 		<Description>Tool for converting a MSBuild project file to VS2017 format and beyond.</Description>
 		<PackageTags>dotnet csproj fsproj vbproj msbuild conversion vs2015 vs14 vs15 vs2017</PackageTags>
-		<Version>4.1.0-beta.2</Version>
+		<Version>4.1.0-beta.4</Version>
 		<AssemblyVersion>4.1.0.0</AssemblyVersion>
 
 		<!-- SourceLink -->
@@ -20,7 +20,7 @@
 	</PropertyGroup>
 
 	<ItemGroup>
-		<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-19270-01" PrivateAssets="All"/>
+		<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-19367-01" PrivateAssets="All"/>
 	</ItemGroup>
 
 </Project>
diff --git a/Project2015To2017.Core/ExtensionsGraph.cs b/Project2015To2017.Core/ExtensionsGraph.cs
@@ -1,13 +1,15 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.Extensions.Logging;
+using Project2015To2017.Definition;
 using Project2015To2017.Transforms;
 
 namespace Project2015To2017
 {
 	public static partial class Extensions
 	{
-		public static IReadOnlyCollection<ITransformation> IterateTransformations(this ITransformationSet set,
+		public static IReadOnlyCollection<ITransformation> CollectAndOrderTransformations(this ITransformationSet set,
 			ILogger logger, ConversionOptions conversionOptions)
 		{
 			var all = set.Transformations(logger, conversionOptions);
@@ -18,9 +20,59 @@ public static IReadOnlyCollection<ITransformation> IterateTransformations(this I
 			TopologicalSort(early, res, logger);
 			TopologicalSort(normal, res, logger);
 			TopologicalSort(late, res, logger);
+
+			foreach (var (transformation, index) in res.Select((x, i) => (x, i)))
+			{
+				logger.LogTrace("{Index}: {TransformName}", index + 1, transformation.GetType().Name);
+			}
+
 			return res;
 		}
 
+		/// <summary>
+		/// Choose and order all transformations from set that are suitable for the project type.
+		/// </summary>
+		/// <param name="set">Transformation set to choose from</param>
+		/// <param name="project">Project to be based upon when determining suitability</param>
+		/// <param name="logger">Logger for transformations to use</param>
+		/// <param name="conversionOptions">Conversion options for transformations to use, or to override suitability choice</param>
+		/// <returns></returns>
+		public static IEnumerable<ITransformation> IterateSuitableTransformations(this ITransformationSet set,
+			Project project, ILogger logger, ConversionOptions conversionOptions)
+		{
+			return set.CollectAndOrderTransformations(logger, conversionOptions).WhereSuitable(project, conversionOptions);
+		}
+
+		/// <summary>
+		/// Choose all transformations from sequence that are suitable for the project type.
+		/// </summary>
+		/// <param name="source">Sequence to choose from</param>
+		/// <param name="project">Project to be based upon when determining suitability</param>
+		/// <param name="conversionOptions">Conversion options for transformations to use, or to override suitability choice</param>
+		/// <returns></returns>
+		public static IEnumerable<ITransformation> WhereSuitable(this IEnumerable<ITransformation> source,
+			Project project, ConversionOptions conversionOptions)
+		{
+			foreach (var transformation in source)
+			{
+				if (project.IsModernProject
+				    && transformation is ILegacyOnlyProjectTransformation
+				    && !conversionOptions.ForceDefaultTransforms.Contains(transformation.GetType().Name))
+				{
+					continue;
+				}
+
+				if (!project.IsModernProject
+				    && transformation is IModernOnlyProjectTransformation
+				    && !conversionOptions.ForceDefaultTransforms.Contains(transformation.GetType().Name))
+				{
+					continue;
+				}
+
+				yield return transformation;
+			}
+		}
+
 		private static void TopologicalSort(
 			IReadOnlyList<ITransformation> source,
 			ICollection<ITransformation> target,

diff --git a/Project2015To2017.Core/ProjectConverter.cs b/Project2015To2017.Core/ProjectConverter.cs
@@ -91,22 +91,8 @@ public Project ProcessProjectFile(FileInfo file, Solution solution, ProjectRefer
 				transform.Transform(project);
 			}
 
-			foreach (var transform in transformationSet.IterateTransformations(logger, conversionOptions))
+			foreach (var transform in transformationSet.IterateSuitableTransformations(project, logger, conversionOptions))
 			{
-				if (project.IsModernProject
-					&& transform is ILegacyOnlyProjectTransformation
-					&& !conversionOptions.ForceDefaultTransforms.Contains(transform.GetType().Name))
-				{
-					continue;
-				}
-
-				if (!project.IsModernProject
-					&& transform is IModernOnlyProjectTransformation
-					&& !conversionOptions.ForceDefaultTransforms.Contains(transform.GetType().Name))
-				{
-					continue;
-				}
-
 				transform.Transform(project);
 			}
 

diff --git a/Project2015To2017.Core/Transforms/BasicSimplifyTransformationSet.cs b/Project2015To2017.Core/Transforms/BasicSimplifyTransformationSet.cs
@@ -0,0 +1,30 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+using Microsoft.Extensions.Logging;
+
+namespace Project2015To2017.Transforms
+{
+	public sealed class BasicSimplifyTransformationSet : ITransformationSet
+	{
+		private readonly Version targetVisualStudioVersion;
+
+		public BasicSimplifyTransformationSet(Version targetVisualStudioVersion)
+		{
+			this.targetVisualStudioVersion = targetVisualStudioVersion;
+		}
+
+		public IReadOnlyCollection<ITransformation> Transformations(
+			ILogger logger,
+			ConversionOptions conversionOptions)
+		{
+			return new ITransformation[]
+			{
+				new PropertyDeduplicationTransformation(),
+				new PropertySimplificationTransformation(targetVisualStudioVersion),
+				new PrimaryProjectPropertiesUpdateTransformation(),
+				new EmptyGroupRemoveTransformation(),
+			};
+		}
+	}
+}
diff --git a/Project2015To2017.Migrate2017.Library/Transforms/FileTransformation.cs b/Project2015To2017.Migrate2017.Library/Transforms/FileTransformation.cs
@@ -38,6 +38,14 @@ public sealed class FileTransformation : ILegacyOnlyProjectTransformation
 			"Antlr3"
 		};
 
+		private static readonly IReadOnlyCollection<string> EnableDefaultItemsPropertyNames = new[]
+		{
+			"EnableDefaultItems",
+			"EnableDefaultCompileItems",
+			"EnableDefaultEmbeddedResourceItems",
+			"EnableDefaultNoneItems",
+		};
+
 		private readonly ILogger logger;
 
 		public FileTransformation(ILogger logger = null)
@@ -64,46 +72,63 @@ public void Transform(Project definition)
 
 			foreach (var includeMatchingWildcard in otherIncludedFilesMatchingWildcard)
 			{
-				logger.LogTrace("Excluding non-Compile code item: " + includeMatchingWildcard);
+				logger.LogTrace("Excluding non-Compile code item: {Item}", includeMatchingWildcard);
 			}
 
-			var wildcardIncludes = keepItems
-				.Where(x => x.Name.LocalName == "Compile")
-				.Select(x => x.ExtractIncludeItemPath().value)
-				.Where(x => x != null && x.Contains("*"))
-				.ToImmutableArray();
-			if (wildcardIncludes.Length > 0)
+			if (definition.IsModernProject)
 			{
-				logger.LogWarning(
-					"Wildcard include detected, please check for erroneous inclusion of additional files.");
-
-				foreach (var wildcard in wildcardIncludes)
+				var foundDefaultWildcardSwitch = EnableDefaultItemsPropertyNames.FirstOrDefault(x => definition.Property(x, true) != null);
+				if (!string.IsNullOrEmpty(foundDefaultWildcardSwitch))
 				{
-					logger.LogTrace("Wildcard include: " + wildcard);
+					logger.LogDebug("Modern project has some default wildcards disabled ({Switch}); still, no glob search will be performed to avoid introducing errors", foundDefaultWildcardSwitch);
+				}
+				else
+				{
+					logger.LogTrace("Modern project has default wildcards, no glob search will be performed");
 				}
 			}
 			else
 			{
-				var referencedItems = definition.ItemGroups
-					.SelectMany(x => x.Elements())
+				var wildcardIncludes = keepItems
 					.Where(x => x.Name.LocalName == "Compile")
 					.Select(x => x.ExtractIncludeItemPath().value)
-					.Where(x => !string.IsNullOrEmpty(x));
-
-				// Do file search, find all real files matching glob pattern
-				var nonReferencedWildcardMatchingItems = definition.FindAllWildcardFiles(definition.CodeFileExtension)
-					.Select(x => definition.ProjectFolder.GetRelativePathTo(x))
-					.Except(referencedItems, Extensions.PathEqualityComparer)
+					.Where(x => x != null && x.Contains("*"))
 					.ToImmutableArray();
+				if (wildcardIncludes.Length > 0)
+				{
+					logger.LogWarning(
+						"Wildcard include detected, please check for erroneous inclusion of additional files.");
 
-				foreach (var includeMatchingWildcard in nonReferencedWildcardMatchingItems)
+					foreach (var wildcard in wildcardIncludes)
+					{
+						logger.LogTrace("Wildcard include: {Item}", wildcard);
+					}
+				}
+				else
 				{
-					logger.LogTrace("Excluding non-Compile-referenced wildcard-matching item: " + includeMatchingWildcard);
+					var referencedItems = definition.ItemGroups
+						.SelectMany(x => x.Elements())
+						.Where(x => x.Name.LocalName == "Compile")
+						.Select(x => x.ExtractIncludeItemPath().value)
+						.Where(x => !string.IsNullOrEmpty(x));
+
+					// Do file search, find all real files matching glob pattern
+					var nonReferencedWildcardMatchingItems = definition
+						.FindAllWildcardFiles(definition.CodeFileExtension)
+						.Select(x => definition.ProjectFolder.GetRelativePathTo(x))
+						.Except(referencedItems, Extensions.PathEqualityComparer)
+						.ToImmutableArray();
+
+					foreach (var includeMatchingWildcard in nonReferencedWildcardMatchingItems)
+					{
+						logger.LogTrace("Excluding non-Compile-referenced wildcard-matching item: {Item}",
+						                includeMatchingWildcard);
+					}
+
+					otherIncludedFilesMatchingWildcard = otherIncludedFilesMatchingWildcard
+						.Union(nonReferencedWildcardMatchingItems, Extensions.PathEqualityComparer)
+						.ToImmutableArray();
 				}
-
-				otherIncludedFilesMatchingWildcard = otherIncludedFilesMatchingWildcard
-					.Union(nonReferencedWildcardMatchingItems, Extensions.PathEqualityComparer)
-					.ToImmutableArray();
 			}
 
 			if (otherIncludedFilesMatchingWildcard.Length > 0)
@@ -131,7 +156,7 @@ public void Transform(Project definition)
 				x.Remove();
 			}
 
-			logger.LogDebug($"Removed {count} include items thanks to Microsoft.NET.Sdk defaults");
+			logger.LogDebug("Removed {Count} include items thanks to Microsoft.NET.Sdk defaults", count);
 		}
 
 		private static bool KeepFileInclusion(XElement x, Project project)

diff --git a/Project2015To2017.Migrate2017.Library/Vs15ModernizationTransformationSet.cs b/Project2015To2017.Migrate2017.Library/Vs15ModernizationTransformationSet.cs
@@ -12,6 +12,7 @@ public sealed class Vs15ModernizationTransformationSet : ITransformationSet
 
 		public static readonly ITransformationSet Instance = new ChainTransformationSet(
 			BasicReadTransformationSet.Instance,
+			new BasicSimplifyTransformationSet(Vs15TransformationSet.TargetVisualStudioVersion),
 			TrueInstance);
 
 		private Vs15ModernizationTransformationSet()
@@ -24,10 +25,6 @@ public IReadOnlyCollection<ITransformation> Transformations(
 		{
 			return new ITransformation[]
 			{
-				// Generic
-				new PrimaryProjectPropertiesUpdateTransformation(),
-				new EmptyGroupRemoveTransformation(),
-				// Modernization
 				new UpgradeDebugTypeTransformation(),
 				new UpgradeUseDefaultOutputPathTransformation(),
 				new UpgradeUseComVisibleDefaultTransformation(),

diff --git a/Project2015To2017.Migrate2017.Library/Vs15TransformationSet.cs b/Project2015To2017.Migrate2017.Library/Vs15TransformationSet.cs
@@ -8,10 +8,13 @@ namespace Project2015To2017.Migrate2017
 {
 	public sealed class Vs15TransformationSet : ITransformationSet
 	{
+		public static readonly Version TargetVisualStudioVersion = new Version(15, 0);
+
 		public static readonly Vs15TransformationSet TrueInstance = new Vs15TransformationSet();
 
 		public static readonly ITransformationSet Instance = new ChainTransformationSet(
 			BasicReadTransformationSet.Instance,
+			new BasicSimplifyTransformationSet(TargetVisualStudioVersion),
 			TrueInstance);
 
 		private Vs15TransformationSet()
@@ -22,17 +25,12 @@ public IReadOnlyCollection<ITransformation> Transformations(
 			ILogger logger,
 			ConversionOptions conversionOptions)
 		{
-			var targetVisualStudioVersion = new Version(15, 0);
 			return new ITransformation[]
 			{
 				// Generic
 				new TargetFrameworkReplaceTransformation(
 					conversionOptions.TargetFrameworks,
 					conversionOptions.AppendTargetFrameworkToOutputPath),
-				new PropertyDeduplicationTransformation(),
-				new PropertySimplificationTransformation(targetVisualStudioVersion),
-				new PrimaryProjectPropertiesUpdateTransformation(),
-				new EmptyGroupRemoveTransformation(),
 				// VS15 migration
 				new FrameworkReferencesTransformation(),
 				new TestProjectPackageReferenceTransformation(logger),

diff --git a/Project2015To2017.Migrate2017.Tool/CommandLogic.cs b/Project2015To2017.Migrate2017.Tool/CommandLogic.cs
@@ -100,6 +100,11 @@ public void ExecuteWizard(
 
 				var writer = new ProjectWriter(facility.Logger, new ProjectWriteOptions { MakeBackups = doBackups });
 
+				var transformations = new ChainTransformationSet(
+						new BasicSimplifyTransformationSet(Vs15TransformationSet.TargetVisualStudioVersion),
+						Vs15TransformationSet.TrueInstance)
+					.CollectAndOrderTransformations(facility.Logger, conversionOptions);
+
 				foreach (var project in legacy)
 				{
 					using (facility.Logger.BeginScope(project.FilePath))
@@ -113,9 +118,7 @@ public void ExecuteWizard(
 							continue;
 						}
 
-						foreach (var transformation in Vs15TransformationSet.TrueInstance.IterateTransformations(
-							facility.Logger,
-							conversionOptions))
+						foreach (var transformation in transformations.WhereSuitable(project, conversionOptions))
 						{
 							try
 							{
@@ -141,6 +144,10 @@ public void ExecuteWizard(
 				Log.Information("It appears you already have everything converted to CPS.");
 				if (AskBinaryChoice("Would you like to process CPS projects to clean up and reformat them?"))
 				{
+					var transformations =
+						new BasicSimplifyTransformationSet(Vs15TransformationSet.TargetVisualStudioVersion)
+							.CollectAndOrderTransformations(facility.Logger, conversionOptions);
+
 					foreach (var project in modern)
 					{
 						using (facility.Logger.BeginScope(project.FilePath))
@@ -154,9 +161,7 @@ public void ExecuteWizard(
 								continue;
 							}
 
-							foreach (var transformation in Vs15TransformationSet.TrueInstance.IterateTransformations(
-								facility.Logger,
-								conversionOptions))
+							foreach (var transformation in transformations.WhereSuitable(project, conversionOptions))
 							{
 								try
 								{
@@ -188,6 +193,11 @@ public void ExecuteWizard(
 
 				var writer = new ProjectWriter(facility.Logger, new ProjectWriteOptions { MakeBackups = doBackups });
 
+				var transformations = new ChainTransformationSet(
+						new BasicSimplifyTransformationSet(Vs15TransformationSet.TargetVisualStudioVersion),
+						Vs15ModernizationTransformationSet.TrueInstance)
+					.CollectAndOrderTransformations(facility.Logger, conversionOptions);
+
 				foreach (var project in projects)
 				{
 					using (facility.Logger.BeginScope(project.FilePath))
@@ -201,10 +211,7 @@ public void ExecuteWizard(
 							continue;
 						}
 
-						foreach (var transformation in Vs15ModernizationTransformationSet.TrueInstance
-							.IterateTransformations(
-								facility.Logger,
-								conversionOptions))
+						foreach (var transformation in transformations.WhereSuitable(project, conversionOptions))
 						{
 							try
 							{