Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate config.GetValue() with [NotNullIfNotNull] #101336

Merged
merged 3 commits into from Apr 29, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libraries/Directory.Build.targets
Expand Up @@ -170,6 +170,7 @@
<!-- Adds Nullable annotation attributes to C# non .NETCoreApp builds. -->
<ItemGroup Condition="'$(Nullable)' != '' and
'$(Nullable)' != 'disable' and
'$(SkipIncludeNullableAttributes)' != 'true' and
'$(MSBuildProjectExtension)' == '.csproj' and
'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\NullableAttributes.cs" Link="System\Diagnostics\CodeAnalysis\NullableAttributes.cs" />
Expand Down
Expand Up @@ -15,6 +15,7 @@ private sealed partial class Emitter
private readonly TypeIndex _typeIndex;
private readonly bool _emitEnumParseMethod;
private readonly bool _emitGenericParseEnum;
private readonly bool _emitNotNullIfNotNull;
private readonly bool _emitThrowIfNullMethod;

private readonly SourceWriter _writer = new();
Expand All @@ -26,6 +27,7 @@ public Emitter(SourceGenerationSpec sourceGenSpec)
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
_emitEnumParseMethod = sourceGenSpec.EmitEnumParseMethod;
_emitGenericParseEnum = sourceGenSpec.EmitGenericParseEnum;
_emitNotNullIfNotNull = sourceGenSpec.EmitNotNullIfNotNull;
_emitThrowIfNullMethod = sourceGenSpec.EmitThrowIfNullMethod;
}

Expand Down
Expand Up @@ -57,6 +57,7 @@ internal sealed partial class Parser(CompilationData compilationData)
ConfigTypes = _createdTypeSpecs.Values.OrderBy(s => s.TypeRef.FullyQualifiedName).ToImmutableEquatableArray(),
EmitEnumParseMethod = _emitEnumParseMethod,
EmitGenericParseEnum = _emitGenericParseEnum,
EmitNotNullIfNotNull = _typeSymbols.NotNullIfNotNullAttribute is not null,
EmitThrowIfNullMethod = IsThrowIfNullMethodToBeEmitted()
};
}
Expand Down
Expand Up @@ -73,6 +73,7 @@ private void EmitGetValueMethods()
if (ShouldEmitMethods(MethodsToGen.ConfigBinder_GetValue_T_key_defaultValue))
{
EmitStartDefinition_Get_Or_GetValue_Overload(MethodsToGen.ConfigBinder_GetValue_T_key_defaultValue, documentation);
EmitNotNullIfNotNull(Identifier.defaultValue);
_writer.WriteLine($"public static T? {Identifier.GetValue}<T>(this {Identifier.IConfiguration} {Identifier.configuration}, string {Identifier.key}, T {Identifier.defaultValue}) => " +
$"(T?)({expressionForGetValueCore}({Identifier.configuration}, typeof(T), {Identifier.key}) ?? {Identifier.defaultValue});");
}
Expand All @@ -87,11 +88,20 @@ private void EmitGetValueMethods()
if (ShouldEmitMethods(MethodsToGen.ConfigBinder_GetValue_TypeOf_key_defaultValue))
{
EmitStartDefinition_Get_Or_GetValue_Overload(MethodsToGen.ConfigBinder_GetValue_TypeOf_key_defaultValue, documentation);
EmitNotNullIfNotNull(Identifier.defaultValue);
_writer.WriteLine($"public static object? {Identifier.GetValue}(this {Identifier.IConfiguration} {Identifier.configuration}, Type {Identifier.type}, string {Identifier.key}, object? {Identifier.defaultValue}) => " +
$"{expressionForGetValueCore}({Identifier.configuration}, {Identifier.type}, {Identifier.key}) ?? {Identifier.defaultValue};");
}
}

private void EmitNotNullIfNotNull(string parameterName)
{
if (_emitNotNullIfNotNull)
{
_writer.WriteLine($"[return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof({parameterName}))]");
}
}

private void EmitBindMethods_ConfigurationBinder()
{
if (!ShouldEmitMethods(MethodsToGen.ConfigBinder_Bind))
Expand Down
Expand Up @@ -64,6 +64,7 @@ internal sealed class KnownTypeSymbols
public INamedTypeSymbol? MemberInfo { get; }
public INamedTypeSymbol? ParameterInfo { get; }
public INamedTypeSymbol? Delegate { get; }
public INamedTypeSymbol? NotNullIfNotNullAttribute { get; }

public KnownTypeSymbols(CSharpCompilation compilation)
{
Expand Down Expand Up @@ -132,6 +133,9 @@ public KnownTypeSymbols(CSharpCompilation compilation)
IntPtr = Compilation.GetSpecialType(SpecialType.System_IntPtr);
UIntPtr = Compilation.GetSpecialType(SpecialType.System_UIntPtr);
Delegate = Compilation.GetSpecialType(SpecialType.System_Delegate);

// Only generate nullable attributes if available
NotNullIfNotNullAttribute = compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute");
}
}
}
Expand Up @@ -12,6 +12,7 @@ public sealed record SourceGenerationSpec
public required ImmutableEquatableArray<TypeSpec> ConfigTypes { get; init; }
public required bool EmitEnumParseMethod { get; set; }
public required bool EmitGenericParseEnum { get; set; }
public required bool EmitNotNullIfNotNull { get; set; }
public required bool EmitThrowIfNullMethod { get; set; }
}
}
Expand Up @@ -32,10 +32,12 @@ public static partial class ConfigurationBinder
[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.")]
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; }
[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.")]
[return: System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
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; }
[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.")]
public static T? GetValue<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.All)] T>(this Microsoft.Extensions.Configuration.IConfiguration configuration, string key) { throw null; }
[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.")]
[return: System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
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; }
[System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("Binding strongly typed objects to configuration values requires generating dynamic code at runtime, for example instantiating generic types.")]
[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.")]
Expand Down
Expand Up @@ -167,6 +167,7 @@ public static void Bind(this IConfiguration configuration, object? instance, Act
/// <param name="defaultValue">The default value to use if no value is found.</param>
/// <returns>The converted value.</returns>
[RequiresUnreferencedCode(TrimmingWarningMessage)]
[return: NotNullIfNotNull(nameof(defaultValue))]
public static T? GetValue<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>(this IConfiguration configuration, string key, T defaultValue)
{
return (T?)GetValue(configuration, typeof(T), key, defaultValue);
Expand Down Expand Up @@ -198,6 +199,7 @@ public static void Bind(this IConfiguration configuration, object? instance, Act
/// <param name="defaultValue">The default value to use if no value is found.</param>
/// <returns>The converted value.</returns>
[RequiresUnreferencedCode(TrimmingWarningMessage)]
[return: NotNullIfNotNull(nameof(defaultValue))]
public static object? GetValue(
this IConfiguration configuration,
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]
Expand Down
Expand Up @@ -326,11 +326,15 @@ public void CanBindToObjectProperty()
[Fact]
public void GetNullValue()
{
var dic = new Dictionary<string, string>
#nullable enable
#pragma warning disable IDE0004 // Cast is redundant

var dic = new Dictionary<string, string?>
{
{"Integer", null},
{"Boolean", null},
{"Nested:Integer", null},
{"String", null},
{"Object", null }
};
var configurationBuilder = new ConfigurationBuilder();
Expand All @@ -341,13 +345,16 @@ public void GetNullValue()
Assert.False(config.GetValue<bool>("Boolean"));
Assert.Equal(0, config.GetValue<int>("Integer"));
Assert.Equal(0, config.GetValue<int>("Nested:Integer"));
Assert.Null(config.GetValue<string>("String"));
Assert.Null(config.GetValue<ComplexOptions>("Object"));

// Generic overloads with default value.
Assert.True(config.GetValue("Boolean", true));
Assert.Equal(1, config.GetValue("Integer", 1));
Assert.Equal(1, config.GetValue("Nested:Integer", 1));
Assert.Equal(new NestedConfig(""), config.GetValue("Object", new NestedConfig("")));
Assert.True((bool)config.GetValue("Boolean", true));
Assert.Equal(1, (int)config.GetValue("Integer", 1));
Assert.Equal(1, (int)config.GetValue("Nested:Integer", 1));
// [NotNullIfNotNull] avoids CS8600: Converting possible null value to non-nullable type.
Assert.Equal("s", (string)config.GetValue("String", "s"));
Assert.Equal(new NestedConfig(""), (NestedConfig)config.GetValue("Object", new NestedConfig("")));

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

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

// GetSection tests.
Assert.False(config.GetSection("Boolean").Get<bool>());
Assert.Equal(0, config.GetSection("Integer").Get<int>());
Assert.Equal(0, config.GetSection("Nested:Integer").Get<int>());
Assert.Null(config.GetSection("Object").Get<ComplexOptions>());

#pragma warning restore IDE0004
#nullable restore
}

[Fact]
Expand Down
Expand Up @@ -39,6 +39,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration

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

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

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

Expand Down
Expand Up @@ -35,6 +35,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
#region IConfiguration extensions.
/// <summary>Extracts the value with the specified key and converts it to the specified type.</summary>
[InterceptsLocation(@"src-0.cs", 12, 20)]
[return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
public static T? GetValue<T>(this IConfiguration configuration, string key, T defaultValue) => (T?)(BindingExtensions.GetValueCore(configuration, typeof(T), key) ?? defaultValue);
#endregion IConfiguration extensions.

Expand Down
Expand Up @@ -35,6 +35,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
#region IConfiguration extensions.
/// <summary>Extracts the value with the specified key and converts it to the specified type.</summary>
[InterceptsLocation(@"src-0.cs", 11, 20)]
[return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(defaultValue))]
public static object? GetValue(this IConfiguration configuration, Type type, string key, object? defaultValue) => BindingExtensions.GetValueCore(configuration, type, key) ?? defaultValue;
#endregion IConfiguration extensions.

Expand Down
Expand Up @@ -8,6 +8,9 @@
<NoWarn>$(NoWarn);SYSLIB1103,SYSLIB1104</NoWarn>
<InterceptorsPreviewNamespaces>$(InterceptorsPreviewNamespaces);Microsoft.Extensions.Configuration.Binder.SourceGeneration</InterceptorsPreviewNamespaces>
<EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>

<!-- Avoid conflict with Microsoft.Extensions.Configuration.Binder (via InternalsVisibleTo) -->
<SkipIncludeNullableAttributes>true</SkipIncludeNullableAttributes>
</PropertyGroup>

<PropertyGroup>
Expand Down