Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions FactoryGenerator/FactoryGenerator.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@
<Nullable>enable</Nullable>
<IncludeBuildOutput>false</IncludeBuildOutput>
<SuppressDependenciesWhenPacking>true</SuppressDependenciesWhenPacking>
<!--
Marks this package as a build-time-only/analyzer dependency. Without this, NuGet has no
signal that FactoryGenerator's analyzer reference should stop at the directly-referencing
project: a project that only transitively depends on FactoryGenerator (e.g. via a
ProjectReference to another library that itself references this package) would ALSO get
the FactoryGenerator analyzer registered, on top of that project's own direct reference —
two separate <Analyzer> items loading the same generator type for one compilation. Roslyn
does not deduplicate across such items, so both instances independently try to add the
same generated-source hint names (e.g. "FactoryGenerator.Declarations/{member}.g.cs"),
and the second AddSource call throws ArgumentException("...must be unique within a
generator"), aborting the whole generator run for that compilation with zero output.
DevelopmentDependency=true is the standard, documented fix for Roslyn analyzer/source
generator NuGet packages (see: "How to create a NuGet package with a Roslyn analyzer") —
it stops the package from flowing to a referencing project's own downstream consumers,
regardless of whether that referencing project explicitly sets PrivateAssets.
-->
<DevelopmentDependency>true</DevelopmentDependency>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Text;
Expand Down Expand Up @@ -577,12 +578,12 @@ private void InitializeLookup()
private static void ArrayDeclarations(string usingStatements, Dictionary<string, string> 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)}
}}");
}

Expand All @@ -598,19 +599,56 @@ public void InvalidateCollectionCaches()

private static void Declarations(string usingStatements, Dictionary<string, string> 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)}}
};
""");
}
}

/// <summary>
/// Groups declaration-dictionary entries by a hint name Roslyn would treat as identical.
/// <c>SourceProductionContext.AddSource</c> compares hint names with
/// <see cref="StringComparer.OrdinalIgnoreCase"/> (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.
/// </summary>
private static List<(string HintName, List<string> Values)> GroupByHintName(Dictionary<string, string> declarations)
{
var indexByHintName = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase);
var groups = new List<(string HintName, List<string> 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<string> {declaration.Value}));
}
}

return groups;
}

private static void MakeArray(Dictionary<string, string> declarations, string name,
string elementTypeFullName, Dictionary<string, List<InjectionData>> interfaceInjectors,
IReadOnlyDictionary<string, string> booleanIdentifiers)
Expand Down
111 changes: 111 additions & 0 deletions Tests/FactoryGenerator.Tests/GeneratorBehaviorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,117 @@ 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 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()
{
Expand Down
Loading