Introduce ProjectLoader class to handle responsibilities of loading a…

… project

src/OmniSharp.MSBuild/MSBuildProjectSystem.cs

@@ -32,7 +32,7 @@ public class MSBuildProjectSystem : IProjectSystem
     {
         private readonly IOmniSharpEnvironment _environment;
         private readonly OmniSharpWorkspace _workspace;
-        private readonly MSBuildInstance _msbuildInstance;
+        private readonly ImmutableDictionary<string, string> _propertyOverrides;
         private readonly DotNetCliService _dotNetCli;
         private readonly MetadataFileReferenceCache _metadataFileReferenceCache;
         private readonly IEventEmitter _eventEmitter;
@@ -45,6 +45,7 @@ public class MSBuildProjectSystem : IProjectSystem
         private readonly Queue<ProjectFileInfo> _projectsToProcess;
         private readonly ProjectFileInfoCollection _projects;
 
+        private ProjectLoader _loader;
         private MSBuildOptions _options;
         private string _solutionFileOrRootPath;
 
@@ -65,7 +66,7 @@ public MSBuildProjectSystem(
         {
             _environment = environment;
             _workspace = workspace;
-            _msbuildInstance = msbuildLocator.RegisteredInstance;
+            _propertyOverrides = msbuildLocator.RegisteredInstance.PropertyOverrides;
             _dotNetCli = dotNetCliService;
             _metadataFileReferenceCache = metadataFileReferenceCache;
             _eventEmitter = eventEmitter;
@@ -89,6 +90,8 @@ public void Initalize(IConfiguration configuration)
                 _logger.LogDebug($"MSBuild environment: {Environment.NewLine}{buildEnvironmentInfo}");
             }
 
+            _loader = new ProjectLoader(_options, _environment.TargetDirectory, _propertyOverrides, _loggerFactory);
+
             var initialProjectPaths = GetInitialProjectPaths();
 
             foreach (var projectPath in initialProjectPaths)
@@ -316,7 +319,7 @@ private ProjectFileInfo LoadProject(string projectFilePath)
 
             try
             {
-                (project, diagnostics) = ProjectFileInfo.Create(projectFilePath, _environment.TargetDirectory, _loggerFactory.CreateLogger<ProjectFileInfo>(), _msbuildInstance, _options);
+                (project, diagnostics) = ProjectFileInfo.Create(projectFilePath, _loader);
 
                 if (project == null)
                 {
@@ -344,7 +347,7 @@ private void OnProjectChanged(string projectFilePath, bool allowAutoRestore)
                     ProjectFileInfo newProjectFileInfo;
                     ImmutableArray<MSBuildDiagnostic> diagnostics;
 
-                    (newProjectFileInfo, diagnostics) = oldProjectFileInfo.Reload(_environment.TargetDirectory, _loggerFactory.CreateLogger<ProjectFileInfo>(), _msbuildInstance, _options);
+                    (newProjectFileInfo, diagnostics) = oldProjectFileInfo.Reload(_loader);
 
                     if (newProjectFileInfo != null)
                     {

src/OmniSharp.MSBuild/Models/MSBuildProjectInfo.cs

@@ -19,7 +19,7 @@ public class MSBuildProjectInfo
         public bool IsExe { get; set; }
         public bool IsUnityProject { get; set; }
 
-        public MSBuildProjectInfo(ProjectFileInfo projectFileInfo)
+        internal MSBuildProjectInfo(ProjectFileInfo projectFileInfo)
         {
             AssemblyName = projectFileInfo.AssemblyName;
             Path = projectFileInfo.FilePath;

src/OmniSharp.MSBuild/Models/MSBuildWorkspaceInfo.cs

@@ -6,7 +6,7 @@ namespace OmniSharp.MSBuild.Models
 {
     public class MSBuildWorkspaceInfo
     {
-        public MSBuildWorkspaceInfo(string solutionFilePath, IEnumerable<ProjectFileInfo> projects, bool excludeSourceFiles)
+        internal MSBuildWorkspaceInfo(string solutionFilePath, IEnumerable<ProjectFileInfo> projects, bool excludeSourceFiles)
         {
             SolutionPath = solutionFilePath;
 

src/OmniSharp.MSBuild/ProjectFile/ItemNames.cs

@@ -0,0 +1,11 @@
+namespace OmniSharp.MSBuild.ProjectFile
+{
+    internal static class ItemNames
+    {
+        public const string Analyzer = nameof(Analyzer);
+        public const string Compile = nameof(Compile);
+        public const string PackageReference = nameof(PackageReference);
+        public const string ProjectReference = nameof(ProjectReference);
+        public const string ReferencePath = nameof(ReferencePath);
+    }
+}

src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs

@@ -0,0 +1,11 @@
+namespace OmniSharp.MSBuild.ProjectFile
+{
+    internal static class MetadataNames
+    {
+        public const string FullPath = nameof(FullPath);
+        public const string IsImplicitlyDefined = nameof(IsImplicitlyDefined);
+        public const string Project = nameof(Project);
+        public const string ReferenceSourceTarget = nameof(ReferenceSourceTarget);
+        public const string Version = nameof(Version);
+    }
+}

src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs

@@ -11,7 +11,7 @@
 
 namespace OmniSharp.MSBuild.ProjectFile
 {
-    public partial class ProjectFileInfo
+    internal partial class ProjectFileInfo
     {
         private class ProjectData
         {

src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs

@@ -4,18 +4,13 @@
 using System.IO;
 using System.Linq;
 using System.Runtime.Versioning;
-using Microsoft.Build.Evaluation;
-using Microsoft.Build.Execution;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
-using Microsoft.Extensions.Logging;
-using OmniSharp.MSBuild.Discovery;
 using OmniSharp.MSBuild.Logging;
-using OmniSharp.Options;
 
 namespace OmniSharp.MSBuild.ProjectFile
 {
-    public partial class ProjectFileInfo
+    internal partial class ProjectFileInfo
     {
         private readonly ProjectData _data;
 
@@ -68,15 +63,14 @@ private ProjectFileInfo(
             _data = data;
         }
 
-        public static (ProjectFileInfo projectFileInfo, ImmutableArray<MSBuildDiagnostic> diagnostics) Create(
-            string filePath, string solutionDirectory, ILogger logger, MSBuildInstance msbuildInstance, MSBuildOptions options = null)
+        public static (ProjectFileInfo projectFileInfo, ImmutableArray<MSBuildDiagnostic> diagnostics) Create(string filePath, ProjectLoader loader)
         {
             if (!File.Exists(filePath))
             {
                 return (null, ImmutableArray<MSBuildDiagnostic>.Empty);
             }
 
-            var (projectInstance, diagnostics) = LoadProject(filePath, solutionDirectory, logger, msbuildInstance, options);
+            var (projectInstance, diagnostics) = loader.BuildProject(filePath);
             if (projectInstance == null)
             {
                 return (null, diagnostics);
@@ -89,107 +83,9 @@ public static (ProjectFileInfo projectFileInfo, ImmutableArray<MSBuildDiagnostic
             return (projectFileInfo, diagnostics);
         }
 
-        private static (ProjectInstance projectInstance, ImmutableArray<MSBuildDiagnostic> diagnostics) LoadProject(
-            string filePath, string solutionDirectory, ILogger logger,
-            MSBuildInstance msbuildInstance, MSBuildOptions options)
+        public (ProjectFileInfo projectFileInfo, ImmutableArray<MSBuildDiagnostic> diagnostics) Reload(ProjectLoader loader)
         {
-            options = options ?? new MSBuildOptions();
-
-            var globalProperties = GetGlobalProperties(msbuildInstance, options, solutionDirectory, logger);
-
-            var collection = new ProjectCollection(globalProperties);
-
-            var toolsVersion = options.ToolsVersion;
-            if (string.IsNullOrEmpty(toolsVersion) || Version.TryParse(toolsVersion, out _))
-            {
-                toolsVersion = collection.DefaultToolsVersion;
-            }
-
-            toolsVersion = GetLegalToolsetVersion(toolsVersion, collection.Toolsets);
-
-            // Evaluate the MSBuild project
-            var project = collection.LoadProject(filePath, toolsVersion);
-
-            var targetFramework = project.GetPropertyValue(PropertyNames.TargetFramework);
-            var targetFrameworks = PropertyConverter.SplitList(project.GetPropertyValue(PropertyNames.TargetFrameworks), ';');
-
-            // If the project supports multiple target frameworks and specific framework isn't
-            // selected, we must pick one before execution. Otherwise, the ResolveReferences
-            // target might not be available to us.
-            if (string.IsNullOrWhiteSpace(targetFramework) && targetFrameworks.Length > 0)
-            {
-                // For now, we'll just pick the first target framework. Eventually, we'll need to
-                // do better and potentially allow OmniSharp hosts to select a target framework.
-                targetFramework = targetFrameworks[0];
-                project.SetProperty(PropertyNames.TargetFramework, targetFramework);
-            }
-            else if (!string.IsNullOrWhiteSpace(targetFramework) && targetFrameworks.Length == 0)
-            {
-                targetFrameworks = ImmutableArray.Create(targetFramework);
-            }
-
-            var projectInstance = project.CreateProjectInstance();
-            var msbuildLogger = new MSBuildLogger(logger);
-            var buildResult = projectInstance.Build(
-                targets: new string[] { TargetNames.Compile, TargetNames.CoreCompile },
-                loggers: new[] { msbuildLogger });
-
-            var diagnostics = msbuildLogger.GetDiagnostics();
-
-            return buildResult
-                ? (projectInstance, diagnostics)
-                : (null, diagnostics);
-        }
-
-        private static string GetLegalToolsetVersion(string toolsVersion, ICollection<Toolset> toolsets)
-        {
-            // It's entirely possible the the toolset specified does not exist. In that case, we'll try to use
-            // the highest version available.
-            var version = new Version(toolsVersion);
-
-            bool exists = false;
-            Version highestVersion = null;
-
-            var legalToolsets = new SortedList<Version, Toolset>(toolsets.Count);
-            foreach (var toolset in toolsets)
-            {
-                // Only consider this toolset if it has a legal version, we haven't seen it, and its path exists.
-                if (Version.TryParse(toolset.ToolsVersion, out var toolsetVersion) &&
-                    !legalToolsets.ContainsKey(toolsetVersion) &&
-                    System.IO.Directory.Exists(toolset.ToolsPath))
-                {
-                    legalToolsets.Add(toolsetVersion, toolset);
-
-                    if (highestVersion == null ||
-                        toolsetVersion > highestVersion)
-                    {
-                        highestVersion = toolsetVersion;
-                    }
-
-                    if (toolsetVersion == version)
-                    {
-                        exists = true;
-                    }
-                }
-            }
-
-            if (highestVersion == null)
-            {
-                throw new InvalidOperationException("No legal MSBuild toolsets available.");
-            }
-
-            if (!exists)
-            {
-                toolsVersion = legalToolsets[highestVersion].ToolsPath;
-            }
-
-            return toolsVersion;
-        }
-
-        public (ProjectFileInfo projectFileInfo, ImmutableArray<MSBuildDiagnostic> diagnostics) Reload(
-            string solutionDirectory, ILogger logger, MSBuildInstance msbuildInstance, MSBuildOptions options = null)
-        {
-            var (projectInstance, diagnostics) = LoadProject(FilePath, solutionDirectory, logger, msbuildInstance, options);
+            var (projectInstance, diagnostics) = loader.BuildProject(FilePath);
             if (projectInstance == null)
             {
                 return (null, diagnostics);
@@ -202,42 +98,12 @@ private static string GetLegalToolsetVersion(string toolsVersion, ICollection<To
         }
 
         public bool IsUnityProject()
-        {
-            return References.Any(filePath =>
-            {
-                var fileName = Path.GetFileName(filePath);
-
-                return string.Equals(fileName, "UnityEngine.dll", StringComparison.OrdinalIgnoreCase)
-                    || string.Equals(fileName, "UnityEditor.dll", StringComparison.OrdinalIgnoreCase);
-            });
-        }
+            => References.Any(filePath =>
+                {
+                    var fileName = Path.GetFileName(filePath);
 
-        private static Dictionary<string, string> GetGlobalProperties(MSBuildInstance msbuildInstance, MSBuildOptions options, string solutionDirectory, ILogger logger)
-        {
-            var globalProperties = new Dictionary<string, string>
-            {
-                { PropertyNames.DesignTimeBuild, "true" },
-                { PropertyNames.BuildingInsideVisualStudio, "true" },
-                { PropertyNames.BuildProjectReferences, "false" },
-                { PropertyNames._ResolveReferenceDependencies, "true" },
-                { PropertyNames.SolutionDir, solutionDirectory + Path.DirectorySeparatorChar },
-
-                // This properties allow the design-time build to handle the Compile target without actually invoking the compiler.
-                // See https://github.com/dotnet/roslyn/pull/4604 for details.
-                { PropertyNames.ProvideCommandLineArgs, "true" },
-                { PropertyNames.SkipCompilerExecution, "true" }
-            };
-
-            globalProperties.AddPropertyOverride(PropertyNames.MSBuildExtensionsPath, options.MSBuildExtensionsPath, msbuildInstance.PropertyOverrides, logger);
-            globalProperties.AddPropertyOverride(PropertyNames.TargetFrameworkRootPath, options.TargetFrameworkRootPath, msbuildInstance.PropertyOverrides, logger);
-            globalProperties.AddPropertyOverride(PropertyNames.RoslynTargetsPath, options.RoslynTargetsPath, msbuildInstance.PropertyOverrides, logger);
-            globalProperties.AddPropertyOverride(PropertyNames.CscToolPath, options.CscToolPath, msbuildInstance.PropertyOverrides, logger);
-            globalProperties.AddPropertyOverride(PropertyNames.CscToolExe, options.CscToolExe, msbuildInstance.PropertyOverrides, logger);
-            globalProperties.AddPropertyOverride(PropertyNames.VisualStudioVersion, options.VisualStudioVersion, msbuildInstance.PropertyOverrides, logger);
-            globalProperties.AddPropertyOverride(PropertyNames.Configuration, options.Configuration, msbuildInstance.PropertyOverrides, logger);
-            globalProperties.AddPropertyOverride(PropertyNames.Platform, options.Platform, msbuildInstance.PropertyOverrides, logger);
-
-            return globalProperties;
-        }
+                    return string.Equals(fileName, "UnityEngine.dll", StringComparison.OrdinalIgnoreCase)
+                        || string.Equals(fileName, "UnityEditor.dll", StringComparison.OrdinalIgnoreCase);
+                });
     }
 }