Merge pull request #1845 from filipw/feature/dotnetcoresdkmandatory

only select MSBuild from VS if .NET SDK Resolvers are present
OmniSharp · Jul 2, 2020 · 8e78f74 · 8e78f74
2 parents 38e2b6b + e86c566
commit 8e78f74
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@ All changes to the project will be documented in this file.
 
 ## [1.35.4] - Not yet released
 * Added LSP handler for the `workspace/symbol` request. (PR: [#1799](https://github.com/OmniSharp/omnisharp-roslyn/pull/1799))
+* Do not use Visual Studio MSBuild if it doesn't have .NET SDK resolver ([#1842](https://github.com/OmniSharp/omnisharp-roslyn/issues/1842), [#1730](https://github.com/OmniSharp/omnisharp-roslyn/issues/1730), PR: [#1845](https://github.com/OmniSharp/omnisharp-roslyn/pull/1845))
 
 ## [1.35.3] - 2020-06-11
 * Added LSP handler for `textDocument/codeAction` request. (PR: [#1795](https://github.com/OmniSharp/omnisharp-roslyn/pull/1795))

diff --git a/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs b/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs
@@ -7,7 +7,7 @@ internal static class Extensions
     {
         public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator, ILogger logger)
         {
-            var bestInstanceFound = GetBestInstance(msbuildLocator, logger, out var invalidVSFound);
+            var bestInstanceFound = GetBestInstance(msbuildLocator, logger, out var invalidVSFound, out var vsWithoutSdkResolver);
 
             if (bestInstanceFound != null)
             {
@@ -16,8 +16,16 @@ public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator,
                 if (invalidVSFound && bestInstanceFound.DiscoveryType == DiscoveryType.StandAlone)
                 {
                     logger.LogWarning(
-                        @"It looks like you have Visual Studio 2019 RTM installed.
- Try updating Visual Studio 2019 to the most recent release to enable better MSBuild support."
+                        @"It looks like you have Visual Studio lower than VS 2019 16.3 installed.
+ Try updating Visual Studio to the most recent release to enable better MSBuild support."
+                    );
+                }
+
+                if (vsWithoutSdkResolver && bestInstanceFound.DiscoveryType == DiscoveryType.StandAlone)
+                {
+                    logger.LogWarning(
+                        @"It looks like you have Visual Studio 2019 installed without .NET Core SDK support which is required by OmniSharp.
+ Try updating Visual Studio 2019 installation with .NET Core SDK to enable better MSBuild support."
                     );
                 }
 
@@ -47,14 +55,15 @@ public static bool HasDotNetSdksResolvers(this MSBuildInstance instance)
         /// Checks if it is MSBuild from Visual Studio 2017 RTM that cannot be used.
         /// </summary>
         public static bool IsInvalidVisualStudio(this MSBuildInstance instance)
-            => instance.Version.Major == 15
-                && instance.Version.Minor == 0
+            => (instance.Version.Major < 16 ||
+                (instance.Version.Major == 16 && instance.Version.Minor < 3))
                 && (instance.DiscoveryType == DiscoveryType.DeveloperConsole
                     || instance.DiscoveryType == DiscoveryType.VisualStudioSetup);
 
-        public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocator, ILogger logger, out bool invalidVSFound)
+        public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocator, ILogger logger, out bool invalidVSFound, out bool vsWithoutSdkResolver)
         {
             invalidVSFound = false;
+            vsWithoutSdkResolver = false;
             MSBuildInstance bestMatchInstance = null;
             var bestMatchScore = 0;
 
@@ -65,6 +74,7 @@ public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocato
                 logger.LogDebug($"MSBuild instance {instance.Name} {instance.Version} scored at {score}");
 
                 invalidVSFound = invalidVSFound || instance.IsInvalidVisualStudio();
+                vsWithoutSdkResolver = vsWithoutSdkResolver || (!instance.IsInvalidVisualStudio() && !instance.HasDotNetSdksResolvers());
 
                 if (bestMatchInstance == null ||
                     score > bestMatchScore ||
@@ -86,21 +96,18 @@ private static int GetInstanceFeatureScore(MSBuildInstance i)
             if (i.DiscoveryType == DiscoveryType.UserOverride)
                 return int.MaxValue;
 
-            if (i.HasDotNetSdksResolvers())
-                score++;
-
             if (i.IsInvalidVisualStudio())
                 return int.MinValue;
             else
                 score++;
 
-            if (i.DiscoveryType == DiscoveryType.StandAlone)
-                score--;
-
-            // VS 2019 should be preferred
-            if (i.Version.Major >= 16 && i.Version.Minor >= 3)
+            // dotnet SDK resolvers are mandatory to use a VS instance
+            if (i.HasDotNetSdksResolvers())
                 score++;
             else
+                return int.MinValue;
+
+            if (i.DiscoveryType == DiscoveryType.StandAlone)
                 score--;
 
             return score;

diff --git a/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs b/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs
@@ -195,22 +195,24 @@ public void RegisterDefaultInstancePrefersSupportedVSLowerVersionInstanceOverSta
             msbuildLocator.DeleteFakeInstancesFolders();
         }
 
-        [Fact]
-        public void RegisterDefaultInstancePrefersStandAloneOverSupportedVSLowerVersionInstanceWithoutDotnetCore()
+        [Theory]
+        [InlineData("16.2.2.3")] // lower than standalone
+        [InlineData("16.6.2.3")] // higher than standalone
+        public void RegisterDefaultInstancePrefersStandAloneOverSupportedVSInstanceWithoutDotnetCore(string vsVersion)
         {
             var msBuildInstances = new[]
             {
                 new MSBuildInstance(
                     "Test Instance",
                     TestIO.GetRandomTempFolderPath(),
-                    Version.Parse("16.2.2.3"),
+                    Version.Parse(vsVersion),
                     DiscoveryType.VisualStudioSetup
                 ),
                 GetStandAloneMSBuildInstance()
             };
 
             var msbuildLocator = new MSFakeLocator(msBuildInstances);
-            var logger = LoggerFactory.CreateLogger(nameof(RegisterDefaultInstancePrefersStandAloneOverSupportedVSLowerVersionInstanceWithoutDotnetCore));
+            var logger = LoggerFactory.CreateLogger(nameof(RegisterDefaultInstancePrefersStandAloneOverSupportedVSInstanceWithoutDotnetCore));
 
             // test
             msbuildLocator.RegisterDefaultInstance(logger);
@@ -224,6 +226,7 @@ public void RegisterDefaultInstancePrefersStandAloneOverSupportedVSLowerVersionI
 
         [Theory]
         [InlineData("15.1.2.3")]
+        [InlineData("15.9.2.3")]
         [InlineData("16.1.2.3")]
         [InlineData("16.2.2.3")]
         public void StandAloneIsPreferredOverUnsupportedVS(string vsVersion)