diff --git a/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs b/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs index e58ecadccb..9325d73039 100644 --- a/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs +++ b/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs @@ -1,4 +1,5 @@ -using Microsoft.Extensions.Logging; +using System.IO; +using Microsoft.Extensions.Logging; namespace OmniSharp.MSBuild.Discovery { @@ -6,34 +7,21 @@ internal static class Extensions { public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator, ILogger logger) { - MSBuildInstance instanceToRegister = null; - var invalidVSFound = false; + var bestInstanceFound = GetBestInstance(msbuildLocator, out var invalidVSFound); - foreach (var instance in msbuildLocator.GetInstances()) - { - if (instance.IsInvalidVisualStudio()) - { - invalidVSFound = true; - } - else - { - instanceToRegister = instance; - break; - } - } - - - if (instanceToRegister != null) + if (bestInstanceFound != null) { // Did we end up choosing the standalone MSBuild because there was an invalid Visual Studio? // If so, provide a helpful message to the user. - if (invalidVSFound && instanceToRegister.DiscoveryType == DiscoveryType.StandAlone) + if (invalidVSFound && bestInstanceFound.DiscoveryType == DiscoveryType.StandAlone) { - logger.LogWarning(@"It looks like you have Visual Studio 2017 RTM installed. -Try updating Visual Studio 2017 to the most recent release to enable better MSBuild support."); + logger.LogWarning( + @"It looks like you have Visual Studio 2017 RTM installed. + Try updating Visual Studio 2017 to the most recent release to enable better MSBuild support." + ); } - msbuildLocator.RegisterInstance(instanceToRegister); + msbuildLocator.RegisterInstance(bestInstanceFound); } else { @@ -41,12 +29,68 @@ public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator, } } + public static bool HasDotNetSdksResolvers(this MSBuildInstance instance) + { + const string dotnetSdkResolver = "Microsoft.DotNet.MSBuildSdkResolver"; + + return File.Exists( + Path.Combine( + instance.MSBuildPath, + "SdkResolvers", + dotnetSdkResolver, + dotnetSdkResolver + ".dll" + ) + ); + } + /// + /// Checks if it is MSBuild from Visual Studio 2017 RTM that cannot be used. + /// public static bool IsInvalidVisualStudio(this MSBuildInstance instance) - // MSBuild from Visual Studio 2017 RTM cannot be used. => instance.Version.Major == 15 - && instance.Version.Minor == 0 - && (instance.DiscoveryType == DiscoveryType.DeveloperConsole - || instance.DiscoveryType == DiscoveryType.VisualStudioSetup); + && instance.Version.Minor == 0 + && (instance.DiscoveryType == DiscoveryType.DeveloperConsole + || instance.DiscoveryType == DiscoveryType.VisualStudioSetup); + + public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocator, out bool invalidVSFound) + { + invalidVSFound = false; + MSBuildInstance bestMatchInstance = null; + var bestMatchScore = 0; + + foreach (var instance in msbuildLocator.GetInstances()) + { + var score = GetInstanceFeatureScore(instance); + + invalidVSFound = invalidVSFound || instance.IsInvalidVisualStudio(); + + if (score > bestMatchScore + || (score == bestMatchScore && instance.Version.Major > (bestMatchInstance?.Version.Major ?? 0))) + { + bestMatchInstance = instance; + bestMatchScore = score; + } + } + + return bestMatchInstance; + } + + private static int GetInstanceFeatureScore(MSBuildInstance i) + { + var score = 0; + + if (i.HasDotNetSdksResolvers()) + score++; + + if (i.IsInvalidVisualStudio()) + return int.MinValue; + else + score++; + + if (i.DiscoveryType == DiscoveryType.StandAlone) + score--; + + return score; + } } } diff --git a/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs b/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs new file mode 100644 index 0000000000..d527ee4a62 --- /dev/null +++ b/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs @@ -0,0 +1,131 @@ +using System; +using OmniSharp.MSBuild.Discovery; +using TestUtility; +using Xunit; +using Xunit.Abstractions; + +namespace OmniSharp.MSBuild.Tests +{ + public class MSBuildSelectionTests : AbstractTestFixture + { + public MSBuildSelectionTests(ITestOutputHelper output) + : base(output) + { + } + + [Fact] + public void RegisterDefaultInstanceFindsTheBestInstanceAvailable() + { + var msBuildInstances = new[] + { + GetInvalidMsBuildInstance(), + // Valid + new MSBuildInstance( + "Test Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("15.1.2.3"), + DiscoveryType.VisualStudioSetup + ).AddDotNetCoreToFakeInstance(), + GetStandAloneMSBuildInstance() + }; + + var msbuildLocator = new MSFakeLocator(msBuildInstances); + var logger = LoggerFactory.CreateLogger(nameof(RegisterDefaultInstanceFindsTheBestInstanceAvailable)); + + // test + msbuildLocator.RegisterDefaultInstance(logger); + + Assert.NotNull(msbuildLocator.RegisteredInstance); + Assert.Same(msBuildInstances[1], msbuildLocator.RegisteredInstance); + + // clean up + msbuildLocator.DeleteFakeInstancesFolders(); + } + + [Fact] + public void RegisterDefaultInstanceFindsTheBestInstanceAvailableEvenWithOtherValidInstances() + { + var msBuildInstances = new[] + { + new MSBuildInstance( + "Valid Test Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("15.3.2.1"), + DiscoveryType.VisualStudioSetup + ), + GetInvalidMsBuildInstance(), + + // Valid + Dotnet Core + new MSBuildInstance( + "Another Valid Test Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("15.1.2.3"), + DiscoveryType.VisualStudioSetup + ).AddDotNetCoreToFakeInstance(), + GetStandAloneMSBuildInstance() + }; + + var msbuildLocator = new MSFakeLocator(msBuildInstances); + + var logger = LoggerFactory.CreateLogger( + nameof(RegisterDefaultInstanceFindsTheBestInstanceAvailableEvenWithOtherValidInstances) + ); + + // test + msbuildLocator.RegisterDefaultInstance(logger); + + Assert.NotNull(msbuildLocator.RegisteredInstance); + Assert.Same(msBuildInstances[2], msbuildLocator.RegisteredInstance); + + // clean up + msbuildLocator.DeleteFakeInstancesFolders(); + } + + [Fact] + public void RegisterDefaultInstanceStillPrefersTheFirstInstance() + { + var msBuildInstances = new[] + { + new MSBuildInstance( + "Test Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("15.1.2.3"), + DiscoveryType.VisualStudioSetup + ), + GetStandAloneMSBuildInstance() + }; + + var msbuildLocator = new MSFakeLocator(msBuildInstances); + var logger = LoggerFactory.CreateLogger(nameof(RegisterDefaultInstanceStillPrefersTheFirstInstance)); + + // test + msbuildLocator.RegisterDefaultInstance(logger); + + Assert.NotNull(msbuildLocator.RegisteredInstance); + Assert.Same(msBuildInstances[0], msbuildLocator.RegisteredInstance); + + // clean up + msbuildLocator.DeleteFakeInstancesFolders(); + } + + private static MSBuildInstance GetStandAloneMSBuildInstance() + { + return new MSBuildInstance( + "Stand Alone :(", + TestIO.GetRandomTempFolderPath(), + Version.Parse("99.0.0.0"), + DiscoveryType.StandAlone + ); + } + + private static MSBuildInstance GetInvalidMsBuildInstance() + { + return new MSBuildInstance( + "Invalid Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("15.0.4.2"), + DiscoveryType.VisualStudioSetup + ); + } + } +} diff --git a/tests/TestUtility/MSFakeLocator.cs b/tests/TestUtility/MSFakeLocator.cs new file mode 100644 index 0000000000..41ca907b41 --- /dev/null +++ b/tests/TestUtility/MSFakeLocator.cs @@ -0,0 +1,34 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using OmniSharp.MSBuild.Discovery; + +namespace TestUtility +{ + public class MSFakeLocator : IMSBuildLocator + { + private readonly ImmutableArray _instances; + + public MSBuildInstance RegisteredInstance { get; private set; } + + public MSFakeLocator(IEnumerable instances) + { + _instances = instances.ToImmutableArray(); + } + + public void RegisterInstance(MSBuildInstance instance) + => RegisteredInstance = instance; + + public ImmutableArray GetInstances() + => _instances; + + public void DeleteFakeInstancesFolders() + { + foreach (var instance in _instances) + { + if (Directory.Exists(instance.MSBuildPath)) + Directory.Delete(instance.MSBuildPath, true); + } + } + } +} diff --git a/tests/TestUtility/TestFolder.cs b/tests/TestUtility/TestFolder.cs new file mode 100644 index 0000000000..7517c96e4f --- /dev/null +++ b/tests/TestUtility/TestFolder.cs @@ -0,0 +1,24 @@ +using System.IO; + +namespace TestUtility +{ + public static class TestIO + { + public static string GetRandomTempFolderPath() + { + var path = Path.Combine( + Path.GetTempPath(), + Path.GetRandomFileName() + ); + + Directory.CreateDirectory(path); + + return path; + } + + public static void TouchFakeFile(string path) + { + File.WriteAllText(path, "just testing :)"); + } + } +} diff --git a/tests/TestUtility/TestHelpers.cs b/tests/TestUtility/TestHelpers.cs index a2f4c34a75..1820c81fa0 100644 --- a/tests/TestUtility/TestHelpers.cs +++ b/tests/TestUtility/TestHelpers.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Reflection; using Microsoft.CodeAnalysis; @@ -7,6 +8,7 @@ using Microsoft.Extensions.Logging; using OmniSharp; using OmniSharp.FileWatching; +using OmniSharp.MSBuild.Discovery; using OmniSharp.Script; using OmniSharp.Services; @@ -24,7 +26,7 @@ public static OmniSharpWorkspace CreateCsxWorkspace(TestFile testFile) public static void AddCsxProjectToWorkspace(OmniSharpWorkspace workspace, TestFile testFile) { var references = GetReferences(); - var scriptHelper = new ScriptProjectProvider(new ScriptOptions(), new OmniSharpEnvironment(), new LoggerFactory(), true); + var scriptHelper = new ScriptProjectProvider(new ScriptOptions(), new OmniSharpEnvironment(), new LoggerFactory(), true); var project = scriptHelper.CreateProject(testFile.FileName, references.Union(new[] { MetadataReference.CreateFromFile(typeof(CommandLineScriptGlobals).GetTypeInfo().Assembly.Location) }), testFile.FileName, typeof(CommandLineScriptGlobals), Enumerable.Empty()); workspace.AddProject(project); @@ -91,5 +93,22 @@ private static IEnumerable GetReferences() return references; } + + public static MSBuildInstance AddDotNetCoreToFakeInstance(this MSBuildInstance instance) + { + const string dotnetSdkResolver = "Microsoft.DotNet.MSBuildSdkResolver"; + + var directory = Path.Combine( + instance.MSBuildPath, + "SdkResolvers", + dotnetSdkResolver + ); + + Directory.CreateDirectory(directory); + + TestIO.TouchFakeFile(Path.Combine(directory, dotnetSdkResolver + ".dll")); + + return instance; + } } }