Show diagnostic beneath unresolved package/project reference in Solut…

…ion Explorer (#5146)

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/AssetsFileTopLevelDependenciesCollectionSourceProvider.cs

@@ -26,17 +26,17 @@ namespace NuGet.VisualStudio.SolutionExplorer
     /// <remarks>
     /// Templates out common code with a bunch of protected methods to override for specific item types.
     /// </remarks>
-    internal abstract class AssetsFileTopLevelDependenciesCollectionSourceProvider<TIdentity, TItem> : DependenciesAttachedCollectionSourceProviderBase
+    internal abstract class AssetsFileTopLevelDependenciesCollectionSourceProvider<TItem> : DependenciesAttachedCollectionSourceProviderBase
         where TItem : class, IRelatableItem
     {
         protected AssetsFileTopLevelDependenciesCollectionSourceProvider(ProjectTreeFlags flags)
             : base(flags)
         {
         }
 
-        protected abstract bool TryGetIdentity(Properties properties, out TIdentity identity);
+        protected abstract bool TryGetLibraryName(Properties properties, [NotNullWhen(returnValue: true)] out string? libraryName);
 
-        protected abstract bool TryGetLibrary(AssetsFileTarget target, TIdentity identity, [NotNullWhen(returnValue: true)] out AssetsFileTargetLibrary? library);
+        protected abstract bool TryGetLibrary(AssetsFileTarget target, string libraryName, [NotNullWhen(returnValue: true)] out AssetsFileTargetLibrary? library);
 
         protected abstract TItem CreateItem(AssetsFileTarget targetData, AssetsFileTargetLibrary library);
 
@@ -51,7 +51,7 @@ protected override bool TryCreateCollectionSource(
         {
             ThreadHelper.ThrowIfNotOnUIThread();
 
-            TIdentity identity;
+            string? libraryName;
 
             if (!ErrorHandler.Succeeded(hierarchyItem.HierarchyIdentity.Hierarchy.GetProperty(
                 hierarchyItem.HierarchyIdentity.ItemID, (int)__VSHPROPID.VSHPROPID_ExtObject, out object projectItemObject)))
@@ -63,7 +63,7 @@ protected override bool TryCreateCollectionSource(
             {
                 Properties? properties = (projectItemObject as ProjectItem)?.Properties;
 
-                if (properties == null || !TryGetIdentity(properties, out identity))
+                if (properties == null || !TryGetLibraryName(properties, out libraryName))
                 {
                     containsCollectionSource = null;
                     return false;
@@ -99,7 +99,7 @@ protected override bool TryCreateCollectionSource(
                     AssetsFileDependenciesSnapshot snapshot = versionedValue.Value;
                     if (snapshot.TryGetTarget(target, out AssetsFileTarget? targetData))
                     {
-                        if (TryGetLibrary(targetData, identity, out AssetsFileTargetLibrary? library))
+                        if (TryGetLibrary(targetData, libraryName, out AssetsFileTargetLibrary? library))
                         {
                             if (item == null)
                             {

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Items/PackageAssemblyItem.cs

@@ -67,7 +67,9 @@ public string? Path
             {
                 get
                 {
-                    return _item.Target.TryResolvePackagePath(_item.Library.Name, _item.Library.Version, out string? fullPath)
+                    return
+                        _item.Library.Version is not null &&
+                        _item.Target.TryResolvePackagePath(_item.Library.Name, _item.Library.Version, out string? fullPath)
                         ? System.IO.Path.GetFullPath(System.IO.Path.Combine(fullPath, _item.Path))
                         : null;
                 }

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Items/PackageBuildFileItem.cs

@@ -40,7 +40,7 @@ public PackageBuildFileItem(AssetsFileTarget target, AssetsFileTargetLibrary lib
             _fileOpener = fileOpener;
         }
 
-        public string? FullPath => Target.TryResolvePackagePath(Library.Name, Library.Version, out string? fullPath)
+        public string? FullPath => Library.Version is not null && Target.TryResolvePackagePath(Library.Name, Library.Version, out string? fullPath)
             ? System.IO.Path.GetFullPath(System.IO.Path.Combine(fullPath, Path))
             : null;
 

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Items/PackageDocumentItem.cs

@@ -68,7 +68,7 @@ public bool Invoke(IEnumerable<object> items, InputSource inputSource, bool prev
             return result;
         }
 
-        public string? FullPath => Target.TryResolvePackagePath(Library.Name, Library.Version, out string? fullPath)
+        public string? FullPath => Library.Version is not null && Target.TryResolvePackagePath(Library.Name, Library.Version, out string? fullPath)
             ? System.IO.Path.GetFullPath(System.IO.Path.Combine(fullPath, Path))
             : null;
 

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Items/PackageReferenceItem.cs

@@ -44,7 +44,7 @@ internal bool TryUpdateState(AssetsFileTarget target, AssetsFileTargetLibrary li
             return true;
         }
 
-        private static string GetCaption(AssetsFileTargetLibrary library) => $"{library.Name} ({library.Version})";
+        private static string GetCaption(AssetsFileTargetLibrary library) => library.Version is not null ? $"{library.Name} ({library.Version})" : library.Name;
 
         public override object Identity => Library.Name;
 
@@ -71,7 +71,7 @@ private sealed class BrowseObject : LocalizableProperties
 
             public BrowseObject(PackageReferenceItem item) => _item = item;
 
-            public override string GetComponentName() => $"{_item.Library.Name} ({_item.Library.Version})";
+            public override string GetComponentName() => _item.Library.Version is not null ? $"{_item.Library.Name} ({_item.Library.Version})" : _item.Library.Name;
 
             public override string GetClassName() => VsResources.PackageReferenceBrowseObjectClassName;
 
@@ -81,11 +81,11 @@ private sealed class BrowseObject : LocalizableProperties
 
             [BrowseObjectDisplayName(nameof(VsResources.PackageReferenceVersionDisplayName))]
             [BrowseObjectDescription(nameof(VsResources.PackageReferenceVersionDescription))]
-            public string Version => _item.Library.Version;
+            public string? Version => _item.Library.Version;
 
             [BrowseObjectDisplayName(nameof(VsResources.PackageReferencePathDisplayName))]
             [BrowseObjectDescription(nameof(VsResources.PackageReferencePathDescription))]
-            public string? Path => _item.Target.TryResolvePackagePath(_item.Library.Name, _item.Library.Version, out string? fullPath) ? fullPath : null;
+            public string? Path => _item.Library.Version is not null && _item.Target.TryResolvePackagePath(_item.Library.Name, _item.Library.Version, out string? fullPath) ? fullPath : null;
         }
     }
 }

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Models/AssetsFileDependenciesSnapshot.cs

@@ -90,13 +90,15 @@ private AssetsFileDependenciesSnapshot(LockFile? lockFile, AssetsFileDependencie
 
                 previous.DataByTarget.TryGetValue(lockFileTarget.Name, out AssetsFileTarget? previousTarget);
 
+                ImmutableArray<AssetsFileLogMessage> logMessages = ParseLogMessages(lockFile, previousTarget, lockFileTarget.Name);
+
                 dataByTarget.Add(
                     lockFileTarget.Name,
                     new AssetsFileTarget(
                         this,
                         lockFileTarget.Name,
-                        ParseLogMessages(lockFile, previousTarget, lockFileTarget.Name),
-                        ParseLibraries(lockFile, lockFileTarget)));
+                        logMessages,
+                        ParseLibraries(lockFile, lockFileTarget, logMessages)));
             }
 
             DataByTarget = dataByTarget.ToImmutable();
@@ -123,22 +125,22 @@ static ImmutableArray<AssetsFileLogMessage> ParseLogMessages(LockFile lockFile,
 
                     j++;
 
-                    if (j < previousLogs.Length && previousLogs[j].Equals(logMessage))
+                    if (j < previousLogs.Length && previousLogs[j].Equals(logMessage, lockFile.PackageSpec.FilePath))
                     {
                         // Unchanged, so use previous value
                         builder.Add(previousLogs[j]);
                     }
                     else
                     {
-                        builder.Add(new AssetsFileLogMessage(logMessage));
+                        builder.Add(new AssetsFileLogMessage(lockFile.PackageSpec.FilePath, logMessage));
                     }
                 }
 
                 return builder.ToImmutable();
             }
         }
 
-        internal static ImmutableDictionary<string, AssetsFileTargetLibrary> ParseLibraries(LockFile lockFile, LockFileTarget lockFileTarget)
+        internal static ImmutableDictionary<string, AssetsFileTargetLibrary> ParseLibraries(LockFile lockFile, LockFileTarget lockFileTarget, ImmutableArray<AssetsFileLogMessage> logMessages)
         {
             ImmutableDictionary<string, AssetsFileTargetLibrary>.Builder builder = ImmutableDictionary.CreateBuilder<string, AssetsFileTargetLibrary>(StringComparer.OrdinalIgnoreCase);
 
@@ -150,6 +152,19 @@ internal static ImmutableDictionary<string, AssetsFileTargetLibrary> ParseLibrar
                 }
             }
 
+            // If a non-existent library is referenced, it will have an error log message, but no entry in "libraries".
+            // We want to show a diagnostic node beneath such nodes in the tree, so need to create a dummy library entry,
+            // otherwise there's nothing to attach that diagnostic to.
+            foreach (AssetsFileLogMessage message in logMessages)
+            {
+                string libraryName = message.LibraryName;
+
+                if (!builder.ContainsKey(libraryName))
+                {
+                    builder.Add(libraryName, AssetsFileTargetLibrary.CreatePlaceholder(libraryName));
+                }
+            }
+
             return builder.ToImmutable();
         }
 

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Models/AssetsFileLibraryType.cs

@@ -11,6 +11,22 @@ namespace NuGet.VisualStudio.SolutionExplorer.Models
     internal enum AssetsFileLibraryType : byte
     {
         Package,
-        Project
+
+        Project,
+
+        /// <summary>
+        /// When it is unknown whether the library represents a package or a project.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// This occurs, for example, when a referenced item is not found. It will be present in the lock file's
+        /// messages, but not elsewhere in the lock file.
+        /// </para>
+        /// <para>
+        /// This enum member exists only to support attaching diagnostic items to their package or project reference
+        /// node in the tree.
+        /// </para>
+        /// </remarks>
+        Unknown
     }
 }

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Models/AssetsFileLogMessage.cs

@@ -3,6 +3,7 @@
 
 #nullable enable
 
+using System.IO;
 using NuGet.Common;
 using NuGet.ProjectModel;
 
@@ -13,13 +14,13 @@ namespace NuGet.VisualStudio.SolutionExplorer.Models
     /// </summary>
     internal readonly struct AssetsFileLogMessage
     {
-        public AssetsFileLogMessage(IAssetsLogMessage logMessage)
+        public AssetsFileLogMessage(string projectFilePath, IAssetsLogMessage logMessage)
         {
             Code = logMessage.Code;
             Level = logMessage.Level;
             WarningLevel = logMessage.WarningLevel;
             Message = logMessage.Message;
-            LibraryName = logMessage.LibraryId;
+            LibraryName = NormalizeLibraryName(logMessage.LibraryId, projectFilePath);
         }
 
         public NuGetLogCode Code { get; }
@@ -28,13 +29,27 @@ public AssetsFileLogMessage(IAssetsLogMessage logMessage)
         public string Message { get; }
         public string LibraryName { get; }
 
-        public bool Equals(IAssetsLogMessage other)
+        public bool Equals(IAssetsLogMessage other, string projectFilePath)
         {
             return other.Code == Code
                 && other.Level == Level
                 && other.WarningLevel == WarningLevel
                 && other.Message == Message
-                && other.LibraryId == LibraryName;
+                && NormalizeLibraryName(other.LibraryId, projectFilePath) == LibraryName;
+        }
+
+        private static string NormalizeLibraryName(string libraryName, string projectFilePath)
+        {
+            // If we have a rooted path for the library in the messages, it is an unresolved project reference.
+            // Other identifiers for this item will be a relative path with respect to the project.
+            // So, we compute that relative path here such that it will match correctly. This enables us to
+            // display diagnostic items beneath unresolved project references in Solution Explorer.
+            if (Path.IsPathRooted(libraryName))
+            {
+                return PathUtility.GetRelativePath(projectFilePath, libraryName);
+            }
+
+            return libraryName;
         }
 
         public override string ToString() => $"{Level} {Code} ({LibraryName}) {Message}";

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Models/AssetsFileTarget.cs

@@ -145,14 +145,12 @@ public bool TryGetDependencies(string libraryName, string? version, out Immutabl
             return true;
         }
 
-        public bool TryGetPackage(string packageId, string version, [NotNullWhen(returnValue: true)] out AssetsFileTargetLibrary? assetsFileLibrary)
+        public bool TryGetPackage(string packageId, [NotNullWhen(returnValue: true)] out AssetsFileTargetLibrary? assetsFileLibrary)
         {
             Requires.NotNull(packageId, nameof(packageId));
-            Requires.NotNull(version, nameof(version));
 
             if (LibraryByName.TryGetValue(packageId, out assetsFileLibrary) &&
-                assetsFileLibrary.Type == AssetsFileLibraryType.Package &&
-                assetsFileLibrary.Version == version)
+                assetsFileLibrary.Type is AssetsFileLibraryType.Package or AssetsFileLibraryType.Unknown)
             {
                 return true;
             }
@@ -166,7 +164,7 @@ public bool TryGetProject(string projectId, [NotNullWhen(returnValue: true)] out
             Requires.NotNull(projectId, nameof(projectId));
 
             if (LibraryByName.TryGetValue(projectId, out assetsFileLibrary) &&
-                assetsFileLibrary.Type == AssetsFileLibraryType.Project)
+                assetsFileLibrary.Type is AssetsFileLibraryType.Project or AssetsFileLibraryType.Unknown)
             {
                 return true;
             }

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/Models/AssetsFileTargetLibrary.cs

@@ -39,6 +39,33 @@ public static bool TryCreate(LockFile lockFile, LockFileTargetLibrary lockFileLi
             return true;
         }
 
+        /// <summary>
+        /// Creates a dummy placeholder for a library in cases where we only know the name of the library.
+        /// </summary>
+        /// <remarks>
+        /// This is useful, for example, when a referenced package does not exist. It will not have an entry in "libraries",
+        /// yet we want to model its presence in our snapshot so that we can display diagnostic nodes to it. For such libraries,
+        /// the version is unknown, which we represent with a <see langword="null"/> value.
+        /// </remarks>
+        public static AssetsFileTargetLibrary CreatePlaceholder(string name)
+        {
+            return new AssetsFileTargetLibrary(name);
+        }
+
+        private AssetsFileTargetLibrary(string name)
+        {
+            Name = name;
+            Version = null;
+            Type = AssetsFileLibraryType.Unknown;
+            Dependencies = ImmutableArray<string>.Empty;
+            FrameworkAssemblies = ImmutableArray<string>.Empty;
+            CompileTimeAssemblies = ImmutableArray<string>.Empty;
+            ContentFiles = ImmutableArray<AssetsFileTargetLibraryContentFile>.Empty;
+            BuildFiles = ImmutableArray<string>.Empty;
+            BuildMultiTargetingFiles = ImmutableArray<string>.Empty;
+            DocumentationFiles = ImmutableArray<string>.Empty;
+        }
+
         private AssetsFileTargetLibrary(LockFileLibrary? library, LockFileTargetLibrary targetLibrary, AssetsFileLibraryType type)
         {
             Name = targetLibrary.Name;
@@ -103,7 +130,15 @@ static bool IsDocumentationFile(string path)
         }
 
         public string Name { get; }
-        public string Version { get; }
+
+        /// <summary>
+        /// Gets the version of the library, or <see langword="null"/> if it is unknown.
+        /// </summary>
+        /// <remarks>
+        /// The version can be unknown for packages that fail to resolve at all, for example when the package
+        /// name is not found. For resolved packages however, this value will always be present.
+        /// </remarks>
+        public string? Version { get; }
         public AssetsFileLibraryType Type { get; }
         public ImmutableArray<string> Dependencies { get; }
         public ImmutableArray<string> FrameworkAssemblies { get; }
@@ -113,6 +148,6 @@ static bool IsDocumentationFile(string path)
         public ImmutableArray<string> BuildMultiTargetingFiles { get; }
         public ImmutableArray<string> DocumentationFiles { get; }
 
-        public override string ToString() => $"{Type} {Name} ({Version}) {Dependencies.Length} {(Dependencies.Length == 1 ? "dependency" : "dependencies")}";
+        public override string ToString() => $"{Type} {Name} ({Version ?? "Unknown"}) {Dependencies.Length} {(Dependencies.Length == 1 ? "dependency" : "dependencies")}";
     }
 }

src/NuGet.Clients/NuGet.VisualStudio.Implementation/SolutionExplorer/PackageReferenceAttachedCollectionSourceProvider.cs

@@ -18,25 +18,23 @@ namespace NuGet.VisualStudio.SolutionExplorer
     [Export(typeof(IAttachedCollectionSourceProvider))]
     [Name(nameof(PackageReferenceAttachedCollectionSourceProvider))]
     [Order(Before = HierarchyItemsProviderNames.Contains)]
-    internal sealed class PackageReferenceAttachedCollectionSourceProvider : AssetsFileTopLevelDependenciesCollectionSourceProvider<(string Name, string Version), PackageReferenceItem>
+    internal sealed class PackageReferenceAttachedCollectionSourceProvider : AssetsFileTopLevelDependenciesCollectionSourceProvider<PackageReferenceItem>
     {
         public PackageReferenceAttachedCollectionSourceProvider()
             : base(DependencyTreeFlags.PackageDependency)
         {
         }
 
-        protected override bool TryGetIdentity(Properties properties, out (string Name, string Version) identity)
+        protected override bool TryGetLibraryName(Properties properties, [NotNullWhen(returnValue: true)] out string? libraryName)
         {
             ThreadHelper.ThrowIfNotOnUIThread();
 
             try
             {
                 if (properties.Item("Name")?.Value is string name &&
-                    properties.Item("Version")?.Value is string version &&
-                    !string.IsNullOrEmpty(name) &&
-                    !string.IsNullOrEmpty(version))
+                    !string.IsNullOrEmpty(name))
                 {
-                    identity = (name, version);
+                    libraryName = name;
                     return true;
                 }
             }
@@ -46,13 +44,13 @@ protected override bool TryGetIdentity(Properties properties, out (string Name,
                 // "Could not find project item with item type 'PackageReference' and include value '...'.
             }
 
-            identity = default;
+            libraryName = null;
             return false;
         }
 
-        protected override bool TryGetLibrary(AssetsFileTarget target, (string Name, string Version) identity, [NotNullWhen(returnValue: true)] out AssetsFileTargetLibrary? library)
+        protected override bool TryGetLibrary(AssetsFileTarget target, string libraryName, [NotNullWhen(returnValue: true)] out AssetsFileTargetLibrary? library)
         {
-            return target.TryGetPackage(identity.Name, identity.Version, out library);
+            return target.TryGetPackage(libraryName, out library);
         }
 
         protected override PackageReferenceItem CreateItem(AssetsFileTarget targetData, AssetsFileTargetLibrary library)