Remove dead code from PackageSpecReferenceDependencyProvider, and use PackageSpec's TargetFrameworkInformation in GetReferenceNearestTargetFrameworkTask (#7065)

Author: nkolev92

src/NuGet.Core/NuGet.Build.Tasks/GetReferenceNearestTargetFrameworkTask.cs

@@ -10,14 +10,14 @@
 using Microsoft.Build.Utilities;
 using NuGet.Common;
 using NuGet.Frameworks;
-
-#if NETFRAMEWORK || NETSTANDARD
+#if NETFRAMEWORK
 using System.Linq;
 #endif
+using NuGet.ProjectModel;
 
 namespace NuGet.Build.Tasks
 {
-    public class GetReferenceNearestTargetFrameworkTask : Microsoft.Build.Utilities.Task
+    public class GetReferenceNearestTargetFrameworkTask : Task
     {
         private const string NEAREST_TARGET_FRAMEWORK = "NearestTargetFramework";
         private const string TARGET_FRAMEWORKS = "TargetFrameworks";
@@ -41,6 +41,7 @@ public class GetReferenceNearestTargetFrameworkTask : Microsoft.Build.Utilities.
         /// </summary>
         public string CurrentProjectTargetPlatform { get; set; }
 
+
         /// <summary>
         /// Optional list of target frameworks to be used as Fallback target frameworks.
         /// </summary>
@@ -140,27 +141,32 @@ private ITaskItem AssignNearestFrameworkForSingleReference(
             var useTargetMonikers = referencedProjectTargetFrameworkMonikers.Length > 0;
             for (int i = 0; i < referencedProjectFrameworks.Length; i++)
             {
-
-                targetFrameworkInformations.Add(new TargetFrameworkInformation(
-                    referencedProjectFrameworks[i],
-                    useTargetMonikers ? referencedProjectTargetFrameworkMonikers[i] : null,
-                    useTargetMonikers ? referencedProjectTargetPlatformMonikers[i] : null));
+                targetFrameworkInformations.Add(new TargetFrameworkInformation()
+                {
+                    TargetAlias = referencedProjectFrameworks[i],
+                    FrameworkName = GetNuGetFramework(
+                                        referencedProjectFrameworks[i],
+                                        useTargetMonikers ? referencedProjectTargetFrameworkMonikers[i] : null,
+                                        useTargetMonikers ? referencedProjectTargetPlatformMonikers[i] : null)
+                });
             }
 
-            // try project framework
-            var nearestNuGetFramework = NuGetFrameworkUtility.GetNearest(targetFrameworkInformations, projectNuGetFramework, GetNuGetFramework);
-            if (nearestNuGetFramework != null)
+            var packageSpec = new PackageSpec(targetFrameworkInformations);
+
+            var nearestNuGetFramework = packageSpec.GetTargetFramework(projectNuGetFramework);
+
+            if (nearestNuGetFramework.FrameworkName != null)
             {
-                itemWithProperties.SetMetadata(NEAREST_TARGET_FRAMEWORK, nearestNuGetFramework._targetFrameworkAlias);
+                itemWithProperties.SetMetadata(NEAREST_TARGET_FRAMEWORK, nearestNuGetFramework.TargetAlias);
                 return itemWithProperties;
             }
 
             // try project fallback frameworks
             foreach (var currentProjectTargetFramework in fallbackNuGetFrameworks)
             {
-                nearestNuGetFramework = NuGetFrameworkUtility.GetNearest(targetFrameworkInformations, currentProjectTargetFramework, GetNuGetFramework);
+                nearestNuGetFramework = packageSpec.GetTargetFramework(currentProjectTargetFramework);
 
-                if (nearestNuGetFramework != null)
+                if (nearestNuGetFramework.FrameworkName != null)
                 {
                     var message = string.Format(CultureInfo.CurrentCulture,
                         Strings.ImportsFallbackWarning,
@@ -175,7 +181,7 @@ private ITaskItem AssignNearestFrameworkForSingleReference(
                     // log NU1702 for ATF on project reference
                     logger.Log(warning);
 
-                    itemWithProperties.SetMetadata(NEAREST_TARGET_FRAMEWORK, nearestNuGetFramework._targetFrameworkAlias);
+                    itemWithProperties.SetMetadata(NEAREST_TARGET_FRAMEWORK, nearestNuGetFramework.TargetAlias);
                     return itemWithProperties;
                 }
             }
@@ -222,34 +228,19 @@ private static bool TryParseFramework(string targetFrameworkMoniker, string targ
             return true;
         }
 
-        private static NuGetFramework GetNuGetFramework(TargetFrameworkInformation targetFrameworkInformation)
+        private static NuGetFramework GetNuGetFramework(string targetAlias, string targetFrameworkMoniker, string targetPlatformMoniker)
         {
             // Legacy path, process targetFrameworks if empty
-            if (string.IsNullOrEmpty(targetFrameworkInformation._targetFrameworkMoniker))
+            if (string.IsNullOrEmpty(targetFrameworkMoniker))
             {
-                return NuGetFramework.Parse(targetFrameworkInformation._targetFrameworkAlias);
+                return NuGetFramework.Parse(targetAlias);
             }
 
             // TargetFrameworkMoniker is always expected to be set. TargetPlatformMoniker will have a `None` value when empty, for frameworks like net5.0.
-            return NuGetFramework.ParseComponents(targetFrameworkInformation._targetFrameworkMoniker,
-                targetFrameworkInformation._targetPlatformMoniker.Equals("None", StringComparison.OrdinalIgnoreCase) ?
+            return NuGetFramework.ParseComponents(targetFrameworkMoniker,
+                targetPlatformMoniker.Equals("None", StringComparison.OrdinalIgnoreCase) ?
                     string.Empty :
-                    targetFrameworkInformation._targetPlatformMoniker);
-        }
-
-        private class TargetFrameworkInformation
-        {
-            internal readonly string _targetFrameworkAlias;
-            internal readonly string _targetFrameworkMoniker;
-            internal readonly string _targetPlatformMoniker;
-
-            public TargetFrameworkInformation(string targetFrameworkAlias, string targetFrameworkMoniker, string targetPlatformMoniker)
-            {
-                _targetFrameworkAlias = targetFrameworkAlias;
-                _targetFrameworkMoniker = targetFrameworkMoniker;
-                _targetPlatformMoniker = targetPlatformMoniker;
-            }
+                    targetPlatformMoniker);
         }
     }
-
 }

src/NuGet.Core/NuGet.ProjectModel/GlobalSuppressions.cs

@@ -21,7 +21,6 @@
 [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void PackagesLockFileFormat.Write(TextWriter textWriter, PackagesLockFile lockFile)', validate parameter 'lockFile' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.PackagesLockFileFormat.Write(System.IO.TextWriter,NuGet.ProjectModel.PackagesLockFile)")]
 [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'string PackagesLockFileUtilities.GetNuGetLockFilePath(PackageSpec project)', validate parameter 'project' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.PackagesLockFileUtilities.GetNuGetLockFilePath(NuGet.ProjectModel.PackageSpec)~System.String")]
 [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'TargetFrameworkInformation PackageSpecExtensions.GetTargetFramework(PackageSpec project, NuGetFramework targetFramework)', validate parameter 'project' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.PackageSpecExtensions.GetTargetFramework(NuGet.ProjectModel.PackageSpec,NuGet.Frameworks.NuGetFramework)~NuGet.ProjectModel.TargetFrameworkInformation")]
-[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'Library PackageSpecReferenceDependencyProvider.GetLibrary(LibraryRange libraryRange, NuGetFramework targetFramework)', validate parameter 'libraryRange' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.PackageSpecReferenceDependencyProvider.GetLibrary(NuGet.LibraryModel.LibraryRange,NuGet.Frameworks.NuGetFramework)~NuGet.LibraryModel.Library")]
 [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'NuGetVersion PackageSpecUtility.SpecifySnapshot(string version, string snapshotValue)', validate parameter 'version' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.PackageSpecUtility.SpecifySnapshot(System.String,System.String)~NuGet.Versioning.NuGetVersion")]
 [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'int LockFileDependencyComparerWithoutContentHash.GetHashCode(LockFileDependency obj)', validate parameter 'obj' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.ProjectLockFile.LockFileDependencyComparerWithoutContentHash.GetHashCode(NuGet.ProjectModel.LockFileDependency)~System.Int32")]
 [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void ProjectRestoreMetadata.FillClone(ProjectRestoreMetadata clone)', validate parameter 'clone' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.ProjectRestoreMetadata.FillClone(NuGet.ProjectModel.ProjectRestoreMetadata)")]
@@ -70,6 +69,5 @@
 [assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "<Pending>", Scope = "member", Target = "~P:NuGet.ProjectModel.LockFile.CentralTransitiveDependencyGroups")]
 [assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "IReadOnlySet<T> is only available in .NET 5+, so we need to use a HashSet<T> here.", Scope = "member", Target = "~P:NuGet.ProjectModel.RestoreAuditProperties.SuppressedAdvisories")]
 [assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.LockFileFormat.Utf8JsonRead(System.IO.Stream,NuGet.Common.ILogger,System.String,NuGet.ProjectModel.LockFileReadFlags)~NuGet.ProjectModel.LockFile")]
-[assembly: SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.PackageSpecReferenceDependencyProvider.AddLibraryProperties(NuGet.LibraryModel.Library,NuGet.ProjectModel.PackageSpec,NuGet.Frameworks.NuGetFramework)")]
 [assembly: SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.ProjectRestoreMetadata.GetSources(System.Collections.Generic.IList{NuGet.Configuration.PackageSource})~System.Collections.Generic.HashSet{System.String}")]
 [assembly: SuppressMessage("Style", "IDE0051:Remove unused private members", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.AssetsLogMessage.#ctor(NuGet.Common.LogLevel,NuGet.Common.NuGetLogCode,System.String,System.String,System.String,System.String,System.Collections.Generic.IReadOnlyList{System.String},System.Int32,System.Int32,System.Int32,System.Int32)")]

src/NuGet.Core/NuGet.ProjectModel/PackageSpecReferenceDependencyProvider.cs

@@ -96,12 +96,13 @@ public bool SupportsType(LibraryDependencyTarget libraryType)
 
         public Library GetLibrary(LibraryRange libraryRange, NuGetFramework targetFramework)
         {
-            var name = libraryRange.Name;
+            if (libraryRange == null) throw new ArgumentNullException(nameof(libraryRange));
+            if (targetFramework == null) throw new ArgumentNullException(nameof(targetFramework));
 
             PackageSpec packageSpec = null;
 
             // This must exist in the external references
-            if (_externalProjectsByUniqueName.TryGetValue(name, out ExternalProjectReference externalReference))
+            if (_externalProjectsByUniqueName.TryGetValue(libraryRange.Name, out ExternalProjectReference externalReference))
             {
                 packageSpec = externalReference.PackageSpec;
             }
@@ -116,30 +117,17 @@ public Library GetLibrary(LibraryRange libraryRange, NuGetFramework targetFramew
 
             var projectStyle = packageSpec?.RestoreMetadata?.ProjectStyle ?? ProjectStyle.Unknown;
 
-            // Read references from external project
-            if (projectStyle == ProjectStyle.PackageReference)
+            if (projectStyle == ProjectStyle.PackageReference ||
+                projectStyle == ProjectStyle.DotnetCliTool)
             {
-                // NETCore
                 dependencies = GetDependenciesFromSpecRestoreMetadata(packageSpec, targetFramework);
             }
             else
             {
-                // UWP
-                dependencies = GetDependenciesFromExternalReference(externalReference, packageSpec, targetFramework);
+                dependencies = GetDependenciesFromExternalReference(externalReference);
             }
 
-            // Remove duplicate dependencies. A reference can exist both in csproj and project.json
-            // dependencies is already ordered by importance here
-            var uniqueDependencies = new List<LibraryDependency>(dependencies.Count);
-            var projectNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
-
-            foreach (var project in dependencies)
-            {
-                if (projectNames.Add(project.Name))
-                {
-                    uniqueDependencies.Add(project);
-                }
-            }
+            List<LibraryDependency> uniqueDependencies = DeduplicateDependencies(dependencies);
 
             Library library = new Library
             {
@@ -169,9 +157,31 @@ public Library GetLibrary(LibraryRange libraryRange, NuGetFramework targetFramew
             }
 
             return library;
+
+            static List<LibraryDependency> DeduplicateDependencies(List<LibraryDependency> dependencies)
+            {
+                if (dependencies.Count == 0)
+                {
+                    return dependencies;
+                }
+                // Remove duplicate dependencies.
+                // dependencies is already ordered by importance here
+                var uniqueDependencies = new List<LibraryDependency>(dependencies.Count);
+                var projectNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+
+                foreach (var project in dependencies)
+                {
+                    if (projectNames.Add(project.Name))
+                    {
+                        uniqueDependencies.Add(project);
+                    }
+                }
+
+                return uniqueDependencies;
+            }
         }
 
-        private void AddLibraryProperties(Library library, PackageSpec packageSpec, NuGetFramework targetFramework)
+        private static void AddLibraryProperties(Library library, PackageSpec packageSpec, NuGetFramework targetFramework)
         {
             var projectStyle = packageSpec.RestoreMetadata?.ProjectStyle ?? ProjectStyle.Unknown;
 
@@ -287,16 +297,8 @@ private List<LibraryDependency> GetDependenciesFromSpecRestoreMetadata(PackageSp
             return dependencies;
         }
 
-        /// <summary>
-        /// UWP Project.json
-        /// </summary>
-        private List<LibraryDependency> GetDependenciesFromExternalReference(
-            ExternalProjectReference externalReference,
-            PackageSpec packageSpec,
-            NuGetFramework targetFramework)
+        private List<LibraryDependency> GetDependenciesFromExternalReference(ExternalProjectReference externalReference)
         {
-            var dependencies = GetSpecDependencies(packageSpec, targetFramework);
-
             if (externalReference != null)
             {
                 var childReferences = GetChildReferences(externalReference);
@@ -308,27 +310,7 @@ private List<LibraryDependency> GetDependenciesFromExternalReference(
                     childReferenceNames,
                     StringComparer.OrdinalIgnoreCase);
 
-                // Set all dependencies from project.json to external if an external match was passed in
-                // This is viral and keeps p2ps from looking into directories when we are going down
-                // a path already resolved by msbuild.
-                for (int i = 0; i < dependencies.Count; i++)
-                {
-                    var d = dependencies[i];
-                    if (IsProject(d) && filteredExternalDependencies.Contains(d.Name))
-                    {
-                        var libraryRange = new LibraryRange(d.LibraryRange) { TypeConstraint = LibraryDependencyTarget.ExternalProject };
-
-                        // Do not push the dependency changes here upwards, as the original package
-                        // spec should not be modified.
-                        dependencies[i] = new LibraryDependency(d) { LibraryRange = libraryRange };
-                    }
-                }
-
-                // Add dependencies passed in externally
-                // These are usually msbuild references which have less metadata, they have
-                // the lowest priority.
-                // Note: Only add in dependencies that are in the filtered list to avoid getting the wrong TxM
-                dependencies.AddRange(childReferences
+                return [.. childReferences
                     .Where(reference => filteredExternalDependencies.Contains(reference.ProjectName))
                     .Select(reference => new LibraryDependency()
                     {
@@ -338,10 +320,10 @@ private List<LibraryDependency> GetDependenciesFromExternalReference(
                             VersionRange = VersionRange.Parse("1.0.0"),
                             TypeConstraint = LibraryDependencyTarget.ExternalProject
                         }
-                    }));
+                    })];
             }
 
-            return dependencies;
+            return [];
         }
 
         internal List<LibraryDependency> GetSpecDependencies(
@@ -415,13 +397,6 @@ static bool IsDependencyPruned(LibraryDependency dependency, IReadOnlyDictionary
             }
         }
 
-        private bool IsProject(LibraryDependency dependency)
-        {
-            var type = dependency.LibraryRange.TypeConstraint;
-
-            return SupportsType(type);
-        }
-
         private List<ExternalProjectReference> GetChildReferences(ExternalProjectReference parent)
         {
             var children = new List<ExternalProjectReference>(parent.ExternalProjectReferences.Count);