LoggerMessage source generator: preserve params, ref readonly, and scoped parameter modifiers#124589
LoggerMessage source generator: preserve params, ref readonly, and scoped parameter modifiers#124589
params, ref readonly, and scoped parameter modifiers#124589Conversation
…enerator Co-authored-by: svick <287848+svick@users.noreply.github.com>
…generator Co-authored-by: svick <287848+svick@users.noreply.github.com>
params, ref readonly, and scoped parameters
Co-authored-by: svick <287848+svick@users.noreply.github.com>
params, ref readonly, and scoped parametersparams, ref readonly, and scoped parameter modifiers
Co-authored-by: svick <287848+svick@users.noreply.github.com>
|
@svick did you read the comment #91678 (comment)? I agree with @ericstj params will be a problematic to support. The semantic of [LoggerMessage(Message = "{arg1} ... {arg2}", Level = LogLevel.Info)]
static partial void Log1(ILogger logger, params object?[] args);The logger message expects to have two parameters. at runtime we should get exactly number of parameters match the message, otherwise should fail. It is better to have the source generator produce a diagnostic for this case and fail it instead of trying to work it out and produce runtime failure. Whatever we do here, we want to ensure the same be applied to https://github.com/dotnet/extensions/tree/main/src/Generators/Microsoft.Gen.Logging. |
I don't understand why that behavior would be desirable here. If you want to have two values, you can add two parameters to the message and two regular parameters to the logging method, there is no need to use If you want to have variable number of values, then with this PR, you can add one parameter to the message, a For example, with the code from the original issue: internal static partial class LogMessages
{
[LoggerMessage(Message = "Message: {message}, {args}", Level = LogLevel.Information)]
internal static partial void MSG(this ILogger logger, string? message, params object?[] args);
}If you call: LogMessages.MSG(logger, "message", 1, 2, 3);Then with this PR and default console logger, you will get: What am I missing? |
|
The whole point of For example, if I write something like the following, would you accept it as valid or invalid? [LoggerMessage(Message = "Message: {message}, Index:{args0} Values:{args1}", Level = LogLevel.Information)]
internal static partial void MSG(this ILogger logger, string? message, params object?[] args);Then calling: LogMessages.MSG(logger, "message", 1, 2, 3);Logically (following C# concepts), this should be valid. This means if we support it, at run time you have validate and ensure Look at https://learn.microsoft.com/en-us/dotnet/api/system.console.writeline?view=net-10.0#system-console-writeline(system-string-system-object()) as a good example when In another word, in logging source gen, the number of message arguments is always known beforehand which defeat the |
I see the message template to be on the same level as the body of the logging method. There, So your example would be invalid, and I think it doesn't violate C# concepts. Or, to be more technical, The C# spec even says:
So I think making it precisely equivalent to a regular array parameter here is fine.
That's fair. Personally, I think the value of |
The
LoggerMessagesource generator droppedparams,ref readonly, andscopedmodifiers from generated partial method implementations, causing CS0758 (mismatched declaration/implementation). C# 13paramscollections (params IEnumerable<T>, etc.) were also untested.Changes
Generator
paramsviaIParameterSymbol.IsParams; detectref readonlyvia(RefKind)4(cross-version safe —RefKind.RefReadOnlyParameteronly exists in Roslyn 4.8+); detectscopedviaparamSymbol.ScopedKind != ScopedKind.Noneunder#if ROSLYN4_4_OR_GREATERLoggerParameter/LoggerParameterSpec: addIsParamsandIsScopedfields;#pragma warning disable CS0649onIsScopedfor Roslyn 3.11 builds where the field is intentionally unassignedGenParameters): emitscoped, qualifier (in/ref/ref readonly), andparamsprefixes in correct orderRoslyn4.0.cs: propagateIsParamsandIsScopedthrough the incremental spec→model conversionTests
ParameterTestExtensions.cs: addUseParamsParameter,UseParamsCollectionParameter,UseRefReadOnlyParameter,UseScopedRefParameter(latter three under#if ROSLYN4_8_OR_GREATER)ParamsParameterOK,ParamsCollectionParameterOK,RefReadOnlyParameterOK,ScopedRefParameterOK,ScopedRefReadOnlyParameterOKTestWithParamsArrayandTestWithParamsCollection(C# 13params IEnumerable<T>)Original prompt
This section details on the original issue you should resolve
<issue_title>Source generation error in LoggerMessageAttribute in Microsoft.Extensions.Logging.Abstractions</issue_title>
<issue_description>### Description
Using 'params' keyword in method parameters "params object?[] args" in partial class under [LoggerMessage] attribute cause Compiler Error CS0758 - in generated code 'params' keyword is absent
Reproduction Steps
Expected behavior
Compile
Actual behavior
in source-generated file LoggerMessage.g.cs, 'params' keyword is absent in method parameters "global::System.Object?[] args"
Regression?
No response
Known Workarounds
No response
Configuration
.NET 7.0
Windows 10 x64
Visual Studio Community 2022
Other information
No response</issue_description>
<agent_instructions>Add support for params (including params collections) in logging methods. Add tests verifying it works.</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@ Tagging subscribers to this area: @dotnet/area-extensions-logging See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed.Issue Details
Description
Using 'params' keyword in method parameters "params object?[] args" in partial class under [LoggerMessage] attribute cause Compiler Error CS0758 - in generated code 'params' keyword is absent
Reproduction Steps
Expected behavior
Compile
Actual behavior
in source-generated file LoggerMessage.g.cs, 'params' keyword is absent in method parameters "global::System.Object?[] args"
Regression?
No response
Known Workarounds
No response
Configuration
.NET 7.0
Windows 10 x64
Visual Studio Community 2022
Other information
No response
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.