Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for transitive NoWarn properties from project references #1642

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion build/Shared/EqualityUtility.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -59,6 +59,31 @@ internal static bool SequenceEqualWithNullCheck<T>(
return self.SequenceEqual(other, comparer);
}

/// <summary>
/// Compares two sets for equality, allowing either sequence to be null.
/// If one is null, both have to be null for equality.
/// </summary>
internal static bool SetEqualWithNullCheck<T>(
this ISet<T> self,
ISet<T> other,
IEqualityComparer<T> comparer = null)
{
bool identityEquals;
if (TryIdentityEquals(self, other, out identityEquals))
{
return identityEquals;
}

if (comparer == null)
{
comparer = EqualityComparer<T>.Default;
}

ISet<T> set = new HashSet<T>(self, comparer);

return set.SetEquals(other);
}

internal static bool DictionaryEquals<TKey, TValue>(
IDictionary<TKey, TValue> self,
IDictionary<TKey, TValue> other,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public async Task<int> 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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
// 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;
using System.Collections.Generic;
using System.Linq;
using NuGet.Common;
using NuGet.Frameworks;
using NuGet.ProjectModel;
using NuGet.Shared;

namespace NuGet.Commands
{

/// <summary>
/// Contains Package specific properties for Warnings.
/// </summary>
public class PackageSpecificWarningProperties
public class PackageSpecificWarningProperties : IEquatable <PackageSpecificWarningProperties>
{

/// <summary>
/// Contains Package specific No warn properties.
/// NuGetLogCode -> LibraryId -> Set of Frameworks.
/// </summary>
private IDictionary<NuGetLogCode, IDictionary<string, ISet<NuGetFramework>>> Properties;
public IDictionary<NuGetLogCode, IDictionary<string, ISet<NuGetFramework>>> Properties { get; private set; }

/// <summary>
/// Extracts PackageSpecific WarningProperties from a PackageSpec
Expand All @@ -36,21 +38,50 @@ 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);
}
}

foreach (var framework in packageSpec.TargetFrameworks)
{
foreach (var dependency in framework.Dependencies)
{
warningProperties.AddRange(dependency.NoWarn, dependency.Name, framework.FrameworkName);
warningProperties.AddRangeOfCodes(dependency.NoWarn, dependency.Name, framework.FrameworkName);
}
}

return warningProperties;
}

/// <summary>
/// Extracts PackageSpecific WarningProperties from a PackageSpec for a specific NuGetFramework
/// </summary>
/// <param name="packageSpec">PackageSpec containing the Dependencies with WarningProperties</param>
/// <param name="framework">NuGetFramework for which the properties should be assessed.</param>
/// <returns>PackageSpecific WarningProperties extracted from a PackageSpec for a specific NuGetFramework</returns>
public static PackageSpecificWarningProperties CreatePackageSpecificWarningProperties(PackageSpec packageSpec,
NuGetFramework framework)
{
// NuGetLogCode -> LibraryId -> Set of Frameworks.
var warningProperties = new PackageSpecificWarningProperties();

foreach (var dependency in packageSpec.Dependencies)
{
warningProperties.AddRangeOfCodes(dependency.NoWarn, dependency.Name, framework);
}

var targetFrameworkInformation = packageSpec
.TargetFrameworks
.First(tfi => tfi.FrameworkName == framework);

foreach (var dependency in targetFrameworkInformation.Dependencies)
{
warningProperties.AddRangeOfCodes(dependency.NoWarn, dependency.Name, framework);
}

return warningProperties;
}

/// <summary>
/// Adds a NuGetLogCode into the NoWarn Set for the specified library Id and target graph.
/// </summary>
Expand Down Expand Up @@ -85,14 +116,28 @@ public void Add(NuGetLogCode code, string libraryId, NuGetFramework framework)
/// <param name="codes">IEnumerable of NuGetLogCode for which no warning should be thrown.</param>
/// <param name="libraryId">Library for which no warning should be thrown.</param>
/// <param name="framework">Target graph for which no warning should be thrown.</param>
public void AddRange(IEnumerable<NuGetLogCode> codes, string libraryId, NuGetFramework framework)
public void AddRangeOfCodes(IEnumerable<NuGetLogCode> codes, string libraryId, NuGetFramework framework)
{
foreach (var code in codes)
{
Add(code, libraryId, framework);
}
}

/// <summary>
/// Adds a list of NuGetLogCode into the NoWarn Set for the specified library Id and target graph.
/// </summary>
/// <param name="code">NuGetLogCode for which no warning should be thrown.</param>
/// <param name="libraryId">Library for which no warning should be thrown.</param>
/// <param name="frameworks">IEnumerable of Target graph for which no warning should be thrown.</param>
public void AddRangeOfFrameworks(NuGetLogCode code, string libraryId, IEnumerable<NuGetFramework> frameworks)
{
foreach (var framework in frameworks)
{
Add(code, libraryId, framework);
}
}

/// <summary>
/// Checks if a NugetLogCode is part of the NoWarn list for the specified library Id and target graph.
/// </summary>
Expand All @@ -107,5 +152,38 @@ 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this method not match Equals? They should both check the same things.

Copy link
Contributor Author

@mishra14 mishra14 Aug 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems acceptable as per the requirements on HashCode -

  1. if x equals y then the hash code of x must equal the hash code of y. Equivalently: if the hash code of x does not equal the hash code of y then x and y must be unequal.
  2. the hash code of x must remain stable while x is in a hash table.

Ref - https://stackoverflow.com/questions/19710028/what-to-return-when-overriding-object-gethashcode-in-classes-with-no-immutable


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 EqualityUtility.DictionaryEquals(
Properties,
other.Properties,
(sv1, ov1) => EqualityUtility.DictionaryEquals(sv1, ov1, (sv2, ov2) => EqualityUtility.SetEqualWithNullCheck(sv2, ov2)));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// 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;
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
{
Expand All @@ -14,12 +17,68 @@ public class RestoreCollectorLogger : LoggerBase, ICollectorLogger
private readonly ILogger _innerLogger;
private readonly ConcurrentQueue<IRestoreLogMessage> _errors;
private readonly bool _hideWarningsAndErrors;
private IEnumerable<RestoreTargetGraph> _restoreTargetGraphs;
private PackageSpec _projectSpec;
private WarningPropertiesCollection _transitiveWarningPropertiesCollection;

public string ProjectPath => _projectSpec?.RestoreMetadata?.ProjectPath;

public IEnumerable<IRestoreLogMessage> Errors => _errors.ToArray();

public WarningPropertiesCollection WarningPropertiesCollection { get; set; }

public string ProjectPath { get; set; }
public WarningPropertiesCollection ProjectWarningPropertiesCollection { 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;
}

/// <summary>
/// Stores a reference to PackageSpec for the project from the restore request.
/// This are used to generate the warning properties for the project.
/// </summary>
/// <param name="projectSpec">PackageSpec to be stored for reference.</param>
public void ApplyRestoreInputs(PackageSpec projectSpec)
{
_projectSpec = projectSpec;

ProjectWarningPropertiesCollection = new WarningPropertiesCollection(
projectSpec.RestoreMetadata?.ProjectWideWarningProperties,
PackageSpecificWarningProperties.CreatePackageSpecificWarningProperties(projectSpec),
projectSpec.TargetFrameworks.Select(f => f.FrameworkName).AsList().AsReadOnly()
);
}

/// <summary>
/// Stores a reference to RestoreTargetGraphs from the restore output.
/// These graphs are used to generate the transitive warning properties.
/// </summary>
/// <param name="restoreTargetGraphs">RestoreTargetGraphs to be stored for reference.</param>
public void ApplyRestoreOutput(IEnumerable<RestoreTargetGraph> restoreTargetGraphs)
{
_restoreTargetGraphs = restoreTargetGraphs;
}

/// <summary>
/// Initializes an instance of the <see cref="RestoreCollectorLogger"/>, while still
Expand Down Expand Up @@ -67,11 +126,11 @@ 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.
if (WarningPropertiesCollection == null || !WarningPropertiesCollection.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))
{
Expand All @@ -92,9 +151,8 @@ public void Log(IRestoreLogMessage message)

public Task LogAsync(IRestoreLogMessage message)
{

// This will be true only when the Message is a Warning and should be suppressed.
if (WarningPropertiesCollection == null || !WarningPropertiesCollection.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))
{
Expand Down Expand Up @@ -142,6 +200,25 @@ protected bool DisplayMessage(IRestoreLogMessage message)
}
}

/// <summary>
/// This method checks if at least one of the warning properties collections is not null and it suppresses the warning.
/// </summary>
/// <param name="message">IRestoreLogMessage to be logged.</param>
/// <returns>bool indicating if the message should be suppressed.</returns>
private bool IsWarningSuppressed(IRestoreLogMessage message)
{

if (ProjectWarningPropertiesCollection != null && ProjectWarningPropertiesCollection.ApplyWarningProperties(message))
{
return true;
}
else
{
// Use transitive warning properties only if the project does not suppress the warning.
return TransitiveWarningPropertiesCollection != null && TransitiveWarningPropertiesCollection.ApplyWarningProperties(message);
}
}

private static IRestoreLogMessage ToRestoreLogMessage(ILogMessage message)
{
var restoreLogMessage = message as IRestoreLogMessage;
Expand Down
Loading