From 4f248f5fc0ded9d6772e3f000d147f916fb264eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 17:23:55 +0000 Subject: [PATCH 1/8] Initial plan From f8301a38488fcd107beb63597423e981950b8f36 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 17:31:48 +0000 Subject: [PATCH 2/8] Add HasNonstandardId telemetry for non-standard package ID characters per feed source Co-authored-by: jeffkl <17556515+jeffkl@users.noreply.github.com> --- .../Telemetry/PackageSourceTelemetry.cs | 16 ++++ .../ProtocolDiagnosticNupkgCopiedEvent.cs | 14 ++++ .../LocalPackageArchiveDownloader.cs | 2 +- .../LocalV2FindPackageByIdResource.cs | 2 +- .../LocalV3FindPackageByIdResource.cs | 2 +- .../PublicAPI/net472/PublicAPI.Unshipped.txt | 2 + .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 + .../FindPackagesByIdNupkgDownloader.cs | 2 +- .../Telemetry/PackageSourceTelemetryTests.cs | 74 +++++++++++++++++++ 9 files changed, 112 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index bdc107c6b6e..57d87a521a9 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using NuGet.Common; @@ -29,6 +30,8 @@ public sealed class PackageSourceTelemetry : IDisposable internal const string EventName = "PackageSourceDiagnostics"; + private static readonly Regex s_nonstandardIdCharRegex = new Regex(@"[^A-Za-z0-9.\-]", RegexOptions.Compiled); + public enum TelemetryAction { Unknown = 0, @@ -203,9 +206,19 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve { data.NupkgCount++; data.NupkgSize += ncEvent.FileSize; + + if (!data.HasNonstandardId && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId)) + { + data.HasNonstandardId = true; + } } } + private static bool HasNonstandardCharacters(string packageId) + { + return s_nonstandardIdCharRegex.IsMatch(packageId); + } + public void Dispose() { ProtocolDiagnostics.HttpEvent -= ProtocolDiagnostics_HttpEvent; @@ -261,6 +274,7 @@ internal static async Task ToTelemetryAsync(Data data, SourceRep telemetry[PropertyNames.Duration.Total] = data.Resources.Values.Sum(r => r.duration.TotalMilliseconds); telemetry[PropertyNames.Nupkgs.Copied] = data.NupkgCount; telemetry[PropertyNames.Nupkgs.Bytes] = data.NupkgSize; + telemetry[PropertyNames.Nupkgs.HasNonstandardId] = data.HasNonstandardId; AddResourceProperties(telemetry, data.Resources); if (data.Http.Requests > 0) @@ -426,6 +440,7 @@ internal class Data internal HttpData Http { get; } internal int NupkgCount { get; set; } internal long NupkgSize { get; set; } + internal bool HasNonstandardId { get; set; } internal Data() { @@ -475,6 +490,7 @@ internal static class Nupkgs { internal const string Copied = "nupkgs.copied"; internal const string Bytes = "nupkgs.bytes"; + internal const string HasNonstandardId = "nupkgs.hasnonstandard_id"; } internal static class Resources diff --git a/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs b/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs index 6a72a8d8185..9f46f1e8ab4 100644 --- a/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs +++ b/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs @@ -8,12 +8,26 @@ public sealed class ProtocolDiagnosticNupkgCopiedEvent public string Source { get; } public long FileSize { get; } + /// + /// Gets the package ID of the copied nupkg, or if not available. + /// + public string PackageId { get; } + public ProtocolDiagnosticNupkgCopiedEvent( string source, long fileSize) + : this(source, fileSize, packageId: null) + { + } + + public ProtocolDiagnosticNupkgCopiedEvent( + string source, + long fileSize, + string packageId) { Source = source; FileSize = fileSize; + PackageId = packageId; } } } diff --git a/src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs b/src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs index 0ca015f1efe..ca249de6c6c 100644 --- a/src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs +++ b/src/NuGet.Core/NuGet.Protocol/LocalPackageArchiveDownloader.cs @@ -189,7 +189,7 @@ public async Task CopyNupkgFileToAsync(string destinationFilePath, Cancell await source.CopyToAsync(destination, bufferSize, cancellationToken); - ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length)); + ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(Source, destination.Length, _packageIdentity.Id)); return true; } diff --git a/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV2FindPackageByIdResource.cs b/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV2FindPackageByIdResource.cs index 33b1af863c1..e2cb9bc145a 100644 --- a/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV2FindPackageByIdResource.cs +++ b/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV2FindPackageByIdResource.cs @@ -176,7 +176,7 @@ public override async Task CopyNupkgToStreamAsync( using (var fileStream = File.OpenRead(info.Path)) { await fileStream.CopyToAsync(destination, cancellationToken); - ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length)); + ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id)); return true; } } diff --git a/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs b/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs index a0fd2c6fd32..fa8be822274 100644 --- a/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs +++ b/src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalV3FindPackageByIdResource.cs @@ -222,7 +222,7 @@ public override async Task CopyNupkgToStreamAsync( fileStream.Position = 0; await fileStream.CopyToAsync(destination, cancellationToken); - ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length)); + ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_source, destination.Length, id)); return true; } } diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt index 7dc5c58110b..938c0d98a7d 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1 +1,3 @@ #nullable enable +~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string +~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string source, long fileSize, string packageId) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 7dc5c58110b..938c0d98a7d 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1 +1,3 @@ #nullable enable +~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string +~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string source, long fileSize, string packageId) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs b/src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs index 6d138ffdf44..c8a2b0f5a21 100644 --- a/src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs +++ b/src/NuGet.Core/NuGet.Protocol/Utility/FindPackagesByIdNupkgDownloader.cs @@ -137,7 +137,7 @@ public async Task CopyNupkgToStreamAsync( try { await stream.CopyToAsync(destination, token); - ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_httpSource.PackageSource, destination.Length)); + ProtocolDiagnostics.RaiseEvent(new ProtocolDiagnosticNupkgCopiedEvent(_httpSource.PackageSource, destination.Length, identity.Id)); } catch when (!token.IsCancellationRequested) { diff --git a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs index 2a0ffc920dc..d30df622792 100644 --- a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs @@ -137,6 +137,79 @@ public void AddNupkgCopiedData_MultipleEvents_AccumulatesCorrectly() Assert.Equal(sizes.Sum(), result.NupkgSize); } + [Theory] + [InlineData("Newtonsoft.Json")] + [InlineData("NuGet.Protocol")] + [InlineData("My-Package.1")] + [InlineData("ALLCAPS")] + [InlineData("alllower")] + [InlineData("123Numeric")] + [InlineData("a")] + public void AddNupkgCopiedData_StandardPackageId_HasNonstandardIdIsFalse(string packageId) + { + // Arrange + var data = CreateDataDictionary(SampleSource); + var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, packageId); + + // Act + PackageSourceTelemetry.AddNupkgCopiedData(nce, data); + + // Assert + var result = Assert.Single(data).Value; + Assert.False(result.HasNonstandardId); + } + + [Theory] + [InlineData("My_Package")] + [InlineData("Package@1.0")] + [InlineData("Ünïcödé")] + [InlineData("Package Name")] + [InlineData("package+extra")] + public void AddNupkgCopiedData_NonstandardPackageId_HasNonstandardIdIsTrue(string packageId) + { + // Arrange + var data = CreateDataDictionary(SampleSource); + var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, packageId); + + // Act + PackageSourceTelemetry.AddNupkgCopiedData(nce, data); + + // Assert + var result = Assert.Single(data).Value; + Assert.True(result.HasNonstandardId); + } + + [Fact] + public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_HasNonstandardIdIsTrue() + { + // Arrange + var data = CreateDataDictionary(SampleSource); + + // Act + PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Standard.Package"), data); + PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Nonstandard_Package"), data); + PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Another.Standard"), data); + + // Assert + var result = Assert.Single(data).Value; + Assert.True(result.HasNonstandardId); + } + + [Fact] + public void AddNupkgCopiedData_NullPackageId_HasNonstandardIdIsFalse() + { + // Arrange + var data = CreateDataDictionary(SampleSource); + var nce = new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000); + + // Act + PackageSourceTelemetry.AddNupkgCopiedData(nce, data); + + // Assert + var result = Assert.Single(data).Value; + Assert.False(result.HasNonstandardId); + } + [Fact] public async Task AddData_IsThreadSafe() { @@ -308,6 +381,7 @@ public async Task ToTelemetry_WithData_CreatesTelemetryProperties(string package Assert.Equal(data.NupkgCount, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Copied]); Assert.Equal(data.NupkgSize, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Bytes]); + Assert.Equal(data.HasNonstandardId, result[PackageSourceTelemetry.PropertyNames.Nupkgs.HasNonstandardId]); Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]); foreach (var resource in data.Resources) From 5fbc829322b96785d92204c5f2e63dd74f9b6416 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 20:03:10 +0000 Subject: [PATCH 3/8] Rename HasNonstandardId to IdContainsNonAsciiCharacter throughout Co-authored-by: jeffkl <17556515+jeffkl@users.noreply.github.com> --- .../Telemetry/PackageSourceTelemetry.cs | 10 +++++----- .../Telemetry/PackageSourceTelemetryTests.cs | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index 57d87a521a9..e83b88396d5 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -207,9 +207,9 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve data.NupkgCount++; data.NupkgSize += ncEvent.FileSize; - if (!data.HasNonstandardId && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId)) + if (!data.IdContainsNonAsciiCharacter && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId)) { - data.HasNonstandardId = true; + data.IdContainsNonAsciiCharacter = true; } } } @@ -274,7 +274,7 @@ internal static async Task ToTelemetryAsync(Data data, SourceRep telemetry[PropertyNames.Duration.Total] = data.Resources.Values.Sum(r => r.duration.TotalMilliseconds); telemetry[PropertyNames.Nupkgs.Copied] = data.NupkgCount; telemetry[PropertyNames.Nupkgs.Bytes] = data.NupkgSize; - telemetry[PropertyNames.Nupkgs.HasNonstandardId] = data.HasNonstandardId; + telemetry[PropertyNames.Nupkgs.IdContainsNonAsciiCharacter] = data.IdContainsNonAsciiCharacter; AddResourceProperties(telemetry, data.Resources); if (data.Http.Requests > 0) @@ -440,7 +440,7 @@ internal class Data internal HttpData Http { get; } internal int NupkgCount { get; set; } internal long NupkgSize { get; set; } - internal bool HasNonstandardId { get; set; } + internal bool IdContainsNonAsciiCharacter { get; set; } internal Data() { @@ -490,7 +490,7 @@ internal static class Nupkgs { internal const string Copied = "nupkgs.copied"; internal const string Bytes = "nupkgs.bytes"; - internal const string HasNonstandardId = "nupkgs.hasnonstandard_id"; + internal const string IdContainsNonAsciiCharacter = "nupkgs.idcontainsnonasciicharacter"; } internal static class Resources diff --git a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs index d30df622792..7360bfe4ee9 100644 --- a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs @@ -145,7 +145,7 @@ public void AddNupkgCopiedData_MultipleEvents_AccumulatesCorrectly() [InlineData("alllower")] [InlineData("123Numeric")] [InlineData("a")] - public void AddNupkgCopiedData_StandardPackageId_HasNonstandardIdIsFalse(string packageId) + public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFalse(string packageId) { // Arrange var data = CreateDataDictionary(SampleSource); @@ -156,7 +156,7 @@ public void AddNupkgCopiedData_StandardPackageId_HasNonstandardIdIsFalse(string // Assert var result = Assert.Single(data).Value; - Assert.False(result.HasNonstandardId); + Assert.False(result.IdContainsNonAsciiCharacter); } [Theory] @@ -165,7 +165,7 @@ public void AddNupkgCopiedData_StandardPackageId_HasNonstandardIdIsFalse(string [InlineData("Ünïcödé")] [InlineData("Package Name")] [InlineData("package+extra")] - public void AddNupkgCopiedData_NonstandardPackageId_HasNonstandardIdIsTrue(string packageId) + public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAsciiCharacterIsTrue(string packageId) { // Arrange var data = CreateDataDictionary(SampleSource); @@ -176,11 +176,11 @@ public void AddNupkgCopiedData_NonstandardPackageId_HasNonstandardIdIsTrue(strin // Assert var result = Assert.Single(data).Value; - Assert.True(result.HasNonstandardId); + Assert.True(result.IdContainsNonAsciiCharacter); } [Fact] - public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_HasNonstandardIdIsTrue() + public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAsciiCharacterIsTrue() { // Arrange var data = CreateDataDictionary(SampleSource); @@ -192,11 +192,11 @@ public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_HasNonstandardIdIs // Assert var result = Assert.Single(data).Value; - Assert.True(result.HasNonstandardId); + Assert.True(result.IdContainsNonAsciiCharacter); } [Fact] - public void AddNupkgCopiedData_NullPackageId_HasNonstandardIdIsFalse() + public void AddNupkgCopiedData_NullPackageId_IdContainsNonAsciiCharacterIsFalse() { // Arrange var data = CreateDataDictionary(SampleSource); @@ -207,7 +207,7 @@ public void AddNupkgCopiedData_NullPackageId_HasNonstandardIdIsFalse() // Assert var result = Assert.Single(data).Value; - Assert.False(result.HasNonstandardId); + Assert.False(result.IdContainsNonAsciiCharacter); } [Fact] @@ -381,7 +381,7 @@ public async Task ToTelemetry_WithData_CreatesTelemetryProperties(string package Assert.Equal(data.NupkgCount, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Copied]); Assert.Equal(data.NupkgSize, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Bytes]); - Assert.Equal(data.HasNonstandardId, result[PackageSourceTelemetry.PropertyNames.Nupkgs.HasNonstandardId]); + Assert.Equal(data.IdContainsNonAsciiCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAsciiCharacter]); Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]); foreach (var resource in data.Resources) From 4f078f3a826cf4f7a39819f78cf2187e25af153d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 20:10:29 +0000 Subject: [PATCH 4/8] Replace Regex with ReadOnlySpan char check in HasNonstandardCharacters Co-authored-by: jeffkl <17556515+jeffkl@users.noreply.github.com> --- .../Telemetry/PackageSourceTelemetry.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index e83b88396d5..d412c216121 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -8,7 +8,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; -using System.Text.RegularExpressions; + using System.Threading; using System.Threading.Tasks; using NuGet.Common; @@ -30,8 +30,6 @@ public sealed class PackageSourceTelemetry : IDisposable internal const string EventName = "PackageSourceDiagnostics"; - private static readonly Regex s_nonstandardIdCharRegex = new Regex(@"[^A-Za-z0-9.\-]", RegexOptions.Compiled); - public enum TelemetryAction { Unknown = 0, @@ -216,7 +214,14 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve private static bool HasNonstandardCharacters(string packageId) { - return s_nonstandardIdCharRegex.IsMatch(packageId); + foreach (char c in packageId.AsSpan()) + { + if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + { + return true; + } + } + return false; } public void Dispose() From edc764f737bf9d1ae9e93330c31e8cbb93b48afe Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 30 Mar 2026 13:11:15 -0700 Subject: [PATCH 5/8] Add telemetry to RestoreCommand as well --- .../Telemetry/PackageSourceTelemetry.cs | 20 ++- .../RestoreCommand/RestoreCommand.cs | 19 +++ .../RestoreCommandTests.cs | 136 +++++++++++++++++- 3 files changed, 162 insertions(+), 13 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index d412c216121..2c5c672ce21 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -204,24 +204,20 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve { data.NupkgCount++; data.NupkgSize += ncEvent.FileSize; - - if (!data.IdContainsNonAsciiCharacter && ncEvent.PackageId != null && HasNonstandardCharacters(ncEvent.PackageId)) - { - data.IdContainsNonAsciiCharacter = true; - } + data.IdContainsNonAsciiCharacter = data.IdContainsNonAsciiCharacter || (ncEvent.PackageId != null && HasNonASCIICharacters(ncEvent.PackageId)); } - } - private static bool HasNonstandardCharacters(string packageId) - { - foreach (char c in packageId.AsSpan()) + bool HasNonASCIICharacters(string packageId) { - if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + foreach (char c in packageId.AsSpan()) { - return true; + if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + { + return true; + } } + return false; } - return false; } public void Dispose() diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 458a9bab325..44e85ee22fa 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -54,6 +54,7 @@ private readonly Dictionary } telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); + telemetry.TelemetryEvent[AnyPackageIdContainsNonASCIICharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonASCIICharacters(i.Key.Name)); telemetry.TelemetryEvent[RestoreSuccess] = success; } @@ -755,6 +757,23 @@ private async Task packagesLockFile, packagesLockFilePath, cacheFile); + + bool HasNonASCIICharacters(string packageId) + { + if (string.IsNullOrWhiteSpace(packageId)) + { + return false; + } + + foreach (char c in packageId.AsSpan()) + { + if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + { + return true; + } + } + return false; + } } /// Run NuGetAudit on the project's resolved restore graphs, and log messages and telemetry with the results. diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs index 25362f9497d..2e7d20cf111 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs @@ -3034,6 +3034,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( ["NoOpDuration"] = value => value.Should().NotBeNull(), ["TotalUniquePackagesCount"] = value => value.Should().Be(1), ["NewPackagesInstalledCount"] = value => value.Should().Be(1), + ["AnyPackageIdContainsNonASCIICharacters"] = value => value.Should().Be(false), ["EvaluateLockFileDuration"] = value => value.Should().NotBeNull(), ["CreateRestoreTargetGraphDuration"] = value => value.Should().NotBeNull(), ["GenerateRestoreGraphDuration"] = value => value.Should().NotBeNull(), @@ -3312,7 +3313,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); - projectInformationEvent.Count.Should().Be(49); + projectInformationEvent.Count.Should().Be(50); projectInformationEvent["RestoreSuccess"].Should().Be(true); projectInformationEvent["NoOpResult"].Should().Be(false); projectInformationEvent["TotalUniquePackagesCount"].Should().Be(2); @@ -3721,6 +3722,139 @@ private static PackageSpec CreatePackageSpec(List tf return packageSpec; } + [Fact] + public async Task ExecuteAsync_WithASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsFalse() + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + var projectName = "TestProject"; + var projectPath = Path.Combine(pathContext.SolutionRoot, projectName); + PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", "My.Package1"); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + new SimpleTestPackageContext("My.Package1", "1.0.0")); + var logger = new TestLogger(); + + var request = new TestRestoreRequest(packageSpec, new PackageSource[] { new PackageSource(pathContext.PackageSource) }, pathContext.UserPackagesFolder, logger) + { + LockFilePath = Path.Combine(projectPath, "project.assets.json"), + ProjectStyle = ProjectStyle.PackageReference, + }; + + // Set-up telemetry service - Important to set-up the service *after* the package source creation call as that emits telemetry! + var telemetryEvents = new ConcurrentQueue(); + var _telemetryService = new Mock(MockBehavior.Loose); + _telemetryService + .Setup(x => x.EmitTelemetryEvent(It.IsAny())) + .Callback(x => telemetryEvents.Enqueue(x)); + + TelemetryActivity.NuGetTelemetryService = _telemetryService.Object; + + // Act + var restoreCommand = new RestoreCommand(request); + RestoreResult result = await restoreCommand.ExecuteAsync(); + + // Assert + result.Success.Should().BeTrue(because: logger.ShowMessages()); + var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); + projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(false); + } + + [Theory] + [InlineData("My_Package")] // underscore + [InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A) + [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) + [InlineData("Package\u00E9")] // Latin small letter e with acute (U+00E9) + [InlineData("\u0410.Package")] // Cyrillic capital A (U+0410) + public async Task ExecuteAsync_WithNonASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId) + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + var projectName = "TestProject"; + var projectPath = Path.Combine(pathContext.SolutionRoot, projectName); + PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", packageId); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + new SimpleTestPackageContext(packageId, "1.0.0")); + var logger = new TestLogger(); + + var request = new TestRestoreRequest(packageSpec, new PackageSource[] { new PackageSource(pathContext.PackageSource) }, pathContext.UserPackagesFolder, logger) + { + LockFilePath = Path.Combine(projectPath, "project.assets.json"), + ProjectStyle = ProjectStyle.PackageReference, + }; + + // Set-up telemetry service - Important to set-up the service *after* the package source creation call as that emits telemetry! + var telemetryEvents = new ConcurrentQueue(); + var _telemetryService = new Mock(MockBehavior.Loose); + _telemetryService + .Setup(x => x.EmitTelemetryEvent(It.IsAny())) + .Callback(x => telemetryEvents.Enqueue(x)); + + TelemetryActivity.NuGetTelemetryService = _telemetryService.Object; + + // Act + var restoreCommand = new RestoreCommand(request); + RestoreResult result = await restoreCommand.ExecuteAsync(); + + // Assert + result.Success.Should().BeTrue(because: logger.ShowMessages()); + var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); + projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true); + } + + [Theory] + [InlineData("My_Package")] // underscore + [InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A) + [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) + public async Task ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId) + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + var projectName = "TestProject"; + var projectPath = Path.Combine(pathContext.SolutionRoot, projectName); + var asciiOnlyPkg = new SimpleTestPackageContext("Some.Package", "1.0.0"); + var nonASCIIPkg = new SimpleTestPackageContext(packageId, "1.0.0"); + asciiOnlyPkg.Dependencies.Add(nonASCIIPkg); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + asciiOnlyPkg, + nonASCIIPkg); + + PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", "Some.Package"); + var logger = new TestLogger(); + + var request = new TestRestoreRequest(packageSpec, new PackageSource[] { new PackageSource(pathContext.PackageSource) }, pathContext.UserPackagesFolder, logger) + { + LockFilePath = Path.Combine(projectPath, "project.assets.json"), + ProjectStyle = ProjectStyle.PackageReference, + }; + + // Set-up telemetry service - Important to set-up the service *after* the package source creation call as that emits telemetry! + var telemetryEvents = new ConcurrentQueue(); + var _telemetryService = new Mock(MockBehavior.Loose); + _telemetryService + .Setup(x => x.EmitTelemetryEvent(It.IsAny())) + .Callback(x => telemetryEvents.Enqueue(x)); + + TelemetryActivity.NuGetTelemetryService = _telemetryService.Object; + + // Act + var restoreCommand = new RestoreCommand(request); + RestoreResult result = await restoreCommand.ExecuteAsync(); + + // Assert + result.Success.Should().BeTrue(because: logger.ShowMessages()); + var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); + projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true); + } + private Task> DoWalkAsync(RemoteDependencyWalker walker, string name, NuGetFramework framework) { var range = new LibraryRange From 65d69481e0f4cab74605efcab206e448b2988d74 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Wed, 15 Apr 2026 09:41:55 -0700 Subject: [PATCH 6/8] Feedback --- .../Telemetry/PackageSourceTelemetry.cs | 18 +++++++++++------ .../RestoreCommand/RestoreCommand.cs | 19 +++++++++--------- .../Telemetry/PackageSourceTelemetryTests.cs | 18 ++++++++--------- .../RestoreCommandTests.cs | 20 +++++++++---------- 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index 2c5c672ce21..15a675c750f 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -204,19 +204,25 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve { data.NupkgCount++; data.NupkgSize += ncEvent.FileSize; - data.IdContainsNonAsciiCharacter = data.IdContainsNonAsciiCharacter || (ncEvent.PackageId != null && HasNonASCIICharacters(ncEvent.PackageId)); + data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter || (ncEvent.PackageId != null && HasNonAlphanumericDotDashOrUnderscoreCharacters(ncEvent.PackageId)); } - bool HasNonASCIICharacters(string packageId) + bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId) { foreach (char c in packageId.AsSpan()) { - if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + if (!IsCharacterAlphanumericDotDashOrUnderscore(c)) { return true; } } + return false; + + bool IsCharacterAlphanumericDotDashOrUnderscore(char c) + { + return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_'; + } } } @@ -275,7 +281,7 @@ internal static async Task ToTelemetryAsync(Data data, SourceRep telemetry[PropertyNames.Duration.Total] = data.Resources.Values.Sum(r => r.duration.TotalMilliseconds); telemetry[PropertyNames.Nupkgs.Copied] = data.NupkgCount; telemetry[PropertyNames.Nupkgs.Bytes] = data.NupkgSize; - telemetry[PropertyNames.Nupkgs.IdContainsNonAsciiCharacter] = data.IdContainsNonAsciiCharacter; + telemetry[PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter] = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter; AddResourceProperties(telemetry, data.Resources); if (data.Http.Requests > 0) @@ -441,7 +447,7 @@ internal class Data internal HttpData Http { get; } internal int NupkgCount { get; set; } internal long NupkgSize { get; set; } - internal bool IdContainsNonAsciiCharacter { get; set; } + internal bool IdContainsNonAlphanumericDotDashOrUnderscoreCharacter { get; set; } internal Data() { @@ -491,7 +497,7 @@ internal static class Nupkgs { internal const string Copied = "nupkgs.copied"; internal const string Bytes = "nupkgs.bytes"; - internal const string IdContainsNonAsciiCharacter = "nupkgs.idcontainsnonasciicharacter"; + internal const string IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = "nupkgs.idcontainsNonAlphanumericDotDashOrUnderscorecharacter"; } internal static class Resources diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 44e85ee22fa..bf3146e6ef8 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -54,7 +54,7 @@ private readonly Dictionary } telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); - telemetry.TelemetryEvent[AnyPackageIdContainsNonASCIICharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonASCIICharacters(i.Key.Name)); + telemetry.TelemetryEvent[AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonAlphanumericDotDashOrUnderscoreCharacters(i.Key.Name)); telemetry.TelemetryEvent[RestoreSuccess] = success; } @@ -758,21 +758,22 @@ private async Task packagesLockFilePath, cacheFile); - bool HasNonASCIICharacters(string packageId) + bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId) { - if (string.IsNullOrWhiteSpace(packageId)) - { - return false; - } - foreach (char c in packageId.AsSpan()) { - if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-')) + if (!IsCharacterAlphanumericDotDashOrUnderscore(c)) { return true; } } + return false; + + bool IsCharacterAlphanumericDotDashOrUnderscore(char c) + { + return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_'; + } } } diff --git a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs index 7360bfe4ee9..e55c2b146a7 100644 --- a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs @@ -145,7 +145,7 @@ public void AddNupkgCopiedData_MultipleEvents_AccumulatesCorrectly() [InlineData("alllower")] [InlineData("123Numeric")] [InlineData("a")] - public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFalse(string packageId) + public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse(string packageId) { // Arrange var data = CreateDataDictionary(SampleSource); @@ -156,7 +156,7 @@ public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFa // Assert var result = Assert.Single(data).Value; - Assert.False(result.IdContainsNonAsciiCharacter); + Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter); } [Theory] @@ -165,7 +165,7 @@ public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFa [InlineData("Ünïcödé")] [InlineData("Package Name")] [InlineData("package+extra")] - public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAsciiCharacterIsTrue(string packageId) + public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue(string packageId) { // Arrange var data = CreateDataDictionary(SampleSource); @@ -176,11 +176,11 @@ public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAsciiCharacterI // Assert var result = Assert.Single(data).Value; - Assert.True(result.IdContainsNonAsciiCharacter); + Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter); } [Fact] - public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAsciiCharacterIsTrue() + public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue() { // Arrange var data = CreateDataDictionary(SampleSource); @@ -192,11 +192,11 @@ public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAscii // Assert var result = Assert.Single(data).Value; - Assert.True(result.IdContainsNonAsciiCharacter); + Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter); } [Fact] - public void AddNupkgCopiedData_NullPackageId_IdContainsNonAsciiCharacterIsFalse() + public void AddNupkgCopiedData_NullPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse() { // Arrange var data = CreateDataDictionary(SampleSource); @@ -207,7 +207,7 @@ public void AddNupkgCopiedData_NullPackageId_IdContainsNonAsciiCharacterIsFalse( // Assert var result = Assert.Single(data).Value; - Assert.False(result.IdContainsNonAsciiCharacter); + Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter); } [Fact] @@ -381,7 +381,7 @@ public async Task ToTelemetry_WithData_CreatesTelemetryProperties(string package Assert.Equal(data.NupkgCount, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Copied]); Assert.Equal(data.NupkgSize, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Bytes]); - Assert.Equal(data.IdContainsNonAsciiCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAsciiCharacter]); + Assert.Equal(data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter]); Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]); foreach (var resource in data.Resources) diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs index 2e7d20cf111..c7b29c1e090 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs @@ -3034,7 +3034,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( ["NoOpDuration"] = value => value.Should().NotBeNull(), ["TotalUniquePackagesCount"] = value => value.Should().Be(1), ["NewPackagesInstalledCount"] = value => value.Should().Be(1), - ["AnyPackageIdContainsNonASCIICharacters"] = value => value.Should().Be(false), + ["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"] = value => value.Should().Be(false), ["EvaluateLockFileDuration"] = value => value.Should().NotBeNull(), ["CreateRestoreTargetGraphDuration"] = value => value.Should().NotBeNull(), ["GenerateRestoreGraphDuration"] = value => value.Should().NotBeNull(), @@ -3723,7 +3723,7 @@ private static PackageSpec CreatePackageSpec(List tf } [Fact] - public async Task ExecuteAsync_WithASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsFalse() + public async Task ExecuteAsync_WithASCIIPackageId_AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharactersIsFalse() { // Arrange using var pathContext = new SimpleTestPathContext(); @@ -3759,7 +3759,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert result.Success.Should().BeTrue(because: logger.ShowMessages()); var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); - projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(false); + projectInformationEvent["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"].Should().Be(false); } [Theory] @@ -3768,7 +3768,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) [InlineData("Package\u00E9")] // Latin small letter e with acute (U+00E9) [InlineData("\u0410.Package")] // Cyrillic capital A (U+0410) - public async Task ExecuteAsync_WithNonASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId) + public async Task ExecuteAsync_WithNonAlphanumericDotDashOrUnderscorePackageId_AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharactersIsTrue(string packageId) { // Arrange using var pathContext = new SimpleTestPathContext(); @@ -3804,28 +3804,28 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert result.Success.Should().BeTrue(because: logger.ShowMessages()); var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); - projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true); + projectInformationEvent["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"].Should().Be(true); } [Theory] [InlineData("My_Package")] // underscore [InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A) [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) - public async Task ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId) + public async Task ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharactersIsTrue(string packageId) { // Arrange using var pathContext = new SimpleTestPathContext(); var projectName = "TestProject"; var projectPath = Path.Combine(pathContext.SolutionRoot, projectName); var asciiOnlyPkg = new SimpleTestPackageContext("Some.Package", "1.0.0"); - var nonASCIIPkg = new SimpleTestPackageContext(packageId, "1.0.0"); - asciiOnlyPkg.Dependencies.Add(nonASCIIPkg); + var NonAlphanumericDotDashOrUnderscorePkg = new SimpleTestPackageContext(packageId, "1.0.0"); + asciiOnlyPkg.Dependencies.Add(NonAlphanumericDotDashOrUnderscorePkg); await SimpleTestPackageUtility.CreateFolderFeedV3Async( pathContext.PackageSource, PackageSaveMode.Defaultv3, asciiOnlyPkg, - nonASCIIPkg); + NonAlphanumericDotDashOrUnderscorePkg); PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", "Some.Package"); var logger = new TestLogger(); @@ -3852,7 +3852,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert result.Success.Should().BeTrue(because: logger.ShowMessages()); var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation")); - projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true); + projectInformationEvent["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"].Should().Be(true); } private Task> DoWalkAsync(RemoteDependencyWalker walker, string name, NuGetFramework framework) From b732fb09f00251d71c3991dff5e1cf4611f08f0c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 18:14:28 +0000 Subject: [PATCH 7/8] Fix nullability annotations in ProtocolDiagnosticNupkgCopiedEvent and PublicAPI files Agent-Logs-Url: https://github.com/NuGet/NuGet.Client/sessions/99e74a70-9115-4726-adbb-4b262f30d6da Co-authored-by: nkolev92 <2878341+nkolev92@users.noreply.github.com> --- .../Events/ProtocolDiagnosticNupkgCopiedEvent.cs | 4 ++-- .../NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt | 4 ++-- .../NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs b/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs index 9f46f1e8ab4..0b2cc79592b 100644 --- a/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs +++ b/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs @@ -11,7 +11,7 @@ public sealed class ProtocolDiagnosticNupkgCopiedEvent /// /// Gets the package ID of the copied nupkg, or if not available. /// - public string PackageId { get; } + public string? PackageId { get; } public ProtocolDiagnosticNupkgCopiedEvent( string source, @@ -23,7 +23,7 @@ public ProtocolDiagnosticNupkgCopiedEvent( public ProtocolDiagnosticNupkgCopiedEvent( string source, long fileSize, - string packageId) + string? packageId) { Source = source; FileSize = fileSize; diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt index 938c0d98a7d..dd5731e4956 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,3 +1,3 @@ #nullable enable -~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string -~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string source, long fileSize, string packageId) -> void +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string? +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string? packageId) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 938c0d98a7d..dd5731e4956 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,3 +1,3 @@ #nullable enable -~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string -~NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string source, long fileSize, string packageId) -> void +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string? +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string? packageId) -> void From 6cb9d5681afc0cc1a94fa7874a7657094e9210f7 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 21 Apr 2026 14:57:57 -0700 Subject: [PATCH 8/8] Feedback, update tests to allow underscore --- .../Telemetry/PackageSourceTelemetry.cs | 2 +- .../Events/ProtocolDiagnosticNupkgCopiedEvent.cs | 8 ++++---- .../PublicAPI/net472/PublicAPI.Unshipped.txt | 4 ++-- .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 4 ++-- .../Telemetry/PackageSourceTelemetryTests.cs | 5 ++--- .../RestoreCommandTests/RestoreCommandTests.cs | 2 -- 6 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs index 15a675c750f..330aea74d7c 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs @@ -204,7 +204,7 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve { data.NupkgCount++; data.NupkgSize += ncEvent.FileSize; - data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter || (ncEvent.PackageId != null && HasNonAlphanumericDotDashOrUnderscoreCharacters(ncEvent.PackageId)); + data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter || (ncEvent.PackageId.Length > 0 && HasNonAlphanumericDotDashOrUnderscoreCharacters(ncEvent.PackageId)); } bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId) diff --git a/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs b/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs index 0b2cc79592b..425b8ae2230 100644 --- a/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs +++ b/src/NuGet.Core/NuGet.Protocol/Events/ProtocolDiagnosticNupkgCopiedEvent.cs @@ -9,21 +9,21 @@ public sealed class ProtocolDiagnosticNupkgCopiedEvent public long FileSize { get; } /// - /// Gets the package ID of the copied nupkg, or if not available. + /// Gets the package ID of the copied nupkg. /// - public string? PackageId { get; } + public string PackageId { get; } public ProtocolDiagnosticNupkgCopiedEvent( string source, long fileSize) - : this(source, fileSize, packageId: null) + : this(source, fileSize, packageId: string.Empty) { } public ProtocolDiagnosticNupkgCopiedEvent( string source, long fileSize, - string? packageId) + string packageId) { Source = source; FileSize = fileSize; diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt index dd5731e4956..1042d399efe 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,3 +1,3 @@ #nullable enable -NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string? -NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string? packageId) -> void +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string! +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string! packageId) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt index dd5731e4956..1042d399efe 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,3 +1,3 @@ #nullable enable -NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string? -NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string? packageId) -> void +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.PackageId.get -> string! +NuGet.Protocol.Events.ProtocolDiagnosticNupkgCopiedEvent.ProtocolDiagnosticNupkgCopiedEvent(string! source, long fileSize, string! packageId) -> void diff --git a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs index e55c2b146a7..790dac6647d 100644 --- a/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs @@ -160,7 +160,6 @@ public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAlphanumericDotDas } [Theory] - [InlineData("My_Package")] [InlineData("Package@1.0")] [InlineData("Ünïcödé")] [InlineData("Package Name")] @@ -187,7 +186,7 @@ public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAlpha // Act PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Standard.Package"), data); - PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Nonstandard_Package"), data); + PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Nonstandard@Package"), data); PackageSourceTelemetry.AddNupkgCopiedData(new ProtocolDiagnosticNupkgCopiedEvent(SampleSource, fileSize: 1000, "Another.Standard"), data); // Assert @@ -196,7 +195,7 @@ public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAlpha } [Fact] - public void AddNupkgCopiedData_NullPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse() + public void AddNupkgCopiedData_EmptyPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse() { // Arrange var data = CreateDataDictionary(SampleSource); diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs index c7b29c1e090..ded805eae6c 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs @@ -3763,7 +3763,6 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Theory] - [InlineData("My_Package")] // underscore [InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A) [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) [InlineData("Package\u00E9")] // Latin small letter e with acute (U+00E9) @@ -3808,7 +3807,6 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Theory] - [InlineData("My_Package")] // underscore [InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A) [InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1) public async Task ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharactersIsTrue(string packageId)