Conversation
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration |
There was a problem hiding this comment.
Pull request overview
Adds a new ConfigurationIgnoreAttribute to the configuration abstractions and updates both the reflection-based binder and the source generator so selected properties can be explicitly excluded from binding.
Changes:
- Introduces
ConfigurationIgnoreAttributeinMicrosoft.Extensions.Configuration.Abstractions(impl + ref surface). - Updates reflection binder and source generator to skip ignored properties during binding and constructor-parameter matching.
- Adds/extends tests and updates package documentation examples to cover ignore behavior and
ErrorOnUnknownConfiguration.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs | Adds a primary-ctor test type with an ignored property to validate ctor/property matching behavior. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Helpers.cs | Extends ComplexOptions with an ignored property for binder test coverage. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs | Adds tests asserting ignored properties aren’t bound and don’t trip ErrorOnUnknownConfiguration, plus ctor-matching behavior. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs | Skips ignored properties in reflection binding and in ctor-parameter/property equivalence checks; adds IsIgnoredProperty. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/TypeIndex.cs | Ensures generator considers property.IsIgnored when deciding if a property is bindable. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/PropertySpec.cs | Adds IsIgnored flag to property spec model used by the generator. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Parser/KnownTypeSymbols.cs | Adds symbol lookup for ConfigurationIgnoreAttribute for generator parsing. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs | Parses [ConfigurationIgnore] and attempts to incorporate it into property/ctor matching. |
| src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/PACKAGE.md | Updates package example to mention/use [ConfigurationIgnore]. |
| src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/ConfigurationIgnoreAttribute.cs | Adds the new public attribute type. |
| src/libraries/Microsoft.Extensions.Configuration.Abstractions/ref/Microsoft.Extensions.Configuration.Abstractions.cs | Adds the attribute to the reference assembly surface area. |
| src/libraries/Microsoft.Extensions.Configuration.Abstractions/README.md | Updates README example to mention/use [ConfigurationIgnore]. |
| var dic = new Dictionary<string, string> | ||
| { | ||
| {"named_property", "value for named property"}, | ||
| {"ignored_property", "this value is ignored"}, |
There was a problem hiding this comment.
The sample uses the config key "ignored_property", which does not match the property name "IgnoredProperty" (and there's no ConfigurationKeyNameAttribute mapping). As a result, the example would keep the default value even without [ConfigurationIgnore], so it doesn’t actually demonstrate the ignore behavior. Consider using the matching key (e.g., "IgnoredProperty") or adding a ConfigurationKeyName mapping so the key would otherwise bind.
| {"ignored_property", "this value is ignored"}, | |
| {"IgnoredProperty", "this value is ignored"}, |
| var dic = new Dictionary<string, string> | ||
| { | ||
| {"named_property", "value for named property"}, | ||
| {"ignored_property", "this value is ignored"}, |
There was a problem hiding this comment.
Same issue as PACKAGE.md: the example sets "ignored_property" in configuration, but the property name is "IgnoredProperty" and no key-name mapping is provided. The value would remain "default" even without [ConfigurationIgnore], so the snippet doesn’t prove the attribute works. Prefer using a matching key ("IgnoredProperty") or add a ConfigurationKeyName mapping for the ignored property.
| {"ignored_property", "this value is ignored"}, | |
| {"IgnoredProperty", "this value is ignored"}, |
| string parameterName = parameter.Name; | ||
|
|
||
| if (properties?.TryGetValue(parameterName, out PropertySpec? propertySpec) is not true) | ||
| { | ||
| (missingParameters ??= new()).Add(parameterName); | ||
| } | ||
| else if (parameter.RefKind is not RefKind.None) | ||
| else if (parameter.RefKind is not RefKind.None || propertySpec.IsIgnored) | ||
| { | ||
| (invalidParameters ??= new()).Add(parameterName); | ||
| } |
There was a problem hiding this comment.
Ctor-parameter matching treats an ignored property as an invalid parameter (leading to the CannotBindToConstructorParameter error), but the reflection binder treats ignored properties as non-bindable and fails with ConstructorParametersDoNotMatchProperties. To keep source-generated binding behavior/error messages consistent with the reflection binder, consider treating propertySpec.IsIgnored as if the property were missing (i.e., add to missingParameters instead of invalidParameters).
| } | ||
|
|
||
| [Fact] | ||
| public void CanIgnoreConfigurationAttributes() |
There was a problem hiding this comment.
Test name is a bit ambiguous: this test is specifically validating [ConfigurationIgnore] behavior, but the name reads like it covers multiple attributes. Consider renaming to something more specific (e.g., CanIgnoreConfigurationIgnoreAttribute).
| public void CanIgnoreConfigurationAttributes() | |
| public void CanIgnoreConfigurationIgnoreAttribute() |
| } | ||
|
|
||
| [Fact] | ||
| public void ThrowOnClassWithPrimaryCtorAndIgnoredProperty() |
There was a problem hiding this comment.
For consistency with other test naming in this file (e.g., DoesNotThrowWhen...), consider using a verb form like "Throws..." / "ThrowsWhen..." rather than "ThrowOn...".
| public void ThrowOnClassWithPrimaryCtorAndIgnoredProperty() | |
| public void ThrowsOnClassWithPrimaryCtorAndIgnoredProperty() |
| return propertyBindingPoint.Value; | ||
| } | ||
|
|
||
| private static bool IsIgnoredProperty(PropertyInfo property) |
There was a problem hiding this comment.
This can be simplified as:
private static bool IsIgnoredProperty(PropertyInfo property)
=> property.IsDefined(typeof(ConfigurationIgnoreAttribute));| { | ||
| TypeSpec propTypeSpec = GetEffectiveTypeSpec(property.TypeRef); | ||
| return IsAccessible() && !IsCollectionAndCannotOverride() && !IsDictWithUnsupportedKey(); | ||
| return IsAccessible() && !property.IsIgnored && !IsCollectionAndCannotOverride() && !IsDictWithUnsupportedKey(); |
There was a problem hiding this comment.
What happen when having a property with the ignore attribute and the configuration contains a matching key to this property? I guess in this case we can end-up with ErrorOnUnknownConfiguration. This will be inconsistent with the runtime binder too even with the changes here because GetAllProperties() which returns all properties (including ignored ones). It is worth adding a test case for that too.
Fixes #125111
Add
ConfigurationIgnoreAttributeso configuration binding can explicitly skip selected properties.Changes
ConfigurationIgnoreAttributetoMicrosoft.Extensions.Configuration.Abstractions(implementation + ref).ErrorOnUnknownConfiguration, and constructor matching with ignored properties.Validation
dotnet buildfor Abstractions and Binder.