Merge pull request #341 from cartermp/imports-fixes

src/MSBuild.Abstractions/BaselineProject.cs

@@ -31,7 +31,7 @@ private static string AdjustTargetTFM(ProjectStyle projectStyle, ProjectOutputTy
                 return MSBuildFacts.Net5Windows;
             }
 
-            if (projectStyle is not ProjectStyle.MSTest && outputType is ProjectOutputType.Library)
+            if (projectStyle is not ProjectStyle.MSTest && projectStyle is not ProjectStyle.Web && outputType is ProjectOutputType.Library)
             {
                 return MSBuildFacts.NetStandard20;
             }

src/MSBuild.Abstractions/MSBuildConversionWorkspace.cs

@@ -34,6 +34,14 @@ public MSBuildConversionWorkspace(ImmutableArray<string> paths, bool noBackup, s
                     continue;
                 }
 
+                // This is a hack, but the only way to handle this is to re-architect try-convert along these lines:
+                // 1. Launch a .NET Framework process using the VS-deployed MSBuild to evaluate a project
+                // 2. Serialize the evaluation model
+                // 3. Use the .NET Core process to load MSBuild, but this time from .NET SDK
+                // 4. Deserialize the evaluation model
+                // 5. Do conversions
+                RemoveTargetsNotLoadableByNETSDKMSBuild(path);
+
                 var root = new MSBuildProjectRootElement(ProjectRootElement.Open(path, collection, preserveFormatting: true));
                 if (IsSupportedProjectType(root, forceWeb))
                 {
@@ -89,6 +97,25 @@ public ImmutableDictionary<string, ImmutableDictionary<string, string>> Determin
             return builder.ToImmutable();
         }
 
+        private void RemoveTargetsNotLoadableByNETSDKMSBuild(string path)
+        {
+            var projectFile = File.ReadAllText(path);
+            if (projectFile is { Length:>0 })
+            {
+                var replacement =
+                    projectFile
+                        // Legacy web project specify these two targets paths. They aren't loadable by the .NET SDK msbuild process, so we just remove them.
+                        // They aren't actually useful as an end result when converting web projects. When people conver to .NET Core they will just use the Web SDK attribute.
+                        .Replace("<Import Project=\"$(VSToolsPath)\\WebApplications\\Microsoft.WebApplication.targets\" Condition=\"'$(VSToolsPath)' != ''\" />", "")
+                        .Replace("<Import Project=\"$(MSBuildExtensionsPath32)\\Microsoft\\VisualStudio\\v10.0\\WebApplications\\Microsoft.WebApplication.targets\" Condition=\"false\" />", "")
+
+                        // Legacy F# projects specify this import. It's not loadable by the .NET SDK MSBuild, and .NET Core-based F# projects don't use it. So we just remove it.
+                        .Replace("<Import Project=\"$(FSharpTargetsPath)\" />", "");
+
+                File.WriteAllText(path, replacement);
+            }
+        }
+
         /// <summary>
         /// Clear out the project's construction model and add a simple SDK-based project to get a baseline.
         /// We need to use the same name as the original csproj and same path so that all the default that derive
@@ -97,7 +124,7 @@ public ImmutableDictionary<string, ImmutableDictionary<string, string>> Determin
         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);
+            var outputType = GetProjectOutputType(root, projectStyle);
             var rootElement = ProjectRootElement.Open(projectFilePath);
 
             rootElement.RemoveAllChildren();
@@ -194,12 +221,15 @@ private bool IsSupportedOutputType(ProjectOutputType type) =>
                 _ => false
             };
 
-        private ProjectOutputType GetProjectOutputType(IProjectRootElement root)
+        private ProjectOutputType GetProjectOutputType(IProjectRootElement root) =>
+            GetProjectOutputType(root, GetProjectStyle(root));
+
+        private ProjectOutputType GetProjectOutputType(IProjectRootElement root, ProjectStyle projectStyle)
         {
-            if (root.Imports.Any(i => i.Project.Contains(WebFacts.WebApplicationTargets, StringComparison.OrdinalIgnoreCase)))
+            if (projectStyle == ProjectStyle.Web)
             {
-                // ASP.NET Core apps use an EXE output type even though ASP.NET apps use Library
-                // Note that this specifically looks for WebApplicationTargets (rather than a System.Web reference) since
+                // ASP.NET Core apps use an EXE output type even though legacy ASP.NET apps use Library
+                // Note that this specifically checks the project style only (rather than a System.Web reference) since
                 // ASP.NET libraries may reference System.Web and should still use a Library output types. Only ASP.NET
                 // apps should convert with Exe output type.
                 return ProjectOutputType.Exe;

src/MSBuild.Abstractions/MSBuildHelpers.cs

@@ -139,13 +139,11 @@ public static bool ConditionToDimensionValues(string condition, out ImmutableDic
         /// <summary>
         /// Given a TFM string, determines if that TFM has an explicit System.ValueTuple reference.
         /// </summary>
-        public static bool FrameworkHasAValueTuple(string tfm)
-        {
-            return !tfm.ContainsIgnoreCase(MSBuildFacts.NetstandardPrelude)
-                && !tfm.ContainsIgnoreCase(MSBuildFacts.NetcoreappPrelude)
-                && (tfm.StartsWith("net", StringComparison.OrdinalIgnoreCase)
-                    && tfm.StartsWith(MSBuildFacts.LowestFrameworkVersionWithSystemValueTuple));
-        }
+        public static bool FrameworkHasAValueTuple(string tfm) =>
+            tfm.StartsWith(MSBuildFacts.NetstandardPrelude, StringComparison.OrdinalIgnoreCase)
+            || tfm.StartsWith(MSBuildFacts.NetcoreappPrelude, StringComparison.OrdinalIgnoreCase)
+            || tfm.StartsWith(MSBuildFacts.Net5, StringComparison.OrdinalIgnoreCase)
+            || tfm.StartsWith(MSBuildFacts.LowestFrameworkVersionWithSystemValueTuple, StringComparison.OrdinalIgnoreCase);
 
         /// <summary>
         /// Gets all Reference items from a given item group.

src/MSBuild.Abstractions/ProjectExtensions.cs

@@ -32,7 +32,9 @@ public static string GetTargetFramework(this IProject project)
             var tfi = project.GetPropertyValue(MSBuildFacts.LegacyTargetFrameworkPropertyNodeName);
             if (string.IsNullOrWhiteSpace(tfi))
             {
-                throw new InvalidOperationException($"{MSBuildFacts.LegacyTargetFrameworkPropertyNodeName} is not set!");
+                // For some befuddling reason, legacy F# projects don't actually have a TFI.
+                // We just assume it's .NET Framework though, because like, that's just what it's gonna be anyways.
+                tfi = ".NETFramework";
             }
 
             var tfv = project.GetPropertyValue(MSBuildFacts.LegacyTargetFrameworkVersionNodeName);

tests/TestData/SmokeTests.FSharpConsoleBaseline/SmokeTests.FSharpConsoleBaseline.fsproj → tests/TestData/SmokeTests.FSharpConsoleCoreBaseline/SmokeTests.FSharpConsoleCoreBaseline.fsproj

@@ -1,7 +1,6 @@
-<?xml version="1.0" encoding="utf-8"?>
-<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>net472</TargetFramework>
+    <TargetFramework>netcoreapp3.1</TargetFramework>
     <OutputType>Exe</OutputType>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
   </PropertyGroup>

tests/TestData/SmokeTests.FSharpConsoleNet5Baseline/AssemblyInfo.fs

@@ -0,0 +1,41 @@
+namespace SmokeTests.LegacyFSharpConsole.AssemblyInfo
+
+open System.Reflection
+open System.Runtime.CompilerServices
+open System.Runtime.InteropServices
+
+// General Information about an assembly is controlled through the following
+// set of attributes. Change these attribute values to modify the information
+// associated with an assembly.
+[<assembly: AssemblyTitle("SmokeTests.LegacyFSharpConsole")>]
+[<assembly: AssemblyDescription("")>]
+[<assembly: AssemblyConfiguration("")>]
+[<assembly: AssemblyCompany("")>]
+[<assembly: AssemblyProduct("SmokeTests.LegacyFSharpConsole")>]
+[<assembly: AssemblyCopyright("Copyright ©  2019")>]
+[<assembly: AssemblyTrademark("")>]
+[<assembly: AssemblyCulture("")>]
+
+// Setting ComVisible to false makes the types in this assembly not visible
+// to COM components.  If you need to access a type in this assembly from
+// COM, set the ComVisible attribute to true on that type.
+[<assembly: ComVisible(false)>]
+
+// The following GUID is for the ID of the typelib if this project is exposed to COM
+[<assembly: Guid("df33a7b9-e0dd-4e09-b9d9-c6d0b7d5a648")>]
+
+// Version information for an assembly consists of the following four values:
+//
+//       Major Version
+//       Minor Version
+//       Build Number
+//       Revision
+//
+// You can specify all the values or you can default the Build and Revision Numbers
+// by using the '*' as shown below:
+// [<assembly: AssemblyVersion("1.0.*")>]
+[<assembly: AssemblyVersion("1.0.0.0")>]
+[<assembly: AssemblyFileVersion("1.0.0.0")>]
+
+do
+    ()

tests/TestData/SmokeTests.FSharpConsoleNet5Baseline/Program.fs

@@ -0,0 +1,8 @@
+// Learn more about F# at http://fsharp.org
+
+open System
+
+[<EntryPoint>]
+let main argv =
+    printfn "Hello World from F#!"
+    0 // return an integer exit code

tests/TestData/SmokeTests.FSharpConsoleBaseline/SmokeTests.FSharpConsoleBaselineNetCoreApp31.fsproj → tests/TestData/SmokeTests.FSharpConsoleNet5Baseline/SmokeTests.FSharpConsoleNet5Baseline.fsproj

@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>net472</TargetFramework>
+    <TargetFramework>net5.0</TargetFramework>
     <OutputType>Exe</OutputType>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
   </PropertyGroup>

tests/end-to-end/Smoke.Tests/BasicConversions.cs

@@ -26,20 +26,20 @@ public BasicSmokeTests(SolutionPathFixture solutionPathFixture, MSBuildFixture m
             solutionPathFixture.SetCurrentDirectory();
         }
 
-        [Fact(Skip = "Legacy F# support is not installed on any build machines")]
+        [Fact]
         public void ConvertsLegacyFSharpConsoleToNetCoreApp31()
         {
             var projectToConvertPath = GetFSharpProjectPath("SmokeTests.LegacyFSharpConsole");
-            var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleBaseline");
+            var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleCoreBaseline");
             AssertConversionWorks(projectToConvertPath, projectBaselinePath, "netcoreapp3.1");
         }
 
-        [Fact(Skip = "Legacy F# support is not installed on any build machines")]
+        [Fact]
         public void ConvertsLegacyFSharpConsoleToNet50()
         {
             var projectToConvertPath = GetFSharpProjectPath("SmokeTests.LegacyFSharpConsole");
-            var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleBaseline");
-            AssertConversionWorks(projectToConvertPath, projectBaselinePath, "netcoreapp3.1");
+            var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleNet5Baseline");
+            AssertConversionWorks(projectToConvertPath, projectBaselinePath, "net5.0");
         }
 
         [Fact]

tests/unit/ProjectConversion.Tests/ProjectExtensionsTests.cs

@@ -34,9 +34,7 @@ public void GetTargetFramework(string targetFramework, string targetFrameworkIde
         }
 
         [Theory]
-        [InlineData(null, null, "v4.6")]
         [InlineData(null, ".NETCoreApp", null)]
-        [InlineData(null, "Unknown", null)]
         public void GetTargetFramework_Throws(string targetFramework, string targetFrameworkIdentifier, string targetFrameworkVersion)
         {
             var properties = new Dictionary<string, string>();