Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,7 @@ private PackageSpec GetPackageSpec(IMSBuildProject project, IReadOnlyDictionary<
restoreMetadata.SdkAnalysisLevel = MSBuildRestoreUtility.GetSdkAnalysisLevel(project.GetProperty("SdkAnalysisLevel"));
restoreMetadata.UseLegacyDependencyResolver = project.IsPropertyTrue("RestoreUseLegacyDependencyResolver");
restoreMetadata.RestoreDoNotWriteDependencyGraphSpec = project.IsPropertyTrue("RestoreDoNotWriteDependencyGraphSpec");
restoreMetadata.RestoreEnableAnalyzerAssets = GetRestoreEnableAnalyzerAssets(project, projectsByTargetFramework.Values);

return (restoreMetadata, targetFrameworkInfos);

Expand All @@ -1207,6 +1208,19 @@ private PackageSpec GetPackageSpec(IMSBuildProject project, IReadOnlyDictionary<
}
}

internal static bool GetRestoreEnableAnalyzerAssets(IMSBuildProject project, IEnumerable<IMSBuildProject> innerBuilds)
{
foreach (IMSBuildProject innerBuild in innerBuilds.NoAllocEnumerate())
{
if (innerBuild.IsPropertyTrue("RestoreEnableAnalyzerAssets"))
{
return true;
}
}

return project.IsPropertyTrue("RestoreEnableAnalyzerAssets");
}

internal static bool GetPackagePruningDefault(IEnumerable<IMSBuildProject> innerBuilds)
{
foreach (var item in innerBuilds.NoAllocEnumerate())
Expand Down
4 changes: 4 additions & 0 deletions src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ Copyright (c) .NET Foundation. All rights reserved.
AND '$(TargetFrameworkIdentifier)' == '.NETCoreApp'
AND $([MSBuild]::VersionGreaterThanOrEquals('$(TargetFrameworkVersion)', '10.0'))">all</NuGetAuditMode>
<NuGetAuditMode Condition=" '$(NuGetAuditMode)' == '' ">direct</NuGetAuditMode>
<!-- Analyzer assets restore defaults to off. An opt-in on the outer build or any target framework enables analyzer assets project-wide. -->
<RestoreEnableAnalyzerAssets Condition="'$(RestoreEnableAnalyzerAssets)' == '' AND '$(TargetFrameworks)' == ''">false</RestoreEnableAnalyzerAssets>
</PropertyGroup>

<!-- Package pruning is enabled for all projects targeting .NET 10 or greater, including multi-targeted projects. -->
Expand Down Expand Up @@ -974,6 +976,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<NETCoreSdkVersion>$(NETCoreSdkVersion)</NETCoreSdkVersion>
<RestoreUseLegacyDependencyResolver>$(RestoreUseLegacyDependencyResolver)</RestoreUseLegacyDependencyResolver>
<RestoreDoNotWriteDependencyGraphSpec>$(RestoreDoNotWriteDependencyGraphSpec)</RestoreDoNotWriteDependencyGraphSpec>
<RestoreEnableAnalyzerAssets>$(RestoreEnableAnalyzerAssets)</RestoreEnableAnalyzerAssets>
</_RestoreGraphEntry>
</ItemGroup>

Expand Down Expand Up @@ -1198,6 +1201,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<RestoreEnablePackagePruning>$(RestoreEnablePackagePruning)</RestoreEnablePackagePruning>
<RestorePackagePruningDefault>$(RestorePackagePruningDefault)</RestorePackagePruningDefault>
<NuGetAuditMode>$(NuGetAuditMode)</NuGetAuditMode>
<RestoreEnableAnalyzerAssets>$(RestoreEnableAnalyzerAssets)</RestoreEnableAnalyzerAssets>
</_RestoreGraphEntry>
</ItemGroup>
</Target>
Expand Down
15 changes: 11 additions & 4 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/LockFileBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,12 @@ public LockFile CreateLockFile(LockFile previousLockFile,

var librariesWithWarnings = new HashSet<LibraryIdentity>();

var rootProjectStyle = project.RestoreMetadata?.ProjectStyle ?? ProjectStyle.Unknown;
var restoreMetadata = project.RestoreMetadata;
var rootProjectStyle = restoreMetadata?.ProjectStyle ?? ProjectStyle.Unknown;

// Analyzer assets are a project-wide opt-in (the RestoreEnableAnalyzerAssets MSBuild property).
// When enabled, analyzer assets are honored for every target framework.
bool restoreEnableAnalyzerAssets = restoreMetadata?.RestoreEnableAnalyzerAssets ?? false;

// Add the targets
foreach (var targetGraph in targetGraphs
Expand All @@ -182,9 +187,9 @@ public LockFile CreateLockFile(LockFile previousLockFile,
var flattenedFlags = IncludeFlagUtils.FlattenDependencyTypes(_includeFlagGraphs, project, targetGraph);

// Check if warnings should be displayed for the current framework.
var tfi = project.GetTargetFramework(targetGraph.Framework);
var tfi = project.GetTargetFramework(targetGraph.TargetAlias);

bool warnForImportsOnGraph = tfi.Warn
bool warnForImportsOnGraph = tfi?.Warn == true
&& (target.TargetFramework is FallbackFramework
|| target.TargetFramework is AssetTargetFallbackFramework);

Expand Down Expand Up @@ -228,7 +233,7 @@ public LockFile CreateLockFile(LockFile previousLockFile,
}

var package = packageInfo.Package;
var libraryDependency = tfi.Dependencies.FirstOrDefault(e => e.Name.Equals(library.Name, StringComparison.OrdinalIgnoreCase));
var libraryDependency = tfi?.Dependencies.FirstOrDefault(e => e.Name.Equals(library.Name, StringComparison.OrdinalIgnoreCase));

(LockFileTargetLibrary targetLibrary, bool usedFallbackFramework, NuGetFramework compileAssetFramework, NuGetFramework runtimeAssetFramework) = LockFileUtils.CreateLockFileTargetLibrary(
libraryDependency?.Aliases,
Expand All @@ -238,6 +243,7 @@ public LockFile CreateLockFile(LockFile previousLockFile,
dependencyType: includeFlags,
targetFrameworkOverride: null,
dependencies: graphItem.Data.Dependencies,
restoreEnableAnalyzerAssets: restoreEnableAnalyzerAssets,
cache: lockFileBuilderCache);

target.Libraries.Add(targetLibrary);
Expand All @@ -258,6 +264,7 @@ public LockFile CreateLockFile(LockFile previousLockFile,
targetFrameworkOverride: nonFallbackFramework,
dependencyType: includeFlags,
dependencies: graphItem.Data.Dependencies,
restoreEnableAnalyzerAssets: restoreEnableAnalyzerAssets,
cache: lockFileBuilderCache);
usedFallbackFramework = !targetLibrary.Equals(targetLibraryWithoutFallback);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class LockFileBuilderCache
private readonly ConcurrentDictionary<CriteriaKey, List<(List<SelectionCriteria>, bool)>> _criteriaSets =
new();

private readonly ConcurrentDictionary<(CriteriaKey, string path, string aliases, LibraryIncludeFlags, int dependencyCount), Lazy<(LockFileTargetLibrary, bool, NuGetFramework, NuGetFramework)>> _lockFileTargetLibraryCache =
private readonly ConcurrentDictionary<(CriteriaKey, string path, string aliases, LibraryIncludeFlags, int dependencyCount, bool restoreEnableAnalyzerAssets), Lazy<(LockFileTargetLibrary, bool, NuGetFramework, NuGetFramework)>> _lockFileTargetLibraryCache =
new();

/// <summary>
Expand Down Expand Up @@ -106,7 +106,7 @@ public ContentItemCollection GetContentItems(LockFileLibrary library, LocalPacka
/// <summary>
/// Try to get a LockFileTargetLibrary from the cache.
/// </summary>
internal (LockFileTargetLibrary, bool, NuGetFramework, NuGetFramework) GetLockFileTargetLibrary(RestoreTargetGraph graph, NuGetFramework framework, LocalPackageInfo localPackageInfo, string aliases, LibraryIncludeFlags libraryIncludeFlags, List<LibraryDependency> dependencies, Func<(LockFileTargetLibrary, bool, NuGetFramework, NuGetFramework)> valueFactory)
internal (LockFileTargetLibrary, bool, NuGetFramework, NuGetFramework) GetLockFileTargetLibrary(RestoreTargetGraph graph, NuGetFramework framework, LocalPackageInfo localPackageInfo, string aliases, LibraryIncludeFlags libraryIncludeFlags, List<LibraryDependency> dependencies, bool restoreEnableAnalyzerAssets, Func<(LockFileTargetLibrary, bool, NuGetFramework, NuGetFramework)> valueFactory)
{
// Comparing RuntimeGraph for equality is very expensive,
// so in case of a request where the RuntimeGraph is not empty we avoid using the cache.
Expand All @@ -116,7 +116,7 @@ public ContentItemCollection GetContentItems(LockFileLibrary library, LocalPacka
localPackageInfo = localPackageInfo ?? throw new ArgumentNullException(nameof(localPackageInfo));
var criteriaKey = new CriteriaKey(graph.TargetGraphName, framework);
var packagePath = localPackageInfo.ExpandedPath;
return _lockFileTargetLibraryCache.GetOrAdd((criteriaKey, packagePath, aliases, libraryIncludeFlags, dependencies.Count),
return _lockFileTargetLibraryCache.GetOrAdd((criteriaKey, packagePath, aliases, libraryIncludeFlags, dependencies.Count, restoreEnableAnalyzerAssets),
key => new Lazy<(LockFileTargetLibrary, bool, NuGetFramework, NuGetFramework)>(valueFactory)).Value;
}

Expand Down
132 changes: 132 additions & 0 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ private readonly Dictionary<RestoreTargetGraph, Dictionary<string, LibraryInclud
private const string PackagePruningRemovablePackagesCount = "Pruning.RemovablePackages.Count";
private const string PackagePruningDirectCount = "Pruning.Pruned.Direct.Count";

// Analyzer assets names
private const string AnalyzerAssetsEnabled = "AnalyzerAssets.Enabled";
private const string AnalyzerAssetsExcluded = "AnalyzerAssets.Excluded";
private const string AnalyzerAssetsPackagesWithAnalyzersCount = "AnalyzerAssets.PackagesWithAnalyzers.Count";
private const string AnalyzerAssetsPackagesWithExcludedAnalyzersCount = "AnalyzerAssets.PackagesWithExcludedAnalyzers.Count";
private const string AnalyzerAssetsExcludedByPrivateAssetsCount = "AnalyzerAssets.ExcludedByPrivateAssets.Count";
private const string AnalyzerAssetsExcludedByExcludeAssetsCount = "AnalyzerAssets.ExcludedByExcludeAssets.Count";

internal readonly bool _enableNewDependencyResolver;
private readonly bool _isLockFileEnabled;

Expand Down Expand Up @@ -357,6 +365,7 @@ public async Task<RestoreResult> ExecuteAsync(CancellationToken token)

telemetry.TelemetryEvent[UpdatedAssetsFile] = restoreResult._isAssetsFileDirty.Value;
telemetry.TelemetryEvent[UpdatedMSBuildFiles] = restoreResult._dirtyMSBuildFiles.Value.Count > 0;
PopulateAnalyzerAssetsTelemetry(telemetry.TelemetryEvent, assetsFile, graphs, _request.Project);

return restoreResult;
}
Expand Down Expand Up @@ -411,6 +420,7 @@ private void InitializeTelemetry(TelemetryActivity telemetry, int httpSourcesCou
}

telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled";
telemetry.TelemetryEvent[AnalyzerAssetsEnabled] = _request.Project.RestoreMetadata?.RestoreEnableAnalyzerAssets ?? false;

PopulatePruningEnabledTelemetry(_request.Project, telemetry.TelemetryEvent);
}
Expand Down Expand Up @@ -456,6 +466,128 @@ internal static void PopulatePruningEnabledTelemetry(PackageSpec project, Teleme
telemetryEvent[PackagePruningFrameworksUnsupportedCount] = pruningNotApplicableCount;
}

/// <summary>
/// Reports analyzer-asset usage so the impact of enabling <c>RestoreEnableAnalyzerAssets</c> by
/// default can be measured ahead of the rollout. The data is derived from the resolved dependency
/// graphs and the package file lists, so it is reported on every restore regardless of whether the
/// feature is currently enabled. This lets us see, before flipping the default, how many packages
/// would stop having their analyzers applied because <c>PrivateAssets</c>/<c>ExcludeAssets</c> would
/// finally be honored. Detection is per package (not per analyzer assembly): the rollout decision is
/// driven by whether a package's analyzers are affected, not by how many assemblies it ships.
/// </summary>
/// <remarks>
/// Analyzers are not runtime-identifier specific, so only the target-framework graphs (those with a
/// null runtime identifier) are inspected to avoid counting the same package once per RID. This runs on
/// the full-restore path only (after the no-op short-circuit), where the dependency graphs are available.
/// </remarks>
private void PopulateAnalyzerAssetsTelemetry(TelemetryEvent telemetryEvent, LockFile assetsFile, List<RestoreTargetGraph> graphs, PackageSpec project)
Comment thread
nkolev92 marked this conversation as resolved.
{
// Identify which packages contribute at least one analyzer assembly, from the always-present
// libraries section. Detection is per package (not per assembly) since the rollout decision is
// driven by whether a package's analyzers are affected, not by how many assemblies it ships.
var packagesWithAnalyzerAssemblies = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (LockFileLibrary library in assetsFile.Libraries.NoAllocEnumerate())
{
foreach (string file in library.Files.NoAllocEnumerate())
{
if (IsAnalyzerAssemblyPath(file))
{
packagesWithAnalyzerAssemblies.Add(GetAnalyzerPackageKey(library.Name, library.Version));
break;
}
}
}

int packagesWithAnalyzers = 0;
int packagesWithExcludedAnalyzers = 0;
int excludedByPrivateAssets = 0;
int excludedByExcludeAssets = 0;

foreach (RestoreTargetGraph graph in graphs.NoAllocEnumerate())
{
// Analyzers are not runtime specific; only inspect the target framework graphs.
if (graph.RuntimeIdentifier != null)
{
continue;
}

Dictionary<string, LibraryIncludeFlags> flattenedFlags = IncludeFlagUtils.FlattenDependencyTypes(_includeFlagGraphs, project, graph);
Comment thread
nkolev92 marked this conversation as resolved.
TargetFrameworkInformation targetFrameworkInformation = project.GetNearestTargetFramework(graph.Framework, graph.TargetAlias);

foreach (GraphItem<RemoteResolveResult> graphItem in graph.Flattened)
{
LibraryIdentity library = graphItem.Key;
if (library.Type != LibraryType.Package)
{
continue;
}

if (!packagesWithAnalyzerAssemblies.Contains(GetAnalyzerPackageKey(library.Name, library.Version)))
{
continue;
}

packagesWithAnalyzers++;

if (!flattenedFlags.TryGetValue(library.Name, out LibraryIncludeFlags includeFlags))
{
includeFlags = ~LibraryIncludeFlags.ContentFiles;
}

if ((includeFlags & LibraryIncludeFlags.Analyzers) != LibraryIncludeFlags.None)
{
continue;
}

// The package contributes analyzers, but they would be filtered out for this project.
packagesWithExcludedAnalyzers++;

// Attribute the exclusion: a direct reference whose own IncludeAssets/ExcludeAssets drops
// analyzers is counted separately from analyzers suppressed transitively via PrivateAssets
// (the default for analyzers), since the transitive case is the surprising one for customers.
LibraryDependency directDependency = targetFrameworkInformation?.Dependencies.FirstOrDefault(
dependency => dependency.Name.Equals(library.Name, StringComparison.OrdinalIgnoreCase));

bool excludedByOwnAssetsFilter = directDependency != null
&& (directDependency.IncludeType & LibraryIncludeFlags.Analyzers) == LibraryIncludeFlags.None;

if (excludedByOwnAssetsFilter)
{
excludedByExcludeAssets++;
}
else
{
excludedByPrivateAssets++;
}
}
}

telemetryEvent[AnalyzerAssetsExcluded] = packagesWithExcludedAnalyzers > 0;
telemetryEvent[AnalyzerAssetsPackagesWithAnalyzersCount] = packagesWithAnalyzers;
telemetryEvent[AnalyzerAssetsPackagesWithExcludedAnalyzersCount] = packagesWithExcludedAnalyzers;
telemetryEvent[AnalyzerAssetsExcludedByPrivateAssetsCount] = excludedByPrivateAssets;
telemetryEvent[AnalyzerAssetsExcludedByExcludeAssetsCount] = excludedByExcludeAssets;
}

private static string GetAnalyzerPackageKey(string id, NuGetVersion version)
{
return id + "/" + version?.ToNormalizedString();
}

/// <summary>
/// Determines whether a package file path is an analyzer assembly. This intentionally mirrors the
/// detection used by ManagedCodeConventions.ManagedCodePatterns.AnalyzerAssemblies (any '.dll' under
/// 'analyzers/' at any depth, excluding satellite '.resources.dll' assemblies), but as a cheap string
/// check so analyzer packages can be counted from the package file list for telemetry even when analyzer
/// assets are not selected (the feature is off), without allocating a content-item collection per package.
/// </summary>
private static bool IsAnalyzerAssemblyPath(string path)
{
return path.StartsWith("analyzers/", StringComparison.Ordinal)
Comment thread
martinrrm marked this conversation as resolved.
&& path.EndsWith(".dll", StringComparison.OrdinalIgnoreCase)
&& !path.EndsWith(".resources.dll", StringComparison.OrdinalIgnoreCase);
}

private async Task<(RestoreResult, bool, CacheFile)> EvaluateNoOpAsync(TelemetryActivity telemetry, CacheFile cacheFile, Stopwatch restoreTime)
{
telemetry.StartIntervalMeasure();
Expand Down
Loading
Loading