Skip to content

Commit 92ca5f3

Browse files
dahlbykdahlbyk
authored
Annotate config.GetValue() with [NotNullIfNotNull] (#101336)
* Annotate GetValue() with [NotNullIfNotNull] * Avoid duplicate NullableAttributes.cs * Annotate generated GetValue() with [NotNullIfNotNull]
1 parent 9b4c5c6 commit 92ca5f3

13 files changed

+49
-6
lines changed

‎src/libraries/Directory.Build.targets‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
<!-- Adds Nullable annotation attributes to C# non .NETCoreApp builds. -->
171171
<ItemGroup Condition="'$(Nullable)' != '' and
172172
'$(Nullable)' != 'disable' and
173+
'$(SkipIncludeNullableAttributes)' != 'true' and
173174
'$(MSBuildProjectExtension)' == '.csproj' and
174175
'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
175176
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\NullableAttributes.cs" Link="System\Diagnostics\CodeAnalysis\NullableAttributes.cs" />

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Emitter.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ private sealed partial class Emitter
1515
private readonly TypeIndex _typeIndex;
1616
private readonly bool _emitEnumParseMethod;
1717
private readonly bool _emitGenericParseEnum;
18+
private readonly bool _emitNotNullIfNotNull;
1819
private readonly bool _emitThrowIfNullMethod;
1920

2021
private readonly SourceWriter _writer = new();
@@ -26,6 +27,7 @@ public Emitter(SourceGenerationSpec sourceGenSpec)
2627
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
2728
_emitEnumParseMethod = sourceGenSpec.EmitEnumParseMethod;
2829
_emitGenericParseEnum = sourceGenSpec.EmitGenericParseEnum;
30+
_emitNotNullIfNotNull = sourceGenSpec.EmitNotNullIfNotNull;
2931
_emitThrowIfNullMethod = sourceGenSpec.EmitThrowIfNullMethod;
3032
}
3133

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ internal sealed partial class Parser(CompilationData compilationData)
5757
ConfigTypes = _createdTypeSpecs.Values.OrderBy(s => s.TypeRef.FullyQualifiedName).ToImmutableEquatableArray(),
5858
EmitEnumParseMethod = _emitEnumParseMethod,
5959
EmitGenericParseEnum = _emitGenericParseEnum,
60+
EmitNotNullIfNotNull = _typeSymbols.NotNullIfNotNullAttribute is not null,
6061
EmitThrowIfNullMethod = IsThrowIfNullMethodToBeEmitted()
6162
};
6263
}

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/ConfigurationBinder.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ private void EmitGetValueMethods()
7373
if (ShouldEmitMethods(MethodsToGen.ConfigBinder_GetValue_T_key_defaultValue))
7474
{
7575
EmitStartDefinition_Get_Or_GetValue_Overload(MethodsToGen.ConfigBinder_GetValue_T_key_defaultValue, documentation);
76+
EmitNotNullIfNotNull(Identifier.defaultValue);
7677
_writer.WriteLine($"public static T? {Identifier.GetValue}<T>(this {Identifier.IConfiguration} {Identifier.configuration}, string {Identifier.key}, T {Identifier.defaultValue}) => " +
7778
$"(T?)({expressionForGetValueCore}({Identifier.configuration}, typeof(T), {Identifier.key}) ?? {Identifier.defaultValue});");
7879
}
@@ -87,11 +88,20 @@ private void EmitGetValueMethods()
8788
if (ShouldEmitMethods(MethodsToGen.ConfigBinder_GetValue_TypeOf_key_defaultValue))
8889
{
8990
EmitStartDefinition_Get_Or_GetValue_Overload(MethodsToGen.ConfigBinder_GetValue_TypeOf_key_defaultValue, documentation);
91+
EmitNotNullIfNotNull(Identifier.defaultValue);
9092
_writer.WriteLine($"public static object? {Identifier.GetValue}(this {Identifier.IConfiguration} {Identifier.configuration}, Type {Identifier.type}, string {Identifier.key}, object? {Identifier.defaultValue}) => " +
9193
$"{expressionForGetValueCore}({Identifier.configuration}, {Identifier.type}, {Identifier.key}) ?? {Identifier.defaultValue};");
9294
}
9395
}
9496

97+
private void EmitNotNullIfNotNull(string parameterName)
98+
{
99+
if (_emitNotNullIfNotNull)
100+
{
101+
_writer.WriteLine($"[return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof({parameterName}))]");
102+
}
103+
}
104+
95105
private void EmitBindMethods_ConfigurationBinder()
96106
{
97107
if (!ShouldEmitMethods(MethodsToGen.ConfigBinder_Bind))

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Parser/KnownTypeSymbols.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ internal sealed class KnownTypeSymbols
6464
public INamedTypeSymbol? MemberInfo { get; }
6565
public INamedTypeSymbol? ParameterInfo { get; }
6666
public INamedTypeSymbol? Delegate { get; }
67+
public INamedTypeSymbol? NotNullIfNotNullAttribute { get; }
6768

6869
public KnownTypeSymbols(CSharpCompilation compilation)
6970
{
@@ -132,6 +133,9 @@ public KnownTypeSymbols(CSharpCompilation compilation)
132133
IntPtr = Compilation.GetSpecialType(SpecialType.System_IntPtr);
133134
UIntPtr = Compilation.GetSpecialType(SpecialType.System_UIntPtr);
134135
Delegate = Compilation.GetSpecialType(SpecialType.System_Delegate);
136+
137+
// Only generate nullable attributes if available
138+
NotNullIfNotNullAttribute = compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute");
135139
}
136140
}
137141
}

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/SourceGenerationSpec.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ public sealed record SourceGenerationSpec
1212
public required ImmutableEquatableArray<TypeSpec> ConfigTypes { get; init; }
1313
public required bool EmitEnumParseMethod { get; set; }
1414
public required bool EmitGenericParseEnum { get; set; }
15+
public required bool EmitNotNullIfNotNull { get; set; }
1516
public required bool EmitThrowIfNullMethod { get; set; }
1617
}
1718
}

‎src/libraries/Microsoft.Extensions.Configuration.Binder/ref/Microsoft.Extensions.Configuration.Binder.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@ public static void Bind(this Microsoft.Extensions.Configuration.IConfiguration c
3232
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("In case the type is non-primitive, the trimmer cannot statically analyze the object's type so its members may be trimmed.")]
3333
public static object? GetValue(this Microsoft.Extensions.Configuration.IConfiguration configuration, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type type, string key) { throw null; }
3434
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("In case the type is non-primitive, the trimmer cannot statically analyze the object's type so its members may be trimmed.")]
35+
[return: System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
3536
public static object? GetValue(this Microsoft.Extensions.Configuration.IConfiguration configuration, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] System.Type type, string key, object? defaultValue) { throw null; }
3637
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("In case the type is non-primitive, the trimmer cannot statically analyze the object's type so its members may be trimmed.")]
3738
public static T? GetValue<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] T>(this Microsoft.Extensions.Configuration.IConfiguration configuration, string key) { throw null; }
3839
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("In case the type is non-primitive, the trimmer cannot statically analyze the object's type so its members may be trimmed.")]
40+
[return: System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
3941
public static T? GetValue<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] T>(this Microsoft.Extensions.Configuration.IConfiguration configuration, string key, T defaultValue) { throw null; }
4042
[System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("Binding strongly typed objects to configuration values requires generating dynamic code at runtime, for example instantiating generic types.")]
4143
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("In case the type is non-primitive, the trimmer cannot statically analyze the object's type so its members may be trimmed.")]

‎src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ public static void Bind(this IConfiguration configuration, object? instance, Act
167167
/// <param name="defaultValue">The default value to use if no value is found.</param>
168168
/// <returns>The converted value.</returns>
169169
[RequiresUnreferencedCode(TrimmingWarningMessage)]
170+
[return: NotNullIfNotNull(nameof(defaultValue))]
170171
public static T? GetValue<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>(this IConfiguration configuration, string key, T defaultValue)
171172
{
172173
return (T?)GetValue(configuration, typeof(T), key, defaultValue);
@@ -198,6 +199,7 @@ public static void Bind(this IConfiguration configuration, object? instance, Act
198199
/// <param name="defaultValue">The default value to use if no value is found.</param>
199200
/// <returns>The converted value.</returns>
200201
[RequiresUnreferencedCode(TrimmingWarningMessage)]
202+
[return: NotNullIfNotNull(nameof(defaultValue))]
201203
public static object? GetValue(
202204
this IConfiguration configuration,
203205
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,15 @@ public void CanBindToObjectProperty()
326326
[Fact]
327327
public void GetNullValue()
328328
{
329-
var dic = new Dictionary<string, string>
329+
#nullable enable
330+
#pragma warning disable IDE0004 // Cast is redundant
331+
332+
var dic = new Dictionary<string, string?>
330333
{
331334
{"Integer", null},
332335
{"Boolean", null},
333336
{"Nested:Integer", null},
337+
{"String", null},
334338
{"Object", null }
335339
};
336340
var configurationBuilder = new ConfigurationBuilder();
@@ -341,13 +345,16 @@ public void GetNullValue()
341345
Assert.False(config.GetValue<bool>("Boolean"));
342346
Assert.Equal(0, config.GetValue<int>("Integer"));
343347
Assert.Equal(0, config.GetValue<int>("Nested:Integer"));
348+
Assert.Null(config.GetValue<string>("String"));
344349
Assert.Null(config.GetValue<ComplexOptions>("Object"));
345350

346351
// Generic overloads with default value.
347-
Assert.True(config.GetValue("Boolean", true));
348-
Assert.Equal(1, config.GetValue("Integer", 1));
349-
Assert.Equal(1, config.GetValue("Nested:Integer", 1));
350-
Assert.Equal(new NestedConfig(""), config.GetValue("Object", new NestedConfig("")));
352+
Assert.True((bool)config.GetValue("Boolean", true));
353+
Assert.Equal(1, (int)config.GetValue("Integer", 1));
354+
Assert.Equal(1, (int)config.GetValue("Nested:Integer", 1));
355+
// [NotNullIfNotNull] avoids CS8600: Converting possible null value to non-nullable type.
356+
Assert.Equal("s", (string)config.GetValue("String", "s"));
357+
Assert.Equal(new NestedConfig(""), (NestedConfig)config.GetValue("Object", new NestedConfig("")));
351358

352359
// Type overloads.
353360
Assert.Null(config.GetValue(typeof(bool), "Boolean"));
@@ -356,16 +363,22 @@ public void GetNullValue()
356363
Assert.Null(config.GetValue(typeof(ComplexOptions), "Object"));
357364

358365
// Type overloads with default value.
366+
// [NotNullIfNotNull] avoids CS8605: Unboxing a possibly null value.
359367
Assert.True((bool)config.GetValue(typeof(bool), "Boolean", true));
360368
Assert.Equal(1, (int)config.GetValue(typeof(int), "Integer", 1));
361369
Assert.Equal(1, (int)config.GetValue(typeof(int), "Nested:Integer", 1));
362-
Assert.Equal(new NestedConfig(""), config.GetValue("Object", new NestedConfig("")));
370+
// [NotNullIfNotNull] avoids CS8600: Converting possible null value to non-nullable type.
371+
Assert.Equal("s", (string)config.GetValue(typeof(string), "String", "s"));
372+
Assert.Equal(new NestedConfig(""), (NestedConfig)config.GetValue("Object", new NestedConfig("")));
363373

364374
// GetSection tests.
365375
Assert.False(config.GetSection("Boolean").Get<bool>());
366376
Assert.Equal(0, config.GetSection("Integer").Get<int>());
367377
Assert.Equal(0, config.GetSection("Nested:Integer").Get<int>());
368378
Assert.Null(config.GetSection("Object").Get<ComplexOptions>());
379+
380+
#pragma warning restore IDE0004
381+
#nullable restore
369382
}
370383

371384
[Fact]

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/netcoreapp/ConfigurationBinder/GetValue.generated.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
3939

4040
/// <summary>Extracts the value with the specified key and converts it to the specified type.</summary>
4141
[InterceptsLocation(@"src-0.cs", 16, 24)]
42+
[return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
4243
public static T? GetValue<T>(this IConfiguration configuration, string key, T defaultValue) => (T?)(BindingExtensions.GetValueCore(configuration, typeof(T), key) ?? defaultValue);
4344

4445
/// <summary>Extracts the value with the specified key and converts it to the specified type.</summary>
@@ -47,6 +48,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
4748

4849
/// <summary>Extracts the value with the specified key and converts it to the specified type.</summary>
4950
[InterceptsLocation(@"src-0.cs", 17, 24)]
51+
[return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
5052
public static object? GetValue(this IConfiguration configuration, Type type, string key, object? defaultValue) => BindingExtensions.GetValueCore(configuration, type, key) ?? defaultValue;
5153
#endregion IConfiguration extensions.
5254

0 commit comments

Comments
 (0)