Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,13 @@ static bool HasImplicitValueWhenNotSpecified(ParameterInfo paramInfo)
return paramInfo.HasDefaultValue
// parameters of type IConfiguration are implicitly populated with provided Configuration
|| paramInfo.ParameterType == typeof(IConfiguration)
|| paramInfo.IsDefined(typeof(ParamArrayAttribute), false)
|| paramInfo.CustomAttributes.Any(a => a.AttributeType.FullName == "System.Runtime.CompilerServices.ParamCollectionAttribute");
|| IsParamCollection(paramInfo);
}

static bool IsParamCollection(ParameterInfo paramInfo)
{
return paramInfo.IsDefined(typeof(ParamArrayAttribute), false)
|| paramInfo.CustomAttributes.Any(a => a.AttributeType.FullName == "System.Runtime.CompilerServices.ParamCollectionAttribute");
}

internal object? GetImplicitValueForNotSpecifiedKey(ParameterInfo parameter, MethodInfo methodToInvoke)
Expand All @@ -431,9 +436,35 @@ static bool HasImplicitValueWhenNotSpecified(ParameterInfo paramInfo)
$"This is not supported when only a `IConfigSection` has been provided. (method '{methodToInvoke}')");
}

if (parameter.IsDefined(typeof(ParamArrayAttribute), false) && parameter.ParameterType.GetElementType() is { } elementType)
if (IsParamCollection(parameter))
{
return Array.CreateInstance(elementType, 0);
var paramType = parameter.ParameterType;

bool isByRefLike = paramType.CustomAttributes.Any(a => a.AttributeType.FullName == "System.Runtime.CompilerServices.IsByRefLikeAttribute");
if (isByRefLike)
{
return parameter.HasDefaultValue ? parameter.DefaultValue : null;
}

if (paramType.GetElementType() is { } elementType)
{
return Array.CreateInstance(elementType, 0);
}

if (paramType.IsGenericType && paramType.IsInterface)
{
var genericTypeArg = paramType.GetGenericArguments()[0];
return Array.CreateInstance(genericTypeArg, 0);
}

if (paramType.IsGenericType && !paramType.IsAbstract)
{
try
{
return Activator.CreateInstance(paramType);
}
catch { }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the catch block here, it might be worth pointing out:

https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#create-methods

which describes the cases in which Activator.CreateInstance is unlikely to succeed, and also the reflection-driven API we'd need to use in order to extend capability to them.

A Serilog.Debugging.SelfLog.WriteLine(...) call that includes the exception message and as much information as possible about the parameter being constructed would help users figure out why these might fail.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hello,
I updated the PR, here is what the log produces:

Unable to create an implicit instance of the params collection type 'Serilog.Settings.Configuration.Tests.BrokenCollection`1[System.String]' for parameter 'list' on method 'DummyBrokenCollection'.
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.InvalidOperationException: I am broken by design!
   at Serilog.Settings.Configuration.Tests.BrokenCollection`1..ctor() in C:\Development\serilog-settings-configuration\test\Serilog.Settings.Configuration.Tests\DummyLoggerConfigurationExtensions.cs:line 72
   at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean wrapExceptions)
   --- End of inner exception stack trace ---
   at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean wrapExceptions)
   at Serilog.Settings.Configuration.ConfigurationReader.GetImplicitValueForNotSpecifiedKey(ParameterInfo parameter, MethodInfo methodToInvoke) in C:\Development\serilog-settings-configuration\src\Serilog.Settings.Configuration\Settings\Configuration\ConfigurationReader.cs:line 464

Let me know if this works for you.

}
}

return parameter.HasDefaultValue ? parameter.DefaultValue : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,26 @@ public void ParamsEnumerableParameter_GracefullyReturnsDefaultValue()
var param = method.GetParameters().Last(); // params IEnumerable<string>

var result = reader.GetImplicitValueForNotSpecifiedKey(param, method);
var array = Assert.IsType<string[]>(result);
Assert.Empty(array);
}

Assert.Null(result);
[Fact]
public void ParamsListParameter_ReturnsEmptyList()
{
var reader = new ConfigurationReader(
JsonStringConfigSource.LoadSection("{}", "Serilog"),
AssemblyFinder.ForSource(ConfigurationAssemblySource.UseLoadedAssemblies),
new ConfigurationReaderOptions());

// Assuming you have a DummyParamsList method in your TestDummies
var method = typeof(DummyLoggerConfigurationExtensions).GetMethod("DummyParamsList")!;
var param = method.GetParameters().Last(); // params List<string>

var result = reader.GetImplicitValueForNotSpecifiedKey(param, method);

var list = Assert.IsType<List<string>>(result);
Assert.Empty(list);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ public static LoggerConfiguration DummyParamsEnumerable(
return loggerSinkConfiguration.Sink(new DummyParamsSink(values.ToArray()));
}

public static LoggerConfiguration DummyParamsList(
this LoggerSinkConfiguration loggerSinkConfiguration,
params System.Collections.Generic.List<string> list)
{
return loggerSinkConfiguration.Sink(new DummyParamsSink(list.ToArray()));
}

public static LoggerConfiguration DummyParamsSpan(
this LoggerSinkConfiguration loggerSinkConfiguration,
params ReadOnlySpan<string> values)
Expand Down
Loading