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

[Group 2] Enable nullable annotations for Microsoft.Extensions.Configuration.Abstractions #57401

Merged
merged 12 commits into from
Oct 6, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ namespace Microsoft.Extensions.Configuration
{
public static partial class ConfigurationExtensions
{
public static Microsoft.Extensions.Configuration.IConfigurationBuilder Add<TSource>(this Microsoft.Extensions.Configuration.IConfigurationBuilder builder, System.Action<TSource> configureSource) where TSource : Microsoft.Extensions.Configuration.IConfigurationSource, new() { throw null; }
public static System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, string>> AsEnumerable(this Microsoft.Extensions.Configuration.IConfiguration configuration) { throw null; }
public static System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, string>> AsEnumerable(this Microsoft.Extensions.Configuration.IConfiguration configuration, bool makePathsRelative) { throw null; }
public static bool Exists(this Microsoft.Extensions.Configuration.IConfigurationSection section) { throw null; }
public static string GetConnectionString(this Microsoft.Extensions.Configuration.IConfiguration configuration, string name) { throw null; }
public static Microsoft.Extensions.Configuration.IConfigurationBuilder Add<TSource>(this Microsoft.Extensions.Configuration.IConfigurationBuilder builder, System.Action<TSource>? configureSource) where TSource : Microsoft.Extensions.Configuration.IConfigurationSource, new() { throw null; }
public static System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, string?>> AsEnumerable(this Microsoft.Extensions.Configuration.IConfiguration configuration) { throw null; }
public static System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, string?>> AsEnumerable(this Microsoft.Extensions.Configuration.IConfiguration configuration, bool makePathsRelative) { throw null; }
public static bool Exists([System.Diagnostics.CodeAnalysis.NotNullWhen(true)] this Microsoft.Extensions.Configuration.IConfigurationSection? section) { throw null; }
public static string? GetConnectionString(this Microsoft.Extensions.Configuration.IConfiguration configuration, string name) { throw null; }
public static Microsoft.Extensions.Configuration.IConfigurationSection GetRequiredSection(this Microsoft.Extensions.Configuration.IConfiguration configuration, string key) { throw null; }
}
[System.AttributeUsageAttribute(System.AttributeTargets.Property)]
Expand All @@ -26,16 +26,17 @@ public static partial class ConfigurationPath
public static readonly string KeyDelimiter;
public static string Combine(System.Collections.Generic.IEnumerable<string> pathSegments) { throw null; }
public static string Combine(params string[] pathSegments) { throw null; }
public static string GetParentPath(string path) { throw null; }
public static string GetSectionKey(string path) { throw null; }
public static string? GetParentPath(string? path) { throw null; }
maxkoshevoi marked this conversation as resolved.
Show resolved Hide resolved
[return: System.Diagnostics.CodeAnalysis.NotNullIfNotNull("path")]
public static string? GetSectionKey(string? path) { throw null; }
}
public static partial class ConfigurationRootExtensions
{
public static string GetDebugView(this Microsoft.Extensions.Configuration.IConfigurationRoot root) { throw null; }
}
public partial interface IConfiguration
{
string this[string key] { get; set; }
string? this[string key] { get; set; }
System.Collections.Generic.IEnumerable<Microsoft.Extensions.Configuration.IConfigurationSection> GetChildren();
Microsoft.Extensions.Primitives.IChangeToken GetReloadToken();
Microsoft.Extensions.Configuration.IConfigurationSection GetSection(string key);
Expand All @@ -49,11 +50,11 @@ public partial interface IConfigurationBuilder
}
public partial interface IConfigurationProvider
{
System.Collections.Generic.IEnumerable<string> GetChildKeys(System.Collections.Generic.IEnumerable<string> earlierKeys, string parentPath);
System.Collections.Generic.IEnumerable<string> GetChildKeys(System.Collections.Generic.IEnumerable<string> earlierKeys, string? parentPath);
Microsoft.Extensions.Primitives.IChangeToken GetReloadToken();
void Load();
void Set(string key, string value);
bool TryGet(string key, out string value);
void Set(string key, string? value);
bool TryGet(string key, [System.Diagnostics.CodeAnalysis.MaybeNullWhen(false)] out string value);
}
public partial interface IConfigurationRoot : Microsoft.Extensions.Configuration.IConfiguration
{
Expand All @@ -64,7 +65,7 @@ public partial interface IConfigurationSection : Microsoft.Extensions.Configurat
{
string Key { get; }
string Path { get; }
string Value { get; set; }
string? Value { get; set; }
}
public partial interface IConfigurationSource
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>enable</Nullable>
</PropertyGroup>
<ItemGroup>
<Compile Include="Microsoft.Extensions.Configuration.Abstractions.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Primitives\ref\Microsoft.Extensions.Primitives.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
maxkoshevoi marked this conversation as resolved.
Show resolved Hide resolved
<ProjectReference Include="$(LibrariesProjectRoot)System.Collections\ref\System.Collections.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime\ref\System.Runtime.csproj" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;

namespace Microsoft.Extensions.Configuration
Expand All @@ -18,7 +19,7 @@ public static class ConfigurationExtensions
/// <param name="builder">The builder to add to.</param>
/// <param name="configureSource">Configures the source secrets.</param>
/// <returns>The <see cref="IConfigurationBuilder"/>.</returns>
public static IConfigurationBuilder Add<TSource>(this IConfigurationBuilder builder, Action<TSource> configureSource) where TSource : IConfigurationSource, new()
public static IConfigurationBuilder Add<TSource>(this IConfigurationBuilder builder, Action<TSource>? configureSource) where TSource : IConfigurationSource, new()
{
var source = new TSource();
configureSource?.Invoke(source);
Expand All @@ -31,37 +32,36 @@ public static class ConfigurationExtensions
/// <param name="configuration">The configuration to enumerate.</param>
/// <param name="name">The connection string key.</param>
/// <returns>The connection string.</returns>
public static string GetConnectionString(this IConfiguration configuration, string name)
public static string? GetConnectionString(this IConfiguration configuration, string name)
{
return configuration?.GetSection("ConnectionStrings")?[name];
return configuration?.GetSection("ConnectionStrings")[name];
}

/// <summary>
/// Get the enumeration of key value pairs within the <see cref="IConfiguration" />
/// </summary>
/// <param name="configuration">The configuration to enumerate.</param>
/// <returns>An enumeration of key value pairs.</returns>
public static IEnumerable<KeyValuePair<string, string>> AsEnumerable(this IConfiguration configuration) => configuration.AsEnumerable(makePathsRelative: false);
public static IEnumerable<KeyValuePair<string, string?>> AsEnumerable(this IConfiguration configuration) => configuration.AsEnumerable(makePathsRelative: false);

/// <summary>
/// Get the enumeration of key value pairs within the <see cref="IConfiguration" />
/// </summary>
/// <param name="configuration">The configuration to enumerate.</param>
/// <param name="makePathsRelative">If true, the child keys returned will have the current configuration's Path trimmed from the front.</param>
/// <returns>An enumeration of key value pairs.</returns>
public static IEnumerable<KeyValuePair<string, string>> AsEnumerable(this IConfiguration configuration, bool makePathsRelative)
public static IEnumerable<KeyValuePair<string, string?>> AsEnumerable(this IConfiguration configuration, bool makePathsRelative)
{
var stack = new Stack<IConfiguration>();
stack.Push(configuration);
var rootSection = configuration as IConfigurationSection;
int prefixLength = (makePathsRelative && rootSection != null) ? rootSection.Path.Length + 1 : 0;
int prefixLength = (makePathsRelative && configuration is IConfigurationSection rootSection) ? rootSection.Path.Length + 1 : 0;
while (stack.Count > 0)
{
IConfiguration config = stack.Pop();
// Don't include the sections value if we are removing paths, since it will be an empty key
if (config is IConfigurationSection section && (!makePathsRelative || config != configuration))
{
yield return new KeyValuePair<string, string>(section.Path.Substring(prefixLength), section.Value);
yield return new KeyValuePair<string, string?>(section.Path.Substring(prefixLength), section.Value);
}
foreach (IConfigurationSection child in config.GetChildren())
{
Expand All @@ -75,7 +75,7 @@ public static IEnumerable<KeyValuePair<string, string>> AsEnumerable(this IConfi
/// </summary>
/// <param name="section">The section to enumerate.</param>
/// <returns><see langword="true" /> if the section has values or children; otherwise, <see langword="false" />.</returns>
public static bool Exists(this IConfigurationSection section)
public static bool Exists([NotNullWhen(true)] this IConfigurationSection? section)
{
if (section == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.Extensions.Configuration
{
Expand Down Expand Up @@ -49,7 +50,8 @@ public static string Combine(IEnumerable<string> pathSegments)
/// </summary>
/// <param name="path">The path.</param>
/// <returns>The last path segment of the path.</returns>
public static string GetSectionKey(string path)
[return: NotNullIfNotNull("path")]
public static string? GetSectionKey(string? path)
{
if (string.IsNullOrEmpty(path))
{
Expand All @@ -65,7 +67,7 @@ public static string GetSectionKey(string path)
/// </summary>
/// <param name="path">The path.</param>
/// <returns>The original path minus the last individual segment found in it. Null if the original path corresponds to a top level node.</returns>
public static string GetParentPath(string path)
public static string? GetParentPath(string? path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly I think path should be marked as non-null the doc suggests it should be a path also it's not intuitive what will happen when it's null. I do not feel strongly about this so fine with me if you think we should keep it as is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't have a strong opinion. One benefit of making path nullable is that user don't need additional null-check in order to call this method (if their variable may be null).

I didn't find any calls to this method here or in aspnet, so don't know how this method it used.

{
if (string.IsNullOrEmpty(path))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ void RecurseChildren(
{
foreach (IConfigurationSection child in children)
{
(string Value, IConfigurationProvider Provider) valueAndProvider = GetValueAndProvider(root, child.Path);
(string? Value, IConfigurationProvider? Provider) valueAndProvider = GetValueAndProvider(root, child.Path);

if (valueAndProvider.Provider != null)
{
Expand Down Expand Up @@ -57,13 +57,13 @@ void RecurseChildren(
return builder.ToString();
}

private static (string Value, IConfigurationProvider Provider) GetValueAndProvider(
private static (string? Value, IConfigurationProvider? Provider) GetValueAndProvider(
IConfigurationRoot root,
string key)
{
foreach (IConfigurationProvider provider in root.Providers.Reverse())
{
if (provider.TryGet(key, out string value))
if (provider.TryGet(key, out string? value))
{
return (value, provider);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public interface IConfiguration
/// </summary>
/// <param name="key">The configuration key.</param>
/// <returns>The configuration value.</returns>
string this[string key] { get; set; }
string? this[string key] { get; set; }

/// <summary>
/// Gets a configuration sub-section with the specified key.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using Microsoft.Extensions.Primitives;

namespace Microsoft.Extensions.Configuration
Expand All @@ -17,14 +18,14 @@ public interface IConfigurationProvider
/// <param name="key">The key.</param>
/// <param name="value">The value.</param>
/// <returns><c>True</c> if a value for the specified key was found, otherwise <c>false</c>.</returns>
bool TryGet(string key, out string value);
bool TryGet(string key, [MaybeNullWhen(false)] out string value);
maxkoshevoi marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Sets a configuration value for the specified key.
/// </summary>
/// <param name="key">The key.</param>
/// <param name="value">The value.</param>
void Set(string key, string value);
void Set(string key, string? value);

/// <summary>
/// Returns a change token if this provider supports change tracking, null otherwise.
Expand All @@ -45,6 +46,6 @@ public interface IConfigurationProvider
/// <param name="earlierKeys">The child keys returned by the preceding providers for the same parent path.</param>
/// <param name="parentPath">The parent path.</param>
/// <returns>The child keys.</returns>
IEnumerable<string> GetChildKeys(IEnumerable<string> earlierKeys, string parentPath);
IEnumerable<string> GetChildKeys(IEnumerable<string> earlierKeys, string? parentPath);
maxkoshevoi marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ public interface IConfigurationSection : IConfiguration
/// <summary>
/// Gets or sets the section value.
/// </summary>
string Value { get; set; }
string? Value { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you decide to keep path as nullable arg in other places should this also accept nullable path?

Copy link
Contributor Author

@maxkoshevoi maxkoshevoi Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. Path will never be set to null. All sections have a path.

Also all usages don't expect null in Path:

Regarding leaving path parameters as null. I'm not objecting removing ?. My only concern is that it will be harder to call those methods by requiring additional null-check on their side.

}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
maxkoshevoi marked this conversation as resolved.
Show resolved Hide resolved
<Nullable>enable</Nullable>
<EnableDefaultItems>true</EnableDefaultItems>
<!-- Use targeting pack references instead of granular ones in the project file. -->
<DisableImplicitAssemblyReferences>false</DisableImplicitAssemblyReferences>
<PackageDescription>Abstractions of key-value pair based configuration.

Commonly Used Types:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal class FakeConfigurationProvider : MemoryConfigurationProvider, IConfigu
public FakeConfigurationProvider(MemoryConfigurationSource source)
: base(source) { }

public new void Set(string key, string value)
public new void Set(string key, string? value)
{
base.Set(key, value);
OnReload();
Expand Down