Skip to content

Commit 1355061

Browse files
Nigusu-AllehuNigusu-Allehu
authored
Log NU1105 error for badly specified framework (#6291)
1 parent 73bd9f4 commit 1355061

8 files changed

Lines changed: 75 additions & 23 deletions

File tree

‎src/NuGet.Core/NuGet.Commands/PublicAPI/net472/PublicAPI.Unshipped.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
#nullable enable
2+
~static NuGet.Commands.SpecValidationUtility.ValidateDependencySpec(NuGet.ProjectModel.DependencyGraphSpec spec, System.Collections.Generic.HashSet<string> projectsToSkip, NuGet.Common.ILogger logger) -> void
23
~static NuGet.Commands.MSBuildRestoreUtility.GetRestoreAuditProperties(NuGet.Commands.IMSBuildItem specItem, System.Collections.Generic.IEnumerable<NuGet.Commands.IMSBuildItem> allItems) -> NuGet.ProjectModel.RestoreAuditProperties

‎src/NuGet.Core/NuGet.Commands/PublicAPI/net8.0/PublicAPI.Unshipped.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
#nullable enable
2+
~static NuGet.Commands.SpecValidationUtility.ValidateDependencySpec(NuGet.ProjectModel.DependencyGraphSpec spec, System.Collections.Generic.HashSet<string> projectsToSkip, NuGet.Common.ILogger logger) -> void
23
~static NuGet.Commands.MSBuildRestoreUtility.GetRestoreAuditProperties(NuGet.Commands.IMSBuildItem specItem, System.Collections.Generic.IEnumerable<NuGet.Commands.IMSBuildItem> allItems) -> NuGet.ProjectModel.RestoreAuditProperties

‎src/NuGet.Core/NuGet.Commands/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
#nullable enable
2+
~static NuGet.Commands.SpecValidationUtility.ValidateDependencySpec(NuGet.ProjectModel.DependencyGraphSpec spec, System.Collections.Generic.HashSet<string> projectsToSkip, NuGet.Common.ILogger logger) -> void
23
~static NuGet.Commands.MSBuildRestoreUtility.GetRestoreAuditProperties(NuGet.Commands.IMSBuildItem specItem, System.Collections.Generic.IEnumerable<NuGet.Commands.IMSBuildItem> allItems) -> NuGet.ProjectModel.RestoreAuditProperties

‎src/NuGet.Core/NuGet.Commands/RestoreCommand/RequestFactory/DependencyGraphSpecRequestProvider.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ private IReadOnlyList<RestoreSummaryRequest> GetRequestsFromItems(RestoreArgs re
7676
projectsWithErrors.Add(projectPath);
7777
}
7878
}
79-
SpecValidationUtility.ValidateDependencySpec(dgFile, projectsWithErrors);
79+
SpecValidationUtility.ValidateDependencySpec(dgFile, projectsWithErrors, restoreContext.Log);
8080

8181
// Create requests
8282
var requests = new ConcurrentBag<RestoreSummaryRequest>();

‎src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/SpecValidationUtility.cs‎

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Globalization;
77
using System.Linq;
8+
using NuGet.Common;
89
using NuGet.Frameworks;
910
using NuGet.LibraryModel;
1011
using NuGet.ProjectModel;
@@ -27,6 +28,13 @@ public static void ValidateDependencySpec(DependencyGraphSpec spec)
2728
/// </summary>
2829
public static void ValidateDependencySpec(DependencyGraphSpec spec, HashSet<string> projectsToSkip)
2930
{
31+
ValidateDependencySpec(spec, projectsToSkip, NullLogger.Instance);
32+
}
33+
34+
public static void ValidateDependencySpec(DependencyGraphSpec spec, HashSet<string> projectsToSkip, ILogger logger)
35+
{
36+
logger ??= NullLogger.Instance;
37+
3038
if (spec == null)
3139
{
3240
throw new ArgumentNullException(nameof(spec));
@@ -39,7 +47,7 @@ public static void ValidateDependencySpec(DependencyGraphSpec spec, HashSet<stri
3947
{
4048
if (!projectsToSkip.Contains(projectSpec.FilePath))
4149
{
42-
ValidateProjectSpec(projectSpec);
50+
ValidateProjectSpec(projectSpec, logger);
4351
}
4452
}
4553

@@ -77,6 +85,11 @@ public static void ValidateDependencySpec(DependencyGraphSpec spec, HashSet<stri
7785
}
7886

7987
public static void ValidateProjectSpec(PackageSpec spec)
88+
{
89+
ValidateProjectSpec(spec, NullLogger.Instance);
90+
}
91+
92+
private static void ValidateProjectSpec(PackageSpec spec, ILogger logger)
8093
{
8194
if (spec == null)
8295
{
@@ -123,15 +136,15 @@ public static void ValidateProjectSpec(PackageSpec spec)
123136
switch (projectStyle)
124137
{
125138
case ProjectStyle.PackageReference:
126-
ValidateProjectSpecPackageReference(spec, files);
139+
ValidateProjectSpecPackageReference(spec, files, logger);
127140
break;
128141

129142
case ProjectStyle.DotnetToolReference:
130-
ValidateProjectSpecPackageReference(spec, files);
143+
ValidateProjectSpecPackageReference(spec, files, logger);
131144
break;
132145

133146
case ProjectStyle.ProjectJson:
134-
ValidateProjectSpecUAP(spec, files);
147+
ValidateProjectSpecUAP(spec, files, logger);
135148
break;
136149

137150
default:
@@ -141,44 +154,56 @@ public static void ValidateProjectSpec(PackageSpec spec)
141154
}
142155
}
143156

144-
private static void ValidateFrameworks(PackageSpec spec, IEnumerable<string> files)
157+
private static void ValidateFrameworks(PackageSpec spec, IEnumerable<string> files, ILogger logger)
145158
{
146-
var frameworks = spec.TargetFrameworks.Select(f => f.FrameworkName).ToArray();
159+
if (spec.TargetFrameworks == null)
160+
{
161+
throw RestoreSpecException.Create(Strings.SpecValidationNoFrameworks, files);
162+
}
147163

148-
// Verify frameworks are valid
149-
foreach (var framework in frameworks.Where(f => !f.IsSpecificFramework))
164+
bool hasInvalidFrameworks = false;
165+
List<NuGetFramework> frameworkNames = new List<NuGetFramework>(spec.TargetFrameworks.Count);
166+
167+
foreach (var framework in spec.TargetFrameworks)
150168
{
151-
var message = string.Format(
152-
CultureInfo.CurrentCulture,
153-
Strings.SpecValidationInvalidFramework,
154-
framework.GetShortFolderName());
169+
frameworkNames.Add(framework.FrameworkName);
155170

156-
throw RestoreSpecException.Create(message, files);
171+
if (!framework.FrameworkName.IsSpecificFramework)
172+
{
173+
hasInvalidFrameworks |= true;
174+
var message = string.Format(CultureInfo.CurrentCulture, Strings.SpecValidationInvalidFramework, framework.TargetAlias);
175+
logger.Log(new RestoreLogMessage(LogLevel.Error, NuGetLogCode.NU1105, message) { FilePath = spec.FilePath, ProjectPath = spec.FilePath });
176+
}
177+
}
178+
179+
if (hasInvalidFrameworks)
180+
{
181+
throw RestoreSpecException.Create(string.Format(CultureInfo.CurrentCulture, Strings.Invalid_Framework), files);
157182
}
158183

159184
// Must have at least 1 framework
160-
if (frameworks.Length < 1)
185+
if (frameworkNames.Count < 1)
161186
{
162187
throw RestoreSpecException.Create(Strings.SpecValidationNoFrameworks, files);
163188
}
164189

165190
// Duplicate frameworks may not exist
166191
// Change in ATF should *not* affect our duplicate check, so we use the full framework comparer.
167-
if (frameworks.Length != frameworks.Distinct(NuGetFrameworkFullComparer.Instance).Count())
192+
if (frameworkNames.Count != frameworkNames.Distinct(NuGetFrameworkFullComparer.Instance).Count())
168193
{
169194
var message = string.Format(
170195
CultureInfo.CurrentCulture,
171196
Strings.SpecValidationDuplicateFrameworks,
172-
string.Join(", ", frameworks.Select(f => f.GetShortFolderName())));
197+
string.Join(", ", frameworkNames.Select(f => f.GetShortFolderName())));
173198

174199
throw RestoreSpecException.Create(message, files);
175200
}
176201
}
177202

178-
private static void ValidateProjectSpecPackageReference(PackageSpec spec, IEnumerable<string> files)
203+
private static void ValidateProjectSpecPackageReference(PackageSpec spec, IEnumerable<string> files, ILogger logger)
179204
{
180205
// Verify frameworks
181-
ValidateFrameworks(spec, files);
206+
ValidateFrameworks(spec, files, logger);
182207

183208
// NETCore may not specify a project.json file
184209
if (!string.IsNullOrEmpty(spec.RestoreMetadata.ProjectJsonPath))
@@ -234,10 +259,10 @@ private static void ValidateProjectSpecPackageReference(PackageSpec spec, IEnume
234259
}
235260
}
236261

237-
private static void ValidateProjectSpecUAP(PackageSpec spec, IEnumerable<string> files)
262+
private static void ValidateProjectSpecUAP(PackageSpec spec, IEnumerable<string> files, ILogger logger)
238263
{
239264
// Verify frameworks
240-
ValidateFrameworks(spec, files);
265+
ValidateFrameworks(spec, files, logger);
241266

242267
// UAP may contain only 1 framework
243268
if (spec.TargetFrameworks.Count != 1)

‎src/NuGet.Core/NuGet.Commands/Strings.Designer.cs‎

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/NuGet.Core/NuGet.Commands/Strings.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,4 +1139,7 @@ NuGet requires HTTPS sources. Refer to https://aka.ms/nuget-https-everywhere for
11391139
<value>A ProjectReference cannot be pruned, {0}.</value>
11401140
<comment>0 - project reference</comment>
11411141
</data>
1142+
<data name="Invalid_Framework" xml:space="preserve">
1143+
<value>One or more invalid frameworks were detected.</value>
1144+
</data>
11421145
</root>

‎test/NuGet.Core.Tests/NuGet.Commands.Test/SpecValidationUtilityTests.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.IO;
67
using System.Linq;
8+
using FluentAssertions;
9+
using Moq;
10+
using NuGet.Common;
711
using NuGet.Frameworks;
812
using NuGet.LibraryModel;
913
using NuGet.ProjectModel;
@@ -53,6 +57,10 @@ public void SpecValidationUtility_VerifyFrameworks_Unsupported()
5357
// Arrange
5458
var spec = new DependencyGraphSpec();
5559
spec.AddRestore("a");
60+
var errors = new List<NuGetLogCode>();
61+
var mockLogger = new Mock<ILogger>();
62+
mockLogger.Setup(l => l.Log(It.IsAny<ILogMessage>()))
63+
.Callback((ILogMessage message) => { errors.Add(message.Code); });
5664

5765
var targetFramework = new TargetFrameworkInformation()
5866
{
@@ -72,8 +80,12 @@ public void SpecValidationUtility_VerifyFrameworks_Unsupported()
7280

7381
spec.AddProject(project);
7482

75-
// Act && Assert
76-
AssertError(spec, "Invalid target framework");
83+
// Act
84+
Action act = () => SpecValidationUtility.ValidateDependencySpec(spec, new HashSet<string>(), mockLogger.Object);
85+
86+
// Assert
87+
act.Should().Throw<RestoreSpecException>().Where(e => e.Message.Contains(project.FilePath));
88+
errors.Should().Contain(NuGetLogCode.NU1105);
7789
}
7890

7991
[Fact]

0 commit comments

Comments
 (0)