From e3e150f52eb1b76685d24d04e22461ef0acdfba1 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Fri, 4 Aug 2017 15:39:48 -0700 Subject: [PATCH 01/19] temp status --- .../AddPackageReferenceCommandRunner.cs | 2 +- .../Logging/RestoreCollectorLogger.cs | 7 +- .../Logging/TransitiveNoWarnUtils.cs | 69 +++++++++++++++++++ .../RestoreCommand/RestoreCommand.cs | 8 ++- .../RestoreCommand/RestoreSummary.cs | 2 +- 5 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs index d9643c10ba1..445de72cb0a 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs @@ -138,7 +138,7 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, } // Ignore the graphs with RID else if (compatibleFrameworks.Count == - restorePreviewResult.Result.CompatibilityCheckResults.Where(r => r.Graph.RuntimeIdentifier == null).Count()) + restorePreviewResult.Result.CompatibilityCheckResults.Where(r => string.IsNullOrEmpty(r.Graph.RuntimeIdentifier)).Count()) { // Package is compatible with all the project TFMs // Add an unconditional package reference to the project diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index 8d63a1b9194..505f3b7ef4a 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -4,8 +4,11 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using NuGet.Common; +using NuGet.ProjectModel; +using NuGet.Shared; namespace NuGet.Commands { @@ -18,6 +21,8 @@ public class RestoreCollectorLogger : LoggerBase, ICollectorLogger public IEnumerable Errors => _errors.ToArray(); public WarningPropertiesCollection WarningPropertiesCollection { get; set; } + + public IDictionary TransitiveWarningPropertiesCollection { get; set; } public string ProjectPath { get; set; } @@ -67,7 +72,7 @@ public RestoreCollectorLogger(ILogger innerLogger) : this(innerLogger, LogLevel.Debug, hideWarningsAndErrors: false) { } - + public void Log(IRestoreLogMessage message) { // This will be true only when the Message is a Warning and should be suppressed. diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs new file mode 100644 index 00000000000..3e076d281aa --- /dev/null +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -0,0 +1,69 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using NuGet.DependencyResolver; +using NuGet.Frameworks; +using NuGet.LibraryModel; +using NuGet.ProjectModel; +using NuGet.Shared; + +namespace NuGet.Commands +{ + internal static class TransitiveNoWarnUtils + { + internal static IDictionary CreateTransitiveWarningPropertiesCollection( + DependencyGraphSpec dgSpec, + IEnumerable targetGraphs, + LibraryIdentity parentProject) + { + var transitiveWarningPropertiesCollection = new Dictionary(); + + foreach (var targetGraph in targetGraphs) + { + if (string.IsNullOrEmpty(targetGraph.RuntimeIdentifier)) + { + TraverseTargetGraph(targetGraph, parentProject); + } + + } + + return transitiveWarningPropertiesCollection; + } + + private static void TraverseTargetGraph(RestoreTargetGraph targetGraph, LibraryIdentity parentProject) + { + var paths = new Dictionary>>(); + var dependencyMapping = new Dictionary>(); + var queue = new Queue>>(); + var seen = new HashSet(); + + // seed the queue with the original flattened graph + // Add all dependencies into a dict for a quick transitive lookup + foreach (var dependency in targetGraph.Flattened.OrderBy(d => d.Key.Name)) + { + dependencyMapping[dependency.Key.Name] = dependency.Data.Dependencies; + queue.Enqueue(Tuple.Create>(dependency.Key.Name, new List { parentProject })); + } + + // seed the queue with the original flattened graph + // Add all dependencies into a dict for a quick transitive lookup + foreach (var dependency in targetGraph.Flattened.OrderBy(d => d.Key.Name)) + { + dependencyMapping[dependency.Key.Name] = dependency.Data.Dependencies; + queue.Enqueue(Tuple.Create>(dependency.Key.Name, new List { parentProject })); + } + + var path = new List { parentProject }; + // start taking one node from the queue and get all of it's dependencies + while (queue.Count > 0) + { + var node = queue.Dequeue(); + + } + } + } +} diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 51926959495..4ae8ecb328b 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -43,6 +43,8 @@ public RestoreCommand(RestoreRequest request) { _request = request ?? throw new ArgumentNullException(nameof(request)); + Debugger.Launch(); + // Validate the lock file version requested if (_request.LockFileVersion < 1 || _request.LockFileVersion > LockFileFormat.Version) { @@ -120,6 +122,10 @@ public async Task ExecuteAsync(CancellationToken token) contextForProject, token); + _logger.TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils + .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, + graphs, new LibraryIdentity(_request.Project.Name, _request.Project.Version, LibraryType.Project)); + // Create assets file var assetsFile = BuildAssetsFile( _request.ExistingLockFile, diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs index 2328200407b..325c2e3b510 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; From e25192e9b33aad73c14ca8d19d8d7d7e7e1736a3 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Mon, 7 Aug 2017 14:59:10 -0700 Subject: [PATCH 02/19] working through merging warning properties --- .../PackageSpecificWarningProperties.cs | 47 +++- .../Logging/TransitiveNoWarnUtils.cs | 214 ++++++++++++++++-- .../RestoreCommand/RestoreCommand.cs | 6 +- 3 files changed, 244 insertions(+), 23 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs index f5db18bf2ef..257fecd4fe5 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using NuGet.Common; using NuGet.Frameworks; using NuGet.ProjectModel; @@ -20,7 +21,7 @@ public class PackageSpecificWarningProperties /// Contains Package specific No warn properties. /// NuGetLogCode -> LibraryId -> Set of Frameworks. /// - private IDictionary>> Properties; + public IDictionary>> Properties { get; private set; } /// /// Extracts PackageSpecific WarningProperties from a PackageSpec @@ -51,6 +52,36 @@ public static PackageSpecificWarningProperties CreatePackageSpecificWarningPrope return warningProperties; } + /// + /// Extracts PackageSpecific WarningProperties from a PackageSpec for a specific NuGetFramework + /// + /// PackageSpec containing the Dependencies with WarningProperties + /// NuGetFramework for which the properties should be assessed. + /// PackageSpecific WarningProperties extracted from a PackageSpec for a specific NuGetFramework + public static PackageSpecificWarningProperties CreatePackageSpecificWarningProperties(PackageSpec packageSpec, + NuGetFramework framework) + { + // NuGetLogCode -> LibraryId -> Set of Frameworks. + var warningProperties = new PackageSpecificWarningProperties(); + + foreach (var dependency in packageSpec.Dependencies) + { + warningProperties.AddRange(dependency.NoWarn, dependency.Name, framework); + } + + var targetFrameworkInformation = packageSpec + .TargetFrameworks + .Where(tfi => tfi.FrameworkName == framework) + .First(); + + foreach (var dependency in targetFrameworkInformation.Dependencies) + { + warningProperties.AddRange(dependency.NoWarn, dependency.Name, framework); + } + + return warningProperties; + } + /// /// Adds a NuGetLogCode into the NoWarn Set for the specified library Id and target graph. /// @@ -93,6 +124,20 @@ public void AddRange(IEnumerable codes, string libraryId, NuGetFra } } + /// + /// Adds a list of NuGetLogCode into the NoWarn Set for the specified library Id and target graph. + /// + /// NuGetLogCode for which no warning should be thrown. + /// Library for which no warning should be thrown. + /// IEnumerable of Target graph for which no warning should be thrown. + public void AddRange(NuGetLogCode code, string libraryId, IEnumerable frameworks) + { + foreach (var framework in frameworks) + { + Add(code, libraryId, framework); + } + } + /// /// Checks if a NugetLogCode is part of the NoWarn list for the specified library Id and target graph. /// diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index 3e076d281aa..24684f0d41c 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -2,31 +2,37 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; -using System.Text; -using NuGet.DependencyResolver; -using NuGet.Frameworks; +using NuGet.Common; using NuGet.LibraryModel; using NuGet.ProjectModel; using NuGet.Shared; namespace NuGet.Commands { - internal static class TransitiveNoWarnUtils + internal class TransitiveNoWarnUtils { + // static should be fine across multiple restore calls as this solely depends on the csproj file of the project. + private static readonly ConcurrentDictionary _warningPropertiesCache = + new ConcurrentDictionary(); + internal static IDictionary CreateTransitiveWarningPropertiesCollection( DependencyGraphSpec dgSpec, IEnumerable targetGraphs, LibraryIdentity parentProject) { var transitiveWarningPropertiesCollection = new Dictionary(); + var parentProjectSpec = GetNodePackageSpec(dgSpec, parentProject.Name); + + var parentWarningProperties = GetNodeWarningProperties(parentProjectSpec); foreach (var targetGraph in targetGraphs) { if (string.IsNullOrEmpty(targetGraph.RuntimeIdentifier)) { - TraverseTargetGraph(targetGraph, parentProject); + TraverseTargetGraph(targetGraph, dgSpec, parentProject, parentWarningProperties); } } @@ -34,36 +40,206 @@ internal static IDictionary CreateTransitiv return transitiveWarningPropertiesCollection; } - private static void TraverseTargetGraph(RestoreTargetGraph targetGraph, LibraryIdentity parentProject) + private static void TraverseTargetGraph(RestoreTargetGraph targetGraph, + DependencyGraphSpec dgSpec, + LibraryIdentity parentProject, + WarningPropertiesCollection parentWarningPropertiesCollection) { - var paths = new Dictionary>>(); + var paths = new Dictionary>(); var dependencyMapping = new Dictionary>(); - var queue = new Queue>>(); + var queue = new Queue>(); + + //TODO seen should have node id and the path warningproperties var seen = new HashSet(); - // seed the queue with the original flattened graph - // Add all dependencies into a dict for a quick transitive lookup - foreach (var dependency in targetGraph.Flattened.OrderBy(d => d.Key.Name)) - { - dependencyMapping[dependency.Key.Name] = dependency.Data.Dependencies; - queue.Enqueue(Tuple.Create>(dependency.Key.Name, new List { parentProject })); - } + // All the packages in parent project's closure. + // Once we have collected data for all of these, we can exit. + var parentPackageDependencies = new HashSet( + targetGraph.Flattened.Where( d => d.Key.Type == LibraryType.Package).Select( d => d.Key.Name)); - // seed the queue with the original flattened graph + // Seed the queue with the original flattened graph // Add all dependencies into a dict for a quick transitive lookup foreach (var dependency in targetGraph.Flattened.OrderBy(d => d.Key.Name)) { dependencyMapping[dependency.Key.Name] = dependency.Data.Dependencies; - queue.Enqueue(Tuple.Create>(dependency.Key.Name, new List { parentProject })); + var queueTuple = Tuple.Create(dependency.Key.Name, dependency.Key.Type ,parentWarningPropertiesCollection); + + // Add the metadata from the parent project here. + queue.Enqueue(queueTuple); } - var path = new List { parentProject }; // start taking one node from the queue and get all of it's dependencies while (queue.Count > 0) { var node = queue.Dequeue(); - + if (!seen.Contains(node.Item1)) + { + var nodeDependencies = dependencyMapping[node.Item1]; + var nodeName = node.Item1; + var nodeType = node.Item2; + var pathWarningProperties = node.Item3; + + // If the node is a project then we need to extract the warning properties and + // add those to the warning properties of the current path. + if (nodeType == LibraryType.Project || nodeType == LibraryType.ExternalProject) + { + // Get the node PackageSpec + var nodeProjectSpec = GetNodePackageSpec(dgSpec, nodeName); + + // Get the WarningPropertiesCollection from the PackageSpec + var nodeWarningProperties = GetNodeWarningProperties(nodeProjectSpec); + + // Merge the WarningPropertiesCollection to the one in the path + var mergedWarningPropertiesCollection = MergeWarningPropertiesCollection(pathWarningProperties, + nodeWarningProperties); + + // Add all the project's dependencies to the Queue with the merged WarningPropertiesCollection + + + } + else if (nodeType == LibraryType.Package) + { + // Evaluate the package properties for the current path + + // If the path does not "NoWarn" for this package then remove the path from parentPackageDependencies + + // If the path has a "NoWarn" for the package then save it to the result + + // If parentPackageDependencies is empty then exit the graph traversal + } + + } + } + } + + private static WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec) + { + return _warningPropertiesCache.GetOrAdd(nodeProjectSpec.RestoreMetadata.ProjectPath, + (s) => new WarningPropertiesCollection() + { + ProjectWideWarningProperties = nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, + PackageSpecificWarningProperties = PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec), + ProjectFrameworks = nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly() + }); + } + + private static PackageSpec GetNodePackageSpec(DependencyGraphSpec dgSpec, string nodeId) + { + return dgSpec + .Projects + .Where(p => string.Equals(p.RestoreMetadata.ProjectName, nodeId, StringComparison.OrdinalIgnoreCase)) + .First(); + } + + private static WarningPropertiesCollection MergeWarningPropertiesCollection(WarningPropertiesCollection first, + WarningPropertiesCollection second) + { + WarningPropertiesCollection result = null; + + if (TryMergeNullObjects(first, second, out object merged)) + { + result = merged as WarningPropertiesCollection; + } + else + { + // Merge Project Wide Warning Properties + var mergedProjectWideWarningProperties = MergeProjectWideWarningProperties( + first.ProjectWideWarningProperties, + second.ProjectWideWarningProperties); + + // Merge Package Specific Warning Properties + var mergedPackageSpecificWarnings = MergePackageSpecificWarningProperties(first.PackageSpecificWarningProperties, + second.PackageSpecificWarningProperties); + } + + return result; + } + + private static WarningProperties MergeProjectWideWarningProperties(WarningProperties first, + WarningProperties second) + { + WarningProperties result = null; + + if (TryMergeNullObjects(first, second, out object merged)) + { + result = merged as WarningProperties; + } + else + { + // Merge WarningsAsErrors Sets. + var mergedWarningsAsErrors = new HashSet(); + mergedWarningsAsErrors.UnionWith(first.WarningsAsErrors); + mergedWarningsAsErrors.UnionWith(second.WarningsAsErrors); + + // Merge NoWarn Sets. + var mergedNoWarn = new HashSet(); + mergedNoWarn.UnionWith(first.NoWarn); + mergedNoWarn.UnionWith(second.NoWarn); + + // Merge AllWarningsAsErrors. If one project treats all warnigs as errors then the chain will too. + var mergedAllWarningsAsErrors = first.AllWarningsAsErrors || second.AllWarningsAsErrors; + + result = new WarningProperties(mergedWarningsAsErrors, + mergedNoWarn, + mergedAllWarningsAsErrors); + } + + return result; + } + + private static PackageSpecificWarningProperties MergePackageSpecificWarningProperties(PackageSpecificWarningProperties first, + PackageSpecificWarningProperties second) + { + PackageSpecificWarningProperties result = null; + + if (TryMergeNullObjects(first, second, out object merged)) + { + result = merged as PackageSpecificWarningProperties; + } + else + { + result = new PackageSpecificWarningProperties(); + foreach (var code in first.Properties.Keys) + { + foreach (var libraryId in first.Properties[code].Keys) + { + result.AddRange(code, libraryId, first.Properties[code][libraryId]); + } + } + } + return result; + } + + /// + /// Try to merge 2 objects if one or both of them are null. + /// + /// First Object to be merged. + /// First Object to be merged. + /// Out Merged Object. + /// Returns true if atleast one of the objects was Null. + /// If none of them is null then the returns false, indicating that the merge failed. + private static bool TryMergeNullObjects(object first, object second, out object merged) + { + merged = null; + var result = false; + + if (first == null && second == null) + { + merged = null; + result = true; } + else if (first == null) + { + merged = second; + result = true; + } + else if (second == null) + { + merged = first; + result = true; + } + + return result; } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 4ae8ecb328b..8827c5b13dc 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -122,9 +122,9 @@ public async Task ExecuteAsync(CancellationToken token) contextForProject, token); - _logger.TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils - .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, - graphs, new LibraryIdentity(_request.Project.Name, _request.Project.Version, LibraryType.Project)); + //_logger.TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils + // .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, + // graphs, new LibraryIdentity(_request.Project.Name, _request.Project.Version, LibraryType.Project)); // Create assets file var assetsFile = BuildAssetsFile( From dbb4e78f3920b849dee1013599adf4c6355be8e4 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Mon, 7 Aug 2017 18:29:30 -0700 Subject: [PATCH 03/19] Adding better quality across and changing tests --- .../PackageSpecificWarningProperties.cs | 35 +++- .../Logging/TransitiveNoWarnUtils.cs | 145 +++++++++++-- .../Logging/WarningPropertiesCollection.cs | 53 ++++- .../RestoreCommand/RestoreCommand.cs | 15 +- .../CollectorLoggerTests.cs | 159 +++++++------- .../WarningPropertiesCollectionTests.cs | 197 ++++++------------ 6 files changed, 353 insertions(+), 251 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs index 257fecd4fe5..2cd14f5fd2c 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs @@ -7,6 +7,7 @@ using NuGet.Common; using NuGet.Frameworks; using NuGet.ProjectModel; +using NuGet.Shared; namespace NuGet.Commands { @@ -14,7 +15,7 @@ namespace NuGet.Commands /// /// Contains Package specific properties for Warnings. /// - public class PackageSpecificWarningProperties + public class PackageSpecificWarningProperties : IEquatable { /// @@ -152,5 +153,37 @@ public bool Contains(NuGetLogCode code, string libraryId, NuGetFramework framewo libraryIdsAndFrameworks.TryGetValue(libraryId, out var frameworkSet) && frameworkSet.Contains(framework); } + + public override int GetHashCode() + { + var hashCode = new HashCodeCombiner(); + + // Add a constant hash for all objects + hashCode.AddObject(1); + + return hashCode.CombinedHash; + } + + public override bool Equals(object obj) + { + return Equals(obj as PackageSpecificWarningProperties); + } + + public bool Equals(PackageSpecificWarningProperties other) + { + if (other == null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return Properties.Keys.OrderedEquals(other.Properties.Keys, (code) => code) && + Properties.Keys.All(c => Properties[c].Keys.OrderedEquals(other.Properties[c].Keys, (id) => id)) && + Properties.Keys.All(c => Properties[c].Keys.All(id => Properties[c][id].SetEquals(other.Properties[c][id]))); + } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index 24684f0d41c..5a5a479f156 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using NuGet.Common; +using NuGet.Frameworks; using NuGet.LibraryModel; using NuGet.ProjectModel; using NuGet.Shared; @@ -47,7 +48,7 @@ private static void TraverseTargetGraph(RestoreTargetGraph targetGraph, { var paths = new Dictionary>(); var dependencyMapping = new Dictionary>(); - var queue = new Queue>(); + var queue = new Queue(); //TODO seen should have node id and the path warningproperties var seen = new HashSet(); @@ -57,47 +58,55 @@ private static void TraverseTargetGraph(RestoreTargetGraph targetGraph, var parentPackageDependencies = new HashSet( targetGraph.Flattened.Where( d => d.Key.Type == LibraryType.Package).Select( d => d.Key.Name)); + var parentTargetFramework = targetGraph.Framework; + // Seed the queue with the original flattened graph // Add all dependencies into a dict for a quick transitive lookup foreach (var dependency in targetGraph.Flattened.OrderBy(d => d.Key.Name)) { dependencyMapping[dependency.Key.Name] = dependency.Data.Dependencies; - var queueTuple = Tuple.Create(dependency.Key.Name, dependency.Key.Type ,parentWarningPropertiesCollection); + var queueNode = new DependencyNode(dependency.Key.Name, GetType(dependency.Key.Type), parentWarningPropertiesCollection); // Add the metadata from the parent project here. - queue.Enqueue(queueTuple); + queue.Enqueue(queueNode); } // start taking one node from the queue and get all of it's dependencies while (queue.Count > 0) { var node = queue.Dequeue(); - if (!seen.Contains(node.Item1)) + if (!seen.Contains(node.Id)) { - var nodeDependencies = dependencyMapping[node.Item1]; - var nodeName = node.Item1; - var nodeType = node.Item2; - var pathWarningProperties = node.Item3; + var nodeDependencies = dependencyMapping[node.Id]; + var nodeName = node.Id; + var nodeType = node.Type; + var pathWarningProperties = node.WarningPropertiesCollection; // If the node is a project then we need to extract the warning properties and // add those to the warning properties of the current path. - if (nodeType == LibraryType.Project || nodeType == LibraryType.ExternalProject) + if (nodeType == LibraryDependencyTarget.Project || nodeType == LibraryDependencyTarget.ExternalProject) { // Get the node PackageSpec var nodeProjectSpec = GetNodePackageSpec(dgSpec, nodeName); // Get the WarningPropertiesCollection from the PackageSpec - var nodeWarningProperties = GetNodeWarningProperties(nodeProjectSpec); + var nodeWarningProperties = GetNodeWarningProperties(nodeProjectSpec, parentTargetFramework); // Merge the WarningPropertiesCollection to the one in the path var mergedWarningPropertiesCollection = MergeWarningPropertiesCollection(pathWarningProperties, nodeWarningProperties); // Add all the project's dependencies to the Queue with the merged WarningPropertiesCollection + foreach (var dependency in dependencyMapping[nodeName].OrderBy(d => d.Name)) + { + var queueTuple = new DependencyNode(dependency.Name, dependency.LibraryRange.TypeConstraint, mergedWarningPropertiesCollection); + // Add the metadata from the parent project here. + queue.Enqueue(queueTuple); + } } - else if (nodeType == LibraryType.Package) + else if (nodeType == LibraryDependencyTarget.Package) { // Evaluate the package properties for the current path @@ -115,12 +124,19 @@ private static void TraverseTargetGraph(RestoreTargetGraph targetGraph, private static WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec) { return _warningPropertiesCache.GetOrAdd(nodeProjectSpec.RestoreMetadata.ProjectPath, - (s) => new WarningPropertiesCollection() - { - ProjectWideWarningProperties = nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, - PackageSpecificWarningProperties = PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec), - ProjectFrameworks = nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly() - }); + (s) => new WarningPropertiesCollection( + nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, + PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec), + nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly())); + } + + private static WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec, NuGetFramework framework) + { + return _warningPropertiesCache.GetOrAdd(nodeProjectSpec.RestoreMetadata.ProjectPath, + (s) => new WarningPropertiesCollection( + nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, + PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec, framework), + nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly())); } private static PackageSpec GetNodePackageSpec(DependencyGraphSpec dgSpec, string nodeId) @@ -148,7 +164,8 @@ private static WarningPropertiesCollection MergeWarningPropertiesCollection(Warn second.ProjectWideWarningProperties); // Merge Package Specific Warning Properties - var mergedPackageSpecificWarnings = MergePackageSpecificWarningProperties(first.PackageSpecificWarningProperties, + var mergedPackageSpecificWarnings = MergePackageSpecificWarningProperties( + first.PackageSpecificWarningProperties, second.PackageSpecificWarningProperties); } @@ -241,5 +258,97 @@ private static bool TryMergeNullObjects(object first, object second, out object return result; } + + private static LibraryDependencyTarget GetType(LibraryType type) + { + if (type == LibraryType.Assembly) + { + return LibraryDependencyTarget.Assembly; + } + else if (type == LibraryType.ExternalProject) + { + return LibraryDependencyTarget.ExternalProject; + } + else if (type == LibraryType.Package) + { + return LibraryDependencyTarget.Package; + } + else if (type == LibraryType.Project) + { + return LibraryDependencyTarget.Project; + } + else if (type == LibraryType.Reference) + { + return LibraryDependencyTarget.Reference; + } + else if (type == LibraryType.WinMD) + { + return LibraryDependencyTarget.WinMD; + } + else + { + return LibraryDependencyTarget.None; + } + } + + /// + /// A simple node class to hold the outgoing dependency edge during the graph walk. + /// + private class DependencyNode : IEquatable + { + // ID of the Node + public string Id { get; } + + // Type of the Node + public LibraryDependencyTarget Type { get; } + + // WarningPropertiesCollection of the path taken to the Node + public WarningPropertiesCollection WarningPropertiesCollection { get; } + + public DependencyNode(string id, LibraryDependencyTarget type, WarningPropertiesCollection warningPropertiesCollection) + { + Id = id ?? throw new ArgumentNullException(nameof(id)); + WarningPropertiesCollection = warningPropertiesCollection ?? throw new ArgumentNullException(nameof(warningPropertiesCollection)); + Type = type; + } + + public DependencyNode(string id, LibraryDependencyTarget type) + { + Id = id ?? throw new ArgumentNullException(nameof(id)); + Type = type; + } + + public override int GetHashCode() + { + var hashCode = new HashCodeCombiner(); + + hashCode.AddStringIgnoreCase(Id); + hashCode.AddObject(Type); + + return hashCode.CombinedHash; + } + + public override bool Equals(object obj) + { + return Equals(obj as DependencyNode); + } + + public bool Equals(DependencyNode other) + { + if (other == null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return string.Equals(Id, other.Id, StringComparison.OrdinalIgnoreCase) && + Type == other.Type && + WarningPropertiesCollection.Equals(other.WarningPropertiesCollection); + } + } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs index 1d4327fb994..6542602a412 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs @@ -16,7 +16,7 @@ namespace NuGet.Commands /// /// Class to hold ProjectWide and PackageSpecific WarningProperties. /// - public class WarningPropertiesCollection + public class WarningPropertiesCollection : IEquatable { private readonly ConcurrentDictionary _getFrameworkCache = new ConcurrentDictionary(); @@ -24,18 +24,27 @@ public class WarningPropertiesCollection /// Contains the target frameworks for the project. /// These are used for no warn filtering in case of a log message without a target graph. /// - public IReadOnlyList ProjectFrameworks { get; set; } = new ReadOnlyCollection(new List()); + public IReadOnlyList ProjectFrameworks { get; } = new ReadOnlyCollection(new List()); /// /// Contains Project wide properties for Warnings. /// - public WarningProperties ProjectWideWarningProperties { get; set; } + public WarningProperties ProjectWideWarningProperties { get; } /// /// Contains Package specific properties for Warnings. /// NuGetLogCode -> LibraryId -> Set of Frameworks. /// - public PackageSpecificWarningProperties PackageSpecificWarningProperties { get; set; } + public PackageSpecificWarningProperties PackageSpecificWarningProperties { get; } + + public WarningPropertiesCollection(WarningProperties projectWideWarningProperties, + PackageSpecificWarningProperties packageSpecificWarningProperties, + IReadOnlyList projectFrameworks) + { + ProjectWideWarningProperties = projectWideWarningProperties; + PackageSpecificWarningProperties = packageSpecificWarningProperties; + ProjectFrameworks = projectFrameworks; + } /// /// Attempts to suppress a warning log message or upgrade it to error log message. @@ -61,7 +70,8 @@ public bool ApplyWarningProperties(IRestoreLogMessage message) if (messageTargetFrameworks.Count == 0) { // Suppress the warning if the code + libraryId combination is suppressed for all project frameworks. - if (ProjectFrameworks.Count > 0 && + if (ProjectFrameworks != null && + ProjectFrameworks.Count > 0 && ProjectFrameworks.All(e => PackageSpecificWarningProperties.Contains(message.Code, message.LibraryId, e))) { return true; @@ -128,5 +138,38 @@ private static NuGetFramework GetNuGetFrameworkFromTargetGraph(string targetGrap return NuGetFramework.Parse(parts[0]); } + + public override int GetHashCode() + { + var hashCode = new HashCodeCombiner(); + + hashCode.AddObject(ProjectWideWarningProperties); + hashCode.AddObject(PackageSpecificWarningProperties); + hashCode.AddSequence(ProjectFrameworks); + + return hashCode.CombinedHash; + } + + public override bool Equals(object obj) + { + return Equals(obj as WarningPropertiesCollection); + } + + public bool Equals(WarningPropertiesCollection other) + { + if (other == null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return ProjectWideWarningProperties.Equals(other.ProjectWideWarningProperties) && + PackageSpecificWarningProperties.Equals(other.PackageSpecificWarningProperties) && + ProjectFrameworks.OrderedEquals(other.ProjectFrameworks, (s) => s.Framework); + } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 8827c5b13dc..bcbf6b0b2ab 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -43,7 +43,7 @@ public RestoreCommand(RestoreRequest request) { _request = request ?? throw new ArgumentNullException(nameof(request)); - Debugger.Launch(); + //Debugger.Launch(); // Validate the lock file version requested if (_request.LockFileVersion < 1 || _request.LockFileVersion > LockFileFormat.Version) @@ -58,12 +58,11 @@ public RestoreCommand(RestoreRequest request) var collectorLogger = new RestoreCollectorLogger(_request.Log, collectorLoggerHideWarningsAndErrors) { ProjectPath = _request.Project.RestoreMetadata?.ProjectPath, - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = request.Project.RestoreMetadata?.ProjectWideWarningProperties, - PackageSpecificWarningProperties = PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(request.Project), - ProjectFrameworks = request.Project.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly() - } + WarningPropertiesCollection = new WarningPropertiesCollection( + request.Project.RestoreMetadata?.ProjectWideWarningProperties, + PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(request.Project), + request.Project.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly() + ) }; _logger = collectorLogger; @@ -123,7 +122,7 @@ public async Task ExecuteAsync(CancellationToken token) token); //_logger.TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils - // .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, + // .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, // graphs, new LibraryIdentity(_request.Project.Name, _request.Project.Version, LibraryType.Project)); // Create assets file diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs index 6ddb026a6fc..29e9a65abf7 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs @@ -368,10 +368,10 @@ public void CollectorLogger_LogsWarningsAsErrorsErrorsForProjectWideWarnAsErrorS var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + null, + null) }; // Act @@ -402,10 +402,10 @@ public void CollectorLogger_LogsWarningsAsErrorsForProjectAllWarningsAsErrors() var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + null, + null) }; // Act @@ -434,10 +434,10 @@ public void CollectorLogger_LogsWarningsAsErrorsForProjectAllWarningsAsErrorsAnd var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + null, + null) }; // Act @@ -468,10 +468,10 @@ public void CollectorLogger_DoesNotLogsWarningsForProjectWideNoWarnSet() var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + null, + null) }; // Act @@ -506,11 +506,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSet() var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null) }; // Act @@ -545,11 +544,10 @@ public void CollectorLogger_LogsWarningsForPackageSpecificNoWarnSetButWarningsWi var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null) }; // Act @@ -586,11 +584,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithMu var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null) }; // Act @@ -627,11 +624,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithLi var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null) }; // Act @@ -668,11 +664,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithLi var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null) }; // Act @@ -711,12 +706,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetForGlo var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { targetFramework } - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework }) }; // Act @@ -753,12 +746,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetForGlo var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { targetFramework, netcoreTargetFramework } - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework, netcoreTargetFramework }) }; // Act @@ -795,12 +786,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithMu var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { targetFramework } - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework }) }; // Act @@ -841,12 +830,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithMu var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { targetFramework, netcoreTargetFramework } - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework, netcoreTargetFramework }) }; // Act @@ -879,10 +866,10 @@ public void CollectorLogger_DoesNotLogsWarningsForProjectWideNoWarnSetAndAllWarn var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + null, + null) }; // Act @@ -911,10 +898,10 @@ public void CollectorLogger_DoesNotLogsWarningsForProjectWideNoWarnSetAndWarnAsE var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + null, + null) }; // Act @@ -943,10 +930,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetAndWar var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + null, + null) }; // Act @@ -983,11 +970,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificAndProjectWideN var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null) }; // Act @@ -1026,11 +1012,10 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetAndPro var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - } + WarningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null) }; // Act diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/WarningPropertiesCollectionTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/WarningPropertiesCollectionTests.cs index 19f6a35d1da..ad788eeb839 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/WarningPropertiesCollectionTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/WarningPropertiesCollectionTests.cs @@ -21,10 +21,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithNoWarn() var noWarnSet = new HashSet { NuGetLogCode.NU1500 }; var warnAsErrorSet = new HashSet { }; var allWarningsAsErrors = false; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var suppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1500, "Warning"); var nonSuppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -42,10 +39,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithWarnAsError() var noWarnSet = new HashSet { }; var warnAsErrorSet = new HashSet { NuGetLogCode.NU1500 }; var allWarningsAsErrors = false; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var upgradedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1500, "Warning"); var nonSuppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -64,10 +58,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithWarnAsErrorAndUndef var noWarnSet = new HashSet { }; var warnAsErrorSet = new HashSet { NuGetLogCode.Undefined }; var allWarningsAsErrors = false; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var nonSuppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.Undefined, "Warning"); var nonSuppressedMessage2 = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -87,10 +78,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithAllWarningsAsErrors var noWarnSet = new HashSet { }; var warnAsErrorSet = new HashSet { }; var allWarningsAsErrors = true; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var upgradedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1500, "Warning"); var upgradedMessage2 = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -109,10 +97,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithAllWarningsAsErrors var noWarnSet = new HashSet { }; var warnAsErrorSet = new HashSet { }; var allWarningsAsErrors = true; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var upgradedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.Undefined, "Warning"); var upgradedMessage2 = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -132,10 +117,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithNoWarnAndWarnAsErro var noWarnSet = new HashSet { NuGetLogCode.NU1500 }; var warnAsErrorSet = new HashSet { NuGetLogCode.NU1500 }; var allWarningsAsErrors = false; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var suppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1500, "Warning"); var nonSuppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -153,10 +135,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithNoWarnAndWarnAsErro var noWarnSet = new HashSet { NuGetLogCode.NU1500 }; var warnAsErrorSet = new HashSet { NuGetLogCode.NU1500 }; var allWarningsAsErrors = true; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var suppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1500, "Warning"); var nonSuppressedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -174,10 +153,7 @@ public void WarningPropertiesCollection_ProjectPropertiesWithWarnAsErrorAndAllWa var noWarnSet = new HashSet { }; var warnAsErrorSet = new HashSet { NuGetLogCode.NU1500 }; var allWarningsAsErrors = true; - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors) - }; + var warningPropertiesCollection = new WarningPropertiesCollection(new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null); var upgradedMessage = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1500, "Warning"); var upgradedMessage2 = new RestoreLogMessage(LogLevel.Warning, NuGetLogCode.NU1601, "Warning"); @@ -204,10 +180,7 @@ public void WarningPropertiesCollection_PackagePropertiesWithFrameworkAndWarning var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection(null, packageSpecificWarningProperties, null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, frameworkString); var nonSuppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1601, "Warning", libraryId, frameworkString); @@ -233,10 +206,7 @@ public void WarningPropertiesCollection_PackagePropertiesWithATFFrameworkAndWarn var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection(null, packageSpecificWarningProperties, null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, frameworkString); @@ -259,10 +229,7 @@ public void WarningPropertiesCollection_PackagePropertiesWithPTFFrameworkAndWarn var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection(null, packageSpecificWarningProperties, null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, frameworkString); @@ -285,11 +252,7 @@ public void WarningPropertiesCollection_PackagePropertiesWithoutFrameworkAndWarn var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { targetFramework } - }; + var warningPropertiesCollection = new WarningPropertiesCollection(null, packageSpecificWarningProperties, new List { targetFramework }); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId); @@ -312,10 +275,7 @@ public void WarningPropertiesCollection_PackagePropertiesWithoutFrameworkAndWarn var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection(null, packageSpecificWarningProperties, null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, "net45"); @@ -342,11 +302,10 @@ public void WarningPropertiesCollection_PackagePropertiesWithNoWarnAndProjectPro var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, frameworkString); @@ -372,11 +331,10 @@ public void WarningPropertiesCollection_PackagePropertiesAndProjectPropertiesWit var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId); @@ -403,15 +361,10 @@ public void WarningPropertiesCollection_PackagePropertiesWithNoWarnAndProjectPro var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List - { - targetFramework - } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework }); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, frameworkString); var suppressedMessage2 = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId); @@ -443,15 +396,10 @@ public void WarningPropertiesCollection_PackagePropertiesWithNoWarnAndProjectPro var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List - { - targetFramework - } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework }); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, frameworkString); var suppressedMessage2 = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId); @@ -485,12 +433,10 @@ public void WarningPropertiesCollection_PackagePropertiesWithNoWarnAndProjectPro var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { targetFramework } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework }); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, frameworkString); var suppressedMessage2 = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId); @@ -527,12 +473,10 @@ public void WarningPropertiesCollection_MessageWithNoTargetGraphAndDependencyWit var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, firstTargetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { firstTargetFramework, secondTargetFramework } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { firstTargetFramework, secondTargetFramework }); var nonSuppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, firstFrameworkString); @@ -567,12 +511,10 @@ public void WarningPropertiesCollection_MessageWithNoTargetGraphAndDependencyWit packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, firstTargetFramework); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, secondTargetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { firstTargetFramework, secondTargetFramework } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { firstTargetFramework, secondTargetFramework }); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId); @@ -603,12 +545,10 @@ public void WarningPropertiesCollection_MessageWithTargetGraphAndDependencyWithN var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, firstTargetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { firstTargetFramework, secondTargetFramework } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { firstTargetFramework, secondTargetFramework }); var nonSuppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, new string[] { firstFrameworkString, secondFrameworkString }); @@ -639,12 +579,10 @@ public void WarningPropertiesCollection_MessageWithTargetGraphAndDependencyWithN packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, firstTargetFramework); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, secondTargetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { firstTargetFramework, secondTargetFramework } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { firstTargetFramework, secondTargetFramework }); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, new string[] { firstFrameworkString, secondFrameworkString }); @@ -672,12 +610,10 @@ public void WarningPropertiesCollection_MessageWithTargetGraphAndDependencyWithN var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties, - ProjectFrameworks = new List { targetFramework } - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + new List { targetFramework}); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, new string[] { frameworkString }); @@ -708,11 +644,10 @@ public void WarningPropertiesCollection_MessageWithTargetGraphAndDependencyWithN var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, firstTargetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null); var nonSuppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, new string[] { firstFrameworkString, secondFrameworkString }); @@ -743,11 +678,10 @@ public void WarningPropertiesCollection_MessageWithTargetGraphAndDependencyWithN packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, firstTargetFramework); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, secondTargetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, new string[] { firstFrameworkString, secondFrameworkString }); @@ -775,11 +709,10 @@ public void WarningPropertiesCollection_MessageWithTargetGraphAndDependencyWithN var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); packageSpecificWarningProperties.Add(NuGetLogCode.NU1500, libraryId, targetFramework); - var warningPropertiesCollection = new WarningPropertiesCollection() - { - ProjectWideWarningProperties = new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), - PackageSpecificWarningProperties = packageSpecificWarningProperties - }; + var warningPropertiesCollection = new WarningPropertiesCollection( + new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), + packageSpecificWarningProperties, + null); var suppressedMessage = RestoreLogMessage.CreateWarning(NuGetLogCode.NU1500, "Warning", libraryId, new string[] { frameworkString }); From 3512c8a05e649a5f617c77cadaaa8e34b431f16b Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Tue, 8 Aug 2017 14:26:11 -0700 Subject: [PATCH 04/19] checkpoint for correctly extracting transitive no warn from a basic case --- .../Logging/RestoreCollectorLogger.cs | 2 +- .../Logging/TransitiveNoWarnUtils.cs | 294 +++++++++++++----- .../RestoreCommand/RestoreCommand.cs | 7 +- 3 files changed, 222 insertions(+), 81 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index 505f3b7ef4a..75faa27cd92 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -22,7 +22,7 @@ public class RestoreCollectorLogger : LoggerBase, ICollectorLogger public WarningPropertiesCollection WarningPropertiesCollection { get; set; } - public IDictionary TransitiveWarningPropertiesCollection { get; set; } + public PackageSpecificWarningProperties TransitiveWarningPropertiesCollection { get; set; } public string ProjectPath { get; set; } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index 5a5a479f156..ce5b971b638 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -1,15 +1,15 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; -using System.Collections.Concurrent; -using System.Collections.Generic; -using System.Linq; using NuGet.Common; using NuGet.Frameworks; using NuGet.LibraryModel; using NuGet.ProjectModel; using NuGet.Shared; +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; namespace NuGet.Commands { @@ -19,13 +19,19 @@ internal class TransitiveNoWarnUtils private static readonly ConcurrentDictionary _warningPropertiesCache = new ConcurrentDictionary(); - internal static IDictionary CreateTransitiveWarningPropertiesCollection( + /// + /// Creates a PackageSpecificWarningProperties for a project generated by traversing the dependency graph. + /// + /// Restore DGSpec containing the complete closure. + /// Parent project restore target graphs. + /// LibraryIdentity of the parent project. + /// + internal static PackageSpecificWarningProperties CreateTransitiveWarningPropertiesCollection( DependencyGraphSpec dgSpec, IEnumerable targetGraphs, - LibraryIdentity parentProject) + PackageSpec parentProjectSpec) { - var transitiveWarningPropertiesCollection = new Dictionary(); - var parentProjectSpec = GetNodePackageSpec(dgSpec, parentProject.Name); + var transitiveNoWarnProperties = new PackageSpecificWarningProperties(); var parentWarningProperties = GetNodeWarningProperties(parentProjectSpec); @@ -33,92 +39,219 @@ internal static IDictionary CreateTransitiv { if (string.IsNullOrEmpty(targetGraph.RuntimeIdentifier)) { - TraverseTargetGraph(targetGraph, dgSpec, parentProject, parentWarningProperties); + var transitiveNoWarnFromTargetGraph = ExtractTransitiveNoWarnProperties( + targetGraph, + dgSpec, + parentProjectSpec, + parentWarningProperties); + + transitiveNoWarnProperties = MergePackageSpecificWarningProperties( + transitiveNoWarnProperties, + transitiveNoWarnFromTargetGraph); } - } - return transitiveWarningPropertiesCollection; + return transitiveNoWarnProperties; } - private static void TraverseTargetGraph(RestoreTargetGraph targetGraph, + /// + /// Traverses a Dependency grpah starting from the parent project in BF style. + /// + /// Parent project restore target graph. + /// Restore DGSpec containing the complete closure. + /// PackageSpec of the parent project. + /// WarningPropertiesCollection of the parent project. + /// PackageSpecificWarningProperties containing all the NoWarn's for each package seen in the graph accumulated while traversing the graph. + private static PackageSpecificWarningProperties ExtractTransitiveNoWarnProperties( + RestoreTargetGraph targetGraph, DependencyGraphSpec dgSpec, - LibraryIdentity parentProject, + PackageSpec parentProjectSpec, WarningPropertiesCollection parentWarningPropertiesCollection) { var paths = new Dictionary>(); var dependencyMapping = new Dictionary>(); var queue = new Queue(); - - //TODO seen should have node id and the path warningproperties - var seen = new HashSet(); + var seen = new HashSet(); + var frameworkReducer = new FrameworkReducer(); + var resultWarningProperties = new PackageSpecificWarningProperties(); // All the packages in parent project's closure. // Once we have collected data for all of these, we can exit. var parentPackageDependencies = new HashSet( - targetGraph.Flattened.Where( d => d.Key.Type == LibraryType.Package).Select( d => d.Key.Name)); + targetGraph.Flattened.Where(d => d.Key.Type == LibraryType.Package).Select(d => d.Key.Name)); var parentTargetFramework = targetGraph.Framework; - // Seed the queue with the original flattened graph // Add all dependencies into a dict for a quick transitive lookup foreach (var dependency in targetGraph.Flattened.OrderBy(d => d.Key.Name)) { - dependencyMapping[dependency.Key.Name] = dependency.Data.Dependencies; - var queueNode = new DependencyNode(dependency.Key.Name, GetType(dependency.Key.Type), parentWarningPropertiesCollection); + // Use the full path for projects and id for packages + var name = string.IsNullOrEmpty(dependency.Data.Match.Path) ? + dependency.Key.Name : + dependency.Data.Match.Path; - // Add the metadata from the parent project here. - queue.Enqueue(queueNode); + dependencyMapping[name.ToLower()] = dependency.Data.Dependencies; } + var parentDependencies = GetDirectDependencies(parentProjectSpec, parentWarningPropertiesCollection, parentTargetFramework); + + // Seed the queue with the parent project's direct dependencies + parentDependencies.OrderBy(d => d.Id).ForEach(d => queue.Enqueue(d)); + + // Add the parent project to the seen set to prevent adding it back to the queue + seen.Add(new DependencyNode(id: parentProjectSpec.RestoreMetadata.ProjectPath.ToLowerInvariant(), + isProject: true, + warningPropertiesCollection: parentWarningPropertiesCollection)); + // start taking one node from the queue and get all of it's dependencies while (queue.Count > 0) { var node = queue.Dequeue(); - if (!seen.Contains(node.Id)) + if (!seen.Contains(node)) { - var nodeDependencies = dependencyMapping[node.Id]; - var nodeName = node.Id; - var nodeType = node.Type; + + var nodeId = node.Id; + var nodeIsProject = node.IsProject; + var nodeDependencies = dependencyMapping[nodeId]; var pathWarningProperties = node.WarningPropertiesCollection; // If the node is a project then we need to extract the warning properties and // add those to the warning properties of the current path. - if (nodeType == LibraryDependencyTarget.Project || nodeType == LibraryDependencyTarget.ExternalProject) + if (nodeIsProject) { // Get the node PackageSpec - var nodeProjectSpec = GetNodePackageSpec(dgSpec, nodeName); + var nodeProjectSpec = GetNodePackageSpec(dgSpec, nodeId); + var nodeTargetFrameworks = nodeProjectSpec.TargetFrameworks.Select(tfi => tfi.FrameworkName); + + var nearestFramework = frameworkReducer.GetNearest(parentTargetFramework, nodeTargetFrameworks); // Get the WarningPropertiesCollection from the PackageSpec - var nodeWarningProperties = GetNodeWarningProperties(nodeProjectSpec, parentTargetFramework); + var nodeWarningProperties = GetNodeWarningProperties(nodeProjectSpec, nearestFramework); // Merge the WarningPropertiesCollection to the one in the path - var mergedWarningPropertiesCollection = MergeWarningPropertiesCollection(pathWarningProperties, + var mergedWarningPropertiesCollection = MergeWarningPropertiesCollection(pathWarningProperties, nodeWarningProperties); // Add all the project's dependencies to the Queue with the merged WarningPropertiesCollection - foreach (var dependency in dependencyMapping[nodeName].OrderBy(d => d.Name)) + foreach (var dependency in dependencyMapping[nodeId].OrderBy(d => d.Name)) { - var queueTuple = new DependencyNode(dependency.Name, dependency.LibraryRange.TypeConstraint, mergedWarningPropertiesCollection); + var queueNode = new DependencyNode(dependency.Name.ToLowerInvariant(), IsProject(dependency.LibraryRange.TypeConstraint), mergedWarningPropertiesCollection); - // Add the metadata from the parent project here. - queue.Enqueue(queueTuple); + if (!seen.Contains(queueNode)) + { + // Add the metadata from the parent project here. + queue.Enqueue(queueNode); + } } } - else if (nodeType == LibraryDependencyTarget.Package) + else { // Evaluate the package properties for the current path + // Check if There was any NoWarn in the path + var pathNoWarnForId = ExtractPathNoWarnProperties(pathWarningProperties, nodeId); - // If the path does not "NoWarn" for this package then remove the path from parentPackageDependencies - - // If the path has a "NoWarn" for the package then save it to the result - - // If parentPackageDependencies is empty then exit the graph traversal + if (pathNoWarnForId.Count > 0) + { + // If the path has a "NoWarn" for the package then save it to the result + resultWarningProperties.AddRange(pathNoWarnForId.AsList(), nodeId, parentTargetFramework); + } + else + { + // If the path does not "NoWarn" for this package then remove the path from parentPackageDependencies + parentPackageDependencies.Remove(nodeId); + + // If parentPackageDependencies is empty then exit the graph traversal + if (parentPackageDependencies.Count == 0) + { + break; + } + } } + } + } + + return resultWarningProperties; + } + + private static IEnumerable GetDirectDependencies( + PackageSpec projectSpec, + WarningPropertiesCollection warningPropertiesCollection, + NuGetFramework targetFramework) + { + var dependencies = new List(); + + var targetFrameworkPackageDependencies = projectSpec + .TargetFrameworks + .Single(tfi => tfi.FrameworkName == targetFramework)? + .Dependencies; + + var targetFrameworkProjectDependencies = projectSpec + .RestoreMetadata? + .TargetFrameworks + .Single(tfi => tfi.FrameworkName == targetFramework)? + .ProjectReferences; + + // Unconditional package references + foreach (var dependency in projectSpec.Dependencies.OrderBy(d => d.Name)) + { + var queueNode = new DependencyNode(dependency.Name.ToLowerInvariant(), IsProject(dependency.LibraryRange.TypeConstraint), warningPropertiesCollection); + dependencies.Add(queueNode); + } + + // target framework specific package references + if (targetFrameworkPackageDependencies != null) + { + foreach (var dependency in targetFrameworkPackageDependencies.OrderBy(d => d.Name)) + { + var queueNode = new DependencyNode( + id: dependency.Name.ToLowerInvariant(), + isProject: IsProject(dependency.LibraryRange.TypeConstraint), + warningPropertiesCollection: warningPropertiesCollection); + + dependencies.Add(queueNode); + } + } + + // target framework specific project references + if (targetFrameworkProjectDependencies != null) + { + foreach (var dependency in targetFrameworkProjectDependencies.OrderBy(d => d.ProjectPath)) + { + var queueNode = new DependencyNode( + id: dependency.ProjectPath.ToLowerInvariant(), + isProject: true, + warningPropertiesCollection: warningPropertiesCollection); + dependencies.Add(queueNode); } } + + return dependencies; + } + + private static ISet ExtractPathNoWarnProperties(WarningPropertiesCollection pathWarningProperties, string id) + { + var result = new HashSet(); + if (pathWarningProperties?.ProjectWideWarningProperties?.NoWarn?.Count > 0) + { + result.UnionWith(pathWarningProperties.ProjectWideWarningProperties.NoWarn); + } + + if (pathWarningProperties?.PackageSpecificWarningProperties?.Properties?.Count > 0) + { + foreach(var codeIdCollection in pathWarningProperties.PackageSpecificWarningProperties.Properties) + { + var code = codeIdCollection.Key; + var IdCollection = codeIdCollection.Value; + if (IdCollection.ContainsKey(id)) + { + result.Add(code); + } + } + } + + return result; } private static WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec) @@ -143,7 +276,7 @@ private static PackageSpec GetNodePackageSpec(DependencyGraphSpec dgSpec, string { return dgSpec .Projects - .Where(p => string.Equals(p.RestoreMetadata.ProjectName, nodeId, StringComparison.OrdinalIgnoreCase)) + .Where(p => string.Equals(p.RestoreMetadata.ProjectPath, nodeId, StringComparison.OrdinalIgnoreCase)) .First(); } @@ -167,6 +300,13 @@ private static WarningPropertiesCollection MergeWarningPropertiesCollection(Warn var mergedPackageSpecificWarnings = MergePackageSpecificWarningProperties( first.PackageSpecificWarningProperties, second.PackageSpecificWarningProperties); + + // Ignore the project frameworks as the final collection will contain the parent project frameworks + + result = new WarningPropertiesCollection( + mergedProjectWideWarningProperties, + mergedPackageSpecificWarnings, + projectFrameworks: null); } return result; @@ -216,14 +356,29 @@ private static PackageSpecificWarningProperties MergePackageSpecificWarningPrope else { result = new PackageSpecificWarningProperties(); - foreach (var code in first.Properties.Keys) + if (first.Properties != null) { - foreach (var libraryId in first.Properties[code].Keys) + foreach (var code in first.Properties.Keys) { - result.AddRange(code, libraryId, first.Properties[code][libraryId]); + foreach (var libraryId in first.Properties[code].Keys) + { + result.AddRange(code, libraryId, first.Properties[code][libraryId]); + } + } + } + + if (second.Properties != null) + { + foreach (var code in second.Properties.Keys) + { + foreach (var libraryId in second.Properties[code].Keys) + { + result.AddRange(code, libraryId, second.Properties[code][libraryId]); + } } } } + return result; } @@ -259,35 +414,16 @@ private static bool TryMergeNullObjects(object first, object second, out object return result; } - private static LibraryDependencyTarget GetType(LibraryType type) + private static bool IsProject(LibraryDependencyTarget type) { - if (type == LibraryType.Assembly) - { - return LibraryDependencyTarget.Assembly; - } - else if (type == LibraryType.ExternalProject) - { - return LibraryDependencyTarget.ExternalProject; - } - else if (type == LibraryType.Package) - { - return LibraryDependencyTarget.Package; - } - else if (type == LibraryType.Project) - { - return LibraryDependencyTarget.Project; - } - else if (type == LibraryType.Reference) + if (type == LibraryDependencyTarget.ExternalProject || + type == LibraryDependencyTarget.Project) { - return LibraryDependencyTarget.Reference; - } - else if (type == LibraryType.WinMD) - { - return LibraryDependencyTarget.WinMD; + return true; } else { - return LibraryDependencyTarget.None; + return false; } } @@ -299,23 +435,24 @@ private class DependencyNode : IEquatable // ID of the Node public string Id { get; } - // Type of the Node - public LibraryDependencyTarget Type { get; } + // bool to indicate if the node is a project node + // if false then the node is a package + public bool IsProject { get; } // WarningPropertiesCollection of the path taken to the Node public WarningPropertiesCollection WarningPropertiesCollection { get; } - public DependencyNode(string id, LibraryDependencyTarget type, WarningPropertiesCollection warningPropertiesCollection) + public DependencyNode(string id, bool isProject, WarningPropertiesCollection warningPropertiesCollection) { Id = id ?? throw new ArgumentNullException(nameof(id)); WarningPropertiesCollection = warningPropertiesCollection ?? throw new ArgumentNullException(nameof(warningPropertiesCollection)); - Type = type; + IsProject = isProject; } - public DependencyNode(string id, LibraryDependencyTarget type) + public DependencyNode(string id, bool isProject) { Id = id ?? throw new ArgumentNullException(nameof(id)); - Type = type; + IsProject = isProject; } public override int GetHashCode() @@ -323,7 +460,7 @@ public override int GetHashCode() var hashCode = new HashCodeCombiner(); hashCode.AddStringIgnoreCase(Id); - hashCode.AddObject(Type); + hashCode.AddObject(IsProject); return hashCode.CombinedHash; } @@ -346,9 +483,14 @@ public bool Equals(DependencyNode other) } return string.Equals(Id, other.Id, StringComparison.OrdinalIgnoreCase) && - Type == other.Type && + IsProject == other.IsProject && WarningPropertiesCollection.Equals(other.WarningPropertiesCollection); } + + public override string ToString() + { + return $"{(IsProject ? "Project" : "Package")}/{Id}"; + } } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index bcbf6b0b2ab..0fd35a61250 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -43,7 +43,7 @@ public RestoreCommand(RestoreRequest request) { _request = request ?? throw new ArgumentNullException(nameof(request)); - //Debugger.Launch(); + Debugger.Launch(); // Validate the lock file version requested if (_request.LockFileVersion < 1 || _request.LockFileVersion > LockFileFormat.Version) @@ -121,9 +121,8 @@ public async Task ExecuteAsync(CancellationToken token) contextForProject, token); - //_logger.TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils - // .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, - // graphs, new LibraryIdentity(_request.Project.Name, _request.Project.Version, LibraryType.Project)); + _logger.TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils + .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, graphs, _request.Project); // Create assets file var assetsFile = BuildAssetsFile( From 5bac353b5359e8257983101ec052c30be98c2c06 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Tue, 8 Aug 2017 17:14:15 -0700 Subject: [PATCH 05/19] Checkpoint with a functional case (A->B->C->Pkg) and lazy initialization --- .../Logging/RestoreCollectorLogger.cs | 34 ++- .../Logging/TransitiveNoWarnUtils.cs | 215 ++++++++---------- .../RestoreCommand/ProjectRestoreCommand.cs | 8 +- .../RestoreCommand/ProjectRestoreRequest.cs | 4 +- .../RestoreCommand/RestoreCommand.cs | 13 +- .../CollectorLoggerTests.cs | 44 ++-- 6 files changed, 167 insertions(+), 151 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index 75faa27cd92..e081b579876 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -20,11 +20,15 @@ public class RestoreCollectorLogger : LoggerBase, ICollectorLogger public IEnumerable Errors => _errors.ToArray(); - public WarningPropertiesCollection WarningPropertiesCollection { get; set; } + public WarningPropertiesCollection ProjectWarningPropertiesCollection { get; set; } - public PackageSpecificWarningProperties TransitiveWarningPropertiesCollection { get; set; } + public WarningPropertiesCollection TransitiveWarningPropertiesCollection { get; set; } - public string ProjectPath { get; set; } + public IEnumerable RestoreTargetGraphs { get; set; } + + public PackageSpec ProjectSpec { get; set; } + + public string ProjectPath => ProjectSpec.RestoreMetadata?.ProjectPath; /// /// Initializes an instance of the , while still @@ -75,8 +79,11 @@ public RestoreCollectorLogger(ILogger innerLogger) public void Log(IRestoreLogMessage message) { + PopulateTransitiveWarningPropertiesCollection(message); + // This will be true only when the Message is a Warning and should be suppressed. - if (WarningPropertiesCollection == null || !WarningPropertiesCollection.ApplyWarningProperties(message)) + if ((ProjectWarningPropertiesCollection == null || !ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) && + (TransitiveWarningPropertiesCollection == null || !TransitiveWarningPropertiesCollection.ApplyWarningProperties(message))) { if (string.IsNullOrEmpty(message.FilePath)) { @@ -97,9 +104,11 @@ public void Log(IRestoreLogMessage message) public Task LogAsync(IRestoreLogMessage message) { + PopulateTransitiveWarningPropertiesCollection(message); // This will be true only when the Message is a Warning and should be suppressed. - if (WarningPropertiesCollection == null || !WarningPropertiesCollection.ApplyWarningProperties(message)) + if ((ProjectWarningPropertiesCollection == null || !ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) && + (TransitiveWarningPropertiesCollection == null || !TransitiveWarningPropertiesCollection.ApplyWarningProperties(message))) { if (string.IsNullOrEmpty(message.FilePath)) { @@ -147,6 +156,21 @@ protected bool DisplayMessage(IRestoreLogMessage message) } } + private void PopulateTransitiveWarningPropertiesCollection(ILogMessage message) + { + // Populate TransitiveWarningPropertiesCollection only if it is null and we have RestoreTargetGraphs. + // This will happen at most once. + if (message.Level == LogLevel.Warning && + TransitiveWarningPropertiesCollection == null && + RestoreTargetGraphs != null && + RestoreTargetGraphs.Any()) + { + TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils.CreateTransitiveWarningPropertiesCollection( + RestoreTargetGraphs, + ProjectSpec); + } + } + private static IRestoreLogMessage ToRestoreLogMessage(ILogMessage message) { var restoreLogMessage = message as IRestoreLogMessage; diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index ce5b971b638..aa3a78e2182 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -10,29 +10,28 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using NuGet.DependencyResolver; namespace NuGet.Commands { internal class TransitiveNoWarnUtils { // static should be fine across multiple restore calls as this solely depends on the csproj file of the project. - private static readonly ConcurrentDictionary _warningPropertiesCache = + private static readonly ConcurrentDictionary _warningPropertiesCache = new ConcurrentDictionary(); /// /// Creates a PackageSpecificWarningProperties for a project generated by traversing the dependency graph. /// - /// Restore DGSpec containing the complete closure. /// Parent project restore target graphs. - /// LibraryIdentity of the parent project. - /// - internal static PackageSpecificWarningProperties CreateTransitiveWarningPropertiesCollection( - DependencyGraphSpec dgSpec, + /// PackageSpec of the parent project. + /// WarningPropertiesCollection with the project frameworks and the transitive package specific no warn properties. + internal static WarningPropertiesCollection CreateTransitiveWarningPropertiesCollection( IEnumerable targetGraphs, PackageSpec parentProjectSpec) { - var transitiveNoWarnProperties = new PackageSpecificWarningProperties(); - + var transitivePackageSpecificProperties = new PackageSpecificWarningProperties(); + var projectFrameworks = new List(); var parentWarningProperties = GetNodeWarningProperties(parentProjectSpec); foreach (var targetGraph in targetGraphs) @@ -40,36 +39,38 @@ internal static PackageSpecificWarningProperties CreateTransitiveWarningProperti if (string.IsNullOrEmpty(targetGraph.RuntimeIdentifier)) { var transitiveNoWarnFromTargetGraph = ExtractTransitiveNoWarnProperties( - targetGraph, - dgSpec, - parentProjectSpec, + targetGraph, + parentProjectSpec.RestoreMetadata.ProjectName, parentWarningProperties); - transitiveNoWarnProperties = MergePackageSpecificWarningProperties( - transitiveNoWarnProperties, + projectFrameworks.Add(targetGraph.Framework); + + transitivePackageSpecificProperties = MergePackageSpecificWarningProperties( + transitivePackageSpecificProperties, transitiveNoWarnFromTargetGraph); } } - return transitiveNoWarnProperties; + return new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: transitivePackageSpecificProperties, + projectFrameworks: projectFrameworks + ); } /// /// Traverses a Dependency grpah starting from the parent project in BF style. /// /// Parent project restore target graph. - /// Restore DGSpec containing the complete closure. - /// PackageSpec of the parent project. + /// File path of the parent project. /// WarningPropertiesCollection of the parent project. /// PackageSpecificWarningProperties containing all the NoWarn's for each package seen in the graph accumulated while traversing the graph. private static PackageSpecificWarningProperties ExtractTransitiveNoWarnProperties( RestoreTargetGraph targetGraph, - DependencyGraphSpec dgSpec, - PackageSpec parentProjectSpec, + string parentProjectName, WarningPropertiesCollection parentWarningPropertiesCollection) { - var paths = new Dictionary>(); - var dependencyMapping = new Dictionary>(); + var dependencyMapping = new Dictionary(); var queue = new Queue(); var seen = new HashSet(); var frameworkReducer = new FrameworkReducer(); @@ -83,23 +84,51 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie var parentTargetFramework = targetGraph.Framework; // Add all dependencies into a dict for a quick transitive lookup - foreach (var dependency in targetGraph.Flattened.OrderBy(d => d.Key.Name)) + foreach (var dependencyGraphItem in targetGraph.Flattened.OrderBy(d => d.Key.Name)) { - // Use the full path for projects and id for packages - var name = string.IsNullOrEmpty(dependency.Data.Match.Path) ? - dependency.Key.Name : - dependency.Data.Match.Path; + WarningPropertiesCollection nodeWarningProperties = null; + + if (IsProject(dependencyGraphItem.Key.Type)) + { + var localMatch = dependencyGraphItem.Data.Match as LocalMatch; + var nodeProjectSpec = GetNodePackageSpec(localMatch); + var nearestFramework = frameworkReducer.GetNearest( + parentTargetFramework, + nodeProjectSpec.TargetFrameworks.Select(tfi => tfi.FrameworkName)); + + // Get the WarningPropertiesCollection from the PackageSpec + nodeWarningProperties = GetNodeWarningProperties(nodeProjectSpec, nearestFramework); + } - dependencyMapping[name.ToLower()] = dependency.Data.Dependencies; + var lookUpNode = new LookUpNode() + { + Dependencies = dependencyGraphItem.Data.Dependencies, + WarningPropertiesCollection = nodeWarningProperties + }; + + dependencyMapping[dependencyGraphItem.Key.Name.ToLower()] = lookUpNode; } - var parentDependencies = GetDirectDependencies(parentProjectSpec, parentWarningPropertiesCollection, parentTargetFramework); + // Get the direct dependencies for the parent project to seed the queue + var parentDependencies = dependencyMapping[parentProjectName]; // Seed the queue with the parent project's direct dependencies - parentDependencies.OrderBy(d => d.Id).ForEach(d => queue.Enqueue(d)); + foreach (var dependency in dependencyMapping[parentProjectName].Dependencies) + { + var queueNode = new DependencyNode( + dependency.Name.ToLowerInvariant(), + IsProject(dependency.LibraryRange.TypeConstraint), + parentWarningPropertiesCollection); + + if (!seen.Contains(queueNode)) + { + // Add the metadata from the parent project here. + queue.Enqueue(queueNode); + } + } // Add the parent project to the seen set to prevent adding it back to the queue - seen.Add(new DependencyNode(id: parentProjectSpec.RestoreMetadata.ProjectPath.ToLowerInvariant(), + seen.Add(new DependencyNode(id: parentProjectName.ToLowerInvariant(), isProject: true, warningPropertiesCollection: parentWarningPropertiesCollection)); @@ -112,30 +141,25 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie var nodeId = node.Id; var nodeIsProject = node.IsProject; - var nodeDependencies = dependencyMapping[nodeId]; + var nodeDependencies = dependencyMapping[nodeId].Dependencies; + var nodeWarningProperties = dependencyMapping[nodeId].WarningPropertiesCollection; var pathWarningProperties = node.WarningPropertiesCollection; // If the node is a project then we need to extract the warning properties and // add those to the warning properties of the current path. if (nodeIsProject) { - // Get the node PackageSpec - var nodeProjectSpec = GetNodePackageSpec(dgSpec, nodeId); - var nodeTargetFrameworks = nodeProjectSpec.TargetFrameworks.Select(tfi => tfi.FrameworkName); - - var nearestFramework = frameworkReducer.GetNearest(parentTargetFramework, nodeTargetFrameworks); - - // Get the WarningPropertiesCollection from the PackageSpec - var nodeWarningProperties = GetNodeWarningProperties(nodeProjectSpec, nearestFramework); - // Merge the WarningPropertiesCollection to the one in the path var mergedWarningPropertiesCollection = MergeWarningPropertiesCollection(pathWarningProperties, nodeWarningProperties); // Add all the project's dependencies to the Queue with the merged WarningPropertiesCollection - foreach (var dependency in dependencyMapping[nodeId].OrderBy(d => d.Name)) + foreach (var dependency in dependencyMapping[nodeId].Dependencies.OrderBy(d => d.Name)) { - var queueNode = new DependencyNode(dependency.Name.ToLowerInvariant(), IsProject(dependency.LibraryRange.TypeConstraint), mergedWarningPropertiesCollection); + var queueNode = new DependencyNode( + dependency.Name.ToLowerInvariant(), + IsProject(dependency.LibraryRange.TypeConstraint), + mergedWarningPropertiesCollection); if (!seen.Contains(queueNode)) { @@ -174,60 +198,9 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie return resultWarningProperties; } - private static IEnumerable GetDirectDependencies( - PackageSpec projectSpec, - WarningPropertiesCollection warningPropertiesCollection, - NuGetFramework targetFramework) + private static PackageSpec GetNodePackageSpec(LocalMatch localMatch) { - var dependencies = new List(); - - var targetFrameworkPackageDependencies = projectSpec - .TargetFrameworks - .Single(tfi => tfi.FrameworkName == targetFramework)? - .Dependencies; - - var targetFrameworkProjectDependencies = projectSpec - .RestoreMetadata? - .TargetFrameworks - .Single(tfi => tfi.FrameworkName == targetFramework)? - .ProjectReferences; - - // Unconditional package references - foreach (var dependency in projectSpec.Dependencies.OrderBy(d => d.Name)) - { - var queueNode = new DependencyNode(dependency.Name.ToLowerInvariant(), IsProject(dependency.LibraryRange.TypeConstraint), warningPropertiesCollection); - dependencies.Add(queueNode); - } - - // target framework specific package references - if (targetFrameworkPackageDependencies != null) - { - foreach (var dependency in targetFrameworkPackageDependencies.OrderBy(d => d.Name)) - { - var queueNode = new DependencyNode( - id: dependency.Name.ToLowerInvariant(), - isProject: IsProject(dependency.LibraryRange.TypeConstraint), - warningPropertiesCollection: warningPropertiesCollection); - - dependencies.Add(queueNode); - } - } - - // target framework specific project references - if (targetFrameworkProjectDependencies != null) - { - foreach (var dependency in targetFrameworkProjectDependencies.OrderBy(d => d.ProjectPath)) - { - var queueNode = new DependencyNode( - id: dependency.ProjectPath.ToLowerInvariant(), - isProject: true, - warningPropertiesCollection: warningPropertiesCollection); - - dependencies.Add(queueNode); - } - } - - return dependencies; + return localMatch.LocalLibrary.Items[KnownLibraryProperties.PackageSpec] as PackageSpec; } private static ISet ExtractPathNoWarnProperties(WarningPropertiesCollection pathWarningProperties, string id) @@ -272,15 +245,8 @@ private static WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly())); } - private static PackageSpec GetNodePackageSpec(DependencyGraphSpec dgSpec, string nodeId) - { - return dgSpec - .Projects - .Where(p => string.Equals(p.RestoreMetadata.ProjectPath, nodeId, StringComparison.OrdinalIgnoreCase)) - .First(); - } - - private static WarningPropertiesCollection MergeWarningPropertiesCollection(WarningPropertiesCollection first, + private static WarningPropertiesCollection MergeWarningPropertiesCollection( + WarningPropertiesCollection first, WarningPropertiesCollection second) { WarningPropertiesCollection result = null; @@ -312,7 +278,8 @@ private static WarningPropertiesCollection MergeWarningPropertiesCollection(Warn return result; } - private static WarningProperties MergeProjectWideWarningProperties(WarningProperties first, + private static WarningProperties MergeProjectWideWarningProperties( + WarningProperties first, WarningProperties second) { WarningProperties result = null; @@ -344,7 +311,8 @@ private static WarningProperties MergeProjectWideWarningProperties(WarningProper return result; } - private static PackageSpecificWarningProperties MergePackageSpecificWarningProperties(PackageSpecificWarningProperties first, + private static PackageSpecificWarningProperties MergePackageSpecificWarningProperties( + PackageSpecificWarningProperties first, PackageSpecificWarningProperties second) { PackageSpecificWarningProperties result = null; @@ -414,17 +382,24 @@ private static bool TryMergeNullObjects(object first, object second, out object return result; } + /// + /// Checks if a LibraryDependencyTarget is a project. + /// + /// LibraryDependencyTarget to be checked. + /// True if a LibraryDependencyTarget is Project or ExternalProject. private static bool IsProject(LibraryDependencyTarget type) { - if (type == LibraryDependencyTarget.ExternalProject || - type == LibraryDependencyTarget.Project) - { - return true; - } - else - { - return false; - } + return (type == LibraryDependencyTarget.ExternalProject || type == LibraryDependencyTarget.Project); + } + + /// + /// Checks if a LibraryType is a project. + /// + /// LibraryType to be checked. + /// True if a LibraryType is Project or ExternalProject. + private static bool IsProject(LibraryType type) + { + return (type == LibraryType.ExternalProject || type == LibraryType.Project); } /// @@ -492,5 +467,17 @@ public override string ToString() return $"{(IsProject ? "Project" : "Package")}/{Id}"; } } + + /// + /// A simple node class to hold the outgoing dependency edge for a quick look up + /// + private class LookUpNode + { + // List of dependencies for this node + public IEnumerable Dependencies { get; set; } + + // If the node is a project then this will hold the + public WarningPropertiesCollection WarningPropertiesCollection { get; set; } + } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs index eb99abdd92c..19fe6e50bc8 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs @@ -22,7 +22,7 @@ namespace NuGet.Commands { internal class ProjectRestoreCommand { - private readonly ILogger _logger; + private readonly RestoreCollectorLogger _logger; private readonly ProjectRestoreRequest _request; @@ -32,7 +32,7 @@ public ProjectRestoreCommand(ProjectRestoreRequest request) _request = request; } - public async Task, RuntimeGraph>> TryRestore(LibraryRange projectRange, + public async Task, RuntimeGraph>> TryRestoreAsync(LibraryRange projectRange, IEnumerable frameworkRuntimePairs, HashSet allInstalledPackages, NuGetv3LocalRepository userPackageFolder, @@ -119,6 +119,10 @@ await InstallPackagesAsync(runtimeGraphs, userPackageFolder.ClearCacheForIds(allInstalledPackages.Select(package => package.Name)); } + // Update the logger with the restore target graphs + // This allows lazy initialization for the Transitive Warning Properties + _logger.RestoreTargetGraphs = graphs; + // Warn for all dependencies that do not have exact matches or // versions that have been bumped up unexpectedly. await UnexpectedDependencyMessages.LogAsync(graphs, _request.Project, _logger); diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreRequest.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreRequest.cs index 4904043172b..f4387524927 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreRequest.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreRequest.cs @@ -21,7 +21,7 @@ public ProjectRestoreRequest( LockFile existingLockFile, Dictionary runtimeGraphCache, ConcurrentDictionary runtimeGraphCacheByPackage, - ILogger log) + RestoreCollectorLogger log) { CacheContext = request.CacheContext; Log = log; @@ -36,7 +36,7 @@ public ProjectRestoreRequest( } public SourceCacheContext CacheContext { get; } - public ILogger Log { get; } + public RestoreCollectorLogger Log { get; } public string PackagesDirectory { get; } public int MaxDegreeOfConcurrency { get; } public LockFile ExistingLockFile { get; } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 0fd35a61250..72fe64fc9d0 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -43,8 +43,6 @@ public RestoreCommand(RestoreRequest request) { _request = request ?? throw new ArgumentNullException(nameof(request)); - Debugger.Launch(); - // Validate the lock file version requested if (_request.LockFileVersion < 1 || _request.LockFileVersion > LockFileFormat.Version) { @@ -57,8 +55,8 @@ public RestoreCommand(RestoreRequest request) var collectorLogger = new RestoreCollectorLogger(_request.Log, collectorLoggerHideWarningsAndErrors) { - ProjectPath = _request.Project.RestoreMetadata?.ProjectPath, - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectSpec = _request.Project, + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( request.Project.RestoreMetadata?.ProjectWideWarningProperties, PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(request.Project), request.Project.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly() @@ -121,9 +119,6 @@ public async Task ExecuteAsync(CancellationToken token) contextForProject, token); - _logger.TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils - .CreateTransitiveWarningPropertiesCollection(_request.DependencyGraphSpec, graphs, _request.Project); - // Create assets file var assetsFile = BuildAssetsFile( _request.ExistingLockFile, @@ -572,7 +567,7 @@ private async Task> ExecuteRestoreAsync( var projectRestoreCommand = new ProjectRestoreCommand(projectRestoreRequest); - var result = await projectRestoreCommand.TryRestore( + var result = await projectRestoreCommand.TryRestoreAsync( projectRange, projectFrameworkRuntimePairs, allInstalledPackages, @@ -619,7 +614,7 @@ private async Task> ExecuteRestoreAsync( // Walk additional runtime graphs for supports checks if (_success && _request.CompatibilityProfiles.Any()) { - var compatibilityResult = await projectRestoreCommand.TryRestore( + var compatibilityResult = await projectRestoreCommand.TryRestoreAsync( projectRange, _request.CompatibilityProfiles, allInstalledPackages, diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs index 29e9a65abf7..9217252e6ec 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs @@ -114,7 +114,13 @@ public void CollectorLogger_DoesNotPassLogCallsToInnerLoggerByDefaultWithFilePat var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object, LogLevel.Debug, hideWarningsAndErrors: false) { - ProjectPath = projectPath + ProjectSpec = new PackageSpec() + { + RestoreMetadata = new ProjectRestoreMetadata() + { + ProjectPath = projectPath + } + } }; // Act @@ -368,7 +374,7 @@ public void CollectorLogger_LogsWarningsAsErrorsErrorsForProjectWideWarnAsErrorS var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null) @@ -402,7 +408,7 @@ public void CollectorLogger_LogsWarningsAsErrorsForProjectAllWarningsAsErrors() var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null) @@ -434,7 +440,7 @@ public void CollectorLogger_LogsWarningsAsErrorsForProjectAllWarningsAsErrorsAnd var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null) @@ -468,7 +474,7 @@ public void CollectorLogger_DoesNotLogsWarningsForProjectWideNoWarnSet() var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null) @@ -506,7 +512,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSet() var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, null) @@ -544,7 +550,7 @@ public void CollectorLogger_LogsWarningsForPackageSpecificNoWarnSetButWarningsWi var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, null) @@ -584,7 +590,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithMu var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, null) @@ -624,7 +630,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithLi var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, null) @@ -664,7 +670,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithLi var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, null) @@ -706,7 +712,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetForGlo var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, new List { targetFramework }) @@ -746,7 +752,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetForGlo var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, new List { targetFramework, netcoreTargetFramework }) @@ -786,7 +792,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithMu var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, new List { targetFramework }) @@ -830,7 +836,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithMu var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, new List { targetFramework, netcoreTargetFramework }) @@ -866,7 +872,7 @@ public void CollectorLogger_DoesNotLogsWarningsForProjectWideNoWarnSetAndAllWarn var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null) @@ -898,7 +904,7 @@ public void CollectorLogger_DoesNotLogsWarningsForProjectWideNoWarnSetAndWarnAsE var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null) @@ -930,7 +936,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetAndWar var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), null, null) @@ -970,7 +976,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificAndProjectWideN var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, null) @@ -1012,7 +1018,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetAndPro var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) { - WarningPropertiesCollection = new WarningPropertiesCollection( + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( new WarningProperties(warnAsErrorSet, noWarnSet, allWarningsAsErrors), packageSpecificWarningProperties, null) From 7b0626024ef18dd86718f3e9f5031a40ace939f8 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Tue, 8 Aug 2017 18:32:21 -0700 Subject: [PATCH 06/19] Adding code to deal with path exclusion logic and fixing test breaks --- .../Logging/RestoreCollectorLogger.cs | 2 +- .../Logging/TransitiveNoWarnUtils.cs | 81 +++++++++++++------ 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index e081b579876..243ca2e37eb 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -28,7 +28,7 @@ public class RestoreCollectorLogger : LoggerBase, ICollectorLogger public PackageSpec ProjectSpec { get; set; } - public string ProjectPath => ProjectSpec.RestoreMetadata?.ProjectPath; + public string ProjectPath => ProjectSpec?.RestoreMetadata?.ProjectPath; /// /// Initializes an instance of the , while still diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index aa3a78e2182..c1310725e2e 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -40,7 +40,7 @@ internal static WarningPropertiesCollection CreateTransitiveWarningPropertiesCol { var transitiveNoWarnFromTargetGraph = ExtractTransitiveNoWarnProperties( targetGraph, - parentProjectSpec.RestoreMetadata.ProjectName, + parentProjectSpec.RestoreMetadata.ProjectName.ToLowerInvariant(), parentWarningProperties); projectFrameworks.Add(targetGraph.Framework); @@ -75,11 +75,12 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie var seen = new HashSet(); var frameworkReducer = new FrameworkReducer(); var resultWarningProperties = new PackageSpecificWarningProperties(); + var packageNoWarn = new Dictionary>(); // All the packages in parent project's closure. // Once we have collected data for all of these, we can exit. var parentPackageDependencies = new HashSet( - targetGraph.Flattened.Where(d => d.Key.Type == LibraryType.Package).Select(d => d.Key.Name)); + targetGraph.Flattened.Where(d => d.Key.Type == LibraryType.Package).Select(d => d.Key.Name.ToLowerInvariant())); var parentTargetFramework = targetGraph.Framework; @@ -150,39 +151,37 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie if (nodeIsProject) { // Merge the WarningPropertiesCollection to the one in the path - var mergedWarningPropertiesCollection = MergeWarningPropertiesCollection(pathWarningProperties, + var mergedWarningProperties = MergeWarningPropertiesCollection(pathWarningProperties, nodeWarningProperties); - // Add all the project's dependencies to the Queue with the merged WarningPropertiesCollection - foreach (var dependency in dependencyMapping[nodeId].Dependencies.OrderBy(d => d.Name)) - { - var queueNode = new DependencyNode( - dependency.Name.ToLowerInvariant(), - IsProject(dependency.LibraryRange.TypeConstraint), - mergedWarningPropertiesCollection); - - if (!seen.Contains(queueNode)) - { - // Add the metadata from the parent project here. - queue.Enqueue(queueNode); - } - } + AddDependenciesToQueue(dependencyMapping[nodeId].Dependencies, + queue, + seen, + mergedWarningProperties); } - else - { - // Evaluate the package properties for the current path - // Check if There was any NoWarn in the path - var pathNoWarnForId = ExtractPathNoWarnProperties(pathWarningProperties, nodeId); + else if (parentPackageDependencies.Contains(nodeId)) + { + // Evaluate the current path for package properties + var packageNoWarnFromPath = ExtractPathNoWarnProperties(pathWarningProperties, nodeId); - if (pathNoWarnForId.Count > 0) + if (packageNoWarn.ContainsKey(nodeId)) { - // If the path has a "NoWarn" for the package then save it to the result - resultWarningProperties.AddRange(pathNoWarnForId.AsList(), nodeId, parentTargetFramework); + // We have seen atleast one path which contained a NoWarn for the package + // We need to update the + packageNoWarn[nodeId].IntersectWith(packageNoWarnFromPath); } else + { + packageNoWarn[nodeId] = packageNoWarnFromPath; + } + + // Check if there was any NoWarn in the path + if (packageNoWarn[nodeId].Count == 0) { // If the path does not "NoWarn" for this package then remove the path from parentPackageDependencies + // This is done because if there are no "NoWarn" in one path, the the warnings must come through + // We no longer care about this package in the graph parentPackageDependencies.Remove(nodeId); // If parentPackageDependencies is empty then exit the graph traversal @@ -191,13 +190,45 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie break; } } + + AddDependenciesToQueue(dependencyMapping[nodeId].Dependencies, + queue, + seen, + pathWarningProperties); } } } + // At the end of the graph traversal add the remaining package no warn lists into the result + foreach(var packageId in packageNoWarn.Keys) + { + resultWarningProperties.AddRange(packageNoWarn[packageId], packageId, parentTargetFramework); + } + return resultWarningProperties; } + private static void AddDependenciesToQueue(IEnumerable dependencies, + Queue queue, + HashSet seen, + WarningPropertiesCollection pathWarningPropertiesCollection) + { + // Add all the project's dependencies to the Queue with the merged WarningPropertiesCollection + foreach (var dependency in dependencies) + { + var queueNode = new DependencyNode( + dependency.Name.ToLowerInvariant(), + IsProject(dependency.LibraryRange.TypeConstraint), + pathWarningPropertiesCollection); + + if (!seen.Contains(queueNode)) + { + // Add the metadata from the parent project here. + queue.Enqueue(queueNode); + } + } + } + private static PackageSpec GetNodePackageSpec(LocalMatch localMatch) { return localMatch.LocalLibrary.Items[KnownLibraryProperties.PackageSpec] as PackageSpec; From 32dd8747c80018342a6c6693f73eabeafee6803d Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Wed, 9 Aug 2017 11:08:24 -0700 Subject: [PATCH 07/19] making TransitiveNoWarnUtils conditional on projectspec and restoremetadata --- .../RestoreCommand/Logging/RestoreCollectorLogger.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index 243ca2e37eb..b33a8f5f10e 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -159,11 +159,13 @@ protected bool DisplayMessage(IRestoreLogMessage message) private void PopulateTransitiveWarningPropertiesCollection(ILogMessage message) { // Populate TransitiveWarningPropertiesCollection only if it is null and we have RestoreTargetGraphs. - // This will happen at most once. + // This will happen at most once and only if we have the project spec with restore metadata. if (message.Level == LogLevel.Warning && TransitiveWarningPropertiesCollection == null && RestoreTargetGraphs != null && - RestoreTargetGraphs.Any()) + RestoreTargetGraphs.Any() && + ProjectSpec != null && + ProjectSpec.RestoreMetadata != null) { TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils.CreateTransitiveWarningPropertiesCollection( RestoreTargetGraphs, From b41d9d3406f573b38702a82bae19a469760f6cfd Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Wed, 9 Aug 2017 15:57:30 -0700 Subject: [PATCH 08/19] Adding scenario tests --- .../RestoreTransitiveLoggingTests.cs | 1179 +++++++++++++++++ .../Test.Utility/TestExtensions.cs | 27 + 2 files changed, 1206 insertions(+) create mode 100644 test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs create mode 100644 test/TestUtilities/Test.Utility/TestExtensions.cs diff --git a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs new file mode 100644 index 00000000000..baa0ce3611f --- /dev/null +++ b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs @@ -0,0 +1,1179 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Diagnostics; +using System.Linq; +using FluentAssertions; +using NuGet.Frameworks; +using NuGet.Test.Utility; +using Test.Utility; +using Xunit; + +namespace NuGet.CommandLine.Test +{ + public class RestoreTransitiveLoggingTests + { + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + public void GivenAProjectReferenceNoWarnsVerifyNoWarning() + { + + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Save(); + + // A -> B + projectA.AddProjectToAllFrameworks(projectB); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[ProjectWide NoWarn NU1603] -> PkgX[NU1603] + public void GivenAProjectReferenceNoWarnsProjectWideVerifyNoWarning() + { + + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Properties.Add("NoWarn", "NU1603"); + projectB.Save(); + + // A -> B + projectA.AddProjectToAllFrameworks(projectB); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + public void GivenAProjectReferenceWithDifferentFrameworkNoWarnsVerifyNoWarning() + { + + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp1.1")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Save(); + + // A -> B + projectA.AddProjectToAllFrameworks(projectB); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB -> ProjC[PkgX NoWarn NU1603] -> PkgX[NU1603] + public void GivenATransitiveProjectReferenceNoWarnsVerifyNoWarning() + { + + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // C -> X + projectC.AddPackageToAllFrameworks(packageX); + projectC.Save(); + + // B -> C + projectB.AddProjectToAllFrameworks(projectC); + projectB.Save(); + + // A -> B + projectA.AddProjectToAllFrameworks(projectB); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB -> ProjC[ProjectWide NoWarn NU1603] -> PkgX[NU1603] + public void GivenATransitiveProjectReferenceNoWarnsProjectWideVerifyNoWarning() + { + + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // C -> X + projectC.AddPackageToAllFrameworks(packageX); + projectC.Properties.Add("NoWarn", "NU1603"); + projectC.Save(); + + // B -> C + projectB.AddProjectToAllFrameworks(projectC); + projectB.Save(); + + // A -> B + projectA.AddProjectToAllFrameworks(projectB); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> PkgX[NU1603] + public void GivenAProjectReferenceNoWarnsButDirectReferenceGeneratesWarningVerifyWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageXWithNoWarn); + projectB.Save(); + + // A -> B + // A -> X + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddPackageToAllFrameworks(packageX); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().Contain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[ProjectWide NoWarn NU1603] -> PkgX[NU1603] + // -> PkgX[NU1603] + public void GivenAProjectReferenceNoWarnsProjectWideButDirectReferenceGeneratesWarningVerifyWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Properties.Add("NoWarn", "NU1603"); + projectB.Save(); + + // A -> B + // A -> X + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddPackageToAllFrameworks(packageX); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().Contain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC[PkgX NoWarn NU1603] -> PkgX[NU1603] + public void GivenMultipleProjectReferencesNoWarnVerifyNoWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageXWithNoWarn); + projectB.Save(); + + // C -> X + projectC.AddPackageToAllFrameworks(packageXWithNoWarn); + projectC.Save(); + + // A -> B + // A -> C + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[ProjectWide NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC[ProjectWide NoWarn NU1603] -> PkgX[NU1603] + public void GivenMultipleProjectReferencesNoWarnProjectWideVerifyNoWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Properties.Add("NoWarn", "NU1603"); + projectB.Save(); + + // C -> X + projectC.AddPackageToAllFrameworks(packageX); + projectC.Properties.Add("NoWarn", "NU1603"); + projectC.Save(); + + // A -> B + // A -> C + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[ProjectWide NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC[ProjectWide NoWarn NU1605] -> PkgX[NU1603] + public void GivenMultipleProjectReferencesNoWarnDifferentWarningsProjectWideVerifyWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Properties.Add("NoWarn", "NU1603"); + projectB.Save(); + + // C -> X + projectC.AddPackageToAllFrameworks(packageX); + projectC.Properties.Add("NoWarn", "NU1605"); + projectC.Save(); + + // A -> B + // A -> C + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.GetSubstringCount("NU1603", ignoreCase: false).Should().Be(3); + } + } + + [Fact] + // Tests ProjA -> ProjB[ProjectWide NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC[PkgX NoWarn NU1603] -> PkgX[NU1603] + public void GivenMultipleProjectReferencesNoWarnMixedVerifyNoWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Properties.Add("NoWarn", "NU1603"); + projectB.Save(); + + // C -> X + projectC.AddPackageToAllFrameworks(packageXWithNoWarn); + projectC.Save(); + + // A -> B + // A -> C + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC -> PkgX[NU1603] + public void GivenMultipleProjectReferencesAndOnePathWarnsVerifyNoWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageXWithNoWarn); + projectB.Save(); + + // C -> X + projectC.AddPackageToAllFrameworks(packageX); + projectC.Save(); + + // A -> B + // A -> C + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().Contain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> PkgX[NU1603] + public void GivenMultipleProjectReferencesNoWarnButDirectReferenceWarnsVerifyWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageXWithNoWarn); + projectB.Save(); + + // C -> X + projectC.AddPackageToAllFrameworks(packageXWithNoWarn); + projectC.Save(); + + // A -> B + // A -> C + // A -> X + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.AddPackageToAllFrameworks(packageX); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.GetSubstringCount("NU1603", ignoreCase: false).Should().Be(1); + } + } + + [Fact] + // Tests ProjA[PkgX NoWarn NU1603] -> ProjB -> PkgX[NU1603] + // -> ProjC -> PkgX[NU1603] + // -> PkgX[NU1603] + public void GivenMultipleProjectReferencesWarnButDirectReferenceNoWarnsVerifyNoWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageX); + projectB.Save(); + + // C -> X + projectC.AddPackageToAllFrameworks(packageX); + projectC.Save(); + + // A -> B + // A -> C + // A -> X + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.AddPackageToAllFrameworks(packageXWithNoWarn); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.GetSubstringCount("NU1603", ignoreCase: false).Should().Be(2); + } + } + + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + public void GivenSinglePointOfReferenceNoWarnsVerifyNoWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageXWithNoWarn); + projectB.Save(); + + // C -> B + projectC.AddProjectToAllFrameworks(projectB); + projectC.Save(); + + // A -> B + // A -> C + // A -> X + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + + [Fact] + // Tests ProjA -> ProjB[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> ProjC[PkgX NoWarn NU1603] -> PkgX[NU1603] + // -> ProjD -> ProjE -> ProjF -> PkgX[NU1603] + public void GivenOneLongPathWarnsVerifyWarning() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = SimpleTestProjectContext.CreateNETCore( + "a", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectB = SimpleTestProjectContext.CreateNETCore( + "b", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectC = SimpleTestProjectContext.CreateNETCore( + "c", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectD = SimpleTestProjectContext.CreateNETCore( + "d", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectE = SimpleTestProjectContext.CreateNETCore( + "e", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + var projectF = SimpleTestProjectContext.CreateNETCore( + "f", + pathContext.SolutionRoot, + NuGetFramework.Parse("netcoreapp2.0")); + + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Referenced but not created + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + // B -> X + projectB.AddPackageToAllFrameworks(packageXWithNoWarn); + projectB.Save(); + + // C -> B + projectC.AddPackageToAllFrameworks(packageXWithNoWarn); + projectC.Save(); + + // F -> X + projectF.AddPackageToAllFrameworks(packageX); + projectF.Save(); + + // E -> F + projectE.AddProjectToAllFrameworks(projectF); + projectE.Save(); + + // D -> E + projectD.AddProjectToAllFrameworks(projectE); + projectD.Save(); + + // A -> B + // A -> C + // A -> D + projectA.AddProjectToAllFrameworks(projectB); + projectA.AddProjectToAllFrameworks(projectC); + projectA.AddProjectToAllFrameworks(projectD); + projectA.Save(); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Projects.Add(projectC); + solution.Projects.Add(projectD); + solution.Projects.Add(projectE); + solution.Projects.Add(projectF); + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().Contain("NU1603"); + } + } + } +} diff --git a/test/TestUtilities/Test.Utility/TestExtensions.cs b/test/TestUtilities/Test.Utility/TestExtensions.cs new file mode 100644 index 00000000000..7300044870e --- /dev/null +++ b/test/TestUtilities/Test.Utility/TestExtensions.cs @@ -0,0 +1,27 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Test.Utility +{ + public static class TestExtensions + { + public static int GetSubstringCount(this string str, string substr, bool ignoreCase) + { + var splitChars = new char[] { '.', '?', '!', ' ', ';', ':', ',', '\r', '\n' }; + var words = str.Split(splitChars, StringSplitOptions.RemoveEmptyEntries); + var comparisonType = ignoreCase ? + StringComparison.OrdinalIgnoreCase : + StringComparison.Ordinal; + + return words + .Where(word => string.Equals(word, substr, comparisonType)) + .Count(); + } + } +} From d2d095a41091ecf61164ee460c91ba67c2b353c7 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Mon, 14 Aug 2017 14:20:39 -0700 Subject: [PATCH 09/19] addressing some PR feedback --- .../RestoreTransitiveLoggingTests.cs | 21 ++++++++++++--- .../Test.Utility/TestExtensions.cs | 27 ------------------- 2 files changed, 17 insertions(+), 31 deletions(-) delete mode 100644 test/TestUtilities/Test.Utility/TestExtensions.cs diff --git a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs index baa0ce3611f..49783455edb 100644 --- a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -668,7 +668,7 @@ public void GivenMultipleProjectReferencesNoWarnDifferentWarningsProjectWideVeri // Assert r.Success.Should().BeTrue(); - r.AllOutput.GetSubstringCount("NU1603", ignoreCase: false).Should().Be(3); + GetSubstringCount(r.AllOutput, "NU1603", ignoreCase: false).Should().Be(3); } } @@ -907,7 +907,7 @@ public void GivenMultipleProjectReferencesNoWarnButDirectReferenceWarnsVerifyWar // Assert r.Success.Should().BeTrue(); - r.AllOutput.GetSubstringCount("NU1603", ignoreCase: false).Should().Be(1); + GetSubstringCount(r.AllOutput, "NU1603", ignoreCase: false).Should().Be(1); } } @@ -988,7 +988,7 @@ public void GivenMultipleProjectReferencesWarnButDirectReferenceNoWarnsVerifyNoW // Assert r.Success.Should().BeTrue(); - r.AllOutput.GetSubstringCount("NU1603", ignoreCase: false).Should().Be(2); + GetSubstringCount(r.AllOutput, "NU1603", ignoreCase: false).Should().Be(2); } } @@ -1175,5 +1175,18 @@ public void GivenOneLongPathWarnsVerifyWarning() r.AllOutput.Should().Contain("NU1603"); } } + + private static int GetSubstringCount(string str, string substr, bool ignoreCase) + { + var splitChars = new char[] { '.', '?', '!', ' ', ';', ':', ',', '\r', '\n' }; + var words = str.Split(splitChars, StringSplitOptions.RemoveEmptyEntries); + var comparisonType = ignoreCase ? + StringComparison.OrdinalIgnoreCase : + StringComparison.Ordinal; + + return words + .Where(word => string.Equals(word, substr, comparisonType)) + .Count(); + } } } diff --git a/test/TestUtilities/Test.Utility/TestExtensions.cs b/test/TestUtilities/Test.Utility/TestExtensions.cs deleted file mode 100644 index 7300044870e..00000000000 --- a/test/TestUtilities/Test.Utility/TestExtensions.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace Test.Utility -{ - public static class TestExtensions - { - public static int GetSubstringCount(this string str, string substr, bool ignoreCase) - { - var splitChars = new char[] { '.', '?', '!', ' ', ';', ':', ',', '\r', '\n' }; - var words = str.Split(splitChars, StringSplitOptions.RemoveEmptyEntries); - var comparisonType = ignoreCase ? - StringComparison.OrdinalIgnoreCase : - StringComparison.Ordinal; - - return words - .Where(word => string.Equals(word, substr, comparisonType)) - .Count(); - } - } -} From aff0915737164947d96ba3a92a63206693126679 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Mon, 21 Aug 2017 15:05:25 -0700 Subject: [PATCH 10/19] Addressing warning properties related PR feedback --- .../PackageSpecificWarningProperties.cs | 23 +++++++------- .../Logging/RestoreCollectorLogger.cs | 31 ++++++++++++------- .../Logging/TransitiveNoWarnUtils.cs | 6 ++-- .../Logging/WarningPropertiesCollection.cs | 4 +-- .../CollectorLoggerTests.cs | 4 +-- .../PackageSpecificWanringPropertiesTests.cs | 2 +- 6 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs index 2cd14f5fd2c..5059b31587b 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -38,7 +38,7 @@ public static PackageSpecificWarningProperties CreatePackageSpecificWarningPrope { foreach (var framework in packageSpec.TargetFrameworks) { - warningProperties.AddRange(dependency.NoWarn, dependency.Name, framework.FrameworkName); + warningProperties.AddRangeOfCodes(dependency.NoWarn, dependency.Name, framework.FrameworkName); } } @@ -46,7 +46,7 @@ public static PackageSpecificWarningProperties CreatePackageSpecificWarningPrope { foreach (var dependency in framework.Dependencies) { - warningProperties.AddRange(dependency.NoWarn, dependency.Name, framework.FrameworkName); + warningProperties.AddRangeOfCodes(dependency.NoWarn, dependency.Name, framework.FrameworkName); } } @@ -67,17 +67,16 @@ public static PackageSpecificWarningProperties CreatePackageSpecificWarningPrope foreach (var dependency in packageSpec.Dependencies) { - warningProperties.AddRange(dependency.NoWarn, dependency.Name, framework); + warningProperties.AddRangeOfCodes(dependency.NoWarn, dependency.Name, framework); } var targetFrameworkInformation = packageSpec .TargetFrameworks - .Where(tfi => tfi.FrameworkName == framework) - .First(); + .First(tfi => tfi.FrameworkName == framework); foreach (var dependency in targetFrameworkInformation.Dependencies) { - warningProperties.AddRange(dependency.NoWarn, dependency.Name, framework); + warningProperties.AddRangeOfCodes(dependency.NoWarn, dependency.Name, framework); } return warningProperties; @@ -117,7 +116,7 @@ public void Add(NuGetLogCode code, string libraryId, NuGetFramework framework) /// IEnumerable of NuGetLogCode for which no warning should be thrown. /// Library for which no warning should be thrown. /// Target graph for which no warning should be thrown. - public void AddRange(IEnumerable codes, string libraryId, NuGetFramework framework) + public void AddRangeOfCodes(IEnumerable codes, string libraryId, NuGetFramework framework) { foreach (var code in codes) { @@ -131,7 +130,7 @@ public void AddRange(IEnumerable codes, string libraryId, NuGetFra /// NuGetLogCode for which no warning should be thrown. /// Library for which no warning should be thrown. /// IEnumerable of Target graph for which no warning should be thrown. - public void AddRange(NuGetLogCode code, string libraryId, IEnumerable frameworks) + public void AddRangeOfFrameworks(NuGetLogCode code, string libraryId, IEnumerable frameworks) { foreach (var framework in frameworks) { @@ -181,9 +180,9 @@ public bool Equals(PackageSpecificWarningProperties other) return true; } - return Properties.Keys.OrderedEquals(other.Properties.Keys, (code) => code) && - Properties.Keys.All(c => Properties[c].Keys.OrderedEquals(other.Properties[c].Keys, (id) => id)) && - Properties.Keys.All(c => Properties[c].Keys.All(id => Properties[c][id].SetEquals(other.Properties[c][id]))); + return EqualityUtility.OrderedEquals(Properties.Keys, other.Properties.Keys, (code) => code) && + Properties.Keys.All(c => EqualityUtility.OrderedEquals(Properties[c].Keys, other.Properties[c].Keys, (id) => id)) && + Properties.Keys.All(c => Properties[c].Keys.All(id => EqualityUtility.OrderedEquals(Properties[c][id], other.Properties[c][id], (fx) => fx))); } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index b33a8f5f10e..fde913ff8d8 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -79,11 +79,8 @@ public RestoreCollectorLogger(ILogger innerLogger) public void Log(IRestoreLogMessage message) { - PopulateTransitiveWarningPropertiesCollection(message); - - // This will be true only when the Message is a Warning and should be suppressed. - if ((ProjectWarningPropertiesCollection == null || !ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) && - (TransitiveWarningPropertiesCollection == null || !TransitiveWarningPropertiesCollection.ApplyWarningProperties(message))) + // This will be true if the message is not or warning or it is not suppressed. + if (!IsWarningSuppressed(message)) { if (string.IsNullOrEmpty(message.FilePath)) { @@ -104,11 +101,8 @@ public void Log(IRestoreLogMessage message) public Task LogAsync(IRestoreLogMessage message) { - PopulateTransitiveWarningPropertiesCollection(message); - - // This will be true only when the Message is a Warning and should be suppressed. - if ((ProjectWarningPropertiesCollection == null || !ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) && - (TransitiveWarningPropertiesCollection == null || !TransitiveWarningPropertiesCollection.ApplyWarningProperties(message))) + // This will be true if the message is not or warning or it is not suppressed. + if (!IsWarningSuppressed(message)) { if (string.IsNullOrEmpty(message.FilePath)) { @@ -156,7 +150,20 @@ protected bool DisplayMessage(IRestoreLogMessage message) } } - private void PopulateTransitiveWarningPropertiesCollection(ILogMessage message) + /// + /// This method checks if at least one of the warning properties collections is not null and it suppresses the warning. + /// + /// IRestoreLogMessage to be logged. + /// bool indicating if the message should be suppressed. + private bool IsWarningSuppressed(IRestoreLogMessage message) + { + TryPopulateTransitiveWarningPropertiesCollection(message); + + return (ProjectWarningPropertiesCollection != null && ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) || + (TransitiveWarningPropertiesCollection != null && TransitiveWarningPropertiesCollection.ApplyWarningProperties(message)); + } + + private void TryPopulateTransitiveWarningPropertiesCollection(ILogMessage message) { // Populate TransitiveWarningPropertiesCollection only if it is null and we have RestoreTargetGraphs. // This will happen at most once and only if we have the project spec with restore metadata. diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index c1310725e2e..d9b4ab5867a 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -202,7 +202,7 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie // At the end of the graph traversal add the remaining package no warn lists into the result foreach(var packageId in packageNoWarn.Keys) { - resultWarningProperties.AddRange(packageNoWarn[packageId], packageId, parentTargetFramework); + resultWarningProperties.AddRangeOfCodes(packageNoWarn[packageId], packageId, parentTargetFramework); } return resultWarningProperties; @@ -361,7 +361,7 @@ private static PackageSpecificWarningProperties MergePackageSpecificWarningPrope { foreach (var libraryId in first.Properties[code].Keys) { - result.AddRange(code, libraryId, first.Properties[code][libraryId]); + result.AddRangeOfFrameworks(code, libraryId, first.Properties[code][libraryId]); } } } @@ -372,7 +372,7 @@ private static PackageSpecificWarningProperties MergePackageSpecificWarningPrope { foreach (var libraryId in second.Properties[code].Keys) { - result.AddRange(code, libraryId, second.Properties[code][libraryId]); + result.AddRangeOfFrameworks(code, libraryId, second.Properties[code][libraryId]); } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs index 6542602a412..a7ca1779c68 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -169,7 +169,7 @@ public bool Equals(WarningPropertiesCollection other) return ProjectWideWarningProperties.Equals(other.ProjectWideWarningProperties) && PackageSpecificWarningProperties.Equals(other.PackageSpecificWarningProperties) && - ProjectFrameworks.OrderedEquals(other.ProjectFrameworks, (s) => s.Framework); + EqualityUtility.OrderedEquals(ProjectFrameworks, other.ProjectFrameworks, (fx) => fx, sequenceComparer: new NuGetFrameworkFullComparer()); } } } diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs index 9217252e6ec..8cee9d93b61 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs @@ -625,7 +625,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithLi var warnAsErrorSet = new HashSet { }; var allWarningsAsErrors = false; var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); - packageSpecificWarningProperties.AddRange(new List { NuGetLogCode.NU1500, NuGetLogCode.NU1601, NuGetLogCode.NU1605}, libraryId, targetFramework); + packageSpecificWarningProperties.AddRangeOfCodes(new List { NuGetLogCode.NU1500, NuGetLogCode.NU1601, NuGetLogCode.NU1605}, libraryId, targetFramework); var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) @@ -665,7 +665,7 @@ public void CollectorLogger_DoesNotLogsWarningsForPackageSpecificNoWarnSetWithLi var warnAsErrorSet = new HashSet { }; var allWarningsAsErrors = false; var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); - packageSpecificWarningProperties.AddRange(new List { NuGetLogCode.NU1500 }, libraryId, targetFramework); + packageSpecificWarningProperties.AddRangeOfCodes(new List { NuGetLogCode.NU1500 }, libraryId, targetFramework); var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object) diff --git a/test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWanringPropertiesTests.cs b/test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWanringPropertiesTests.cs index cf001b9cd24..58bf9074980 100644 --- a/test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWanringPropertiesTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWanringPropertiesTests.cs @@ -54,7 +54,7 @@ public void PackageSpecificWarningProperties_AddsRangeValueWithGlobalTFM() var libraryId = "test_libraryId"; var targetFramework = NuGetFramework.Parse("net45"); var properties = new PackageSpecificWarningProperties(); - properties.AddRange(codes, libraryId, targetFramework); + properties.AddRangeOfCodes(codes, libraryId, targetFramework); // Assert foreach (var code in codes) From fd2954df566bd4df91b3c9b8f9194f5bec4fbbd8 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Mon, 21 Aug 2017 15:30:58 -0700 Subject: [PATCH 11/19] Addressing feedback on TransitiveNoWarnUtil --- .../Logging/RestoreCollectorLogger.cs | 4 +- .../Logging/TransitiveNoWarnUtils.cs | 58 ++++++++++--------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index fde913ff8d8..31ed3dfc2b4 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -174,7 +174,9 @@ private void TryPopulateTransitiveWarningPropertiesCollection(ILogMessage messag ProjectSpec != null && ProjectSpec.RestoreMetadata != null) { - TransitiveWarningPropertiesCollection = TransitiveNoWarnUtils.CreateTransitiveWarningPropertiesCollection( + var transitiveNoWarnUtils = new TransitiveNoWarnUtils(); + + TransitiveWarningPropertiesCollection = transitiveNoWarnUtils.CreateTransitiveWarningPropertiesCollection( RestoreTargetGraphs, ProjectSpec); } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index d9b4ab5867a..1ab533cc911 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using NuGet.Common; @@ -16,9 +16,9 @@ namespace NuGet.Commands { internal class TransitiveNoWarnUtils { - // static should be fine across multiple restore calls as this solely depends on the csproj file of the project. - private static readonly ConcurrentDictionary _warningPropertiesCache = - new ConcurrentDictionary(); + // static should be fine across multiple calls as this solely depends on the csproj file of the project. + private readonly ConcurrentDictionary> _warningPropertiesPerFrameworkCache = + new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); /// /// Creates a PackageSpecificWarningProperties for a project generated by traversing the dependency graph. @@ -26,13 +26,16 @@ internal class TransitiveNoWarnUtils /// Parent project restore target graphs. /// PackageSpec of the parent project. /// WarningPropertiesCollection with the project frameworks and the transitive package specific no warn properties. - internal static WarningPropertiesCollection CreateTransitiveWarningPropertiesCollection( + internal WarningPropertiesCollection CreateTransitiveWarningPropertiesCollection( IEnumerable targetGraphs, PackageSpec parentProjectSpec) { var transitivePackageSpecificProperties = new PackageSpecificWarningProperties(); var projectFrameworks = new List(); - var parentWarningProperties = GetNodeWarningProperties(parentProjectSpec); + var parentWarningProperties = new WarningPropertiesCollection( + parentProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, + PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(parentProjectSpec), + parentProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly()); foreach (var targetGraph in targetGraphs) { @@ -40,7 +43,7 @@ internal static WarningPropertiesCollection CreateTransitiveWarningPropertiesCol { var transitiveNoWarnFromTargetGraph = ExtractTransitiveNoWarnProperties( targetGraph, - parentProjectSpec.RestoreMetadata.ProjectName.ToLowerInvariant(), + parentProjectSpec.RestoreMetadata.ProjectName, parentWarningProperties); projectFrameworks.Add(targetGraph.Framework); @@ -65,22 +68,22 @@ internal static WarningPropertiesCollection CreateTransitiveWarningPropertiesCol /// File path of the parent project. /// WarningPropertiesCollection of the parent project. /// PackageSpecificWarningProperties containing all the NoWarn's for each package seen in the graph accumulated while traversing the graph. - private static PackageSpecificWarningProperties ExtractTransitiveNoWarnProperties( + private PackageSpecificWarningProperties ExtractTransitiveNoWarnProperties( RestoreTargetGraph targetGraph, string parentProjectName, WarningPropertiesCollection parentWarningPropertiesCollection) { - var dependencyMapping = new Dictionary(); + var dependencyMapping = new Dictionary(StringComparer.OrdinalIgnoreCase); var queue = new Queue(); var seen = new HashSet(); var frameworkReducer = new FrameworkReducer(); var resultWarningProperties = new PackageSpecificWarningProperties(); - var packageNoWarn = new Dictionary>(); + var packageNoWarn = new Dictionary>(StringComparer.OrdinalIgnoreCase); // All the packages in parent project's closure. // Once we have collected data for all of these, we can exit. var parentPackageDependencies = new HashSet( - targetGraph.Flattened.Where(d => d.Key.Type == LibraryType.Package).Select(d => d.Key.Name.ToLowerInvariant())); + targetGraph.Flattened.Where(d => d.Key.Type == LibraryType.Package).Select(d => d.Key.Name)); var parentTargetFramework = targetGraph.Framework; @@ -107,7 +110,7 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie WarningPropertiesCollection = nodeWarningProperties }; - dependencyMapping[dependencyGraphItem.Key.Name.ToLower()] = lookUpNode; + dependencyMapping[dependencyGraphItem.Key.Name] = lookUpNode; } // Get the direct dependencies for the parent project to seed the queue @@ -117,7 +120,7 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie foreach (var dependency in dependencyMapping[parentProjectName].Dependencies) { var queueNode = new DependencyNode( - dependency.Name.ToLowerInvariant(), + dependency.Name, IsProject(dependency.LibraryRange.TypeConstraint), parentWarningPropertiesCollection); @@ -129,7 +132,7 @@ private static PackageSpecificWarningProperties ExtractTransitiveNoWarnPropertie } // Add the parent project to the seen set to prevent adding it back to the queue - seen.Add(new DependencyNode(id: parentProjectName.ToLowerInvariant(), + seen.Add(new DependencyNode(id: parentProjectName, isProject: true, warningPropertiesCollection: parentWarningPropertiesCollection)); @@ -217,7 +220,7 @@ private static void AddDependenciesToQueue(IEnumerable depend foreach (var dependency in dependencies) { var queueNode = new DependencyNode( - dependency.Name.ToLowerInvariant(), + dependency.Name, IsProject(dependency.LibraryRange.TypeConstraint), pathWarningPropertiesCollection); @@ -231,7 +234,7 @@ private static void AddDependenciesToQueue(IEnumerable depend private static PackageSpec GetNodePackageSpec(LocalMatch localMatch) { - return localMatch.LocalLibrary.Items[KnownLibraryProperties.PackageSpec] as PackageSpec; + return (PackageSpec) localMatch.LocalLibrary.Items[KnownLibraryProperties.PackageSpec]; } private static ISet ExtractPathNoWarnProperties(WarningPropertiesCollection pathWarningProperties, string id) @@ -258,18 +261,17 @@ private static ISet ExtractPathNoWarnProperties(WarningPropertiesC return result; } - private static WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec) + private WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec, NuGetFramework framework) { - return _warningPropertiesCache.GetOrAdd(nodeProjectSpec.RestoreMetadata.ProjectPath, - (s) => new WarningPropertiesCollection( - nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, - PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec), - nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly())); - } + var key = nodeProjectSpec.RestoreMetadata.ProjectPath; - private static WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec, NuGetFramework framework) - { - return _warningPropertiesCache.GetOrAdd(nodeProjectSpec.RestoreMetadata.ProjectPath, + if (!_warningPropertiesPerFrameworkCache.ContainsKey(key)) + { + _warningPropertiesPerFrameworkCache[key] = + new ConcurrentDictionary(new NuGetFrameworkFullComparer()); + } + + return _warningPropertiesPerFrameworkCache[key].GetOrAdd(framework, (s) => new WarningPropertiesCollection( nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec, framework), @@ -488,8 +490,8 @@ public bool Equals(DependencyNode other) return true; } - return string.Equals(Id, other.Id, StringComparison.OrdinalIgnoreCase) && - IsProject == other.IsProject && + return IsProject == other.IsProject && + string.Equals(Id, other.Id, StringComparison.OrdinalIgnoreCase) && WarningPropertiesCollection.Equals(other.WarningPropertiesCollection); } From 2bb5484d5a1be8dfaabfc8e28b168a5703b5c8c0 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Mon, 21 Aug 2017 18:28:53 -0700 Subject: [PATCH 12/19] Adding unit tests for TransitiveNoWarnUtils --- .../Logging/TransitiveNoWarnUtils.cs | 44 +-- .../TransitiveNoWarnUtilsTests.cs | 305 ++++++++++++++++++ 2 files changed, 327 insertions(+), 22 deletions(-) create mode 100644 test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index 1ab533cc911..ebda258e8bb 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -14,7 +14,7 @@ namespace NuGet.Commands { - internal class TransitiveNoWarnUtils + public class TransitiveNoWarnUtils { // static should be fine across multiple calls as this solely depends on the csproj file of the project. private readonly ConcurrentDictionary> _warningPropertiesPerFrameworkCache = @@ -26,7 +26,7 @@ internal class TransitiveNoWarnUtils /// Parent project restore target graphs. /// PackageSpec of the parent project. /// WarningPropertiesCollection with the project frameworks and the transitive package specific no warn properties. - internal WarningPropertiesCollection CreateTransitiveWarningPropertiesCollection( + public WarningPropertiesCollection CreateTransitiveWarningPropertiesCollection( IEnumerable targetGraphs, PackageSpec parentProjectSpec) { @@ -211,6 +211,23 @@ private PackageSpecificWarningProperties ExtractTransitiveNoWarnProperties( return resultWarningProperties; } + private WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec, NuGetFramework framework) + { + var key = nodeProjectSpec.RestoreMetadata.ProjectPath; + + if (!_warningPropertiesPerFrameworkCache.ContainsKey(key)) + { + _warningPropertiesPerFrameworkCache[key] = + new ConcurrentDictionary(new NuGetFrameworkFullComparer()); + } + + return _warningPropertiesPerFrameworkCache[key].GetOrAdd(framework, + (s) => new WarningPropertiesCollection( + nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, + PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec, framework), + nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly())); + } + private static void AddDependenciesToQueue(IEnumerable dependencies, Queue queue, HashSet seen, @@ -237,7 +254,7 @@ private static PackageSpec GetNodePackageSpec(LocalMatch localMatch) return (PackageSpec) localMatch.LocalLibrary.Items[KnownLibraryProperties.PackageSpec]; } - private static ISet ExtractPathNoWarnProperties(WarningPropertiesCollection pathWarningProperties, string id) + public static ISet ExtractPathNoWarnProperties(WarningPropertiesCollection pathWarningProperties, string id) { var result = new HashSet(); if (pathWarningProperties?.ProjectWideWarningProperties?.NoWarn?.Count > 0) @@ -261,23 +278,6 @@ private static ISet ExtractPathNoWarnProperties(WarningPropertiesC return result; } - private WarningPropertiesCollection GetNodeWarningProperties(PackageSpec nodeProjectSpec, NuGetFramework framework) - { - var key = nodeProjectSpec.RestoreMetadata.ProjectPath; - - if (!_warningPropertiesPerFrameworkCache.ContainsKey(key)) - { - _warningPropertiesPerFrameworkCache[key] = - new ConcurrentDictionary(new NuGetFrameworkFullComparer()); - } - - return _warningPropertiesPerFrameworkCache[key].GetOrAdd(framework, - (s) => new WarningPropertiesCollection( - nodeProjectSpec.RestoreMetadata?.ProjectWideWarningProperties, - PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(nodeProjectSpec, framework), - nodeProjectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly())); - } - private static WarningPropertiesCollection MergeWarningPropertiesCollection( WarningPropertiesCollection first, WarningPropertiesCollection second) @@ -344,7 +344,7 @@ private static WarningProperties MergeProjectWideWarningProperties( return result; } - private static PackageSpecificWarningProperties MergePackageSpecificWarningProperties( + public static PackageSpecificWarningProperties MergePackageSpecificWarningProperties( PackageSpecificWarningProperties first, PackageSpecificWarningProperties second) { @@ -391,7 +391,7 @@ private static PackageSpecificWarningProperties MergePackageSpecificWarningPrope /// Out Merged Object. /// Returns true if atleast one of the objects was Null. /// If none of them is null then the returns false, indicating that the merge failed. - private static bool TryMergeNullObjects(object first, object second, out object merged) + public static bool TryMergeNullObjects(object first, object second, out object merged) { merged = null; var result = false; diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs new file mode 100644 index 00000000000..5cd9cd30053 --- /dev/null +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs @@ -0,0 +1,305 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Text; +using NuGet.Common; +using NuGet.Frameworks; +using NuGet.ProjectModel; +using Xunit; +using FluentAssertions; +using System.Diagnostics; + +namespace NuGet.Commands.Test +{ + public class TransitiveNoWarnUtilsTests + { + + // Tests for TransitiveNoWarnUtils.ExtractPathNoWarnProperties + [Fact] + public void ExtractPathNoWarnProperties_ReturnsEmptySetIfPathPropertiesAreNull() + { + // Arrange & Act + var extractedNoWarnSet = TransitiveNoWarnUtils.ExtractPathNoWarnProperties(null, "test_id"); + + // Assert + extractedNoWarnSet.Should().NotBeNull(); + extractedNoWarnSet.Should().BeEmpty(); + } + + [Fact] + public void ExtractPathNoWarnProperties_CorrectlyReadsProjectWideNoWarns() + { + // Arrange + var noWarnSet = new HashSet { NuGetLogCode.NU1601, NuGetLogCode.NU1603 }; + var warningsAsErrorSet = new HashSet { }; + + var projectWideWarningProperties = new WarningProperties( + warningsAsErrors: warningsAsErrorSet, + noWarn: noWarnSet, + allWarningsAsErrors: false + ); + + var warningPropertiesCollection = new WarningPropertiesCollection( + projectWideWarningProperties: projectWideWarningProperties, + packageSpecificWarningProperties: null, + projectFrameworks: null + ); + + // Act + var extractedNoWarnSet = TransitiveNoWarnUtils.ExtractPathNoWarnProperties(warningPropertiesCollection, "test_id"); + + // Assert + extractedNoWarnSet.Should().NotBeNullOrEmpty(); + extractedNoWarnSet.Should().BeEquivalentTo(noWarnSet); + } + + [Fact] + public void ExtractPathNoWarnProperties_CorrectlyReadsPackageSpecificNoWarns() + { + // Arrange + var packageId = "test_package"; + var framework = NuGetFramework.Parse("net461"); + var expectedNoWarnSet = new HashSet { NuGetLogCode.NU1603, NuGetLogCode.NU1605 }; + var noWarnSet = new HashSet { }; + var warningsAsErrorSet = new HashSet { }; + + var projectWideWarningProperties = new WarningProperties( + warningsAsErrors: warningsAsErrorSet, + noWarn: noWarnSet, + allWarningsAsErrors: false + ); + + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + packageSpecificWarningProperties.Add(NuGetLogCode.NU1603, packageId, framework); + packageSpecificWarningProperties.Add(NuGetLogCode.NU1605, packageId, framework); + packageSpecificWarningProperties.Add(NuGetLogCode.NU1701, "other_package", framework); + + var warningPropertiesCollection = new WarningPropertiesCollection( + projectWideWarningProperties: projectWideWarningProperties, + packageSpecificWarningProperties: packageSpecificWarningProperties, + projectFrameworks: null + ); + + // Act + var extractedNoWarnSet = TransitiveNoWarnUtils.ExtractPathNoWarnProperties(warningPropertiesCollection, packageId); + + // Assert + extractedNoWarnSet.Should().NotBeNullOrEmpty(); + extractedNoWarnSet.Should().BeEquivalentTo(expectedNoWarnSet); + } + + + [Fact] + public void ExtractPathNoWarnProperties_CorrectlyReadsPackageSpecificAndProjectWideNoWarns() + { + // Arrange + var packageId = "test_package"; + var framework = NuGetFramework.Parse("net461"); + var expectedNoWarnSet = new HashSet { NuGetLogCode.NU1601 , NuGetLogCode.NU1603, NuGetLogCode.NU1605 }; + var noWarnSet = new HashSet { NuGetLogCode.NU1601, NuGetLogCode.NU1605 }; + var warningsAsErrorSet = new HashSet { }; + + var projectWideWarningProperties = new WarningProperties( + warningsAsErrors: warningsAsErrorSet, + noWarn: noWarnSet, + allWarningsAsErrors: false + ); + + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + packageSpecificWarningProperties.Add(NuGetLogCode.NU1603, packageId, framework); + packageSpecificWarningProperties.Add(NuGetLogCode.NU1605, packageId, framework); + packageSpecificWarningProperties.Add(NuGetLogCode.NU1701, "other_package", framework); + + var warningPropertiesCollection = new WarningPropertiesCollection( + projectWideWarningProperties: projectWideWarningProperties, + packageSpecificWarningProperties: packageSpecificWarningProperties, + projectFrameworks: null + ); + + // Act + var extractedNoWarnSet = TransitiveNoWarnUtils.ExtractPathNoWarnProperties(warningPropertiesCollection, packageId); + + // Assert + extractedNoWarnSet.Should().NotBeNullOrEmpty(); + extractedNoWarnSet.Should().BeEquivalentTo(expectedNoWarnSet); + } + + + // Tests for TransitiveNoWarnUtils.TryMergeNullObjects + [Fact] + public void TryMergeNullObjects_ReturnsNullIfBothAreNull() + { + // Arrange + object mergedObject; + object first = null; + object second = null; + + // Act + var success = TransitiveNoWarnUtils.TryMergeNullObjects(first, second, out mergedObject); + + // Assert + success.Should().BeTrue(); + mergedObject.Should().BeNull(); + } + + [Fact] + public void TryMergeNullObjects_ReturnsFirstIfNotNull() + { + // Arrange + object mergedObject; + object first = new object(); + object second = null; + + // Act + var success = TransitiveNoWarnUtils.TryMergeNullObjects(first, second, out mergedObject); + + // Assert + success.Should().BeTrue(); + mergedObject.Should().Be(first); + } + + [Fact] + public void TryMergeNullObjects_ReturnsSecondIfNotNull() + { + // Arrange + object mergedObject; + object first = null; + object second = new object(); + + // Act + var success = TransitiveNoWarnUtils.TryMergeNullObjects(first, second, out mergedObject); + + // Assert + success.Should().BeTrue(); + mergedObject.Should().Be(second); + } + + [Fact] + public void TryMergeNullObjects_ReturnsFailureIfNoneNull() + { + // Arrange + object mergedObject; + object first = new object(); + object second = new object(); + + // Act + var success = TransitiveNoWarnUtils.TryMergeNullObjects(first, second, out mergedObject); + + // Assert + success.Should().BeFalse(); + mergedObject.Should().BeNull(); + } + + // Tests for TransitiveNoWarnUtils.MergePackageSpecificWarningProperties + [Fact] + public void MergePackageSpecificWarningProperties_ReturnsNullIfBothAreNull() + { + // Arrange + PackageSpecificWarningProperties first = null; + PackageSpecificWarningProperties second = null; + + // Act + var merged = TransitiveNoWarnUtils.MergePackageSpecificWarningProperties(first, second); + + // Assert + merged.Should().BeNull(); + } + + [Fact] + public void MergePackageSpecificWarningProperties_ReturnsFirstIfNotNull() + { + // Arrange + PackageSpecificWarningProperties first = new PackageSpecificWarningProperties(); + PackageSpecificWarningProperties second = null; + + // Act + var merged = TransitiveNoWarnUtils.MergePackageSpecificWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Should().Be(first); + } + + [Fact] + public void MergePackageSpecificWarningProperties_MergesEmptyCollections() + { + // Arrange + var first = new PackageSpecificWarningProperties(); + var second = new PackageSpecificWarningProperties(); + + // Act + var merged = TransitiveNoWarnUtils.MergePackageSpecificWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Properties.Should().BeNull(); + } + + [Fact] + public void MergePackageSpecificWarningProperties_MergesNonEmptyCollections() + { + // Arrange + var packageId1 = "test_id1"; + var packageId2 = "test_id2"; + var net461 = NuGetFramework.Parse("net461"); + var netcoreapp = NuGetFramework.Parse("netcoreapp2.0"); + var expectedResult = new PackageSpecificWarningProperties(); + expectedResult.AddRangeOfCodes( + new List { NuGetLogCode.NU1601, NuGetLogCode.NU1605 }, + packageId1, + net461); + expectedResult.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId2, + new List { net461, netcoreapp }); + expectedResult.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId1, + new List { net461, netcoreapp }); + expectedResult.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId2, + new List { net461, netcoreapp }); + expectedResult.AddRangeOfFrameworks( + NuGetLogCode.NU1604, + packageId1, + new List { net461, netcoreapp }); + + + var first = new PackageSpecificWarningProperties(); + first.AddRangeOfCodes( + new List { NuGetLogCode.NU1601, NuGetLogCode.NU1605 }, + packageId1, + net461); + first.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId2, + new List { net461, netcoreapp }); + first.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId1, + new List { net461, netcoreapp }); + + var second = new PackageSpecificWarningProperties(); + second.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId2, + new List { net461, netcoreapp }); + second.AddRangeOfFrameworks( + NuGetLogCode.NU1604, + packageId1, + new List { net461, netcoreapp }); + + + // Act + var merged = TransitiveNoWarnUtils.MergePackageSpecificWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Properties.Should().NotBeNull(); + merged.ShouldBeEquivalentTo(expectedResult); + } + } +} From bac6ebdb9bd0eabb0980b59ab3b3c3ef94a2a6cc Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Tue, 22 Aug 2017 11:51:06 -0700 Subject: [PATCH 13/19] Adding more unit tests --- .../Logging/TransitiveNoWarnUtils.cs | 35 ++++- .../NuGet.ProjectModel/WarningProperties.cs | 3 +- .../TransitiveNoWarnUtilsTests.cs | 130 +++++++++++++++++- 3 files changed, 157 insertions(+), 11 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index ebda258e8bb..245fdb1af7a 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -278,7 +278,16 @@ public static ISet ExtractPathNoWarnProperties(WarningPropertiesCo return result; } - private static WarningPropertiesCollection MergeWarningPropertiesCollection( + /// + /// Merge 2 WarningPropertiesCollection objects. + /// This method will combine the warning properties from both the collections. + /// + /// First Object to be merged. + /// Second Object to be merged. + /// Returns a WarningPropertiesCollection with the combined warning properties. + /// Returns the reference to one of the inputs if the other input is Null. + /// Returns a Null if both the input properties are Null. + public static WarningPropertiesCollection MergeWarningPropertiesCollection( WarningPropertiesCollection first, WarningPropertiesCollection second) { @@ -311,7 +320,16 @@ private static WarningPropertiesCollection MergeWarningPropertiesCollection( return result; } - private static WarningProperties MergeProjectWideWarningProperties( + /// + /// Merge 2 WarningProperties objects. + /// This method will combine the warning properties from both the collections. + /// + /// First Object to be merged. + /// Second Object to be merged. + /// Returns a WarningProperties with the combined warning properties. + /// Returns the reference to one of the inputs if the other input is Null. + /// Returns a Null if both the input properties are Null. + public static WarningProperties MergeProjectWideWarningProperties( WarningProperties first, WarningProperties second) { @@ -344,6 +362,15 @@ private static WarningProperties MergeProjectWideWarningProperties( return result; } + /// + /// Merge 2 PackageSpecificWarningProperties objects. + /// This method will combine the warning properties from both the collections. + /// + /// First Object to be merged. + /// Second Object to be merged. + /// Returns a PackageSpecificWarningProperties with the combined warning properties. + /// Will return the reference to one of the inputs if the other input is Null. + /// Returns a Null if both the input properties are Null. public static PackageSpecificWarningProperties MergePackageSpecificWarningProperties( PackageSpecificWarningProperties first, PackageSpecificWarningProperties second) @@ -387,7 +414,7 @@ public static PackageSpecificWarningProperties MergePackageSpecificWarningProper /// Try to merge 2 objects if one or both of them are null. /// /// First Object to be merged. - /// First Object to be merged. + /// Second Object to be merged. /// Out Merged Object. /// Returns true if atleast one of the objects was Null. /// If none of them is null then the returns false, indicating that the merge failed. @@ -502,7 +529,7 @@ public override string ToString() } /// - /// A simple node class to hold the outgoing dependency edge for a quick look up + /// A simple node class to hold the outgoing dependency edge for a quick look up. /// private class LookUpNode { diff --git a/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs b/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs index a5ab2b1c246..4fe56e87b5f 100644 --- a/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs +++ b/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -44,7 +44,6 @@ public override int GetHashCode() { var hashCode = new HashCodeCombiner(); - hashCode.AddObject(AllWarningsAsErrors); hashCode.AddSequence(WarningsAsErrors); hashCode.AddSequence(NoWarn); diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs index 5cd9cd30053..868a8c1b915 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs @@ -149,7 +149,7 @@ public void TryMergeNullObjects_ReturnsFirstIfNotNull() { // Arrange object mergedObject; - object first = new object(); + var first = new object(); object second = null; // Act @@ -166,7 +166,7 @@ public void TryMergeNullObjects_ReturnsSecondIfNotNull() // Arrange object mergedObject; object first = null; - object second = new object(); + var second = new object(); // Act var success = TransitiveNoWarnUtils.TryMergeNullObjects(first, second, out mergedObject); @@ -181,8 +181,8 @@ public void TryMergeNullObjects_ReturnsFailureIfNoneNull() { // Arrange object mergedObject; - object first = new object(); - object second = new object(); + var first = new object(); + var second = new object(); // Act var success = TransitiveNoWarnUtils.TryMergeNullObjects(first, second, out mergedObject); @@ -211,7 +211,7 @@ public void MergePackageSpecificWarningProperties_ReturnsNullIfBothAreNull() public void MergePackageSpecificWarningProperties_ReturnsFirstIfNotNull() { // Arrange - PackageSpecificWarningProperties first = new PackageSpecificWarningProperties(); + var first = new PackageSpecificWarningProperties(); PackageSpecificWarningProperties second = null; // Act @@ -222,6 +222,21 @@ public void MergePackageSpecificWarningProperties_ReturnsFirstIfNotNull() merged.Should().Be(first); } + [Fact] + public void MergePackageSpecificWarningProperties_ReturnsSecondIfNotNull() + { + // Arrange + PackageSpecificWarningProperties first = null; + var second = new PackageSpecificWarningProperties(); + + // Act + var merged = TransitiveNoWarnUtils.MergePackageSpecificWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Should().Be(second); + } + [Fact] public void MergePackageSpecificWarningProperties_MergesEmptyCollections() { @@ -301,5 +316,110 @@ public void MergePackageSpecificWarningProperties_MergesNonEmptyCollections() merged.Properties.Should().NotBeNull(); merged.ShouldBeEquivalentTo(expectedResult); } + + // Tests for TransitiveNoWarnUtils.MergeProjectWideWarningProperties + [Fact] + public void MergeProjectWideWarningProperties_ReturnsNullIfBothAreNull() + { + // Arrange + WarningProperties first = null; + WarningProperties second = null; + + // Act + var merged = TransitiveNoWarnUtils.MergeProjectWideWarningProperties(first, second); + + // Assert + merged.Should().BeNull(); + } + + [Fact] + public void MergeProjectWideWarningProperties_ReturnsFirstIfNotNull() + { + // Arrange + var first = new WarningProperties(); + WarningProperties second = null; + + // Act + var merged = TransitiveNoWarnUtils.MergeProjectWideWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Should().Be(first); + } + + [Fact] + public void MergeProjectWideWarningProperties_ReturnsSecondIfNotNull() + { + // Arrange + WarningProperties first = null; + var second = new WarningProperties(); + + // Act + var merged = TransitiveNoWarnUtils.MergeProjectWideWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Should().Be(second); + } + + [Fact] + public void MergeProjectWideWarningProperties_MergesEmptyCollections() + { + // Arrange + var first = new WarningProperties(); + var second = new WarningProperties(); + + // Act + var merged = TransitiveNoWarnUtils.MergeProjectWideWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.AllWarningsAsErrors.Should().BeFalse(); + merged.WarningsAsErrors.Should().BeEmpty(); + merged.NoWarn.Should().BeEmpty(); + } + + [Theory] + [InlineData("NU1603, NU1605", "NU1701", true, "", "", false, "NU1603, NU1605", "NU1701", true)] + [InlineData( "", "", false, "NU1603, NU1605", "NU1701", true, "NU1603, NU1605", "NU1701", true)] + [InlineData("NU1603, NU1701", "", false, "NU1603, NU1701", "", false, "NU1603, NU1701", "", false)] + [InlineData("", "NU1603, NU1701", false, "", "NU1603, NU1701", false, "", "NU1603, NU1701", false)] + [InlineData("NU1601", "NU1602, NU1603", false, "NU1604", "NU1605, NU1701", false, "NU1601, NU1604", "NU1602, NU1603, NU1605, NU1701", false)] + [InlineData("NU1601", "NU1602, NU1603", true, "NU1604", "NU1605, NU1701", false, "NU1601, NU1604", "NU1602, NU1603, NU1605, NU1701", true)] + [InlineData("", "", false, "", "", false, "", "", false)] + [InlineData("", "", true, "", "", false, "", "", true)] + [InlineData("", "", false, "", "", true, "", "", true)] + [InlineData("", "", true, "", "", true, "", "", true)] + public void MergeProjectWideWarningProperties_MergesNonEmptyCollections( + string firstNoWarn, string firstWarningsAsErrors, bool firstAllWarningsAsErrors, + string secondNoWarn, string secondWarningsAsErrors, bool secondAllWarningsAsErrors, + string expectedNoWarn, string expectedWarningsAsErrors, bool expectedAllWarningsAsErrors) + { + // Arrange + var first = new WarningProperties( + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(firstWarningsAsErrors)), + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(firstNoWarn)), + firstAllWarningsAsErrors); + + var second = new WarningProperties( + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(secondWarningsAsErrors)), + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(secondNoWarn)), + secondAllWarningsAsErrors); + + var expected = new WarningProperties( + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(expectedWarningsAsErrors)), + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(expectedNoWarn)), + expectedAllWarningsAsErrors); + + // Act + var merged = TransitiveNoWarnUtils.MergeProjectWideWarningProperties(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.AllWarningsAsErrors.ShouldBeEquivalentTo(expected.AllWarningsAsErrors); + merged.WarningsAsErrors.ShouldBeEquivalentTo(expected.WarningsAsErrors); + merged.NoWarn.ShouldBeEquivalentTo(expected.NoWarn); + merged.ShouldBeEquivalentTo(expected); + } } } From 206783dc05504d78b5f4a155c3de7b24d4039493 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Tue, 22 Aug 2017 18:47:15 -0700 Subject: [PATCH 14/19] Adding unit tests related to Merging warning properties and dependency equality --- build/Shared/EqualityUtility.cs | 27 +- .../PackageSpecificWarningProperties.cs | 6 +- .../Logging/RestoreCollectorLogger.cs | 15 +- .../Logging/TransitiveNoWarnUtils.cs | 25 +- .../Logging/WarningPropertiesCollection.cs | 13 +- .../NuGet.ProjectModel/WarningProperties.cs | 4 +- .../TransitiveNoWarnUtilsTests.cs | 631 +++++++++++++++++- 7 files changed, 683 insertions(+), 38 deletions(-) diff --git a/build/Shared/EqualityUtility.cs b/build/Shared/EqualityUtility.cs index 11c57acaaa2..91fbbd114b0 100644 --- a/build/Shared/EqualityUtility.cs +++ b/build/Shared/EqualityUtility.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -59,6 +59,31 @@ internal static bool SequenceEqualWithNullCheck( return self.SequenceEqual(other, comparer); } + /// + /// Compares two sets for equality, allowing either sequence to be null. + /// If one is null, both have to be null for equality. + /// + internal static bool SetEqualWithNullCheck( + this ISet self, + ISet other, + IEqualityComparer comparer = null) + { + bool identityEquals; + if (TryIdentityEquals(self, other, out identityEquals)) + { + return identityEquals; + } + + if (comparer == null) + { + comparer = EqualityComparer.Default; + } + + ISet set = new HashSet(self, comparer); + + return set.SetEquals(other); + } + internal static bool DictionaryEquals( IDictionary self, IDictionary other, diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs index 5059b31587b..e8959af8ad4 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs @@ -180,9 +180,9 @@ public bool Equals(PackageSpecificWarningProperties other) return true; } - return EqualityUtility.OrderedEquals(Properties.Keys, other.Properties.Keys, (code) => code) && - Properties.Keys.All(c => EqualityUtility.OrderedEquals(Properties[c].Keys, other.Properties[c].Keys, (id) => id)) && - Properties.Keys.All(c => Properties[c].Keys.All(id => EqualityUtility.OrderedEquals(Properties[c][id], other.Properties[c][id], (fx) => fx))); + return EqualityUtility.SequenceEqualWithNullCheck(Properties.Keys.OrderBy(c => c), other.Properties.Keys.OrderBy(c => c)) && + Properties.Keys.All(c => EqualityUtility.OrderedEquals(Properties[c].Keys, other.Properties[c].Keys, (id) => id, StringComparer.OrdinalIgnoreCase, StringComparer.OrdinalIgnoreCase)) && + Properties.Keys.All(c => Properties[c].Keys.All(id => EqualityUtility.SetEqualWithNullCheck(Properties[c][id], other.Properties[c][id], new NuGetFrameworkFullComparer()))); } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index 31ed3dfc2b4..1c3cdd5f9dc 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -157,10 +157,17 @@ protected bool DisplayMessage(IRestoreLogMessage message) /// bool indicating if the message should be suppressed. private bool IsWarningSuppressed(IRestoreLogMessage message) { - TryPopulateTransitiveWarningPropertiesCollection(message); - - return (ProjectWarningPropertiesCollection != null && ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) || - (TransitiveWarningPropertiesCollection != null && TransitiveWarningPropertiesCollection.ApplyWarningProperties(message)); + + if (ProjectWarningPropertiesCollection != null && ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) + { + return true; + } + else + { + // Initialize transitive warning properties only if the project does not suppress the warning. + TryPopulateTransitiveWarningPropertiesCollection(message); + return TransitiveWarningPropertiesCollection != null && TransitiveWarningPropertiesCollection.ApplyWarningProperties(message); + } } private void TryPopulateTransitiveWarningPropertiesCollection(ILogMessage message) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index 245fdb1af7a..14b94409ad8 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -117,19 +117,10 @@ private PackageSpecificWarningProperties ExtractTransitiveNoWarnProperties( var parentDependencies = dependencyMapping[parentProjectName]; // Seed the queue with the parent project's direct dependencies - foreach (var dependency in dependencyMapping[parentProjectName].Dependencies) - { - var queueNode = new DependencyNode( - dependency.Name, - IsProject(dependency.LibraryRange.TypeConstraint), - parentWarningPropertiesCollection); - - if (!seen.Contains(queueNode)) - { - // Add the metadata from the parent project here. - queue.Enqueue(queueNode); - } - } + AddDependenciesToQueue(parentDependencies.Dependencies, + queue, + seen, + parentWarningPropertiesCollection); // Add the parent project to the seen set to prevent adding it back to the queue seen.Add(new DependencyNode(id: parentProjectName, @@ -465,7 +456,7 @@ private static bool IsProject(LibraryType type) /// /// A simple node class to hold the outgoing dependency edge during the graph walk. /// - private class DependencyNode : IEquatable + public class DependencyNode : IEquatable { // ID of the Node public string Id { get; } @@ -484,12 +475,6 @@ public DependencyNode(string id, bool isProject, WarningPropertiesCollection war IsProject = isProject; } - public DependencyNode(string id, bool isProject) - { - Id = id ?? throw new ArgumentNullException(nameof(id)); - IsProject = isProject; - } - public override int GetHashCode() { var hashCode = new HashCodeCombiner(); diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs index a7ca1779c68..56997f15c7b 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs @@ -24,7 +24,7 @@ public class WarningPropertiesCollection : IEquatable - public IReadOnlyList ProjectFrameworks { get; } = new ReadOnlyCollection(new List()); + public IReadOnlyList ProjectFrameworks { get; } /// /// Contains Project wide properties for Warnings. @@ -43,7 +43,7 @@ public WarningPropertiesCollection(WarningProperties projectWideWarningPropertie { ProjectWideWarningProperties = projectWideWarningProperties; PackageSpecificWarningProperties = packageSpecificWarningProperties; - ProjectFrameworks = projectFrameworks; + ProjectFrameworks = projectFrameworks ?? new ReadOnlyCollection(new List()); } /// @@ -70,8 +70,7 @@ public bool ApplyWarningProperties(IRestoreLogMessage message) if (messageTargetFrameworks.Count == 0) { // Suppress the warning if the code + libraryId combination is suppressed for all project frameworks. - if (ProjectFrameworks != null && - ProjectFrameworks.Count > 0 && + if (ProjectFrameworks.Count > 0 && ProjectFrameworks.All(e => PackageSpecificWarningProperties.Contains(message.Code, message.LibraryId, e))) { return true; @@ -167,9 +166,9 @@ public bool Equals(WarningPropertiesCollection other) return true; } - return ProjectWideWarningProperties.Equals(other.ProjectWideWarningProperties) && - PackageSpecificWarningProperties.Equals(other.PackageSpecificWarningProperties) && - EqualityUtility.OrderedEquals(ProjectFrameworks, other.ProjectFrameworks, (fx) => fx, sequenceComparer: new NuGetFrameworkFullComparer()); + return EqualityUtility.EqualsWithNullCheck(ProjectWideWarningProperties, other.ProjectWideWarningProperties) && + EqualityUtility.EqualsWithNullCheck(PackageSpecificWarningProperties, other.PackageSpecificWarningProperties) && + EqualityUtility.OrderedEquals(ProjectFrameworks, other.ProjectFrameworks, (fx) => fx.Framework, orderComparer: StringComparer.OrdinalIgnoreCase, sequenceComparer: new NuGetFrameworkFullComparer()); } } } diff --git a/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs b/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs index 4fe56e87b5f..e24f251e242 100644 --- a/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs +++ b/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs @@ -68,8 +68,8 @@ public bool Equals(WarningProperties other) } return AllWarningsAsErrors == other.AllWarningsAsErrors && - WarningsAsErrors.SetEquals(other.WarningsAsErrors) && - NoWarn.SetEquals(other.NoWarn); + EqualityUtility.SetEqualWithNullCheck(WarningsAsErrors, other.WarningsAsErrors) && + EqualityUtility.SetEqualWithNullCheck(NoWarn, other.NoWarn); } } } diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs index 868a8c1b915..2aa8fddc99d 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs @@ -295,7 +295,7 @@ public void MergePackageSpecificWarningProperties_MergesNonEmptyCollections() first.AddRangeOfFrameworks( NuGetLogCode.NU1701, packageId1, - new List { net461, netcoreapp }); + new List { netcoreapp }); var second = new PackageSpecificWarningProperties(); second.AddRangeOfFrameworks( @@ -306,6 +306,10 @@ public void MergePackageSpecificWarningProperties_MergesNonEmptyCollections() NuGetLogCode.NU1604, packageId1, new List { net461, netcoreapp }); + second.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId1, + new List { net461 }); // Act @@ -421,5 +425,630 @@ public void MergeProjectWideWarningProperties_MergesNonEmptyCollections( merged.NoWarn.ShouldBeEquivalentTo(expected.NoWarn); merged.ShouldBeEquivalentTo(expected); } + + // Tests for TransitiveNoWarnUtils.MergeWarningPropertiesCollection + [Fact] + public void MergeWarningPropertiesCollection_ReturnsNullIfBothAreNull() + { + // Arrange + WarningPropertiesCollection first = null; + WarningPropertiesCollection second = null; + + // Act + var merged = TransitiveNoWarnUtils.MergeWarningPropertiesCollection(first, second); + + // Assert + merged.Should().BeNull(); + } + + [Fact] + public void MergeWarningPropertiesCollection_ReturnsFirstIfNotNull() + { + // Arrange + var first = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: null + ); + WarningPropertiesCollection second = null; + + // Act + var merged = TransitiveNoWarnUtils.MergeWarningPropertiesCollection(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Should().Be(first); + } + + [Fact] + public void MergeWarningPropertiesCollection_ReturnsSecondIfNotNull() + { + // Arrange + WarningPropertiesCollection first = null; + var second = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: null + ); + + // Act + var merged = TransitiveNoWarnUtils.MergeWarningPropertiesCollection(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.Should().Be(second); + } + + [Fact] + public void MergeWarningPropertiesCollection_MergesEmptyCollections() + { + // Arrange + var first = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: null + ); + var second = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: null + ); + + // Act + var merged = TransitiveNoWarnUtils.MergeWarningPropertiesCollection(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.ProjectWideWarningProperties.Should().BeNull(); + merged.PackageSpecificWarningProperties.Should().BeNull(); + merged.ProjectFrameworks.Should().BeEmpty(); + } + + [Fact] + public void MergeWarningPropertiesCollection_MergesNonEmptyCollections1() + { + // Arrange + var first = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: new List { NuGetFramework.Parse("net461"), NuGetFramework.Parse("netcoreapp1.0") }.AsReadOnly() + ); + var second = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: new List { NuGetFramework.Parse("net45"), NuGetFramework.Parse("netcoreapp1.1") }.AsReadOnly() + ); + + // Act + var merged = TransitiveNoWarnUtils.MergeWarningPropertiesCollection(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.ProjectWideWarningProperties.Should().BeNull(); + merged.PackageSpecificWarningProperties.Should().BeNull(); + merged.ProjectFrameworks.Should().BeEmpty(); + } + + [Fact] + public void MergeWarningPropertiesCollection_MergesNonEmptyCollections2() + { + // Arrange + var first = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: new List { NuGetFramework.Parse("net461"), NuGetFramework.Parse("netcoreapp1.0") }.AsReadOnly() + ); + var second = new WarningPropertiesCollection( + projectWideWarningProperties: null, + packageSpecificWarningProperties: null, + projectFrameworks: new List { NuGetFramework.Parse("net45"), NuGetFramework.Parse("netcoreapp1.1") }.AsReadOnly() + ); + + // Act + var merged = TransitiveNoWarnUtils.MergeWarningPropertiesCollection(first, second); + + // Assert + merged.Should().NotBeNull(); + merged.ProjectWideWarningProperties.Should().BeNull(); + merged.PackageSpecificWarningProperties.Should().BeNull(); + merged.ProjectFrameworks.Should().BeEmpty(); + } + + // Tests for TransitiveNoWarnUtils.DependencyNode equality + [Fact] + public void DependencyNodeEqualitySucceeds_NodesAreNull() + { + // Arrange + TransitiveNoWarnUtils.DependencyNode first = null; + TransitiveNoWarnUtils.DependencyNode second = null; + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(1); + } + + [Fact] + public void DependencyNodeEqualitySucceeds_OneNodeIsNull() + { + // Arrange + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection(null, null, null)); + + TransitiveNoWarnUtils.DependencyNode second = null; + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(2); + } + + [Fact] + public void DependencyNodeEqualitySucceeds_NodesAreSame() + { + // Arrange + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection(null, null, null)); + + // Act + var seen = new HashSet + { + first + }; + + // Assert + seen.Count.Should().Be(1); + } + + [Fact] + public void DependencyNodeEqualitySucceeds_NodesHaveSameInternalObjects() + { + // Arrange + var projectWideWarningProperties = new WarningProperties(); + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + var projectFrameworks = new List(); + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + packageSpecificWarningProperties, + projectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + packageSpecificWarningProperties, + projectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(1); + } + + [Theory] + [InlineData("NU1605, NU1701", "NU1602, NU1603", false, "NU1701, NU1605", "NU1603, NU1602", false)] + [InlineData("NU1605, NU1701", "NU1602, NU1603", true, "NU1701, NU1605", "NU1603, NU1602", true)] + [InlineData("", "", false, "", "", false)] + [InlineData("", "", true, "", "", true)] + public void DependencyNodeEqualitySucceeds_NodesHaveEquivalentWarningProperties( + string firstNoWarn, string firstWarningsAsErrors, bool firstAllWarningsAsErrors, + string secondNoWarn, string secondWarningsAsErrors, bool secondAllWarningsAsErrors) + { + // Arrange + var packageId1 = "test_id1"; + var packageId2 = "test_id2"; + var net461 = NuGetFramework.Parse("net461"); + var netcoreapp = NuGetFramework.Parse("netcoreapp2.0"); + + // First + var firstProjectWideWarningProperties = new WarningProperties( + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(firstWarningsAsErrors)), + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(firstNoWarn)), + firstAllWarningsAsErrors); + + var firstPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + firstPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601, NuGetLogCode.NU1605 }, + packageId1, + net461); + firstPackageSpecificWarningProperties.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId2, + new List { net461, netcoreapp }); + firstPackageSpecificWarningProperties.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId1, + new List { net461 }); + + var firstProjectFrameworks = new List { net461, netcoreapp }; + + // Second + var secondProjectWideWarningProperties = new WarningProperties( + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(secondWarningsAsErrors)), + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(secondNoWarn)), + secondAllWarningsAsErrors); + + var secondPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + secondPackageSpecificWarningProperties.AddRangeOfFrameworks( + NuGetLogCode.NU1701, + packageId1, + new List { net461 }); + secondPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1701, NuGetLogCode.NU1605, NuGetLogCode.NU1601 }, + packageId1, + net461); + + var secondProjectFrameworks = new List { netcoreapp, net461 }; + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + firstProjectWideWarningProperties, + firstPackageSpecificWarningProperties, + firstProjectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + firstProjectWideWarningProperties, + firstPackageSpecificWarningProperties, + firstProjectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(1); + } + + [Fact] + public void DependencyNodeEqualityFails_NodesHaveDifferentMetaData() + { + // Arrange + var projectWideWarningProperties = new WarningProperties(); + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + var projectFrameworks = new List(); + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + packageSpecificWarningProperties, + projectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: false, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + packageSpecificWarningProperties, + projectFrameworks + )); + + var third = new TransitiveNoWarnUtils.DependencyNode( + id: "test_other", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + packageSpecificWarningProperties, + projectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second, + third + }; + + // Assert + seen.Count.Should().Be(3); + } + + [Theory] + [InlineData("NU1605, NU1701", "", false, "", "", false)] + [InlineData("NU1605, NU1701", "", false, "NU1701, NU1603", "", false)] + [InlineData("", "NU1602, NU1603", true, "", "", true)] + [InlineData("", "NU1602, NU1603", true, "", "NU1605, NU1602", true)] + [InlineData("NU1605, NU1701", "NU1602, NU1603", true, "NU1701, NU1605", "NU1603, NU1602", false)] + [InlineData("NU1605, NU1701", "NU1602, NU1603", false, "NU1701, NU1605", "NU1603, NU1602", true)] + [InlineData("", "", true, "", "", false)] + [InlineData("", "", false, "", "", true)] + public void DependencyNodeEqualityFails_NodesHaveDifferentProjectWideWarningProperties( + string firstNoWarn, string firstWarningsAsErrors, bool firstAllWarningsAsErrors, + string secondNoWarn, string secondWarningsAsErrors, bool secondAllWarningsAsErrors) + { + // Arrange + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + var projectFrameworks = new List(); + + var firstProjectWideWarningProperties = new WarningProperties( + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(firstWarningsAsErrors)), + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(firstNoWarn)), + firstAllWarningsAsErrors); + + var secondProjectWideWarningProperties = new WarningProperties( + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(secondWarningsAsErrors)), + new HashSet(MSBuildRestoreUtility.GetNuGetLogCodes(secondNoWarn)), + secondAllWarningsAsErrors); + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + firstProjectWideWarningProperties, + packageSpecificWarningProperties, + projectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + secondProjectWideWarningProperties, + packageSpecificWarningProperties, + projectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(2); + } + + [Fact] + public void DependencyNodeEqualityFails_NodesHaveDifferentFrameworksInPackageSpecificNoWarn() + { + // Arrange + var packageId1 = "test_id1"; + var net461 = NuGetFramework.Parse("net461"); + var netcoreapp = NuGetFramework.Parse("netcoreapp2.0"); + + var projectWideWarningProperties = new WarningProperties(); + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + var projectFrameworks = new List { net461, netcoreapp }; + + var firstPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + firstPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601 }, + packageId1, + net461); + + var secondPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + secondPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601 }, + packageId1, + netcoreapp); + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + firstPackageSpecificWarningProperties, + projectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + secondPackageSpecificWarningProperties, + projectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(2); + } + + [Fact] + public void DependencyNodeEqualityFails_NodesHaveDifferentNoWarnCodesInPackageSpecificNoWarn() + { + // Arrange + var packageId1 = "test_id1"; + var net461 = NuGetFramework.Parse("net461"); + + var projectWideWarningProperties = new WarningProperties(); + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + var projectFrameworks = new List { net461 }; + + var firstPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + firstPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601 }, + packageId1, + net461); + + var secondPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + secondPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1602 }, + packageId1, + net461); + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + firstPackageSpecificWarningProperties, + projectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + secondPackageSpecificWarningProperties, + projectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(2); + } + + [Fact] + public void DependencyNodeEqualityFails_NodesHaveDifferentPackageIdsInPackageSpecificNoWarn() + { + // Arrange + var packageId1 = "test_id1"; + var packageId2 = "test_id2"; + var net461 = NuGetFramework.Parse("net461"); + + var projectWideWarningProperties = new WarningProperties(); + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + var projectFrameworks = new List { net461 }; + + var firstPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + firstPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601 }, + packageId1, + net461); + + var secondPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + secondPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601 }, + packageId2, + net461); + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + firstPackageSpecificWarningProperties, + projectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + secondPackageSpecificWarningProperties, + projectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(2); + } + + [Fact] + public void DependencyNodeEqualityFails_NodesHaveDifferentProjectFrameworks() + { + // Arrange + var packageId1 = "test_id1"; + var net461 = NuGetFramework.Parse("net461"); + var net45 = NuGetFramework.Parse("net45"); + var netcoreapp = NuGetFramework.Parse("netcoreapp1.0"); + + var projectWideWarningProperties = new WarningProperties(); + var packageSpecificWarningProperties = new PackageSpecificWarningProperties(); + var firstProjectFrameworks = new List { net461, netcoreapp }; + var secondProjectFrameworks = new List { netcoreapp, net45, net461 }; + + var firstPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + firstPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601 }, + packageId1, + net461); + + var secondPackageSpecificWarningProperties = new PackageSpecificWarningProperties(); + secondPackageSpecificWarningProperties.AddRangeOfCodes( + new List { NuGetLogCode.NU1601 }, + packageId1, + net461); + + var first = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + firstPackageSpecificWarningProperties, + firstProjectFrameworks + )); + + var second = new TransitiveNoWarnUtils.DependencyNode( + id: "test", + isProject: true, + warningPropertiesCollection: new WarningPropertiesCollection( + projectWideWarningProperties, + secondPackageSpecificWarningProperties, + secondProjectFrameworks + )); + + // Act + var seen = new HashSet + { + first, + second + }; + + // Assert + seen.Count.Should().Be(2); + } } } From 2aca4d502a7bd58f842041fc705456d096e92616 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Wed, 23 Aug 2017 20:06:02 -0700 Subject: [PATCH 15/19] Improving perf by fixing seen set handling --- .../Logging/PackageSpecificWarningProperties.cs | 7 ++++--- .../RestoreCommand/Logging/TransitiveNoWarnUtils.cs | 7 +++---- ...esTests.cs => PackageSpecificWarningPropertiesTests.cs} | 0 3 files changed, 7 insertions(+), 7 deletions(-) rename test/NuGet.Core.Tests/NuGet.Common.Test/{PackageSpecificWanringPropertiesTests.cs => PackageSpecificWarningPropertiesTests.cs} (100%) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs index e8959af8ad4..5d0ee79dce1 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/PackageSpecificWarningProperties.cs @@ -180,9 +180,10 @@ public bool Equals(PackageSpecificWarningProperties other) return true; } - return EqualityUtility.SequenceEqualWithNullCheck(Properties.Keys.OrderBy(c => c), other.Properties.Keys.OrderBy(c => c)) && - Properties.Keys.All(c => EqualityUtility.OrderedEquals(Properties[c].Keys, other.Properties[c].Keys, (id) => id, StringComparer.OrdinalIgnoreCase, StringComparer.OrdinalIgnoreCase)) && - Properties.Keys.All(c => Properties[c].Keys.All(id => EqualityUtility.SetEqualWithNullCheck(Properties[c][id], other.Properties[c][id], new NuGetFrameworkFullComparer()))); + return EqualityUtility.DictionaryEquals( + Properties, + other.Properties, + (sv1, ov1) => EqualityUtility.DictionaryEquals(sv1, ov1, (sv2, ov2) => EqualityUtility.SetEqualWithNullCheck(sv2, ov2))); } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index 14b94409ad8..95bea6bdd00 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -133,7 +133,9 @@ private PackageSpecificWarningProperties ExtractTransitiveNoWarnProperties( var node = queue.Dequeue(); if (!seen.Contains(node)) { - + // Add the node to seen set + seen.Add(node); + var nodeId = node.Id; var nodeIsProject = node.IsProject; var nodeDependencies = dependencyMapping[nodeId].Dependencies; @@ -232,11 +234,8 @@ private static void AddDependenciesToQueue(IEnumerable depend IsProject(dependency.LibraryRange.TypeConstraint), pathWarningPropertiesCollection); - if (!seen.Contains(queueNode)) - { // Add the metadata from the parent project here. queue.Enqueue(queueNode); - } } } diff --git a/test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWanringPropertiesTests.cs b/test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWarningPropertiesTests.cs similarity index 100% rename from test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWanringPropertiesTests.cs rename to test/NuGet.Core.Tests/NuGet.Common.Test/PackageSpecificWarningPropertiesTests.cs From 288abdad88f5f9de1a734482e9f789e86585f796 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Thu, 24 Aug 2017 14:48:49 -0700 Subject: [PATCH 16/19] Adding func tests with dense solution --- .../RestoreTransitiveLoggingTests.cs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs index 49783455edb..fa56c14c1e5 100644 --- a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreTransitiveLoggingTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; using FluentAssertions; @@ -1176,6 +1177,86 @@ public void GivenOneLongPathWarnsVerifyWarning() } } + + // Densely connected solutions containing 5, 10, 20 and 50 projects + + [Theory] + [InlineData(5)] + [InlineData(10)] + [InlineData(20)] + [InlineData(50)] + public void GivenDenseSolutionWithMultiplePathsVerifyNoWarn(int projectCount) + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projects = new List(); + + var framework = NuGetFramework.Parse("net461"); + + // Referenced but not created + var packageXWithNoWarn = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0", + NoWarn = "NU1603" + }; + + // Created in the source + var packageX11 = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.1" + }; + + SimpleTestPackageUtility.CreatePackages(pathContext.PackageSource, packageX11); + + for (var i = 0; i < projectCount; i++) + { + var project = SimpleTestProjectContext.CreateNETCore( + "project_" + i, + pathContext.SolutionRoot, + framework); + + projects.Add(project); + } + + for (var i = 1; i < projects.Count(); i++) + { + var project = projects[i]; + project.AddPackageToAllFrameworks(packageXWithNoWarn); + } + + for (var i = 0; i < projects.Count() - 1; i++) + { + var projectA = projects[i]; + for (var j = i+1; j < projects.Count(); j++) + { + var projectB = projects[j]; + projectA.AddProjectToAllFrameworks(projectB); + } + } + + foreach (var project in projects) + { + project.Save(); + solution.Projects.Add(project); + } + + solution.Create(pathContext.SolutionRoot); + + // Act + var r = Util.RestoreSolution(pathContext, expectedExitCode: 0); + + // Assert + r.Success.Should().BeTrue(); + r.AllOutput.Should().NotContain("NU1603"); + } + } + private static int GetSubstringCount(string str, string substr, bool ignoreCase) { var splitChars = new char[] { '.', '?', '!', ' ', ';', ':', ',', '\r', '\n' }; From b2610a1891c729dd531009c4ecf3b9414bc3f32a Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Thu, 24 Aug 2017 17:41:33 -0700 Subject: [PATCH 17/19] removing warnAsError from merge --- .../RestoreCommand/Logging/TransitiveNoWarnUtils.cs | 6 +++--- src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs index 95bea6bdd00..c83a7bef296 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/TransitiveNoWarnUtils.cs @@ -332,9 +332,8 @@ public static WarningProperties MergeProjectWideWarningProperties( else { // Merge WarningsAsErrors Sets. + // No need to merge warnings as errors as they are not used transitively. var mergedWarningsAsErrors = new HashSet(); - mergedWarningsAsErrors.UnionWith(first.WarningsAsErrors); - mergedWarningsAsErrors.UnionWith(second.WarningsAsErrors); // Merge NoWarn Sets. var mergedNoWarn = new HashSet(); @@ -344,7 +343,8 @@ public static WarningProperties MergeProjectWideWarningProperties( // Merge AllWarningsAsErrors. If one project treats all warnigs as errors then the chain will too. var mergedAllWarningsAsErrors = first.AllWarningsAsErrors || second.AllWarningsAsErrors; - result = new WarningProperties(mergedWarningsAsErrors, + result = new WarningProperties( + mergedWarningsAsErrors, mergedNoWarn, mergedAllWarningsAsErrors); } diff --git a/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs b/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs index e24f251e242..637581435c7 100644 --- a/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs +++ b/src/NuGet.Core/NuGet.ProjectModel/WarningProperties.cs @@ -16,20 +16,23 @@ public class WarningProperties : IEquatable /// /// List of Warning Codes that should be treated as Errors. /// - public ISet WarningsAsErrors { get; } = new HashSet(); + public ISet WarningsAsErrors { get; } /// /// List of Warning Codes that should be ignored. /// - public ISet NoWarn { get; } = new HashSet(); + public ISet NoWarn { get; } /// /// Indicates if all warnings should be ignored. /// - public bool AllWarningsAsErrors { get; set; } = false; + public bool AllWarningsAsErrors { get; set; } public WarningProperties() { + WarningsAsErrors = new HashSet(); + NoWarn = new HashSet(); + AllWarningsAsErrors = false; } public WarningProperties(ISet warningsAsErrors, ISet noWarn, bool allWarningsAsErrors) From 6f725f73dcef7ceb689afe3a4a3525e4a70c9dd6 Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Fri, 25 Aug 2017 10:55:13 -0700 Subject: [PATCH 18/19] fixing tests to match the perf changes --- .../TransitiveNoWarnUtilsTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs index 2aa8fddc99d..ed06ee56d6d 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/TransitiveNoWarnUtilsTests.cs @@ -384,12 +384,12 @@ public void MergeProjectWideWarningProperties_MergesEmptyCollections() } [Theory] - [InlineData("NU1603, NU1605", "NU1701", true, "", "", false, "NU1603, NU1605", "NU1701", true)] - [InlineData( "", "", false, "NU1603, NU1605", "NU1701", true, "NU1603, NU1605", "NU1701", true)] + [InlineData("NU1603, NU1605", "NU1701", true, "", "", false, "NU1603, NU1605", "", true)] + [InlineData( "", "", false, "NU1603, NU1605", "NU1701", true, "NU1603, NU1605", "", true)] [InlineData("NU1603, NU1701", "", false, "NU1603, NU1701", "", false, "NU1603, NU1701", "", false)] - [InlineData("", "NU1603, NU1701", false, "", "NU1603, NU1701", false, "", "NU1603, NU1701", false)] - [InlineData("NU1601", "NU1602, NU1603", false, "NU1604", "NU1605, NU1701", false, "NU1601, NU1604", "NU1602, NU1603, NU1605, NU1701", false)] - [InlineData("NU1601", "NU1602, NU1603", true, "NU1604", "NU1605, NU1701", false, "NU1601, NU1604", "NU1602, NU1603, NU1605, NU1701", true)] + [InlineData("", "NU1603, NU1701", false, "", "NU1603, NU1701", false, "", "", false)] + [InlineData("NU1601", "NU1602, NU1603", false, "NU1604", "NU1605, NU1701", false, "NU1601, NU1604", "", false)] + [InlineData("NU1601", "NU1602, NU1603", true, "NU1604", "NU1605, NU1701", false, "NU1601, NU1604", "", true)] [InlineData("", "", false, "", "", false, "", "", false)] [InlineData("", "", true, "", "", false, "", "", true)] [InlineData("", "", false, "", "", true, "", "", true)] @@ -421,7 +421,7 @@ public void MergeProjectWideWarningProperties_MergesNonEmptyCollections( // Assert merged.Should().NotBeNull(); merged.AllWarningsAsErrors.ShouldBeEquivalentTo(expected.AllWarningsAsErrors); - merged.WarningsAsErrors.ShouldBeEquivalentTo(expected.WarningsAsErrors); + merged.WarningsAsErrors.Should().BeEmpty(); merged.NoWarn.ShouldBeEquivalentTo(expected.NoWarn); merged.ShouldBeEquivalentTo(expected); } From 1697f19a5af6910947a2d1565de91a335d1c4e2d Mon Sep 17 00:00:00 2001 From: Ankit Mishra Date: Fri, 25 Aug 2017 13:27:04 -0700 Subject: [PATCH 19/19] Reducung visible propertites in restore collector logger --- .../Logging/RestoreCollectorLogger.cs | 82 +++++++++++++------ .../RestoreCommand/ProjectRestoreCommand.cs | 4 +- .../RestoreCommand/RestoreCommand.cs | 15 +--- .../CollectorLoggerTests.cs | 2 +- 4 files changed, 62 insertions(+), 41 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs index 1c3cdd5f9dc..3c0fa78e4ce 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs @@ -17,18 +17,68 @@ public class RestoreCollectorLogger : LoggerBase, ICollectorLogger private readonly ILogger _innerLogger; private readonly ConcurrentQueue _errors; private readonly bool _hideWarningsAndErrors; + private IEnumerable _restoreTargetGraphs; + private PackageSpec _projectSpec; + private WarningPropertiesCollection _transitiveWarningPropertiesCollection; + + public string ProjectPath => _projectSpec?.RestoreMetadata?.ProjectPath; public IEnumerable Errors => _errors.ToArray(); public WarningPropertiesCollection ProjectWarningPropertiesCollection { get; set; } - public WarningPropertiesCollection TransitiveWarningPropertiesCollection { get; set; } - - public IEnumerable RestoreTargetGraphs { get; set; } + public WarningPropertiesCollection TransitiveWarningPropertiesCollection + { + get + { + if (_transitiveWarningPropertiesCollection == null) + { + // Populate TransitiveWarningPropertiesCollection only if it is null and we have RestoreTargetGraphs. + // This will happen at most once and only if we have the project spec with restore metadata. + if (_restoreTargetGraphs != null && + _restoreTargetGraphs.Any() && + _projectSpec != null && + _projectSpec.RestoreMetadata != null) + { + var transitiveNoWarnUtils = new TransitiveNoWarnUtils(); + + TransitiveWarningPropertiesCollection = transitiveNoWarnUtils.CreateTransitiveWarningPropertiesCollection( + _restoreTargetGraphs, + _projectSpec); + } + } + + return _transitiveWarningPropertiesCollection; + } + + set => _transitiveWarningPropertiesCollection = value; + } + + /// + /// Stores a reference to PackageSpec for the project from the restore request. + /// This are used to generate the warning properties for the project. + /// + /// PackageSpec to be stored for reference. + public void ApplyRestoreInputs(PackageSpec projectSpec) + { + _projectSpec = projectSpec; - public PackageSpec ProjectSpec { get; set; } + ProjectWarningPropertiesCollection = new WarningPropertiesCollection( + projectSpec.RestoreMetadata?.ProjectWideWarningProperties, + PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(projectSpec), + projectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly() + ); + } - public string ProjectPath => ProjectSpec?.RestoreMetadata?.ProjectPath; + /// + /// Stores a reference to RestoreTargetGraphs from the restore output. + /// These graphs are used to generate the transitive warning properties. + /// + /// RestoreTargetGraphs to be stored for reference. + public void ApplyRestoreOutput(IEnumerable restoreTargetGraphs) + { + _restoreTargetGraphs = restoreTargetGraphs; + } /// /// Initializes an instance of the , while still @@ -164,31 +214,11 @@ private bool IsWarningSuppressed(IRestoreLogMessage message) } else { - // Initialize transitive warning properties only if the project does not suppress the warning. - TryPopulateTransitiveWarningPropertiesCollection(message); + // Use transitive warning properties only if the project does not suppress the warning. return TransitiveWarningPropertiesCollection != null && TransitiveWarningPropertiesCollection.ApplyWarningProperties(message); } } - private void TryPopulateTransitiveWarningPropertiesCollection(ILogMessage message) - { - // Populate TransitiveWarningPropertiesCollection only if it is null and we have RestoreTargetGraphs. - // This will happen at most once and only if we have the project spec with restore metadata. - if (message.Level == LogLevel.Warning && - TransitiveWarningPropertiesCollection == null && - RestoreTargetGraphs != null && - RestoreTargetGraphs.Any() && - ProjectSpec != null && - ProjectSpec.RestoreMetadata != null) - { - var transitiveNoWarnUtils = new TransitiveNoWarnUtils(); - - TransitiveWarningPropertiesCollection = transitiveNoWarnUtils.CreateTransitiveWarningPropertiesCollection( - RestoreTargetGraphs, - ProjectSpec); - } - } - private static IRestoreLogMessage ToRestoreLogMessage(ILogMessage message) { var restoreLogMessage = message as IRestoreLogMessage; diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs index 19fe6e50bc8..055487fc03b 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -121,7 +121,7 @@ await InstallPackagesAsync(runtimeGraphs, // Update the logger with the restore target graphs // This allows lazy initialization for the Transitive Warning Properties - _logger.RestoreTargetGraphs = graphs; + _logger.ApplyRestoreOutput(graphs); // Warn for all dependencies that do not have exact matches or // versions that have been bumped up unexpectedly. diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 72fe64fc9d0..ae389998f01 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -52,17 +52,8 @@ public RestoreCommand(RestoreRequest request) var collectorLoggerHideWarningsAndErrors = request.Project.RestoreSettings.HideWarningsAndErrors || request.HideWarningsAndErrors; - - var collectorLogger = new RestoreCollectorLogger(_request.Log, collectorLoggerHideWarningsAndErrors) - { - ProjectSpec = _request.Project, - ProjectWarningPropertiesCollection = new WarningPropertiesCollection( - request.Project.RestoreMetadata?.ProjectWideWarningProperties, - PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(request.Project), - request.Project.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly() - ) - }; - + var collectorLogger = new RestoreCollectorLogger(_request.Log, collectorLoggerHideWarningsAndErrors); + collectorLogger.ApplyRestoreInputs(_request.Project); _logger = collectorLogger; } diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs index 8cee9d93b61..e7a64ed9eac 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/CollectorLoggerTests.cs @@ -114,7 +114,7 @@ public void CollectorLogger_DoesNotPassLogCallsToInnerLoggerByDefaultWithFilePat var innerLogger = new Mock(); var collector = new RestoreCollectorLogger(innerLogger.Object, LogLevel.Debug, hideWarningsAndErrors: false) { - ProjectSpec = new PackageSpec() + _projectSpec = new PackageSpec() { RestoreMetadata = new ProjectRestoreMetadata() {