Handle imports better and fix some bugs

2020-10-12 12:24:28 -07:00 · 2020-10-12 12:24:28 -07:00 · 92d10a9944
--- a/.gitignore
+++ b/.gitignore
@ -358,4 +358,6 @@ MigrationBackup/
 **/*/launchSettings.json
 **/*/FolderProfile.pubxml

-.tools/*
+.tools/*
+
+.vscode/
--- a/src/Directory.Build.props
+++ b/src/Directory.Build.props
@ -10,7 +10,7 @@
  </PropertyGroup>
  
  <PropertyGroup>
-    <LangVersion>8.0</LangVersion>
+    <LangVersion>preview</LangVersion>
    <Nullable>enable</Nullable>
    <DiscoverEditorConfigFiles>true</DiscoverEditorConfigFiles>
  </PropertyGroup>
--- a/src/MSBuild.Abstractions/BaselineProject.cs
+++ b/src/MSBuild.Abstractions/BaselineProject.cs
@ -27,12 +27,12 @@ namespace MSBuild.Abstractions

        private static string AdjustTargetTFM(ProjectStyle projectStyle, ProjectOutputType outputType, string candidateTargetTFM)
        {
-            if (candidateTargetTFM.ContainsIgnoreCase(MSBuildFacts.Net5) && projectStyle == ProjectStyle.WindowsDesktop)
+            if (candidateTargetTFM.ContainsIgnoreCase(MSBuildFacts.Net5) && projectStyle is ProjectStyle.WindowsDesktop)
            {
                return MSBuildFacts.Net5Windows;
            }

-            if (outputType == ProjectOutputType.Library)
+            if (projectStyle is not ProjectStyle.MSTest && outputType is ProjectOutputType.Library)
            {
                return MSBuildFacts.NetStandard20;
            }
--- a/src/MSBuild.Abstractions/MSBuildConversionWorkspace.cs
+++ b/src/MSBuild.Abstractions/MSBuildConversionWorkspace.cs
@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
+using System.Diagnostics.CodeAnalysis;

 using Microsoft.Build.Construction;
 using Microsoft.Build.Evaluation;
@ -36,24 +37,27 @@ namespace MSBuild.Abstractions
                var root = new MSBuildProjectRootElement(ProjectRootElement.Open(path, collection, preserveFormatting: true));
                if (IsSupportedProjectType(root))
                {
-                    if (!noBackup)
-                    {
-                        // Since git doesn't track the new '.old' addition in your changeset,
-                        // failing to overwrite will crash the tool if you have one in your directory.
-                        // This can be common if you're using the tool a few times and forget to delete the backup.
-                        File.Copy(path, path + ".old", overwrite: true);
-                    }

                    var configurations = DetermineConfigurations(root);

                    var unconfiguredProject = new UnconfiguredProject(configurations);
                    unconfiguredProject.LoadProjects(collection, globalProperties, path);

-                    var baseline = CreateSdkBaselineProject(path, unconfiguredProject.FirstConfiguredProject, root, configurations, tfm, keepCurrentTFMs);
-                    root.Reload(throwIfUnsavedChanges: false, preserveFormatting: true);

-                    var item = new MSBuildConversionWorkspaceItem(root, unconfiguredProject, baseline);
-                    items.Add(item);
+                    if (TryCreateSdkBaselineProject(path, unconfiguredProject.FirstConfiguredProject, root, configurations, tfm, keepCurrentTFMs, out var baseline))
+                    {
+                        if (!noBackup)
+                        {
+                            // Since git doesn't track the new '.old' addition in your changeset,
+                            // failing to overwrite will crash the tool if you have one in your directory.
+                            // This can be common if you're using the tool a few times and forget to delete the backup.
+                            File.Copy(path, path + ".old", overwrite: true);
+                        }
+
+                        root.Reload(throwIfUnsavedChanges: false, preserveFormatting: true);
+                        var item = new MSBuildConversionWorkspaceItem(root, unconfiguredProject, baseline.Value);
+                        items.Add(item);
+                    }
                }
            }

@ -79,7 +83,7 @@ namespace MSBuild.Abstractions
                                builder.Add(dimensionValuePair.Value, dimensionValueDictionary.ToImmutableDictionary());
                            }
                        }
-                    }                   
+                    }
                }
            }
            return builder.ToImmutable();
@ -90,7 +94,7 @@ namespace MSBuild.Abstractions
        /// We need to use the same name as the original csproj and same path so that all the default that derive
        /// from name\path get the right values (there are a lot of them).
        /// </summary>
-        private BaselineProject CreateSdkBaselineProject(string projectFilePath, IProject project, IProjectRootElement root, ImmutableDictionary<string, ImmutableDictionary<string, string>> configurations, string tfm, bool keepCurrentTFMs)
+        private bool TryCreateSdkBaselineProject(string projectFilePath, IProject project, IProjectRootElement root, ImmutableDictionary<string, ImmutableDictionary<string, string>> configurations, string tfm, bool keepCurrentTFMs, [NotNullWhen(true)] out BaselineProject? baselineProject)
        {
            var projectStyle = GetProjectStyle(root);
            var outputType = GetProjectOutputType(root);
@ -111,7 +115,8 @@ namespace MSBuild.Abstractions
                            : DesktopFacts.WinSDKAttribute; // pre-.NET 5 apps need a special SDK attribute.
                    break;
                default:
-                    throw new NotSupportedException($"This project has custom imports in a manner that's not supported. '{projectFilePath}'");
+                    baselineProject = null;
+                    return false;
            }

            var propGroup = rootElement.AddPropertyGroup();
@ -166,7 +171,8 @@ namespace MSBuild.Abstractions
                    ? MSBuildFacts.Net5Windows
                    : tfm;

-            return new BaselineProject(newProject, propertiesInTheBaseline, projectStyle, outputType, tfm, keepCurrentTFMs);
+            baselineProject = new BaselineProject(newProject, propertiesInTheBaseline, projectStyle, outputType, tfm, keepCurrentTFMs);
+            return true;
        }

        private bool IsSupportedOutputType(ProjectOutputType type) =>
@ -231,21 +237,13 @@ namespace MSBuild.Abstractions
                }
                else
                {
-                    var lastImport = imports.Last();
-                    var lastImportFileName = Path.GetFileName(lastImport.Project);
+                    var cleansedImports = imports.Select(import => Path.GetFileName(import.Project));
+                    var allImportsConvertibleToSdk =
+                        cleansedImports.All(import =>
+                            MSBuildFacts.PropsConvertibleToSDK.Contains(import, StringComparer.OrdinalIgnoreCase) ||
+                            MSBuildFacts.TargetsConvertibleToSDK.Contains(import, StringComparer.OrdinalIgnoreCase));

-                    if (firstImportFileName == FSharpFacts.FSharpTargetsPathVariableName)
-                    {
-                        firstImportFileName = Path.GetFileName(FSharpFacts.FSharpTargetsPath);
-                    }
-
-                    if (lastImportFileName == FSharpFacts.FSharpTargetsPathVariableName)
-                    {
-                        lastImportFileName = Path.GetFileName(FSharpFacts.FSharpTargetsPath);
-                    }
-
-                    if (MSBuildFacts.PropsConvertibleToSDK.Contains(firstImportFileName, StringComparer.OrdinalIgnoreCase) &&
-                        MSBuildFacts.TargetsConvertibleToSDK.Contains(lastImportFileName, StringComparer.OrdinalIgnoreCase))
+                    if (allImportsConvertibleToSdk)
                    {
                        if (MSBuildHelpers.IsNETFrameworkMSTestProject(projectRootElement))
                        {
@ -262,6 +260,26 @@ namespace MSBuild.Abstractions
                    }
                    else
                    {
+                        Console.WriteLine("This project has custom imports that are not accepted by try-convert.");
+                        Console.WriteLine("Unexpected custom imports were found:");
+
+                        var customImports =
+                            cleansedImports.Where(import =>
+                                !(MSBuildFacts.PropsConvertibleToSDK.Contains(import, StringComparer.OrdinalIgnoreCase) ||
+                                  MSBuildFacts.TargetsConvertibleToSDK.Contains(import, StringComparer.OrdinalIgnoreCase)));
+
+                        foreach (var import in customImports)
+                        {
+                            Console.WriteLine($"\t{import}");
+                        }
+
+                        Console.WriteLine("The following imports are considered valid for conversion:");
+
+                        foreach (var import in MSBuildFacts.TargetsConvertibleToSDK.Union(MSBuildFacts.PropsConvertibleToSDK))
+                        {
+                            Console.WriteLine($"\t{import}");
+                        }
+
                        // It's something else, no idea what though
                        return ProjectStyle.Custom;
                    }
@ -327,7 +345,7 @@ namespace MSBuild.Abstractions
                    {
                        return true;
                    }
-                }
+            }

            static void PrintGuidMessage(IEnumerable<string> allSupportedProjectTypeGuids, IEnumerable<string> allReadProjectTypeGuids)
            {
--- a/src/MSBuild.Conversion.Facts/MSBuildFacts.cs
+++ b/src/MSBuild.Conversion.Facts/MSBuildFacts.cs
@ -26,7 +26,8 @@ namespace MSBuild.Conversion.Facts
            "Microsoft.Portable.CSharp.targets",
            "Microsoft.Portable.VisualBasic.targets",
            "Microsoft.FSharp.Targets",
-            "MSTest.TestAdapter.targets"
+            "MSTest.TestAdapter.targets",
+            "Microsoft.TestTools.targets"
        );

        /// <summary>
--- a/src/try-convert/Program.cs
+++ b/src/try-convert/Program.cs
@ -101,6 +101,12 @@ namespace MSBuild.Conversion
                noBackup = noBackup || diffOnly;
                var msbuildWorkspace = workspaceLoader.LoadWorkspace(workspacePath, noBackup, tfm, keepCurrentTfms);

+                if (msbuildWorkspace.WorkspaceItems.Length is 0)
+                {
+                    Console.WriteLine("No projects converted.");
+                    return 0;
+                }
+
                foreach (var item in msbuildWorkspace.WorkspaceItems)
                {
                    if (diffOnly)
--- a/tests/TestData/SmokeTests.LegacyMSTest/Properties/AssemblyInfo.cs
+++ b/tests/TestData/SmokeTests.LegacyMSTest/Properties/AssemblyInfo.cs
@ -0,0 +1,20 @@
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+[assembly: AssemblyTitle("UnitTestProject1")]
+[assembly: AssemblyDescription("")]
+[assembly: AssemblyConfiguration("")]
+[assembly: AssemblyCompany("")]
+[assembly: AssemblyProduct("UnitTestProject1")]
+[assembly: AssemblyCopyright("Copyright ©  2020")]
+[assembly: AssemblyTrademark("")]
+[assembly: AssemblyCulture("")]
+
+[assembly: ComVisible(false)]
+
+[assembly: Guid("a2de09a2-5c2a-4b5e-b700-9c14be5af757")]
+
+// [assembly: AssemblyVersion("1.0.*")]
+[assembly: AssemblyVersion("1.0.0.0")]
+[assembly: AssemblyFileVersion("1.0.0.0")]
--- a/tests/TestData/SmokeTests.LegacyMSTest/SmokeTests.LegacyMSTest.csproj
+++ b/tests/TestData/SmokeTests.LegacyMSTest/SmokeTests.LegacyMSTest.csproj
@ -0,0 +1,68 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.props" Condition="Exists('..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.props')" />
+  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <ProjectGuid>{A2DE09A2-5C2A-4B5E-B700-9C14BE5AF757}</ProjectGuid>
+    <OutputType>Library</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <RootNamespace>UnitTestProject1</RootNamespace>
+    <AssemblyName>UnitTestProject1</AssemblyName>
+    <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{3AC096D0-A1C2-E12C-1390-A8335801FDAB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">15.0</VisualStudioVersion>
+    <VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\$(VisualStudioVersion)\UITestExtensionPackages</ReferencePath>
+    <IsCodedUITest>False</IsCodedUITest>
+    <TestProjectType>UnitTest</TestProjectType>
+    <NuGetPackageImportStamp>
+    </NuGetPackageImportStamp>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+    <DebugSymbols>true</DebugSymbols>
+    <DebugType>full</DebugType>
+    <Optimize>false</Optimize>
+    <OutputPath>bin\Debug\</OutputPath>
+    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <ErrorReport>prompt</ErrorReport>
+    <WarningLevel>4</WarningLevel>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+    <DebugType>pdbonly</DebugType>
+    <Optimize>true</Optimize>
+    <OutputPath>bin\Release\</OutputPath>
+    <DefineConstants>TRACE</DefineConstants>
+    <ErrorReport>prompt</ErrorReport>
+    <WarningLevel>4</WarningLevel>
+  </PropertyGroup>
+  <ItemGroup>
+    <Reference Include="Microsoft.VisualStudio.TestPlatform.TestFramework, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
+      <HintPath>..\packages\MSTest.TestFramework.2.1.1\lib\net45\Microsoft.VisualStudio.TestPlatform.TestFramework.dll</HintPath>
+    </Reference>
+    <Reference Include="Microsoft.VisualStudio.TestPlatform.TestFramework.Extensions, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
+      <HintPath>..\packages\MSTest.TestFramework.2.1.1\lib\net45\Microsoft.VisualStudio.TestPlatform.TestFramework.Extensions.dll</HintPath>
+    </Reference>
+    <Reference Include="System" />
+    <Reference Include="System.Core" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="UnitTest1.cs" />
+    <Compile Include="Properties\AssemblyInfo.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <None Include="packages.config" />
+  </ItemGroup>
+  <Import Project="$(VSToolsPath)\TeamTest\Microsoft.TestTools.targets" Condition="Exists('$(VSToolsPath)\TeamTest\Microsoft.TestTools.targets')" />
+  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
+  <Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
+    <PropertyGroup>
+      <ErrorText>This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them.  For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}.</ErrorText>
+    </PropertyGroup>
+    <Error Condition="!Exists('..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.props')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.props'))" />
+    <Error Condition="!Exists('..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.targets')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.targets'))" />
+  </Target>
+  <Import Project="..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.targets" Condition="Exists('..\packages\MSTest.TestAdapter.2.1.1\build\net45\MSTest.TestAdapter.targets')" />
+</Project>
--- a/tests/TestData/SmokeTests.LegacyMSTest/UnitTest1.cs
+++ b/tests/TestData/SmokeTests.LegacyMSTest/UnitTest1.cs
@ -0,0 +1,14 @@
+using System;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace UnitTestProject1
+{
+    [TestClass]
+    public class UnitTest1
+    {
+        [TestMethod]
+        public void TestMethod1()
+        {
+        }
+    }
+}
--- a/tests/TestData/SmokeTests.LegacyMSTest/packages.config
+++ b/tests/TestData/SmokeTests.LegacyMSTest/packages.config
@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="utf-8"?>
+<packages>
+  <package id="MSTest.TestAdapter" version="2.1.1" targetFramework="net472" />
+  <package id="MSTest.TestFramework" version="2.1.1" targetFramework="net472" />
+</packages>
--- a/tests/TestData/SmokeTests.MStestCoreBaseline/Properties/AssemblyInfo.cs
+++ b/tests/TestData/SmokeTests.MStestCoreBaseline/Properties/AssemblyInfo.cs
@ -0,0 +1,20 @@
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+[assembly: AssemblyTitle("UnitTestProject1")]
+[assembly: AssemblyDescription("")]
+[assembly: AssemblyConfiguration("")]
+[assembly: AssemblyCompany("")]
+[assembly: AssemblyProduct("UnitTestProject1")]
+[assembly: AssemblyCopyright("Copyright ©  2020")]
+[assembly: AssemblyTrademark("")]
+[assembly: AssemblyCulture("")]
+
+[assembly: ComVisible(false)]
+
+[assembly: Guid("a2de09a2-5c2a-4b5e-b700-9c14be5af757")]
+
+// [assembly: AssemblyVersion("1.0.*")]
+[assembly: AssemblyVersion("1.0.0.0")]
+[assembly: AssemblyFileVersion("1.0.0.0")]
--- a/tests/TestData/SmokeTests.MStestCoreBaseline/SmokeTests.MSTestCoreBaseline.csproj
+++ b/tests/TestData/SmokeTests.MStestCoreBaseline/SmokeTests.MSTestCoreBaseline.csproj
@ -0,0 +1,15 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <RootNamespace>UnitTestProject1</RootNamespace>
+    <AssemblyName>UnitTestProject1</AssemblyName>
+  </PropertyGroup>
+  <PropertyGroup>
+    <TargetFramework>netcoreapp3.1</TargetFramework>
+    <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
+  </PropertyGroup>
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.*" />
+    <PackageReference Include="MSTest.TestAdapter" Version="2.1.1" />
+    <PackageReference Include="MSTest.TestFramework" Version="2.1.1" />
+  </ItemGroup>
+</Project>
--- a/tests/TestData/SmokeTests.MStestCoreBaseline/UnitTest1.cs
+++ b/tests/TestData/SmokeTests.MStestCoreBaseline/UnitTest1.cs
@ -0,0 +1,14 @@
+using System;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace UnitTestProject1
+{
+    [TestClass]
+    public class UnitTest1
+    {
+        [TestMethod]
+        public void TestMethod1()
+        {
+        }
+    }
+}
--- a/tests/TestData/SmokeTests.MStestCoreBaseline/packages.config
+++ b/tests/TestData/SmokeTests.MStestCoreBaseline/packages.config
@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="utf-8"?>
+<packages>
+  <package id="MSTest.TestAdapter" version="2.1.1" targetFramework="net472" />
+  <package id="MSTest.TestFramework" version="2.1.1" targetFramework="net472" />
+</packages>
--- a/tests/end-to-end/Smoke.Tests/BasicConversions.cs
+++ b/tests/end-to-end/Smoke.Tests/BasicConversions.cs
@ -74,6 +74,14 @@ namespace SmokeTests
            AssertConversionWorks(projectToConvertPath, projectBaselinePath, "net5.0-windows");
        }

+        [Fact]
+        public void ConvertsLegacyMSTest()
+        {
+            var projectToConvertPath = GetCSharpProjectPath("SmokeTests.LegacyMSTest");
+            var projectBaselinePath = GetCSharpProjectPath("SmokeTests.MSTestCoreBaseline");
+            AssertConversionWorks(projectToConvertPath, projectBaselinePath, "netcoreapp3.1");
+        }
+
        private void AssertConversionWorks(string projectToConvertPath, string projectBaselinePath, string targetTFM)
        {
            var (baselineRootElement, convertedRootElement) = GetRootElementsForComparison(projectToConvertPath, projectBaselinePath, targetTFM);