From f2e778c5e3200609045cccc72b605692192dd554 Mon Sep 17 00:00:00 2001 From: Ryan Hurey Date: Wed, 29 Jul 2026 20:47:10 -0400 Subject: [PATCH 1/5] Fix external type resolution failing when the type's dependencies are not deployed with the generator --- .../src/TypeFactory.cs | 15 +- .../ExternalTypeReferenceResolver.cs | 108 ++++-- .../src/Utilities/NugetAssemblyResolver.cs | 356 ++++++++++++++++++ .../src/Utilities/NugetPackageResolver.cs | 87 +++++ .../ExternalTypeReferenceResolverTests.cs | 75 +++- .../DependencyPackageSource.cs | 17 + .../DependentPackageSource.cs | 22 ++ .../test/common/FakeNuGetPackage.cs | 29 +- 8 files changed, 670 insertions(+), 39 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependencyPackageSource.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependentPackageSource.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs index a29b1f99c1d..f1d05ed6adc 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/TypeFactory.cs @@ -358,13 +358,14 @@ protected virtual ModelFactoryProvider CreateModelFactoryCore(IEnumerable= {externalProperties.MinVersion}) was not found in the NuGet cache or any configured feed"; + // Prefer the concrete reason recorded by the resolver (missing package, unloadable assembly, + // unsatisfied dependency, ...) so the message points at the actual problem. + var details = ExternalTypeReferenceResolver.GetFailureReason(externalProperties) + ?? (string.IsNullOrEmpty(externalProperties.Package) + ? "no package metadata was provided" + : string.IsNullOrEmpty(externalProperties.MinVersion) + ? $"package '{externalProperties.Package}' was not found in the NuGet cache or any configured feed" + : $"package '{externalProperties.Package}' (>= {externalProperties.MinVersion}) was not found in the NuGet cache or any configured feed"); CodeModelGenerator.Instance.Emitter.ReportDiagnostic( "unsupported-external-type", diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs index f7849a27c78..2cbd8b3fcbe 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs @@ -38,11 +38,21 @@ internal static class ExternalTypeReferenceResolver // the entries are released when the generator is collected. private static readonly ConditionalWeakTable _cacheStates = new(); + /// + /// Outcome of a single resolution attempt. is null when resolution failed, in + /// which case explains why so callers can report an accurate diagnostic + /// (a missing package and an unloadable type are very different problems to debug). + /// + private sealed record ResolutionResult(Type? Type, string? FailureReason) + { + public static readonly ResolutionResult NotAttempted = new(null, null); + } + private sealed class CacheState { - // Cached resolution per (Package, Identity, MinVersion) key. Value is the loaded Type, or - // null if resolution was attempted and failed (so we don't keep re-trying). - public readonly ConcurrentDictionary> Resolved = + // Cached resolution per (Package, Identity, MinVersion) key. Value carries the loaded Type, or + // the reason resolution failed (so we don't keep re-trying). + public readonly ConcurrentDictionary> Resolved = new(StringComparer.Ordinal); // Tracks assembly file paths that have already been added as Roslyn metadata references, so @@ -118,10 +128,27 @@ public static async Task ResolveAllAsync() var state = GetState(CodeModelGenerator.Instance); var key = MakeKey(external); - var lazy = state.Resolved.GetOrAdd(key, _ => new Lazy( - () => Task.Run(() => ResolveAsync(external)).GetAwaiter().GetResult(), + var lazy = state.Resolved.GetOrAdd(key, _ => new Lazy( + () => Task.Run(() => ResolveResultAsync(external)).GetAwaiter().GetResult(), LazyThreadSafetyMode.ExecutionAndPublication)); - return lazy.Value; + return lazy.Value.Type; + } + + /// + /// Returns the reason the most recent resolution attempt for failed, or + /// null when it succeeded or was never attempted. + /// + public static string? GetFailureReason(InputExternalTypeMetadata? external) + { + if (external == null) + { + return null; + } + + var state = GetState(CodeModelGenerator.Instance); + return state.Resolved.TryGetValue(MakeKey(external), out var lazy) && lazy.IsValueCreated + ? lazy.Value.FailureReason + : null; } /// @@ -129,14 +156,27 @@ public static async Task ResolveAllAsync() /// internal static void Reset() { - // Drop the entry entirely; the next call rebuilds a fresh CacheState on demand. - _cacheStates.Remove(CodeModelGenerator.Instance); + try + { + // Drop the entry entirely; the next call rebuilds a fresh CacheState on demand. + _cacheStates.Remove(CodeModelGenerator.Instance); + } + catch (InvalidOperationException) + { + // No generator is installed yet, so there is no per-generator state to drop. This happens + // when Reset runs before the first generator is loaded (e.g. a test fixture's SetUp). + } + + NugetAssemblyResolver.Reset(); } private static string MakeKey(InputExternalTypeMetadata external) => $"{external.Package}|{external.Identity}|{external.MinVersion ?? string.Empty}"; private static async Task ResolveAsync(InputExternalTypeMetadata external) + => (await ResolveResultAsync(external)).Type; + + private static async Task ResolveResultAsync(InputExternalTypeMetadata external) { var generator = CodeModelGenerator.Instance; var state = GetState(generator); @@ -162,10 +202,14 @@ private static string MakeKey(InputExternalTypeMetadata external) => catch (Exception ex) { generator.Emitter?.Debug($"Could not load NuGet settings while resolving '{external.Identity}': {ex.Message}"); - CacheResult(state, key, null); - return null; + return CacheResult(state, key, new ResolutionResult(null, $"NuGet settings could not be loaded ({ex.Message})")); } + // The external assembly is loaded into the default AssemblyLoadContext, which cannot see the + // package's own dependencies. Install the NuGet probing hook before loading anything so that + // base types and interfaces declared in dependency packages can be resolved. + NugetAssemblyResolver.EnsureRegistered(globalPackagesFolder); + string? assemblyPath = NugetPackageResolver.FindPackageAssembly( globalPackagesFolder, external.Package!, external.MinVersion); @@ -197,21 +241,30 @@ private static string MakeKey(InputExternalTypeMetadata external) => if (assemblyPath == null || !File.Exists(assemblyPath)) { - CacheResult(state, key, null); - return null; + var versionQualifier = string.IsNullOrEmpty(external.MinVersion) + ? string.Empty + : $" (>= {external.MinVersion})"; + return CacheResult(state, key, new ResolutionResult( + null, + $"package '{external.Package}'{versionQualifier} was not found in the NuGet cache or any configured feed")); } + // Pin every package in this package's dependency closure before loading it, so the resolving + // hook binds dependencies to the versions NuGet selected rather than guessing from assembly + // versions (which are routinely lower than the package versions that ship them). + NugetAssemblyResolver.RegisterPackageClosure(globalPackagesFolder, assemblyPath); + byte[] assemblyBytes; try { assemblyBytes = await File.ReadAllBytesAsync(assemblyPath).ConfigureAwait(false); - } - catch (Exception ex) + } catch (Exception ex) { generator.Emitter?.Debug( $"Failed to read assembly '{assemblyPath}' for external type '{external.Identity}': {ex.Message}"); - CacheResult(state, key, null); - return null; + return CacheResult(state, key, new ResolutionResult( + null, + $"assembly '{assemblyPath}' could not be read ({ex.Message})")); } Type? loadedType; @@ -226,16 +279,20 @@ private static string MakeKey(InputExternalTypeMetadata external) => { generator.Emitter?.Debug( $"Failed to load assembly '{assemblyPath}' for external type '{external.Identity}': {ex.Message}"); - CacheResult(state, key, null); - return null; + return CacheResult(state, key, new ResolutionResult( + null, + $"assembly '{assemblyPath}' could not be loaded ({ex.Message})")); } if (loadedType == null) { + // Either the type genuinely isn't in the assembly, or one of its dependencies could not be + // satisfied even with the NuGet probing hook installed - GetType reports both as null. generator.Emitter?.Debug( - $"Assembly '{assemblyPath}' does not contain external type '{external.Identity}'."); - CacheResult(state, key, null); - return null; + $"Assembly '{assemblyPath}' does not declare external type '{external.Identity}', or one of its dependencies could not be resolved."); + return CacheResult(state, key, new ResolutionResult( + null, + $"assembly '{assemblyPath}' was loaded but does not declare the type, or one of the type's dependencies could not be resolved")); } // Register the dll as a Roslyn metadata reference exactly once per assembly path so that @@ -248,18 +305,19 @@ private static string MakeKey(InputExternalTypeMetadata external) => $"Added metadata reference for external type '{external.Identity}' from {assemblyPath}"); } - CacheResult(state, key, loadedType); - return loadedType; + CacheResult(state, key, new ResolutionResult(loadedType, null)); + return new ResolutionResult(loadedType, null); } - private static void CacheResult(CacheState state, string key, Type? result) + private static ResolutionResult CacheResult(CacheState state, string key, ResolutionResult result) { // Replace whatever Lazy is in the slot with one that already has the computed value. // AddOrUpdate ensures we don't lose a concurrent write. - var precomputed = new Lazy(() => result, LazyThreadSafetyMode.PublicationOnly); + var precomputed = new Lazy(() => result, LazyThreadSafetyMode.PublicationOnly); // Force the value to be materialized so IsValueCreated is true. _ = precomputed.Value; state.Resolved.AddOrUpdate(key, precomputed, (_, _) => precomputed); + return result; } private static void CollectExternalTypes(InputLibrary library, IDictionary collected) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs new file mode 100644 index 00000000000..c5e1f3f40c7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs @@ -0,0 +1,356 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Runtime.Loader; +using System.Threading; +using Microsoft.CodeAnalysis; +using NuGet.Frameworks; +using NuGet.Packaging; +using NuGet.Packaging.Core; +using NuGet.Versioning; + +namespace Microsoft.TypeSpec.Generator.Utilities +{ + /// + /// Satisfies the transitive assembly references of external-type assemblies loaded by + /// by probing the NuGet global packages folder. + /// + /// + /// External-type assemblies are loaded with into the default + /// , which resolves dependencies only from the generator's own + /// trusted-platform-assemblies list. A package such as Azure.AI.Extensions.OpenAI depends on + /// Azure.Core and OpenAI, neither of which the generator references, so without this + /// hook Assembly.GetType(identity, throwOnError: false) silently returns null for any + /// type whose base type lives in a dependency. The external type then appears unresolvable and is + /// generated instead of referenced. + /// + internal static class NugetAssemblyResolver + { + // Cached resolution per assembly simple name. A null value means resolution was attempted and + // failed, so repeated references to the same missing dependency don't re-probe the file system. + private static readonly ConcurrentDictionary _resolved = + new(StringComparer.OrdinalIgnoreCase); + + // Tracks assembly paths already registered as Roslyn metadata references. + private static readonly ConcurrentDictionary _registeredReferences = + new(StringComparer.OrdinalIgnoreCase); + + // Package id -> package version, for every package in the dependency closure of an external-type + // package. Populated from .nuspec files so dependencies bind to the version NuGet would have + // chosen rather than a version guessed from the referencing assembly's version number. + private static readonly ConcurrentDictionary _closureVersions = + new(StringComparer.OrdinalIgnoreCase); + + // Package version directories whose .nuspec has already been walked. + private static readonly ConcurrentDictionary _walkedPackages = + new(StringComparer.OrdinalIgnoreCase); + + // Guards against a dependency cycle re-entering resolution for the same name on this thread. + [ThreadStatic] + private static HashSet? _inProgress; + + private static string? _globalPackagesFolder; + private static int _hookInstalled; + + /// + /// Installs the hook (once per process) and records the + /// NuGet global packages folder to probe. Safe to call repeatedly. + /// + public static void EnsureRegistered(string globalPackagesFolder) + { + if (string.IsNullOrEmpty(globalPackagesFolder)) + { + return; + } + + Volatile.Write(ref _globalPackagesFolder, globalPackagesFolder); + + if (Interlocked.Exchange(ref _hookInstalled, 1) == 0) + { + AssemblyLoadContext.Default.Resolving += OnResolving; + } + } + + /// + /// Records the dependency closure of the package that ships by + /// walking its .nuspec transitively, so the resolving hook can bind each dependency to the + /// package version NuGet would have selected. + /// + /// + /// This is required because an assembly reference carries an assembly version, which for many + /// packages is deliberately lower than the package version that ships it. System.ClientModel + /// 1.14.0, for example, ships assembly version 1.9.0.0; treating that as a package version resolves the + /// unrelated 1.9.0 package and produces a at type-load time. + /// + public static void RegisterPackageClosure(string globalPackagesFolder, string packageAssemblyPath) + { + if (string.IsNullOrEmpty(globalPackagesFolder) || string.IsNullOrEmpty(packageAssemblyPath)) + { + return; + } + + // Layout is {globalPackagesFolder}/{id}/{version}/lib/{tfm}/{assembly}.dll + var versionDir = Path.GetDirectoryName(Path.GetDirectoryName(Path.GetDirectoryName(packageAssemblyPath))); + var packageDir = Path.GetDirectoryName(versionDir); + if (versionDir == null || packageDir == null) + { + return; + } + + WalkPackage(globalPackagesFolder, Path.GetFileName(packageDir), Path.GetFileName(versionDir)); + } + + private static void WalkPackage(string globalPackagesFolder, string packageId, string version) + { + if (!NuGetVersion.TryParse(version, out var parsedVersion)) + { + return; + } + + // Mirror NuGet's "highest version wins" unification when the same package appears more than once. + var winner = _closureVersions.AddOrUpdate( + packageId, + parsedVersion, + (_, existing) => existing >= parsedVersion ? existing : parsedVersion); + if (winner != parsedVersion) + { + return; + } + + if (!_walkedPackages.TryAdd($"{packageId}/{parsedVersion.ToNormalizedString()}", 0)) + { + return; + } + + var versionDir = Path.Combine(globalPackagesFolder, packageId.ToLowerInvariant(), version.ToLowerInvariant()); + string? nuspecPath; + try + { + nuspecPath = Directory.Exists(versionDir) + ? Directory.EnumerateFiles(versionDir, "*.nuspec").FirstOrDefault() + : null; + } + catch (Exception ex) + { + Debug($"Failed to enumerate '{versionDir}' while walking package dependencies: {ex.Message}"); + return; + } + + if (nuspecPath == null) + { + Debug($"No .nuspec found for package '{packageId}' {version}; its dependencies will not be pinned."); + return; + } + + IReadOnlyList groups; + try + { + groups = new NuspecReader(nuspecPath).GetDependencyGroups().ToList(); + } + catch (Exception ex) + { + Debug($"Failed to read '{nuspecPath}': {ex.Message}"); + return; + } + + foreach (var dependency in SelectNearestGroup(groups)) + { + var dependencyVersion = dependency.VersionRange?.MinVersion; + if (dependencyVersion != null) + { + WalkPackage(globalPackagesFolder, dependency.Id, dependencyVersion.ToNormalizedString()); + } + } + } + + private static IEnumerable SelectNearestGroup(IReadOnlyList groups) + { + if (groups.Count == 0) + { + return Array.Empty(); + } + + var nearest = new FrameworkReducer().GetNearest(NugetPackageResolver.CurrentFramework, groups.Select(g => g.TargetFramework)); + var group = nearest != null + ? groups.FirstOrDefault(g => g.TargetFramework.Equals(nearest)) + : null; + + return group?.Packages ?? Array.Empty(); + } + + /// + /// Clears the cached dependency resolutions. Loaded assemblies cannot be unloaded from the default + /// context, so this only resets the probe/metadata-reference bookkeeping. + /// + internal static void Reset() + { + _resolved.Clear(); + _registeredReferences.Clear(); + _closureVersions.Clear(); + _walkedPackages.Clear(); + } + + private static Assembly? OnResolving(AssemblyLoadContext context, AssemblyName name) + { + var simpleName = name.Name; + var globalPackagesFolder = Volatile.Read(ref _globalPackagesFolder); + if (string.IsNullOrEmpty(simpleName) || string.IsNullOrEmpty(globalPackagesFolder)) + { + return null; + } + + if (_resolved.TryGetValue(simpleName, out var cached)) + { + return cached; + } + + var inProgress = _inProgress ??= new HashSet(StringComparer.OrdinalIgnoreCase); + if (!inProgress.Add(simpleName)) + { + return null; + } + + try + { + // Prefer an assembly the host already provides, even when its version is lower than the one + // requested. The default context only asks us to resolve a name it could not satisfy itself, but + // it will still satisfy *other*, lower-versioned requests for that same name from its own + // assemblies. Loading a second copy from the cache would therefore leave two assemblies with the + // same simple name in the process, and any type they both declare (System.Memory.Data's + // System.BinaryData, for example) would exist twice, so signatures mentioning it fail to bind + // with a MissingMethodException. Reusing the host's copy keeps exactly one identity per name. + var assembly = UseHostAssembly(simpleName) ?? Probe(globalPackagesFolder, simpleName, name.Version); + _resolved[simpleName] = assembly; + return assembly; + } + finally + { + inProgress.Remove(simpleName); + } + } + + /// + /// Returns the host's own copy of when it has one, ignoring version. + /// + private static Assembly? UseHostAssembly(string simpleName) + { + foreach (var loaded in AssemblyLoadContext.Default.Assemblies) + { + if (string.Equals(loaded.GetName().Name, simpleName, StringComparison.OrdinalIgnoreCase)) + { + Debug($"Reusing already-loaded assembly '{loaded.GetName()}' for dependency '{simpleName}'."); + return loaded; + } + } + + // Not loaded yet, but it may still be in the generator's deployment. Ask for it by simple name only, + // which matches any version there; a version-qualified request is what failed to get us here. + try + { + var assembly = AssemblyLoadContext.Default.LoadFromAssemblyName(new AssemblyName(simpleName)); + Debug($"Resolved dependency '{simpleName}' to the generator's own '{assembly.GetName()}'."); + return assembly; + } + catch (Exception) + { + // The generator does not deploy this assembly; fall back to the NuGet cache. + return null; + } + } + + private static Assembly? Probe(string globalPackagesFolder, string simpleName, Version? version) + { + // NuGet package ids and assembly simple names match for the overwhelming majority of packages. + // Prefer the version recorded from the dependency closure: an assembly reference only carries an + // assembly version, which is frequently lower than the package version that ships it, so deriving + // a package version from it can silently bind an incompatible package. + string? assemblyPath = null; + if (_closureVersions.TryGetValue(simpleName, out var closureVersion)) + { + assemblyPath = NugetPackageResolver.FindPackageAssemblyInVersion( + globalPackagesFolder, simpleName, closureVersion.ToNormalizedString()); + if (assemblyPath == null) + { + Debug($"Dependency '{simpleName}' {closureVersion.ToNormalizedString()} is in the closure but is not installed in '{globalPackagesFolder}'."); + } + else + { + // Dependencies of a dependency are only discoverable once we know which version won. + RegisterPackageClosure(globalPackagesFolder, assemblyPath); + } + } + + // Fall back to a version-range search, then to any cached version, for packages that were not + // reachable through the closure (for example when a .nuspec is missing from the cache). + if (assemblyPath == null && version != null) + { + assemblyPath = NugetPackageResolver.FindPackageAssembly(globalPackagesFolder, simpleName, version.ToString()); + } + assemblyPath ??= NugetPackageResolver.FindPackageAssembly(globalPackagesFolder, simpleName); + + if (assemblyPath == null) + { + Debug($"Could not locate dependency assembly '{simpleName}' under '{globalPackagesFolder}'."); + return null; + } + + byte[] assemblyBytes; + try + { + // Load from bytes so we never hold a file handle on a package in the global cache. + assemblyBytes = File.ReadAllBytes(assemblyPath); + } + catch (Exception ex) + { + Debug($"Failed to read dependency assembly '{assemblyPath}': {ex.Message}"); + return null; + } + + Assembly assembly; + try + { + assembly = Assembly.Load(assemblyBytes); + } + catch (Exception ex) + { + Debug($"Failed to load dependency assembly '{assemblyPath}': {ex.Message}"); + return null; + } + + // Make the dependency visible to Roslyn too, so generated code that inherits from or references + // types declared in it still binds inside the generated-code workspace. + if (_registeredReferences.TryAdd(assemblyPath, 0)) + { + try + { + CodeModelGenerator.Instance.AddMetadataReference(MetadataReference.CreateFromImage(assemblyBytes)); + } + catch (Exception ex) + { + Debug($"Failed to add metadata reference for dependency assembly '{assemblyPath}': {ex.Message}"); + } + } + + Debug($"Resolved dependency assembly '{simpleName}' from '{assemblyPath}'."); + return assembly; + } + + private static void Debug(string message) + { + try + { + CodeModelGenerator.Instance.Emitter?.Debug(message); + } + catch (InvalidOperationException) + { + // No generator is installed (e.g. a unit test resolving types outside of a generation run). + } + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs index 12cd7ea42fa..b7e089152d9 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs @@ -8,6 +8,7 @@ using System.Threading; using System.Threading.Tasks; using NuGet.Configuration; +using NuGet.Frameworks; using NuGet.Protocol; using NuGet.Protocol.Core.Types; using NuGet.Versioning; @@ -79,6 +80,92 @@ internal static class NugetPackageResolver return null; } + /// + /// Searches for an assembly belonging to in one specific installed + /// version of the package. Unlike this performs no version + /// selection, so callers that already know the exact version (e.g. from a resolved dependency + /// closure) cannot accidentally bind to a different one. + /// + public static string? FindPackageAssemblyInVersion(string globalPackagesFolder, string packageName, string version) + { + var versionDir = Path.Combine(globalPackagesFolder, packageName.ToLowerInvariant(), version.ToLowerInvariant()); + return Directory.Exists(versionDir) ? TryFindNearestAssemblyInVersionDir(versionDir, packageName) : null; + } + + /// + /// Picks the lib/ asset closest to the framework the generator is running on, the same way NuGet + /// would for a project targeting that framework. + /// + /// + /// This differs from , which probes + /// and therefore prefers + /// netstandard2.0. That ordering is fine for collecting compile-time references, but assemblies + /// resolved here are loaded into the running generator, where a netstandard2.0 asset can bind + /// against compatibility shims that duplicate types the shared framework already provides. + /// + private static string? TryFindNearestAssemblyInVersionDir(string versionDir, string packageName) + { + var libDir = Path.Combine(versionDir, "lib"); + if (!Directory.Exists(libDir)) + { + return null; + } + + var candidates = Directory.GetDirectories(libDir) + .Select(dir => (Dir: dir, Framework: ParseFrameworkFolder(Path.GetFileName(dir)))) + .Where(c => c.Framework != null && File.Exists(Path.Combine(c.Dir, $"{packageName}.dll"))) + .ToList(); + + if (candidates.Count == 0) + { + return null; + } + + var nearest = new FrameworkReducer().GetNearest(CurrentFramework, candidates.Select(c => c.Framework!)); + var match = nearest != null + ? candidates.First(c => c.Framework!.Equals(nearest)) + : candidates[0]; + + return Path.Combine(match.Dir, $"{packageName}.dll"); + } + + private static NuGetFramework? ParseFrameworkFolder(string folderName) + { + try + { + var framework = NuGetFramework.ParseFolder(folderName); + return framework.IsUnsupported ? null : framework; + } + catch (ArgumentException) + { + return null; + } + } + + /// + /// The framework the generator itself is running on, used to choose lib/ assets that are safe to + /// load into this process. + /// + internal static NuGetFramework CurrentFramework { get; } = ResolveCurrentFramework(); + + private static NuGetFramework ResolveCurrentFramework() + { + var frameworkName = AppContext.TargetFrameworkName; + if (!string.IsNullOrEmpty(frameworkName)) + { + try + { + return NuGetFramework.Parse(frameworkName); + } + catch (ArgumentException) + { + // Fall through to the runtime-version based approximation below. + } + } + + return NuGetFramework.Parse($".NETCoreApp,Version=v{Environment.Version.Major}.{Environment.Version.Minor}"); + } + private static string? TryFindAssemblyInVersionDir(string versionDir, string packageName) { foreach (var tfm in NugetPackageDownloader.PreferredDotNetFrameworkVersions) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs index a507018bace..1bb441953ee 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -136,6 +137,64 @@ public void TryResolve_ReturnsNullForUnknownPackage() var resolved = ExternalTypeReferenceResolver.TryResolve(external); Assert.IsNull(resolved); + StringAssert.Contains( + "was not found in the NuGet cache", + ExternalTypeReferenceResolver.GetFailureReason(external), + "A missing package should be reported as a missing package."); + } + + [Test] + public void TryResolve_ResolvesTypeWhoseBaseTypeLivesInAnotherPackage() + { + var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache"); + const string basePkg = "Test.Dependency.Base"; + const string leafPkg = "Test.Dependent.Leaf"; + const string leafTypeName = "Test.Dependent.Leaf.DerivedFromDependencyType"; + + var baseDll = CreateFakeNuGetPackage(nugetCacheDir, basePkg, "1.0.0", template: "DependencyPackageSource"); + CreateFakeNuGetPackage( + nugetCacheDir, + leafPkg, + "1.0.0", + template: "DependentPackageSource", + basePackage: basePkg, + referencedAssemblyPaths: [baseDll]); + + var external = new InputExternalTypeMetadata(leafTypeName, leafPkg, "1.0.0"); + + var resolved = ExternalTypeReferenceResolver.TryResolve(external); + + // Assembly.Load puts the leaf assembly in the default AssemblyLoadContext, which cannot see + // the dependency package. Without the NuGet probing hook, GetType silently returns null here + // and the external type gets generated instead of referenced. + Assert.IsNotNull( + resolved, + $"Resolver should load '{leafTypeName}' even though its base type lives in '{basePkg}'. " + + $"Failure reason: {ExternalTypeReferenceResolver.GetFailureReason(external)}"); + Assert.AreEqual(leafTypeName, resolved!.FullName); + + // The whole point of resolving the dependency is that the type is fully usable afterwards: + // CSharpType's constructor reads BaseType, IsValueType and IsEnum, all of which throw or + // misreport when the dependency assembly is unavailable. + Assert.AreEqual($"{basePkg}.DependencyBaseType", resolved.BaseType?.FullName); + Assert.IsFalse(resolved.IsValueType); + Assert.IsNull(ExternalTypeReferenceResolver.GetFailureReason(external)); + } + + [Test] + public void TryResolve_ReportsFailureReasonWhenTypeMissingFromAssembly() + { + var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache"); + const string pkgName = "Test.MissingType.Package"; + CreateFakeNuGetPackage(nugetCacheDir, pkgName, "1.0.0"); + + var external = new InputExternalTypeMetadata($"{pkgName}.NotDeclaredAnywhere", pkgName, null); + + Assert.IsNull(ExternalTypeReferenceResolver.TryResolve(external)); + StringAssert.Contains( + "does not declare the type", + ExternalTypeReferenceResolver.GetFailureReason(external), + "A located-but-unusable assembly must not be reported as a missing package."); } [Test] @@ -176,17 +235,23 @@ public async Task ResolveAllAsync_ResolvesExternalTypesFromInputLibrary() "Pre-walk should add the metadata reference exactly once."); } - private static string CreateFakeNuGetPackage(string nugetCacheDir, string packageName, string version) + private static string CreateFakeNuGetPackage( + string nugetCacheDir, + string packageName, + string version, + string template = "PackageSource", + string? basePackage = null, + IEnumerable? referencedAssemblyPaths = null) { // Load the source template from TestData and substitute the package name + version. The // template embeds an [assembly: AssemblyVersion("$VERSION$")] attribute so tests can verify // which dll was loaded by inspecting Assembly.GetName().Version. Disk + compile + emit are // delegated to the shared FakeNuGetPackage helper. - var template = Helpers.GetExpectedFromFile(method: "PackageSource"); - var source = template + var source = Helpers.GetExpectedFromFile(method: template) .Replace("$PACKAGE$", packageName) - .Replace("$VERSION$", version); - return FakeNuGetPackage.Create(nugetCacheDir, packageName, version, source); + .Replace("$VERSION$", version) + .Replace("$BASEPACKAGE$", basePackage ?? string.Empty); + return FakeNuGetPackage.Create(nugetCacheDir, packageName, version, source, referencedAssemblyPaths); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependencyPackageSource.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependencyPackageSource.cs new file mode 100644 index 00000000000..d9ca5628b8d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependencyPackageSource.cs @@ -0,0 +1,17 @@ +// Token-substituted template source used by ExternalTypeReferenceResolverTests to emit the +// *dependency* half of a two-package fake NuGet graph. Tokens replaced at compile time by the +// CreateFakeNuGetPackage helper: +// $PACKAGE$ -> the package / namespace name (e.g. "Test.Dependency.Base") +// $VERSION$ -> the assembly version embedded as an AssemblyVersionAttribute +// +// This file is excluded from compilation by the test project (see the +// `` rule in the .csproj). + +using System.Reflection; + +[assembly: AssemblyVersion("$VERSION$")] + +namespace $PACKAGE$ +{ + public class DependencyBaseType { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependentPackageSource.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependentPackageSource.cs new file mode 100644 index 00000000000..89dda37351f --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/ExternalTypeReferenceResolverTests/DependentPackageSource.cs @@ -0,0 +1,22 @@ +// Token-substituted template source used by ExternalTypeReferenceResolverTests to emit the +// *dependent* half of a two-package fake NuGet graph. The declared type derives from a type that +// lives in a different package, which is the shape that previously defeated resolution: loading the +// assembly succeeds, but Assembly.GetType returns null because the base type's assembly cannot be +// found by the default AssemblyLoadContext. +// +// Tokens replaced at compile time by the CreateFakeNuGetPackage helper: +// $PACKAGE$ -> the package / namespace name (e.g. "Test.Dependent.Leaf") +// $VERSION$ -> the assembly version embedded as an AssemblyVersionAttribute +// $BASEPACKAGE$ -> the namespace of the dependency package declaring DependencyBaseType +// +// This file is excluded from compilation by the test project (see the +// `` rule in the .csproj). + +using System.Reflection; + +[assembly: AssemblyVersion("$VERSION$")] + +namespace $PACKAGE$ +{ + public class DerivedFromDependencyType : $BASEPACKAGE$.DependencyBaseType { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs index f9ddd25fd41..29653ca4083 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Collections.Generic; using System.IO; +using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using NUnit.Framework; @@ -20,16 +22,39 @@ public static class FakeNuGetPackage /// Compiles into a netstandard2.0 assembly and writes it to the /// NuGet cache layout. Returns the absolute path to the emitted dll. /// - public static string Create(string nugetCacheDir, string packageName, string version, string sourceCode) + /// The fake NuGet global packages folder to write into. + /// The package id, which is also used as the assembly name. + /// The package version, used as the version directory name. + /// The C# source to compile into the package assembly. + /// + /// Paths to other assemblies the source depends on. Use this to build a package whose public + /// types derive from types in another fake package, which is how the dependency-resolution + /// behavior of the external-type resolver is exercised. + /// + public static string Create( + string nugetCacheDir, + string packageName, + string version, + string sourceCode, + IEnumerable? referencedAssemblyPaths = null) { var pkgDir = Path.Combine( nugetCacheDir, packageName.ToLowerInvariant(), version, "lib", "netstandard2.0"); Directory.CreateDirectory(pkgDir); + var references = new List + { + MetadataReference.CreateFromFile(typeof(object).Assembly.Location) + }; + if (referencedAssemblyPaths != null) + { + references.AddRange(referencedAssemblyPaths.Select(p => MetadataReference.CreateFromFile(p))); + } + var compilation = CSharpCompilation.Create( packageName, [CSharpSyntaxTree.ParseText(sourceCode)], - [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)], + references, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); var dllPath = Path.Combine(pkgDir, $"{packageName}.dll"); From abfa462ef3eb428b8b468b101b4070baa61af40a Mon Sep 17 00:00:00 2001 From: Ryan Hurey Date: Thu, 30 Jul 2026 12:07:59 -0400 Subject: [PATCH 2/5] Address PR feedback: scope the NuGet resolver per run and make the test require closure walking Make NugetAssemblyResolver an instance owned by ExternalTypeReferenceResolver's per-generator cache state instead of a process-wide static. Only the default AssemblyLoadContext's single Resolving event stays static; it dispatches to the currently active resolver so repeated runs in one process cannot accumulate handlers. Record when a dependency is satisfied by an older copy the generator deploys and surface those notes in the resolution failure reason, so a type that fails to load because of the substitution reports why. Preferring the host copy remains the behavior: loading the closure copy would leave two assemblies with the same simple name in the process, which is the condition the fix exists to avoid. Emit a .nuspec from FakeNuGetPackage and give the dependency test a package version that differs from its assembly version, plus a higher-versioned decoy package that lacks the base type. The assembly-version fallback selects the decoy, so the test now passes only when the dependency is pinned from the .nuspec closure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a3c5d2db-ec93-428f-b474-05e7e58cad60 --- .../ExternalTypeReferenceResolver.cs | 40 +++- .../src/Utilities/NugetAssemblyResolver.cs | 200 +++++++++++------- .../ExternalTypeReferenceResolverTests.cs | 35 ++- .../test/common/FakeNuGetPackage.cs | 50 ++++- 4 files changed, 231 insertions(+), 94 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs index 2cbd8b3fcbe..23b39933526 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs @@ -59,6 +59,24 @@ private sealed class CacheState // the same dll isn't registered twice when it contains multiple referenced types. public readonly ConcurrentDictionary AddedAssemblyRefs = new(StringComparer.OrdinalIgnoreCase); + + private readonly object _assemblyResolverLock = new(); + private NugetAssemblyResolver? _assemblyResolver; + + /// + /// The dependency resolver for this generation run, created on first use and reused for every + /// external type resolved by the same generator. + /// + public NugetAssemblyResolver GetAssemblyResolver(string globalPackagesFolder, CodeModelGenerator generator) + { + lock (_assemblyResolverLock) + { + return _assemblyResolver ??= new NugetAssemblyResolver( + globalPackagesFolder, + message => generator.Emitter?.Debug(message), + generator.AddMetadataReference); + } + } } private static CacheState GetState(CodeModelGenerator generator) => @@ -167,7 +185,9 @@ internal static void Reset() // when Reset runs before the first generator is loaded (e.g. a test fixture's SetUp). } - NugetAssemblyResolver.Reset(); + // The dropped CacheState owned the dependency resolver, so stop routing loads to it. Assemblies + // it already loaded stay in the default context; they cannot be unloaded. + NugetAssemblyResolver.Deactivate(); } private static string MakeKey(InputExternalTypeMetadata external) => @@ -206,9 +226,10 @@ private static async Task ResolveResultAsync(InputExternalType } // The external assembly is loaded into the default AssemblyLoadContext, which cannot see the - // package's own dependencies. Install the NuGet probing hook before loading anything so that - // base types and interfaces declared in dependency packages can be resolved. - NugetAssemblyResolver.EnsureRegistered(globalPackagesFolder); + // package's own dependencies. Activate this run's NuGet probing resolver before loading + // anything so that base types and interfaces declared in dependency packages can be resolved. + var assemblyResolver = state.GetAssemblyResolver(globalPackagesFolder, generator); + assemblyResolver.Activate(); string? assemblyPath = NugetPackageResolver.FindPackageAssembly( globalPackagesFolder, external.Package!, external.MinVersion); @@ -252,13 +273,14 @@ private static async Task ResolveResultAsync(InputExternalType // Pin every package in this package's dependency closure before loading it, so the resolving // hook binds dependencies to the versions NuGet selected rather than guessing from assembly // versions (which are routinely lower than the package versions that ship them). - NugetAssemblyResolver.RegisterPackageClosure(globalPackagesFolder, assemblyPath); + assemblyResolver.RegisterPackageClosure(assemblyPath); byte[] assemblyBytes; try { assemblyBytes = await File.ReadAllBytesAsync(assemblyPath).ConfigureAwait(false); - } catch (Exception ex) + } + catch (Exception ex) { generator.Emitter?.Debug( $"Failed to read assembly '{assemblyPath}' for external type '{external.Identity}': {ex.Message}"); @@ -281,7 +303,8 @@ private static async Task ResolveResultAsync(InputExternalType $"Failed to load assembly '{assemblyPath}' for external type '{external.Identity}': {ex.Message}"); return CacheResult(state, key, new ResolutionResult( null, - $"assembly '{assemblyPath}' could not be loaded ({ex.Message})")); + $"assembly '{assemblyPath}' could not be loaded ({ex.Message})" + + assemblyResolver.DescribeDowngradedDependencies())); } if (loadedType == null) @@ -292,7 +315,8 @@ private static async Task ResolveResultAsync(InputExternalType $"Assembly '{assemblyPath}' does not declare external type '{external.Identity}', or one of its dependencies could not be resolved."); return CacheResult(state, key, new ResolutionResult( null, - $"assembly '{assemblyPath}' was loaded but does not declare the type, or one of the type's dependencies could not be resolved")); + $"assembly '{assemblyPath}' was loaded but does not declare the type, or one of the type's dependencies could not be resolved" + + assemblyResolver.DescribeDowngradedDependencies())); } // Register the dll as a Roslyn metadata reference exactly once per assembly path so that diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs index c5e1f3f40c7..48c1b2a3326 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs @@ -29,54 +29,99 @@ namespace Microsoft.TypeSpec.Generator.Utilities /// hook Assembly.GetType(identity, throwOnError: false) silently returns null for any /// type whose base type lives in a dependency. The external type then appears unresolvable and is /// generated instead of referenced. + /// + /// One instance services one generation run and is owned by that run's resolver cache state. The + /// only genuinely process-wide pieces are kept static and documented below: the default context's + /// single Resolving event, and the fact that assemblies loaded into it can never be unloaded. + /// /// - internal static class NugetAssemblyResolver + internal sealed class NugetAssemblyResolver { + // The default AssemblyLoadContext exposes exactly one process-wide Resolving event, so the handler is + // installed once and dispatches to whichever resolver is currently servicing a generation run. Keeping + // the subscription static rather than per-instance means repeated runs in one process - the norm under + // test - cannot accumulate handlers. + private static int _hookInstalled; + private static NugetAssemblyResolver? _active; + + private readonly string _globalPackagesFolder; + private readonly Action _debug; + private readonly Action _addMetadataReference; + // Cached resolution per assembly simple name. A null value means resolution was attempted and // failed, so repeated references to the same missing dependency don't re-probe the file system. - private static readonly ConcurrentDictionary _resolved = + private readonly ConcurrentDictionary _resolved = new(StringComparer.OrdinalIgnoreCase); // Tracks assembly paths already registered as Roslyn metadata references. - private static readonly ConcurrentDictionary _registeredReferences = + private readonly ConcurrentDictionary _registeredReferences = new(StringComparer.OrdinalIgnoreCase); // Package id -> package version, for every package in the dependency closure of an external-type // package. Populated from .nuspec files so dependencies bind to the version NuGet would have // chosen rather than a version guessed from the referencing assembly's version number. - private static readonly ConcurrentDictionary _closureVersions = + private readonly ConcurrentDictionary _closureVersions = new(StringComparer.OrdinalIgnoreCase); // Package version directories whose .nuspec has already been walked. - private static readonly ConcurrentDictionary _walkedPackages = + private readonly ConcurrentDictionary _walkedPackages = new(StringComparer.OrdinalIgnoreCase); - // Guards against a dependency cycle re-entering resolution for the same name on this thread. - [ThreadStatic] - private static HashSet? _inProgress; + // Dependencies that were satisfied by an older copy the generator already deploys, keyed by simple + // name. Surfaced in resolution failures so a type that fails to load because of the substitution + // reports why instead of just appearing to be missing. + private readonly ConcurrentDictionary _downgradedDependencies = + new(StringComparer.OrdinalIgnoreCase); - private static string? _globalPackagesFolder; - private static int _hookInstalled; + // Guards against a dependency cycle re-entering resolution for the same name on this thread. + private readonly ThreadLocal> _inProgress = + new(() => new HashSet(StringComparer.OrdinalIgnoreCase)); + + /// The NuGet global packages folder to probe. + /// Receives trace messages describing each resolution decision. + /// + /// Registers a resolved dependency with the generation run's Roslyn workspaces. + /// + public NugetAssemblyResolver( + string globalPackagesFolder, + Action debug, + Action addMetadataReference) + { + _globalPackagesFolder = globalPackagesFolder; + _debug = debug; + _addMetadataReference = addMetadataReference; + } /// - /// Installs the hook (once per process) and records the - /// NuGet global packages folder to probe. Safe to call repeatedly. + /// Makes this the resolver that services dependency loads the default context cannot satisfy, + /// installing the process-wide hook on first use. Safe to call repeatedly. /// - public static void EnsureRegistered(string globalPackagesFolder) + public void Activate() { - if (string.IsNullOrEmpty(globalPackagesFolder)) - { - return; - } - - Volatile.Write(ref _globalPackagesFolder, globalPackagesFolder); + Volatile.Write(ref _active, this); if (Interlocked.Exchange(ref _hookInstalled, 1) == 0) { - AssemblyLoadContext.Default.Resolving += OnResolving; + AssemblyLoadContext.Default.Resolving += static (context, name) => + Volatile.Read(ref _active)?.OnResolving(context, name); } } + /// + /// Stops routing dependency loads to any resolver. Assemblies already loaded into the default + /// context stay loaded - they cannot be unloaded - so this only detaches the bookkeeping. + /// + public static void Deactivate() => Volatile.Write(ref _active, null); + + /// + /// Describes the dependencies that were satisfied by an older copy shipped with the generator, or + /// null when every dependency was resolved at or above the version that was asked for. + /// + public string? DescribeDowngradedDependencies() => + _downgradedDependencies.IsEmpty + ? null + : string.Join("; ", _downgradedDependencies.Values.OrderBy(v => v, StringComparer.Ordinal)); + /// /// Records the dependency closure of the package that ships by /// walking its .nuspec transitively, so the resolving hook can bind each dependency to the @@ -88,9 +133,9 @@ public static void EnsureRegistered(string globalPackagesFolder) /// 1.14.0, for example, ships assembly version 1.9.0.0; treating that as a package version resolves the /// unrelated 1.9.0 package and produces a at type-load time. /// - public static void RegisterPackageClosure(string globalPackagesFolder, string packageAssemblyPath) + public void RegisterPackageClosure(string packageAssemblyPath) { - if (string.IsNullOrEmpty(globalPackagesFolder) || string.IsNullOrEmpty(packageAssemblyPath)) + if (string.IsNullOrEmpty(packageAssemblyPath)) { return; } @@ -103,10 +148,10 @@ public static void RegisterPackageClosure(string globalPackagesFolder, string pa return; } - WalkPackage(globalPackagesFolder, Path.GetFileName(packageDir), Path.GetFileName(versionDir)); + WalkPackage(Path.GetFileName(packageDir), Path.GetFileName(versionDir)); } - private static void WalkPackage(string globalPackagesFolder, string packageId, string version) + private void WalkPackage(string packageId, string version) { if (!NuGetVersion.TryParse(version, out var parsedVersion)) { @@ -128,7 +173,7 @@ private static void WalkPackage(string globalPackagesFolder, string packageId, s return; } - var versionDir = Path.Combine(globalPackagesFolder, packageId.ToLowerInvariant(), version.ToLowerInvariant()); + var versionDir = Path.Combine(_globalPackagesFolder, packageId.ToLowerInvariant(), version.ToLowerInvariant()); string? nuspecPath; try { @@ -138,13 +183,13 @@ private static void WalkPackage(string globalPackagesFolder, string packageId, s } catch (Exception ex) { - Debug($"Failed to enumerate '{versionDir}' while walking package dependencies: {ex.Message}"); + _debug($"Failed to enumerate '{versionDir}' while walking package dependencies: {ex.Message}"); return; } if (nuspecPath == null) { - Debug($"No .nuspec found for package '{packageId}' {version}; its dependencies will not be pinned."); + _debug($"No .nuspec found for package '{packageId}' {version}; its dependencies will not be pinned."); return; } @@ -155,7 +200,7 @@ private static void WalkPackage(string globalPackagesFolder, string packageId, s } catch (Exception ex) { - Debug($"Failed to read '{nuspecPath}': {ex.Message}"); + _debug($"Failed to read '{nuspecPath}': {ex.Message}"); return; } @@ -164,7 +209,7 @@ private static void WalkPackage(string globalPackagesFolder, string packageId, s var dependencyVersion = dependency.VersionRange?.MinVersion; if (dependencyVersion != null) { - WalkPackage(globalPackagesFolder, dependency.Id, dependencyVersion.ToNormalizedString()); + WalkPackage(dependency.Id, dependencyVersion.ToNormalizedString()); } } } @@ -184,23 +229,10 @@ private static IEnumerable SelectNearestGroup(IReadOnlyList

(); } - ///

- /// Clears the cached dependency resolutions. Loaded assemblies cannot be unloaded from the default - /// context, so this only resets the probe/metadata-reference bookkeeping. - /// - internal static void Reset() - { - _resolved.Clear(); - _registeredReferences.Clear(); - _closureVersions.Clear(); - _walkedPackages.Clear(); - } - - private static Assembly? OnResolving(AssemblyLoadContext context, AssemblyName name) + private Assembly? OnResolving(AssemblyLoadContext context, AssemblyName name) { var simpleName = name.Name; - var globalPackagesFolder = Volatile.Read(ref _globalPackagesFolder); - if (string.IsNullOrEmpty(simpleName) || string.IsNullOrEmpty(globalPackagesFolder)) + if (string.IsNullOrEmpty(simpleName) || string.IsNullOrEmpty(_globalPackagesFolder)) { return null; } @@ -210,7 +242,7 @@ internal static void Reset() return cached; } - var inProgress = _inProgress ??= new HashSet(StringComparer.OrdinalIgnoreCase); + var inProgress = _inProgress.Value!; if (!inProgress.Add(simpleName)) { return null; @@ -225,7 +257,7 @@ internal static void Reset() // same simple name in the process, and any type they both declare (System.Memory.Data's // System.BinaryData, for example) would exist twice, so signatures mentioning it fail to bind // with a MissingMethodException. Reusing the host's copy keeps exactly one identity per name. - var assembly = UseHostAssembly(simpleName) ?? Probe(globalPackagesFolder, simpleName, name.Version); + var assembly = UseHostAssembly(simpleName, name.Version) ?? Probe(simpleName, name.Version); _resolved[simpleName] = assembly; return assembly; } @@ -236,15 +268,45 @@ internal static void Reset() } /// - /// Returns the host's own copy of when it has one, ignoring version. + /// Returns the host's own copy of when it has one, regardless of version, + /// recording a downgrade when that copy is older than the version that was requested. /// - private static Assembly? UseHostAssembly(string simpleName) + private Assembly? UseHostAssembly(string simpleName, Version? requestedVersion) + { + var hostAssembly = FindHostAssembly(simpleName); + if (hostAssembly == null) + { + return null; + } + + var hostVersion = hostAssembly.GetName().Version; + if (requestedVersion != null && hostVersion != null && hostVersion < requestedVersion) + { + // Not fatal on its own: assembly versions are routinely older than the reference that names + // them, and using the host's copy is the only option that keeps one identity per simple name. + // It is recorded so that a type which does fail to load can say why. + var note = + $"'{simpleName}' was requested at version {requestedVersion} but the generator supplies " + + $"{hostVersion}, which was used to keep a single identity for the types it declares"; + if (_downgradedDependencies.TryAdd(simpleName, note)) + { + _debug($"Dependency version downgrade: {note}."); + } + } + else + { + _debug($"Reusing the generator's '{hostAssembly.GetName()}' for dependency '{simpleName}'."); + } + + return hostAssembly; + } + + private static Assembly? FindHostAssembly(string simpleName) { foreach (var loaded in AssemblyLoadContext.Default.Assemblies) { if (string.Equals(loaded.GetName().Name, simpleName, StringComparison.OrdinalIgnoreCase)) { - Debug($"Reusing already-loaded assembly '{loaded.GetName()}' for dependency '{simpleName}'."); return loaded; } } @@ -253,9 +315,7 @@ internal static void Reset() // which matches any version there; a version-qualified request is what failed to get us here. try { - var assembly = AssemblyLoadContext.Default.LoadFromAssemblyName(new AssemblyName(simpleName)); - Debug($"Resolved dependency '{simpleName}' to the generator's own '{assembly.GetName()}'."); - return assembly; + return AssemblyLoadContext.Default.LoadFromAssemblyName(new AssemblyName(simpleName)); } catch (Exception) { @@ -264,7 +324,7 @@ internal static void Reset() } } - private static Assembly? Probe(string globalPackagesFolder, string simpleName, Version? version) + private Assembly? Probe(string simpleName, Version? version) { // NuGet package ids and assembly simple names match for the overwhelming majority of packages. // Prefer the version recorded from the dependency closure: an assembly reference only carries an @@ -274,15 +334,15 @@ internal static void Reset() if (_closureVersions.TryGetValue(simpleName, out var closureVersion)) { assemblyPath = NugetPackageResolver.FindPackageAssemblyInVersion( - globalPackagesFolder, simpleName, closureVersion.ToNormalizedString()); + _globalPackagesFolder, simpleName, closureVersion.ToNormalizedString()); if (assemblyPath == null) { - Debug($"Dependency '{simpleName}' {closureVersion.ToNormalizedString()} is in the closure but is not installed in '{globalPackagesFolder}'."); + _debug($"Dependency '{simpleName}' {closureVersion.ToNormalizedString()} is in the closure but is not installed in '{_globalPackagesFolder}'."); } else { // Dependencies of a dependency are only discoverable once we know which version won. - RegisterPackageClosure(globalPackagesFolder, assemblyPath); + RegisterPackageClosure(assemblyPath); } } @@ -290,13 +350,13 @@ internal static void Reset() // reachable through the closure (for example when a .nuspec is missing from the cache). if (assemblyPath == null && version != null) { - assemblyPath = NugetPackageResolver.FindPackageAssembly(globalPackagesFolder, simpleName, version.ToString()); + assemblyPath = NugetPackageResolver.FindPackageAssembly(_globalPackagesFolder, simpleName, version.ToString()); } - assemblyPath ??= NugetPackageResolver.FindPackageAssembly(globalPackagesFolder, simpleName); + assemblyPath ??= NugetPackageResolver.FindPackageAssembly(_globalPackagesFolder, simpleName); if (assemblyPath == null) { - Debug($"Could not locate dependency assembly '{simpleName}' under '{globalPackagesFolder}'."); + _debug($"Could not locate dependency assembly '{simpleName}' under '{_globalPackagesFolder}'."); return null; } @@ -308,7 +368,7 @@ internal static void Reset() } catch (Exception ex) { - Debug($"Failed to read dependency assembly '{assemblyPath}': {ex.Message}"); + _debug($"Failed to read dependency assembly '{assemblyPath}': {ex.Message}"); return null; } @@ -319,7 +379,7 @@ internal static void Reset() } catch (Exception ex) { - Debug($"Failed to load dependency assembly '{assemblyPath}': {ex.Message}"); + _debug($"Failed to load dependency assembly '{assemblyPath}': {ex.Message}"); return null; } @@ -329,28 +389,16 @@ internal static void Reset() { try { - CodeModelGenerator.Instance.AddMetadataReference(MetadataReference.CreateFromImage(assemblyBytes)); + _addMetadataReference(MetadataReference.CreateFromImage(assemblyBytes)); } catch (Exception ex) { - Debug($"Failed to add metadata reference for dependency assembly '{assemblyPath}': {ex.Message}"); + _debug($"Failed to add metadata reference for dependency assembly '{assemblyPath}': {ex.Message}"); } } - Debug($"Resolved dependency assembly '{simpleName}' from '{assemblyPath}'."); + _debug($"Resolved dependency assembly '{simpleName}' from '{assemblyPath}'."); return assembly; } - - private static void Debug(string message) - { - try - { - CodeModelGenerator.Instance.Emitter?.Debug(message); - } - catch (InvalidOperationException) - { - // No generator is installed (e.g. a unit test resolving types outside of a generation run). - } - } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs index 1bb441953ee..65966107ab2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/ExternalTypeReferenceResolverTests.cs @@ -151,14 +151,30 @@ public void TryResolve_ResolvesTypeWhoseBaseTypeLivesInAnotherPackage() const string leafPkg = "Test.Dependent.Leaf"; const string leafTypeName = "Test.Dependent.Leaf.DerivedFromDependencyType"; - var baseDll = CreateFakeNuGetPackage(nugetCacheDir, basePkg, "1.0.0", template: "DependencyPackageSource"); + // The base package ships package version 2.0.0 but assembly version 1.0.0.0, mirroring real + // packages (System.ClientModel 1.14.0 ships assembly version 1.9.0.0). Only its .nuspec says + // 2.0.0 is the right version to load. + var baseDll = CreateFakeNuGetPackage( + nugetCacheDir, + basePkg, + "2.0.0", + template: "DependencyPackageSource", + assemblyVersion: "1.0.0.0"); + + // A decoy at a higher package version that does *not* declare DependencyBaseType. Resolving the + // dependency from the assembly version in the leaf's reference (1.0.0.0) searches for the + // highest package at or above 1.0.0 and would land here, leaving the base type unloadable. The + // test therefore only passes when the dependency is pinned from the .nuspec closure. + CreateFakeNuGetPackage(nugetCacheDir, basePkg, "3.0.0", assemblyVersion: "1.0.0.0"); + CreateFakeNuGetPackage( nugetCacheDir, leafPkg, "1.0.0", template: "DependentPackageSource", basePackage: basePkg, - referencedAssemblyPaths: [baseDll]); + referencedAssemblyPaths: [baseDll], + dependencies: [(basePkg, "[2.0.0, )")]); var external = new InputExternalTypeMetadata(leafTypeName, leafPkg, "1.0.0"); @@ -241,17 +257,22 @@ private static string CreateFakeNuGetPackage( string version, string template = "PackageSource", string? basePackage = null, - IEnumerable? referencedAssemblyPaths = null) + IEnumerable? referencedAssemblyPaths = null, + string? assemblyVersion = null, + IEnumerable<(string Id, string VersionRange)>? dependencies = null) { // Load the source template from TestData and substitute the package name + version. The // template embeds an [assembly: AssemblyVersion("$VERSION$")] attribute so tests can verify - // which dll was loaded by inspecting Assembly.GetName().Version. Disk + compile + emit are - // delegated to the shared FakeNuGetPackage helper. + // which dll was loaded by inspecting Assembly.GetName().Version. The assembly version defaults + // to the package version but can be set independently, because real packages routinely ship an + // assembly version lower than their package version. Disk + compile + emit are delegated to the + // shared FakeNuGetPackage helper. var source = Helpers.GetExpectedFromFile(method: template) .Replace("$PACKAGE$", packageName) - .Replace("$VERSION$", version) + .Replace("$VERSION$", assemblyVersion ?? version) .Replace("$BASEPACKAGE$", basePackage ?? string.Empty); - return FakeNuGetPackage.Create(nugetCacheDir, packageName, version, source, referencedAssemblyPaths); + return FakeNuGetPackage.Create( + nugetCacheDir, packageName, version, source, referencedAssemblyPaths, dependencies); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs index 29653ca4083..87c0cb6f525 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -31,16 +32,24 @@ public static class FakeNuGetPackage /// types derive from types in another fake package, which is how the dependency-resolution /// behavior of the external-type resolver is exercised. /// + /// + /// Package id / version-range pairs to declare in the emitted .nuspec's + /// .NETStandard2.0 dependency group, e.g. ("Other.Package", "[2.0.0, )"). The + /// resolver reads these to pin each dependency to a package version, so use them whenever a test + /// needs the package version to differ from the assembly version it ships. + /// public static string Create( string nugetCacheDir, string packageName, string version, string sourceCode, - IEnumerable? referencedAssemblyPaths = null) + IEnumerable? referencedAssemblyPaths = null, + IEnumerable<(string Id, string VersionRange)>? dependencies = null) { - var pkgDir = Path.Combine( - nugetCacheDir, packageName.ToLowerInvariant(), version, "lib", "netstandard2.0"); + var versionDir = Path.Combine(nugetCacheDir, packageName.ToLowerInvariant(), version); + var pkgDir = Path.Combine(versionDir, "lib", "netstandard2.0"); Directory.CreateDirectory(pkgDir); + WriteNuspec(versionDir, packageName, version, dependencies); var references = new List { @@ -65,5 +74,40 @@ public static string Create( } return dllPath; } + + /// + /// Writes the .nuspec that a restored package always carries in its version directory. The + /// external-type resolver walks these to build the dependency closure, so a package without one is + /// resolved only by the assembly-version fallback path. + /// + private static void WriteNuspec( + string versionDir, + string packageName, + string version, + IEnumerable<(string Id, string VersionRange)>? dependencies) + { + var dependencyElements = string.Concat( + (dependencies ?? []).Select(d => + $" {Environment.NewLine}")); + + var nuspec = + $""" + + + + {packageName} + {version} + Test + Fake package emitted by tests. + + + {dependencyElements} + + + + """; + + File.WriteAllText(Path.Combine(versionDir, $"{packageName.ToLowerInvariant()}.nuspec"), nuspec); + } } } From 0b760e9c2c4ffac65fa49caa5dd7ccb65c7c1dc7 Mon Sep 17 00:00:00 2001 From: Ryan Hurey Date: Thu, 30 Jul 2026 12:13:54 -0400 Subject: [PATCH 3/5] Return the cached ResolutionResult on the success path Match the failure paths and avoid allocating an identical result twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a3c5d2db-ec93-428f-b474-05e7e58cad60 --- .../src/Utilities/ExternalTypeReferenceResolver.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs index 23b39933526..4fd2bb69ce3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs @@ -329,8 +329,7 @@ private static async Task ResolveResultAsync(InputExternalType $"Added metadata reference for external type '{external.Identity}' from {assemblyPath}"); } - CacheResult(state, key, new ResolutionResult(loadedType, null)); - return new ResolutionResult(loadedType, null); + return CacheResult(state, key, new ResolutionResult(loadedType, null)); } private static ResolutionResult CacheResult(CacheState state, string key, ResolutionResult result) From 5d5161790bb52c8c68a22107f604a4b864c32b2b Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 30 Jul 2026 11:21:52 -0700 Subject: [PATCH 4/5] fix(csharp): address NuGet resolver review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67f4b975-2057-47dd-8364-2d2b6ac1c29e --- .../src/Utilities/NugetAssemblyResolver.cs | 23 ++-- .../src/Utilities/NugetPackageDownloader.cs | 14 ++- .../src/Utilities/NugetPackageResolver.cs | 26 ++++- .../Utilities/NugetAssemblyResolverTests.cs | 102 ++++++++++++++++++ .../PackageSource.cs | 9 ++ .../test/common/FakeNuGetPackage.cs | 14 +++ 6 files changed, 170 insertions(+), 18 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/NugetAssemblyResolverTests.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/NugetAssemblyResolverTests/PackageSource.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs index 48c1b2a3326..5bb9228918a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetAssemblyResolver.cs @@ -102,8 +102,8 @@ public void Activate() if (Interlocked.Exchange(ref _hookInstalled, 1) == 0) { - AssemblyLoadContext.Default.Resolving += static (context, name) => - Volatile.Read(ref _active)?.OnResolving(context, name); + AssemblyLoadContext.Default.Resolving += static (_, name) => + Volatile.Read(ref _active)?.Resolve(name); } } @@ -173,17 +173,24 @@ private void WalkPackage(string packageId, string version) return; } - var versionDir = Path.Combine(_globalPackagesFolder, packageId.ToLowerInvariant(), version.ToLowerInvariant()); + if (!NugetPackageResolver.TryFindPackageInCache( + _globalPackagesFolder, + packageId, + parsedVersion, + out var packageInfo)) + { + _debug($"Package '{packageId}' {version} was not found while walking package dependencies."); + return; + } + string? nuspecPath; try { - nuspecPath = Directory.Exists(versionDir) - ? Directory.EnumerateFiles(versionDir, "*.nuspec").FirstOrDefault() - : null; + nuspecPath = Directory.EnumerateFiles(packageInfo.ExpandedPath, "*.nuspec").FirstOrDefault(); } catch (Exception ex) { - _debug($"Failed to enumerate '{versionDir}' while walking package dependencies: {ex.Message}"); + _debug($"Failed to enumerate '{packageInfo.ExpandedPath}' while walking package dependencies: {ex.Message}"); return; } @@ -229,7 +236,7 @@ private static IEnumerable SelectNearestGroup(IReadOnlyList

(); } - private Assembly? OnResolving(AssemblyLoadContext context, AssemblyName name) + internal Assembly? Resolve(AssemblyName name) { var simpleName = name.Name; if (string.IsNullOrEmpty(simpleName) || string.IsNullOrEmpty(_globalPackagesFolder)) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageDownloader.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageDownloader.cs index 6256832866a..7d6467132d8 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageDownloader.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageDownloader.cs @@ -17,7 +17,6 @@ using NuGet.Packaging.Signing; using NuGet.Protocol; using NuGet.Protocol.Core.Types; -using NuGet.Repositories; using NuGet.Versioning; using ILogger = NuGet.Common.ILogger; using PackageArchiveDownloader = NuGet.Protocol.LocalPackageArchiveDownloader; @@ -32,8 +31,6 @@ internal class NugetPackageDownloader private readonly ISettings _nugetSettings; private readonly IReadOnlyList _availableSources; private readonly HashSet _targetFrameworks; - // cspell: disable-next-line - private readonly NuGetv3LocalRepository _localRepo; public NugetPackageDownloader( string packageName, @@ -43,12 +40,10 @@ public NugetPackageDownloader( { _nugetSettings = settings; _targetFrameworks = targetFrameworks is null - ? [ ..PreferredDotNetFrameworkVersions ] + ? [.. PreferredDotNetFrameworkVersions] : [.. targetFrameworks]; _availableSources = GetPrimaryPackageSources(settings); _globalNugetPackagePath = SettingsUtility.GetGlobalPackagesFolder(settings); - // cspell: disable-next-line - _localRepo = new NuGetv3LocalRepository(_globalNugetPackagePath); _packageName = packageName; _packageVersion = packageVersion; } @@ -71,8 +66,11 @@ protected virtual async Task PackageExistsInSource( protected virtual bool TryFindPackageInCache(string packageName, NuGetVersion version, out NuGet.Repositories.LocalPackageInfo? packageInfo) { - packageInfo = _localRepo.FindPackage(packageName, version); - return packageInfo != null; + return NugetPackageResolver.TryFindPackageInCache( + _globalNugetPackagePath, + packageName, + version, + out packageInfo); } protected virtual bool DirectoryExists(string path) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs index b7e089152d9..e7367009900 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; using System.Threading; @@ -11,6 +12,7 @@ using NuGet.Frameworks; using NuGet.Protocol; using NuGet.Protocol.Core.Types; +using NuGet.Repositories; using NuGet.Versioning; namespace Microsoft.TypeSpec.Generator.Utilities @@ -88,8 +90,28 @@ internal static class NugetPackageResolver ///

public static string? FindPackageAssemblyInVersion(string globalPackagesFolder, string packageName, string version) { - var versionDir = Path.Combine(globalPackagesFolder, packageName.ToLowerInvariant(), version.ToLowerInvariant()); - return Directory.Exists(versionDir) ? TryFindNearestAssemblyInVersionDir(versionDir, packageName) : null; + if (!NuGetVersion.TryParse(version, out var parsedVersion) + || !TryFindPackageInCache(globalPackagesFolder, packageName, parsedVersion, out var packageInfo)) + { + return null; + } + + return TryFindNearestAssemblyInVersionDir(packageInfo.ExpandedPath, packageName); + } + + /// + /// Uses NuGet's local repository implementation to locate one exact package version in the global + /// packages folder. + /// + public static bool TryFindPackageInCache( + string globalPackagesFolder, + string packageName, + NuGetVersion version, + [NotNullWhen(true)] out NuGet.Repositories.LocalPackageInfo? packageInfo) + { + var localRepository = new NuGetv3LocalRepository(globalPackagesFolder); + packageInfo = localRepository.FindPackage(packageName, version); + return packageInfo != null; } /// diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/NugetAssemblyResolverTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/NugetAssemblyResolverTests.cs new file mode 100644 index 00000000000..876aa2f4211 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/NugetAssemblyResolverTests.cs @@ -0,0 +1,102 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; +using Microsoft.TypeSpec.Generator.Tests.Common; +using Microsoft.TypeSpec.Generator.Utilities; +using NUnit.Framework; + +namespace Microsoft.TypeSpec.Generator.Tests.Utilities +{ + [NonParallelizable] + public class NugetAssemblyResolverTests + { + private string? _tempDirectory; + + [SetUp] + public void Setup() + { + _tempDirectory = Path.Combine(Path.GetTempPath(), "TestArtifacts", Guid.NewGuid().ToString()); + Directory.CreateDirectory(_tempDirectory); + } + + [TearDown] + public void Cleanup() + { + NugetAssemblyResolver.Deactivate(); + Directory.Delete(_tempDirectory!, true); + } + + [Test] + public void Resolve_ReturnsNullAndLogsWhenAssemblyCannotBeLoaded() + { + var packageName = $"Test.InvalidAssembly.{Guid.NewGuid():N}"; + var assemblyPath = Path.Combine( + _tempDirectory!, + packageName.ToLowerInvariant(), + "1.0.0", + "lib", + "netstandard2.0", + $"{packageName}.dll"); + Directory.CreateDirectory(Path.GetDirectoryName(assemblyPath)!); + File.WriteAllText(assemblyPath, "not an assembly"); + var debugMessages = new List(); + var resolver = new NugetAssemblyResolver(_tempDirectory!, debugMessages.Add, _ => { }); + + var resolved = resolver.Resolve(new AssemblyName($"{packageName}, Version=1.0.0.0")); + + Assert.IsNull(resolved); + Assert.That(debugMessages, Has.Some.Contains("Failed to load dependency assembly")); + } + + [Test] + public void Resolve_ReturnsAssemblyWhenMetadataReferenceRegistrationFails() + { + var packageName = $"Test.MetadataFailure.P{Guid.NewGuid():N}"; + CreateFakeNuGetPackage(packageName); + var debugMessages = new List(); + var resolver = new NugetAssemblyResolver( + _tempDirectory!, + debugMessages.Add, + _ => throw new InvalidOperationException("metadata failure")); + + var resolved = resolver.Resolve(new AssemblyName($"{packageName}, Version=1.0.0.0")); + + Assert.IsNotNull(resolved); + Assert.AreEqual(packageName, resolved!.GetName().Name); + Assert.That(debugMessages, Has.Some.Contains("Failed to add metadata reference")); + } + + [Test] + public void RegisterPackageClosure_LogsMalformedNuspecWithoutThrowing() + { + var packageName = $"Test.MalformedNuspec.P{Guid.NewGuid():N}"; + var assemblyPath = CreateFakeNuGetPackage(packageName); + var versionDirectory = Path.GetDirectoryName( + Path.GetDirectoryName( + Path.GetDirectoryName(assemblyPath)))!; + File.WriteAllText( + Path.Combine(versionDirectory, $"{packageName.ToLowerInvariant()}.nuspec"), + "(); + var resolver = new NugetAssemblyResolver(_tempDirectory!, debugMessages.Add, _ => { }); + + Assert.DoesNotThrow(() => resolver.RegisterPackageClosure(assemblyPath)); + Assert.That(debugMessages, Has.Some.Contains("Failed to read")); + } + + private string CreateFakeNuGetPackage(string packageName) + { + var source = Helpers.GetExpectedFromFile(method: "PackageSource") + .Replace("$PACKAGE$", packageName); + return FakeNuGetPackage.Create( + _tempDirectory!, + packageName, + "1.0.0", + source); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/NugetAssemblyResolverTests/PackageSource.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/NugetAssemblyResolverTests/PackageSource.cs new file mode 100644 index 00000000000..91e0f201db6 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/NugetAssemblyResolverTests/PackageSource.cs @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +namespace $PACKAGE$ +{ + public class PackageType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs index 87c0cb6f525..e512ebde889 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs @@ -50,6 +50,7 @@ public static string Create( var pkgDir = Path.Combine(versionDir, "lib", "netstandard2.0"); Directory.CreateDirectory(pkgDir); WriteNuspec(versionDir, packageName, version, dependencies); + WritePackageMetadata(versionDir); var references = new List { @@ -109,5 +110,18 @@ private static void WriteNuspec( File.WriteAllText(Path.Combine(versionDir, $"{packageName.ToLowerInvariant()}.nuspec"), nuspec); } + + private static void WritePackageMetadata(string versionDir) + { + var metadata = + """ + { + "version": 2, + "contentHash": "VGVzdCBwYWNrYWdlIGNvbnRlbnQgaGFzaA==", + "source": "https://example.invalid/v3/index.json" + } + """; + File.WriteAllText(Path.Combine(versionDir, ".nupkg.metadata"), metadata); + } } } From 8c4288846632b6bf1906efba9b756ecde61023c9 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 30 Jul 2026 11:48:09 -0700 Subject: [PATCH 5/5] chore(csharp): suppress resolver spell check false positives Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67f4b975-2057-47dd-8364-2d2b6ac1c29e --- .../src/Utilities/NugetPackageResolver.cs | 1 + .../test/common/FakeNuGetPackage.cs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs index e7367009900..2c2883a3534 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs @@ -109,6 +109,7 @@ public static bool TryFindPackageInCache( NuGetVersion version, [NotNullWhen(true)] out NuGet.Repositories.LocalPackageInfo? packageInfo) { + // cspell: disable-next-line var localRepository = new NuGetv3LocalRepository(globalPackagesFolder); packageInfo = localRepository.FindPackage(packageName, version); return packageInfo != null; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs index e512ebde889..4d19793b04b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/FakeNuGetPackage.cs @@ -113,6 +113,7 @@ private static void WriteNuspec( private static void WritePackageMetadata(string versionDir) { + // cspell: disable var metadata = """ { @@ -121,6 +122,7 @@ private static void WritePackageMetadata(string versionDir) "source": "https://example.invalid/v3/index.json" } """; + // cspell: enable File.WriteAllText(Path.Combine(versionDir, ".nupkg.metadata"), metadata); } }