Skip to content

Commit

Permalink
Warn when dot missing from target framework strings
Browse files Browse the repository at this point in the history
  • Loading branch information
zkat committed Oct 6, 2020
1 parent 5a70626 commit e59ea92
Show file tree
Hide file tree
Showing 16 changed files with 273 additions and 4 deletions.
1 change: 0 additions & 1 deletion src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ private static Version GetLicenseExpressionVersion(IPackTaskRequest<IMSBuildItem
return version;
}


private LockFile GetAssetsFile(IPackTaskRequest<IMSBuildItem> request)
{
if (request.PackItem == null)
Expand Down
5 changes: 5 additions & 0 deletions src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -857,5 +857,10 @@ public enum NuGetLogCode
/// Undefined package warning
/// </summary>
NU5500 = 5500,

/// <summary>
/// InvalidUndottedFrameworkWarning
/// </summary>
NU5501 = 5501,
}
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode
NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode
NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode
NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode

NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode
NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode

NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ public int GetHashCode(ContentItem obj)
var hashCode = 0;
foreach (var property in obj.Properties)
{
if (property.Key.EndsWith("_raw"))
{
continue;
}
hashCode ^= property.Key.GetHashCode();
hashCode ^= property.Value.GetHashCode();
}
Expand All @@ -264,6 +268,12 @@ public bool Equals(ContentItem x, ContentItem y)

foreach (var xProperty in x.Properties)
{
if (xProperty.Key.EndsWith("_raw"))
{
// We've started storing raw versions of each key, but
// we don't want that to affect results.
continue;
}
object yValue;
if (!y.Properties.TryGetValue(xProperty.Key, out yValue))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ internal override bool TryMatch(
Path = path
};
}

item.Properties.Add(_token + "_raw", substring);
item.Properties.Add(_token, value);
}
endIndex = delimiterIndex;
Expand Down
2 changes: 2 additions & 0 deletions src/NuGet.Core/NuGet.Packaging/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -309,3 +309,5 @@
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "API already shipped", Scope = "member", Target = "~M:NuGet.Packaging.PackageExtractor.InstallFromSourceAsync(System.String,NuGet.Packaging.Core.PackageIdentity,System.Func{System.IO.Stream,System.Threading.Tasks.Task},NuGet.Packaging.VersionFolderPathResolver,NuGet.Packaging.PackageExtractionContext,System.Threading.CancellationToken,System.Guid)~System.Threading.Tasks.Task{System.Boolean}")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "API already shipped", Scope = "member", Target = "~M:NuGet.Packaging.PackageExtractor.InstallFromSourceAsync(NuGet.Packaging.Core.PackageIdentity,NuGet.Packaging.IPackageDownloader,NuGet.Packaging.VersionFolderPathResolver,NuGet.Packaging.PackageExtractionContext,System.Threading.CancellationToken,System.Guid)~System.Threading.Tasks.Task{System.Boolean}")]
[assembly: SuppressMessage("ApiDesign", "RS0027:Public API with optional parameter(s) should have the most parameters amongst its public overloads.", Justification = "API already shipped", Scope = "member", Target = "~M:NuGet.Packaging.PackageArchiveReader.#ctor(System.String,NuGet.Frameworks.IFrameworkNameProvider,NuGet.Frameworks.IFrameworkCompatibilityProvider)")]
[assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "These are not localized strings", Scope = "member", Target = "~M:NuGet.Packaging.Rules.InvalidUndottedFrameworkRule.Validate(NuGet.Packaging.PackageArchiveReader)~System.Collections.Generic.IEnumerable{NuGet.Common.PackagingLogMessage}")]
[assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "These are not localized strings", Scope = "member", Target = "~M:NuGet.ContentModel.ContentItemCollection.GroupComparer.Equals(NuGet.ContentModel.ContentItem,NuGet.ContentModel.ContentItem)~System.Boolean")]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkWarning.get -> string
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkWarning.get -> string
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkWarning.get -> string

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/NuGet.Core/NuGet.Packaging/Rules/AnalysisResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@
<data name="InvalidPrereleaseDependencyWarning" xml:space="preserve">
<value>A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "{0}" or update the version field in the nuspec.</value>
</data>
<data name="InvalidUndottedFrameworkWarning" xml:space="preserve">
<value>Packaged folders must include dots in their framework version numbers. This is required as of .NET 5.0. Please rename the following folder(s) to include dots as needed (e.g. 'net50' to 'net5.0'): {0}</value>
<comment>0 - comma-separated list of paths</comment>
</data>
<data name="LegacyVersionWarning" xml:space="preserve">
<value>The package version '{0}' uses SemVer 2.0.0 or components of SemVer 1.0.0 that are not supported on legacy clients. Change the package version to a SemVer 1.0.0 string. If the version contains a release label it must start with a letter. This message can be ignored if the package is not intended for older clients.</value>
</data>
Expand Down
103 changes: 103 additions & 0 deletions src/NuGet.Core/NuGet.Packaging/Rules/InvalidUndottedFrameworkRule.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// 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.Globalization;
using System.IO;
using System.Linq;
using NuGet.Client;
using NuGet.Common;
using NuGet.ContentModel;
using NuGet.Frameworks;
using NuGet.Packaging.Core;
using NuGet.RuntimeModel;

namespace NuGet.Packaging.Rules
{
internal class InvalidUndottedFrameworkRule : IPackageRule
{
public string MessageFormat { get; }

public InvalidUndottedFrameworkRule(string messageFormat)
{
MessageFormat = messageFormat;
}

public IEnumerable<PackagingLogMessage> Validate(PackageArchiveReader builder)
{
var set = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (var file in builder.GetFiles().Select(t => PathUtility.GetPathWithDirectorySeparator(t)))
{
set.Add(file);
}

var managedCodeConventions = new ManagedCodeConventions(new RuntimeGraph());
var collection = new ContentItemCollection();
collection.Load(set.Select(path => path.Replace('\\', '/')).ToArray());

var patterns = managedCodeConventions.Patterns;

var frameworkPatterns = new List<PatternSet>()
{
patterns.RuntimeAssemblies,
patterns.CompileRefAssemblies,
patterns.CompileLibAssemblies,
patterns.NativeLibraries,
patterns.ResourceAssemblies,
patterns.MSBuildFiles,
patterns.ContentFiles,
patterns.ToolsAssemblies,
patterns.EmbedAssemblies,
patterns.MSBuildTransitiveFiles
};
var warnPaths = new HashSet<string>();

foreach (var pattern in frameworkPatterns)
{
IEnumerable<ContentItemGroup> targetedItemGroups = ContentExtractor.GetContentForPattern(collection, pattern);
foreach (ContentItemGroup group in targetedItemGroups)
{
foreach (ContentItem item in group.Items)
{
var frameworkString = (string)item.Properties["tfm_raw"];
var framework = (NuGetFramework)item.Properties["tfm"];
if (framework == null)
{
continue;
}

if (framework.Version.Major >= 5 &&
StringComparer.OrdinalIgnoreCase.Equals(FrameworkConstants.FrameworkIdentifiers.NetCoreApp, framework.Framework))
{
var dotIndex = frameworkString.IndexOf('.');
var dashIndex = frameworkString.IndexOf('-');
var frameworkVersionHasDots = (dashIndex > -1 && dotIndex > -1 && dotIndex < dashIndex) || (dashIndex == -1 && dotIndex > -1);
if (!frameworkVersionHasDots)
{
warnPaths.Add(item.Path);
}
}
}
}
}

var output = new List<PackagingLogMessage>();

if (warnPaths.Count > 0)
{
output.Add(CreatePackageIssue(string.Join(", ", warnPaths)));
}

return output;
}

private PackagingLogMessage CreatePackageIssue(string target)
{
return PackagingLogMessage.CreateWarning(
string.Format(CultureInfo.CurrentCulture, MessageFormat, target),
NuGetLogCode.NU5501);
}
}
}

1 change: 1 addition & 0 deletions src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public static class RuleSet
private static readonly ReadOnlyCollection<IPackageRule> _packageCreationRules = new ReadOnlyCollection<IPackageRule>(
new IPackageRule[] {
new InvalidFrameworkFolderRule(AnalysisResources.InvalidFrameworkWarning),
new InvalidUndottedFrameworkRule(AnalysisResources.InvalidUndottedFrameworkWarning),
new MisplacedAssemblyUnderLibRule(AnalysisResources.AssemblyDirectlyUnderLibWarning),
new MisplacedAssemblyOutsideLibRule(AnalysisResources.AssemblyOutsideLibWarning),
new MisplacedScriptFileRule(AnalysisResources.ScriptOutsideToolsWarning),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using NuGet.Commands;
using NuGet.Frameworks;
using NuGet.Packaging;
using NuGet.Packaging.Core;
using NuGet.Test.Utility;
using Xunit;

Expand Down Expand Up @@ -64,6 +65,137 @@ public void PackTaskLogic_ProducesBasicPackage()
}
}

[Fact]
public void PackTaskLogic_WhenUsingCsproj_WarnsMissingDot()
{
// Arrange
using (var testDir = TestDirectory.Create())
{
var tc = new TestContext(testDir);

tc.Request.PackageFiles = new MSBuildItem[] {
tc.AddContentToProject("", "def.dll", "hello world", new Dictionary<string, string>()
{
{"BuildAction", "None"},
{"Pack", "true" },
{"PackagePath", "lib/net50" }
})
};
tc.Request.ContentTargetFolders = new string[] { "lib", "content", "contentFiles" };

// Act
tc.BuildPackage();

// Assert
var logger = (TestLogger)tc.Request.Logger;
var messages = logger.WarningMessages.ToArray();
Assert.True(messages[0].Contains("lib/net50/def.dll"));
Assert.True(messages[0].Contains("include dot"));
}
}

[Fact]
public void PackTaskLogic_WarnsMissingDot_UsingNuspec()
{
// Arrange
using (var testDir = TestDirectory.Create())
{
// Arrange
string nuspec = @"<?xml version=""1.0"" encoding=""utf-8""?>
<package >
<metadata>
<id>bar</id>
<version>0.0.0</version>
<title>bartitle</title>
<authors>kat</authors>
<requireLicenseAcceptance>true</requireLicenseAcceptance>
<license type=""expression"">MIT</license>
<description>desc</description>
<releaseNotes>release notes</releaseNotes>
<copyright>msft</copyright>
<tags>foo bar</tags>
<dependencies>
<group targetFramework=""net50"">
<dependency id=""Newtonsoft.Json"" version=""12.0.3""/>
</group>
</dependencies>
</metadata>
</package>";
string nuspecPath = Path.Combine(testDir, "bar.nuspec");
File.WriteAllText(nuspecPath, nuspec);

var tc = new TestContext(testDir);
tc.Request.NuspecFile = nuspecPath;
tc.Request.NuspecBasePath = testDir;

var net50WinDllDir = Path.Combine(testDir, "lib", "net50");
var net50WinDllPath = Path.Combine(net50WinDllDir, "a.dll");

Directory.CreateDirectory(net50WinDllDir);
File.WriteAllBytes(net50WinDllPath, new byte[0]);

// Act
tc.BuildPackage();

// Assert
var logger = (TestLogger)tc.Request.Logger;
var messages = logger.WarningMessages.ToArray();
Assert.True(messages[0].Contains("net50"));
Assert.True(messages[0].Contains("include dots"));
}
}

[Fact]
public void PackTaskLogic_WhenDotInPlatformOnly_WarnsMissingDot_UsingNuspec()
{
// Arrange
using (var testDir = TestDirectory.Create())
{
// Arrange
string nuspec = @"<?xml version=""1.0"" encoding=""utf-8""?>
<package >
<metadata>
<id>bar</id>
<version>0.0.0</version>
<title>bartitle</title>
<authors>kat</authors>
<requireLicenseAcceptance>true</requireLicenseAcceptance>
<license type=""expression"">MIT</license>
<description>desc</description>
<releaseNotes>release notes</releaseNotes>
<copyright>msft</copyright>
<tags>foo bar</tags>
<dependencies>
<group targetFramework=""net50-windows7.0"">
<dependency id=""Newtonsoft.Json"" version=""12.0.3""/>
</group>
</dependencies>
</metadata>
</package>";
string nuspecPath = Path.Combine(testDir, "bar.nuspec");
File.WriteAllText(nuspecPath, nuspec);

var tc = new TestContext(testDir);
tc.Request.NuspecFile = nuspecPath;
tc.Request.NuspecBasePath = testDir;

var net50WinDllDir = Path.Combine(testDir, "lib", "net50-windows7.0");
var net50WinDllPath = Path.Combine(net50WinDllDir, "a.dll");

Directory.CreateDirectory(net50WinDllDir);
File.WriteAllBytes(net50WinDllPath, new byte[0]);

// Act
tc.BuildPackage();

// Assert
var logger = (TestLogger)tc.Request.Logger;
var messages = logger.WarningMessages.ToArray();
Assert.True(messages[0].Contains("net50-windows7.0"));
Assert.True(messages[0].Contains("include dots"));
}
}

[Fact]
public void PackTaskLogic_SplitsTags()
{
Expand Down

0 comments on commit e59ea92

Please sign in to comment.