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

LoggerMessageGenerator uses a private struct as the TState type argument to ILogger.Log #90226

Open
slaneyrw opened this issue Aug 9, 2023 · 1 comment
Labels
area-Extensions-Logging source-generator Indicates an issue with a source generator feature
Milestone

Comments

@slaneyrw
Copy link

slaneyrw commented Aug 9, 2023

Description

Using the LoggerMessageGenerator source generator for logger messages generates code that is hard ( or impossible depending on the mocking library ) to using in Unit testing as the generated code allows the compiler to infer the type for TState.

The generator can create a private struct that encapsulates all the logger msg template's arguments and uses that as the TState argument. Otherwise the default LoggerMessage.Define is used

Using LoggerMessage.Define directly causes the type argument to other private/internal types including Microsoft.Extensions.Logging.FormattedLogValues, Microsoft.Extensions.Logging.LogValuesFormatter or Microsoft.Extensions.Logging.LoggerMessage.LogValues flavors,

There is a discussion about whether these should be made public again for the same testing reasons.

#67577

Please specify the type argument for TState explicitly to IReadOnlyList<KeyValuePair<string, object>>.

Reproduction Steps

Add a LoggerMessage attribute to a ILogger extension method as per current recommentations.

Inspect generated source. Invocation to logger.Log relies on implicit compiler inference.

Expected behavior

All calls to ILogger.Log<TState>(...) should use public type definitions for the TState argument

Actual behavior

private generated struct used as the TState type argument

Regression?

No

Known Workarounds

None known

Configuration

No response

Other information

Another possibility is to make the types internal. This would then allow the use of InternalsVisibleTo for known test projects and allow the type is to be used directly.

This is only a stop-gap as this pattern fails for library references as InternalsVisibleTo can not know the assembly(s) in advanced

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 9, 2023
@ghost
Copy link

ghost commented Aug 9, 2023

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Using the LoggerMessageGenerator source generator for logger messages generates code that is hard ( or impossible depending on the mocking library ) to using in Unit testing as the generated code allows the compiler to infer the type for TState.

The generator creates a private struct that encapsulates all the logger msg template's arguments and using that as the TState argument. Using the Logger extension methods directly causes the type argument to be Microsoft.Extensions.Logging.FormattedLogValues, which was made internal There is a discussion about whether is should be made public again for the same testing reasons.

#67577

Please specify the type argument for TState explicitly to IReadOnlyList<KeyValuePair<string, object>>.

Reproduction Steps

Add a LoggerMessage attribute to a ILogger extension method as per current recommentations.

Inspect generated source. Invocation to logger.Log relies on implicit compiler inference.

Expected behavior

All calls to ILogger.Log<TState>(...) should use public type definitions for the TState argument

Actual behavior

private generated struct used as the TState type argument

Regression?

No

Known Workarounds

None known

Configuration

No response

Other information

Another possibility is to make the types internal. This would then allow the use of InternalsVisibleTo for known test projects and allow the type is to be used directly.

This is only a stop-gap as this pattern fails for library references as InternalsVisibleTo can not know the assembly(s) in advanced

Author: slaneyrw
Assignees: -
Labels:

area-Extensions-Logging

Milestone: -

@layomia layomia added this to the Future milestone Aug 9, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 9, 2023
@tarekgh tarekgh added the source-generator Indicates an issue with a source generator feature label Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-Logging source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

3 participants