From da7c35f403ae1b4e5dff7996833e30b23e843ab9 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Sun, 27 Sep 2026 14:35:59 +0100 Subject: [PATCH] fix: attribute name reading failed --- docs/wiki/Attribute-Data-Models.md | 29 ++++++++++++++++++- package.json | 2 +- .../Models.cs | 17 ++++++++--- .../ServiceRegistrationEmitter.cs | 10 ++++++- .../Helpers/AttributeDataModelLibrary.cs | 12 +++++++- .../AttributeDataModelGeneratorTests.cs | 15 ++++++++++ 6 files changed, 77 insertions(+), 8 deletions(-) diff --git a/docs/wiki/Attribute-Data-Models.md b/docs/wiki/Attribute-Data-Models.md index f97ea52..26ff310 100644 --- a/docs/wiki/Attribute-Data-Models.md +++ b/docs/wiki/Attribute-Data-Models.md @@ -17,7 +17,7 @@ The generator emits marker attributes into your compilation: | --- | --- | | `[Generate(Type targetAttribute)]` | Placed on a `readonly partial record struct` to opt into generation. | | `[Generate(string targetAttribute)]` | Resolves the attribute by fully-qualified name. Use when the attribute type is not available in the generator's compilation (e.g. `LengthAttribute` in .NET 8+ or a self-generated attribute). | -| `[Property]` | A record parameter is populated from a named attribute property (the property name is inferred from the parameter name unless overridden). | +| `[Property]` | A record parameter is populated from a named attribute property (the property name is inferred from the parameter name unless overridden). When combined with `[Argument]` on the same parameter, the named argument is read first. | | `[Property(string name)]` | Explicit named property source. | | `[Property(..., DefaultValue = ...)]` | Fallback value when the named property is not present. | | `[Argument]` | Populated from a constructor argument by parameter name. | @@ -105,6 +105,33 @@ public readonly partial record struct StringLengthAttributeData( ); ``` +## Constructor and named arguments on the same property + +A property can declare both sources: + +```csharp +[Generate(typeof(GenerateServiceAttribute))] +public readonly partial record struct GenerateServiceAttributeData( + [Argument("lifetime", IsEnum = true, DefaultValue = "…ServiceLifetime.Singleton")] string? Lifetime, + [Argument("name")] [Property] string? Name +); +``` + +The named argument is read first. A named argument assigns the property/field *after* the constructor +runs, so it is the effective value whenever a caller supplies both — mirroring the attribute instance's +own assignment order. Reading it first also prevents an omitted optional constructor parameter's default +from shadowing an explicitly set property: + +```csharp +[GenerateService(Name = "Billing")] // reads "Billing" +[GenerateService(ServiceLifetime.Scoped, "Billing")] // reads "Billing" +``` + +> [!IMPORTANT] +> Before this rule, the constructor argument was read first, so +> `[GenerateService(Name = "Billing")]` resolved to the `name` parameter's default (`null`) and the +> explicitly set property was silently ignored. + ## Nested models Any property whose type is itself annotated with `[Generate]` can be populated as a nested model. diff --git a/package.json b/package.json index c20d121..2fa2581 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "purview-sourcegenerator-framework", - "version": "1.0.0-prerelease.52", + "version": "1.0.0-prerelease.53", "license": "MIT", "author": { "name": "Kieron Lanning", diff --git a/src/src/SourceGeneratorFramework.ExampleGenerator/Models.cs b/src/src/SourceGeneratorFramework.ExampleGenerator/Models.cs index 9d66717..21d4450 100644 --- a/src/src/SourceGeneratorFramework.ExampleGenerator/Models.cs +++ b/src/src/SourceGeneratorFramework.ExampleGenerator/Models.cs @@ -28,8 +28,10 @@ public enum ServiceLifetime /// Initializes a new instance of the class. /// /// The service lifetime. +/// The optional service name. [AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)] -public sealed class GenerateServiceAttribute(ServiceLifetime lifetime = ServiceLifetime.Singleton) : Attribute +public sealed class GenerateServiceAttribute(ServiceLifetime lifetime = ServiceLifetime.Singleton, string? name = null) + : Attribute { /// /// Gets the service lifetime. @@ -37,14 +39,21 @@ public sealed class GenerateServiceAttribute(ServiceLifetime lifetime = ServiceL public ServiceLifetime Lifetime { get; } = lifetime; /// - /// Gets or sets the optional service name. + /// Gets or sets the optional service name. This property can be supplied either as the constructor's + /// name argument or as a named argument ([GenerateService(Name = "…")]); the named + /// argument wins when both are supplied because it is assigned after the constructor runs. /// - public string? Name { get; set; } + public string? Name { get; set; } = name; } /// /// Attribute data model for . /// +/// +/// demonstrates a property mapped from both a constructor argument and a named +/// argument: the named argument is read first so an explicitly set property is never shadowed by the +/// constructor parameter's default. +/// [Generate(typeof(GenerateServiceAttribute))] public readonly partial record struct GenerateServiceAttributeData( [Argument( @@ -53,7 +62,7 @@ public readonly partial record struct GenerateServiceAttributeData( DefaultValue = "Purview.SourceGeneratorFramework.Examples.ServiceLifetime.Singleton" )] string? Lifetime, - [Property] string? Name + [Argument("name")] [Property] string? Name ); /// diff --git a/src/src/SourceGeneratorFramework.ExampleGenerator/ServiceRegistrationEmitter.cs b/src/src/SourceGeneratorFramework.ExampleGenerator/ServiceRegistrationEmitter.cs index 0884277..f9a8921 100644 --- a/src/src/SourceGeneratorFramework.ExampleGenerator/ServiceRegistrationEmitter.cs +++ b/src/src/SourceGeneratorFramework.ExampleGenerator/ServiceRegistrationEmitter.cs @@ -58,10 +58,18 @@ options with { DefaultValue = lifetimeValues[0].FullName, }, + new("name", PurviewTypeLibrary.System.String.MakeNullable(cw)) + { + DefaultValue = "null", + }, ], } ), - body => body.Assignment("Lifetime", "lifetime") + body => + { + body.Assignment("Lifetime", "lifetime"); + body.Assignment("Name", "name"); + } ); cw.Property( diff --git a/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs b/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs index e084c07..a93ad0e 100644 --- a/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs +++ b/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs @@ -423,6 +423,13 @@ static ParameterAttributeInfo ReadParameterAttributes( var ctorAttribute = GetAttribute(parameter, GeneratorTypeLibrary.Attirbutes.ArgumentAttribute); var isEnum = false; + + // Constructor sources are added from this index so the named-argument source below can be placed + // ahead of them. A named argument assigns the property/field after the constructor runs, so it is + // the effective value whenever both map to the same model property. Reading it first also stops an + // omitted optional constructor parameter's default from shadowing an explicitly set property. + var constructorSourceIndex = sources.Count; + if (ctorAttribute is not null && !hasExclusive) { var ctorName = GetCtorPropertyName(ctorAttribute); @@ -457,7 +464,10 @@ static ParameterAttributeInfo ReadParameterAttributes( ); isEnum = isEnum || GetNamedArgument(namedAttribute, "IsEnum", false); - sources.Add(new PropertySource(AttributePropertySource.NamedArgument, namedName ?? propertyName, -1)); + sources.Insert( + constructorSourceIndex, + new PropertySource(AttributePropertySource.NamedArgument, namedName ?? propertyName, -1) + ); if (namedDefaultValue is not null) { diff --git a/src/tests/SourceGeneratorFramework.Generators.UnitTests/AttributeDataModelGeneratorTests.cs b/src/tests/SourceGeneratorFramework.Generators.UnitTests/AttributeDataModelGeneratorTests.cs index 6950d11..fd38848 100644 --- a/src/tests/SourceGeneratorFramework.Generators.UnitTests/AttributeDataModelGeneratorTests.cs +++ b/src/tests/SourceGeneratorFramework.Generators.UnitTests/AttributeDataModelGeneratorTests.cs @@ -209,6 +209,21 @@ await Assert .That(generated) .Contains("if (!attributeData.TryGetNamedArgument(\"GenerateOptions\", out generateOptions))"); await Assert.That(generated).Contains("generateOptions = true"); + + // A named argument is assigned after the constructor runs, so when both a constructor argument and + // a named argument map to the same property the named argument must be read first. This also stops + // an omitted optional constructor parameter's default from shadowing an explicitly set property. + var namedIndex = generated!.IndexOf("TryGetNamedArgument(\"GenerateOptions\"", StringComparison.Ordinal); + var ctorIndex = generated.IndexOf( + "TryGetConstructorArgument(\"generateOptions\"", + StringComparison.Ordinal + ); + await Assert.That(namedIndex).IsGreaterThanOrEqualTo(0); + await Assert.That(ctorIndex).IsGreaterThanOrEqualTo(0); + await Assert + .That(namedIndex) + .IsLessThan(ctorIndex) + .Because("the named argument must be read before the constructor argument"); } [Test]