From 2fcb9a02016da496e08971d294d8907295d5328f Mon Sep 17 00:00:00 2001 From: caran Date: Mon, 24 Aug 2026 16:39:07 +0200 Subject: [PATCH 1/2] Fix analyzer transitively flowing to non-referencing projects FactoryGenerator.csproj never set DevelopmentDependency=true, the standard/documented setting for Roslyn analyzer/source-generator NuGet packages. Without it, a project that only transitively depends on FactoryGenerator (e.g. via a ProjectReference to another library that itself references it directly) also gets the FactoryGenerator analyzer registered on top of that project's own direct reference. Roslyn does not deduplicate separate items even when they load the same generator, so both instances independently try to AddSource the same generated-source hint names (e.g. FactoryGenerator.Declarations/{member}.g.cs for an IContainer-typed constructor parameter). The second AddSource call throws ArgumentException('...must be unique within a generator'), aborting the whole generator run for that compilation with zero output - which is what produces a cascading 'CS0234: ...Generated does not exist' in consuming code, since the generator failed to emit anything. DevelopmentDependency=true stops the analyzer from flowing to a referencing project's own downstream consumers, regardless of whether that project's own PackageReference sets PrivateAssets. Verified via dotnet pack that the resulting nuspec now correctly emits true. This setting has never been present in this file's history (confirmed via git log), so this is a pre-existing packaging gap, not something introduced by the recently-merged perf work. Also adds a regression test covering a related, previously-untested (but safe) scenario: two separate injections both taking an IContainer-typed external constructor parameter with the identical name - confirms that case is correctly deduplicated within a single generator run and was not itself the source of the crash. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- FactoryGenerator/FactoryGenerator.csproj | 17 ++++++ .../GeneratorBehaviorTests.cs | 52 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/FactoryGenerator/FactoryGenerator.csproj b/FactoryGenerator/FactoryGenerator.csproj index 3746997..dff5013 100644 --- a/FactoryGenerator/FactoryGenerator.csproj +++ b/FactoryGenerator/FactoryGenerator.csproj @@ -7,6 +7,23 @@ enable false true + + true diff --git a/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs b/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs index ed521d0..2c868f8 100644 --- a/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs +++ b/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs @@ -92,6 +92,58 @@ public class ImplicitlyInjected : IMarker generatedSource.ShouldContain("ImplicitlyInjected"); } + [Test] + public void GeneratorSupportsIdenticallyNamedContainerParametersAcrossDifferentInjections() + { + // Two unrelated injections both take an IContainer-typed external constructor parameter + // with the exact same identifier name ("lifeTimeScope"). Both end up as + // declarations[parameter.Name] = "..." in GenerateCode; since that's a Dictionary, the + // second assignment must just overwrite the first (same key) rather than producing two + // separate AddSource calls for the same hint name — pins down that this legitimate, + // realistic naming collision within a single generator run doesn't throw. + const string source = """ + using FactoryGenerator; + using FactoryGenerator.Attributes; + + namespace Sample + { + public interface IServiceA + { + } + + public interface IServiceB + { + } + + [Inject] + public class ServiceA : IServiceA + { + public ServiceA(IContainer lifeTimeScope) + { + } + } + + [Inject] + public class ServiceB : IServiceB + { + public ServiceB(IContainer lifeTimeScope) + { + } + } + } + """; + + var compilation = CreateCompilation(source); + var (runResult, outputCompilation) = RunGenerator(compilation); + var generatorResult = runResult.Results[0]; + + generatorResult.Exception.ShouldBeNull(); + outputCompilation.GetDiagnostics() + .Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Error) + .ToArray() + .ShouldBeEmpty(); + } + [Test] public void GeneratorSupportsBooleanKeysThatAreNotIdentifiers() { From dc406c39f23571c18ee3ec7dd00a861284261de1 Mon Sep 17 00:00:00 2001 From: caran Date: Mon, 24 Aug 2026 17:12:12 +0200 Subject: [PATCH 2/2] Fix generator crash on external parameter names differing only by case CONFIRMED root cause, reproduced end-to-end against the real downstream consumer's repository: Roslyn's SourceProductionContext.AddSource compares hint names with StringComparer.OrdinalIgnoreCase (verified against dotnet/roslyn's GeneratorDriver.cs and its own test suite), but GenerateCode's declarations/rrayDeclarations dictionaries used the default (ordinal, case-SENSITIVE) comparer for their keys. Those keys double as generated hint names AND valid C# identifiers - e.g. two unrelated injections each taking an external IContainer-typed constructor parameter named 'lifeTimeScope' and 'lifetimeScope' are both completely valid on their own (C# identifiers are case-sensitive), so both survived as separate dictionary entries, but then both independently tried to become their own generated source file. Roslyn's case-insensitive uniqueness check rejected the second one, throwing ArgumentException('...must be unique within a generator') and aborting the entire generator run for that compilation with zero output - the exact reported regression (CS8785 warning, cascading CS0234 for the missing Generated namespace). This is a long-standing latent bug (the affected dictionaries have used the default comparer since the project's initial commit), not something introduced by the recently-merged perf work - it was just never previously triggered by two same-name-different-case identifiers landing in the same compilation. Fix: since a partial class's members can be split across any number of source files (or consolidated into one) with zero effect on the compiled result, group declarations by a case-insensitive hint name and merge any colliding entries into a single generated file instead of emitting one AddSource call per dictionary key. This preserves every declaration (both differently-cased members still get generated and remain independently usable) rather than silently dropping one via a case-insensitive dictionary overwrite, or crashing the whole generator. Validated three ways: 1. Full test suite: 91/91 passing, including two new regression tests (identical external-parameter names, and names differing only by case). 2. Reproduced the exact crash live against the actual downstream consumer's real WeConfig.Application project (confirmed two classes, SessionFactory and Engine, with external IContainer parameters 'lifeTimeScope'/'lifetimeScope' reachable from the same compilation). 3. Patched the locally-cached FactoryGenerator analyzer DLL with this fix and rebuilt WeConfig.Application end-to-end: build now succeeds with 0 errors (previously failed with the reported CS8785/CS0234). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...FactoryGenerator.ContainerCodeTemplates.cs | 52 +++++++++++++--- .../GeneratorBehaviorTests.cs | 59 +++++++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/FactoryGenerator/Generation/FactoryGenerator.ContainerCodeTemplates.cs b/FactoryGenerator/Generation/FactoryGenerator.ContainerCodeTemplates.cs index 3b8802e..6e70232 100644 --- a/FactoryGenerator/Generation/FactoryGenerator.ContainerCodeTemplates.cs +++ b/FactoryGenerator/Generation/FactoryGenerator.ContainerCodeTemplates.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Text; @@ -577,12 +578,12 @@ private void InitializeLookup() private static void ArrayDeclarations(string usingStatements, Dictionary arrayDeclarations, string className, SourceProductionContext context) { var cacheInvalidations = string.Join("\n ", arrayDeclarations.Keys.Select(name => $"m_{name} = null;")); - foreach (var declaration in arrayDeclarations) + foreach (var group in GroupByHintName(arrayDeclarations)) { - context.AddSource($"FactoryGenerator.Collection.Declarations/{declaration.Key}.g.cs", $@"{usingStatements} + context.AddSource($"FactoryGenerator.Collection.Declarations/{group.HintName}.g.cs", $@"{usingStatements} public partial class {className} {{ - {declaration.Value} + {string.Join("\n ", group.Values)} }}"); } @@ -598,19 +599,56 @@ public void InvalidateCollectionCaches() private static void Declarations(string usingStatements, Dictionary declarations, string className, SourceProductionContext context) { - foreach (var declaration in declarations) + foreach (var group in GroupByHintName(declarations)) { - context.AddSource($"FactoryGenerator.Declarations/{declaration.Key}.g.cs", + context.AddSource($"FactoryGenerator.Declarations/{group.HintName}.g.cs", $$""" {{usingStatements}} public partial class {{className}} { - {{declaration.Value}} + {{string.Join("\n ", group.Values)}} }; """); } } + /// + /// Groups declaration-dictionary entries by a hint name Roslyn would treat as identical. + /// SourceProductionContext.AddSource compares hint names with + /// (matching how the names typically also map + /// to files on a case-insensitive file system), but the dictionaries this feeds are keyed + /// with the default (ordinal, case-SENSITIVE) comparer — because the keys are also valid, + /// distinct C# member/field names, and C# identifiers genuinely are case-sensitive (e.g. two + /// different injections independently choosing external "IContainer"-typed constructor + /// parameters named "lifeTimeScope" and "lifetimeScope" are both completely valid, unrelated + /// C# programs on their own). Naively emitting one hint name per dictionary key would crash + /// the entire generator with "hintName ... must be unique within a generator" as soon as two + /// such keys collided only by case. Since a partial class's members can be split across any + /// number of source files (or consolidated into one) with zero effect on the compiled + /// result, the fix is to detect that case-only collision here and merge the colliding + /// declarations into a single generated file/hint name — preserving every declaration + /// instead of losing one to a silent dictionary overwrite or crashing outright. + /// + private static List<(string HintName, List Values)> GroupByHintName(Dictionary declarations) + { + var indexByHintName = new Dictionary(StringComparer.OrdinalIgnoreCase); + var groups = new List<(string HintName, List Values)>(); + foreach (var declaration in declarations) + { + if (indexByHintName.TryGetValue(declaration.Key, out var index)) + { + groups[index].Values.Add(declaration.Value); + } + else + { + indexByHintName[declaration.Key] = groups.Count; + groups.Add((declaration.Key, new List {declaration.Value})); + } + } + + return groups; + } + private static void MakeArray(Dictionary declarations, string name, string elementTypeFullName, Dictionary> interfaceInjectors, IReadOnlyDictionary booleanIdentifiers) diff --git a/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs b/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs index 2c868f8..a542d23 100644 --- a/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs +++ b/Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs @@ -144,6 +144,65 @@ public ServiceB(IContainer lifeTimeScope) .ShouldBeEmpty(); } + [Test] + public void GeneratorSupportsExternalParameterNamesDifferingOnlyByCase() + { + // Real-world reproduction: two unrelated injections each take an IContainer-typed external + // constructor parameter whose names differ ONLY by case ("lifeTimeScope" vs "lifetimeScope"). + // Roslyn's SourceProductionContext.AddSource compares hint names with + // StringComparer.OrdinalIgnoreCase, but GenerateCode's `declarations` dictionary used the + // default (ordinal, case-SENSITIVE) comparer — so both names survived as separate dictionary + // entries (correctly, since they're both valid, distinct C# identifiers) but then crashed + // the whole generator with "hintName ... must be unique within a generator" as soon as BOTH + // tried to become separate generated files. Pins down the fix: colliding declarations must + // be merged into a single generated file instead. + const string source = """ + using FactoryGenerator; + using FactoryGenerator.Attributes; + + namespace Sample + { + public interface IServiceA + { + } + + public interface IServiceB + { + } + + [Inject] + public class ServiceA : IServiceA + { + public ServiceA(IContainer lifeTimeScope) + { + } + } + + [Inject] + public class ServiceB : IServiceB + { + public ServiceB(IContainer lifetimeScope) + { + } + } + } + """; + + var compilation = CreateCompilation(source); + var (runResult, outputCompilation) = RunGenerator(compilation); + var generatorResult = runResult.Results[0]; + + generatorResult.Exception.ShouldBeNull(); + outputCompilation.GetDiagnostics() + .Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Error) + .ToArray() + .ShouldBeEmpty(); + + var generatedSource = string.Join(Environment.NewLine, generatorResult.GeneratedSources.Select(sourceResult => sourceResult.SourceText.ToString())); + generatedSource.ShouldContain("IContainer lifeTimeScope => this;"); + generatedSource.ShouldContain("IContainer lifetimeScope => this;"); + } + [Test] public void GeneratorSupportsBooleanKeysThatAreNotIdentifiers() {