Clean up unresolved dependencies check

src/OmniSharp.Abstractions/Eventing/IEventEmitterExtensions.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using OmniSharp.Models.Events;
 
 namespace OmniSharp.Eventing
@@ -29,5 +30,16 @@ public static void RestoreFinished(this IEventEmitter emitter, string projectPat
                     Succeeded = succeeded
                 });
         }
+
+        public static void UnresolvedDepdendencies(this IEventEmitter emitter, string projectFilePath, IEnumerable<PackageDependency> unresolvedDependencies)
+        {
+            emitter.Emit(
+                EventTypes.UnresolvedDependencies,
+                new UnresolvedDependenciesMessage
+                {
+                    FileName = projectFilePath,
+                    UnresolvedDependencies = unresolvedDependencies
+                });
+        }
     }
 }

src/OmniSharp.DotNet/DotNetProjectSystem.cs

@@ -302,29 +302,30 @@ private void UpdateUnresolvedDependencies(ProjectState state, bool allowRestore)
         {
             var libraryManager = state.ProjectContext.LibraryManager;
             var allDiagnostics = libraryManager.GetAllDiagnostics();
-            var unresolved = libraryManager.GetLibraries().Where(dep => !dep.Resolved);
-            var needRestore = allDiagnostics.Any(diag => diag.ErrorCode == ErrorCodes.NU1006) || unresolved.Any();
+            var unresolvedLibraries = libraryManager.GetLibraries().Where(dep => !dep.Resolved);
+            var needRestore = allDiagnostics.Any(diag => diag.ErrorCode == ErrorCodes.NU1006) || unresolvedLibraries.Any();
 
             if (needRestore)
             {
+                var unresolvedDependencies = unresolvedLibraries.Select(library =>
+                    new PackageDependency
+                    {
+                        Name = library.Identity.Name,
+                        Version = library.Identity.Version?.ToString()
+                    });
+
+                var projectFile = state.ProjectContext.ProjectFile;
+
                 if (allowRestore && _enableRestorePackages)
                 {
-                    _dotNetCliService.RestoreAsync(state.ProjectContext.ProjectFile.ProjectDirectory, onFailure: () =>
+                    _dotNetCliService.RestoreAsync(projectFile.ProjectDirectory, onFailure: () =>
                     {
-                        _eventEmitter.Emit(EventTypes.UnresolvedDependencies, new UnresolvedDependenciesMessage()
-                        {
-                            FileName = state.ProjectContext.ProjectFile.ProjectFilePath,
-                            UnresolvedDependencies = unresolved.Select(d => new PackageDependency { Name = d.Identity.Name, Version = d.Identity.Version?.ToString() })
-                        });
+                        _eventEmitter.UnresolvedDepdendencies(projectFile.ProjectFilePath, unresolvedDependencies);
                     });
                 }
                 else
                 {
-                    _eventEmitter.Emit(EventTypes.UnresolvedDependencies, new UnresolvedDependenciesMessage()
-                    {
-                        FileName = state.ProjectContext.ProjectFile.ProjectFilePath,
-                        UnresolvedDependencies = unresolved.Select(d => new PackageDependency { Name = d.Identity.Name, Version = d.Identity.Version?.ToString() })
-                    });
+                    _eventEmitter.UnresolvedDepdendencies(projectFile.ProjectFilePath, unresolvedDependencies);
                 }
             }
         }

src/OmniSharp.MSBuild/MSBuildProjectSystem.cs

@@ -9,14 +9,14 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
-using NuGet.ProjectModel;
 using OmniSharp.Eventing;
 using OmniSharp.FileWatching;
 using OmniSharp.Models.Events;
 using OmniSharp.Models.WorkspaceInformation;
 using OmniSharp.MSBuild.Models;
 using OmniSharp.MSBuild.Models.Events;
 using OmniSharp.MSBuild.ProjectFile;
+using OmniSharp.MSBuild.Resolution;
 using OmniSharp.MSBuild.SolutionParsing;
 using OmniSharp.Options;
 using OmniSharp.Services;
@@ -34,6 +34,7 @@ public class MSBuildProjectSystem : IProjectSystem
         private readonly IFileSystemWatcher _fileSystemWatcher;
         private readonly ILoggerFactory _loggerFactory;
         private readonly ILogger _logger;
+        private readonly PackageDependencyResolver _packageDepedencyResolver;
 
         private readonly object _gate = new object();
         private readonly Queue<ProjectFileInfo> _projectsToProcess;
@@ -67,6 +68,7 @@ public MSBuildProjectSystem(
             _projects = new ProjectFileInfoCollection();
             _projectsToProcess = new Queue<ProjectFileInfo>();
             _logger = loggerFactory.CreateLogger<MSBuildProjectSystem>();
+            _packageDepedencyResolver = new PackageDependencyResolver(loggerFactory);
         }
 
         public void Initalize(IConfiguration configuration)
@@ -362,7 +364,7 @@ private void OnProjectChanged(string projectFilePath, bool allowAutoRestore)
                         _projects[projectFilePath] = newProjectFileInfo;
 
                         UpdateProject(newProjectFileInfo);
-                        CheckForUnresolvedDependences(newProjectFileInfo, oldProjectFileInfo, allowAutoRestore);
+                        CheckForUnresolvedDependences(newProjectFileInfo, allowAutoRestore);
                     }
                 }
 
@@ -532,116 +534,32 @@ private void UpdateReferences(Project project, ImmutableArray<string> references
             }
         }
 
-        private void CheckForUnresolvedDependences(ProjectFileInfo projectFileInfo, ProjectFileInfo previousProjectFileInfo = null, bool allowAutoRestore = false)
+        private void CheckForUnresolvedDependences(ProjectFileInfo projectFile, bool allowAutoRestore)
         {
-            List<PackageDependency> unresolvedDependencies;
-
-            if (!File.Exists(projectFileInfo.ProjectAssetsFile))
-            {
-                // Simplest case: If there's no lock file and the project file has package references,
-                // there are certainly unresolved dependencies.
-                unresolvedDependencies = CreatePackageDependencies(projectFileInfo.PackageReferences);
-            }
-            else
-            {
-                // Note: This is a bit of misnmomer. It's entirely possible that a package reference has been removed
-                // and a restore needs to happen in order to update project.assets.json file. Otherwise, the MSBuild targets
-                // will still resolve the removed reference as a reference in the user's project. In that case, the package
-                // reference isn't so much "unresolved" as "incorrectly resolved".
-                IEnumerable<PackageReference> unresolvedPackageReferences;
-
-                // Did the project file change? Diff the package references and see if there are unresolved dependencies.
-                if (previousProjectFileInfo != null)
-                {
-                    var remainingPackageReferences = new HashSet<PackageReference>(previousProjectFileInfo.PackageReferences);
-                    var packageReferencesToAdd = new HashSet<PackageReference>();
-
-                    foreach (var packageReference in projectFileInfo.PackageReferences)
-                    {
-                        if (remainingPackageReferences.Contains(packageReference))
-                        {
-                            remainingPackageReferences.Remove(packageReference);
-                        }
-                        else
-                        {
-                            packageReferencesToAdd.Add(packageReference);
-                        }
-                    }
-
-                    unresolvedPackageReferences = packageReferencesToAdd.Concat(remainingPackageReferences);
-                }
-                else
-                {
-                    // Finally, if the project.assets.json file exists but there's no old project to compare against,
-                    // we'll just check to ensure that all of the project's package references can be found in the
-                    // current project.assets.json file.
-
-                    var lockFileFormat = new LockFileFormat();
-                    var lockFile = lockFileFormat.Read(projectFileInfo.ProjectAssetsFile);
-
-                    unresolvedPackageReferences = projectFileInfo.PackageReferences
-                        .Where(pr => !ContainsPackageReference(lockFile, pr));
-                }
-
-                unresolvedDependencies = CreatePackageDependencies(unresolvedPackageReferences);
-            }
-
-            if (unresolvedDependencies.Count > 0)
+            var unresolvedPackageReferences = _packageDepedencyResolver.FindUnresolvedPackageReferences(projectFile);
+            if (unresolvedPackageReferences.IsEmpty)
             {
-                if (allowAutoRestore && _options.EnablePackageAutoRestore)
-                {
-                    _dotNetCli.RestoreAsync(projectFileInfo.Directory, onFailure: () =>
-                    {
-                        FireUnresolvedDependenciesEvent(projectFileInfo, unresolvedDependencies);
-                    });
-                }
-                else
-                {
-                    FireUnresolvedDependenciesEvent(projectFileInfo, unresolvedDependencies);
-                }
+                return;
             }
-        }
 
-        private List<PackageDependency> CreatePackageDependencies(IEnumerable<PackageReference> packageReferences)
-        {
-            var list = new List<PackageDependency>();
-
-            foreach (var packageReference in packageReferences)
-            {
-                var dependency = new PackageDependency
+            var unresolvedDependencies = unresolvedPackageReferences.Select(packageReference =>
+                new PackageDependency
                 {
                     Name = packageReference.Dependency.Id,
                     Version = packageReference.Dependency.VersionRange.ToNormalizedString()
-                };
-
-                list.Add(dependency);
-            }
-
-            return list;
-        }
+                });
 
-        private static bool ContainsPackageReference(LockFile lockFile, PackageReference reference)
-        {
-            foreach (var library in lockFile.Libraries)
+            if (allowAutoRestore && _options.EnablePackageAutoRestore)
             {
-                if (string.Equals(library.Name, reference.Dependency.Id, StringComparison.OrdinalIgnoreCase) &&
-                    reference.Dependency.VersionRange.Satisfies(library.Version))
+                _dotNetCli.RestoreAsync(projectFile.Directory, onFailure: () =>
                 {
-                    return true;
-                }
-            }
-
-            return false;
-        }
-
-        private void FireUnresolvedDependenciesEvent(ProjectFileInfo projectFileInfo, List<PackageDependency> unresolvedDependencies)
-        {
-            _eventEmitter.Emit(EventTypes.UnresolvedDependencies,
-                new UnresolvedDependenciesMessage()
-                {
-                    FileName = projectFileInfo.FilePath,
-                    UnresolvedDependencies = unresolvedDependencies
+                    _eventEmitter.UnresolvedDepdendencies(projectFile.FilePath, unresolvedDependencies);
                 });
+            }
+            else
+            {
+                _eventEmitter.UnresolvedDepdendencies(projectFile.FilePath, unresolvedDependencies);
+            }
         }
 
         private ProjectFileInfo GetProjectFileInfo(string path)

src/OmniSharp.MSBuild/Resolution/PackageDependencyResolver.cs

@@ -0,0 +1,108 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.IO;
+using System.Linq;
+using Microsoft.Extensions.Logging;
+using NuGet.ProjectModel;
+using OmniSharp.MSBuild.ProjectFile;
+
+namespace OmniSharp.MSBuild.Resolution
+{
+    internal class PackageDependencyResolver
+    {
+        private readonly ILogger _logger;
+
+        public PackageDependencyResolver(ILoggerFactory loggerFactory)
+        {
+            _logger = loggerFactory.CreateLogger<PackageDependencyResolver>();
+        }
+
+        public ImmutableArray<PackageReference> FindUnresolvedPackageReferences(ProjectFileInfo projectFile)
+        {
+            if (projectFile.PackageReferences.Length == 0)
+            {
+                return ImmutableArray<PackageReference>.Empty;
+            }
+
+            // If the lock file does not exist, all of the package references are unresolved.
+            if (!File.Exists(projectFile.ProjectAssetsFile))
+            {
+                return projectFile.PackageReferences;
+            }
+
+            var lockFileFormat = new LockFileFormat();
+            var lockFile = lockFileFormat.Read(projectFile.ProjectAssetsFile);
+
+            return FindUnresolvedPackageReferencesInLockFile(projectFile, lockFile);
+        }
+
+        private ImmutableArray<PackageReference> FindUnresolvedPackageReferencesInLockFile(ProjectFileInfo projectFile, LockFile lockFile)
+        {
+            var libraryMap = CreateLibraryMap(lockFile);
+
+            var unresolved = ImmutableArray.CreateBuilder<PackageReference>();
+
+            // Iterate through each package reference and see if we can find a library with the same name
+            // that satisfies the reference's version range in the lock file.
+
+            foreach (var reference in projectFile.PackageReferences)
+            {
+                if (!libraryMap.TryGetValue(reference.Dependency.Id, out var libraries))
+                {
+                    _logger.LogWarning($"{projectFile.Name}: Did not find '{reference.Dependency.Id}' in lock file.");
+                    unresolved.Add(reference);
+                }
+                else
+                {
+                    var found = false;
+                    foreach (var library in libraries)
+                    {
+                        if (reference.Dependency.VersionRange.Satisfies(library.Version))
+                        {
+                            found = true;
+                            break;
+                        }
+                    }
+
+                    if (!found)
+                    {
+                        var referenceText = reference.IsImplicitlyDefined
+                            ? "implicit package reference"
+                            : "package reference";
+
+                        var versions = string.Join(", ", libraries.Select(l => '"' + l.Version.ToString() + '"'));
+
+                        _logger.LogWarning($"{projectFile.Name}: Found {referenceText} '{reference.Dependency.Id}', but none of the versions in the lock file ({versions}) satisfy {reference.Dependency.VersionRange}");
+                        unresolved.Add(reference);
+                    }
+                }
+            }
+
+            return unresolved.ToImmutable();
+        }
+
+        private static Dictionary<string, List<LockFileLibrary>> CreateLibraryMap(LockFile lockFile)
+        {
+            // Create map of all libraries in the lock file by their name.
+            // Note that the map's key is case-insensitive.
+
+            var libraryMap = new Dictionary<string, List<LockFileLibrary>>(
+                capacity: lockFile.Libraries.Count,
+                comparer: StringComparer.OrdinalIgnoreCase);
+
+            foreach (var library in lockFile.Libraries)
+            {
+                if (!libraryMap.TryGetValue(library.Name, out var libraries))
+                {
+                    libraries = new List<LockFileLibrary>();
+                    libraryMap.Add(library.Name, libraries);
+                }
+
+                libraries.Add(library);
+            }
+
+            return libraryMap;
+        }
+    }
+}