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

LoggerMessage source generator does not work with logger from primary constructor. #91121

Open
stmimon opened this issue Aug 25, 2023 · 6 comments · May be fixed by #101660
Open

LoggerMessage source generator does not work with logger from primary constructor. #91121

stmimon opened this issue Aug 25, 2023 · 6 comments · May be fixed by #101660
Labels
area-Extensions-Logging in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@stmimon
Copy link

stmimon commented Aug 25, 2023

Description

The source generator for LoggerMessage does not work if the logger is injected from the primary constructor.

Reproduction Steps

Create the project.

  • dotnet new classlib
  • dotnet add package Microsoft.Extensions.Logging --version 8.0.0-preview.7.23375.6

Add the following c# file to the project.

using Microsoft.Extensions.Logging;

namespace Test;

public partial class LoggerFromPrimaryConstructor(ILogger<LoggerFromPrimaryConstructor> logger)
{
    [LoggerMessage(Level = LogLevel.Debug, Message = "This is a test")]
    public partial void LogTest();
}

Expected behavior

The source generator emits code with similar behaviour as if I had written the following.

public partial class LogWithExplicitProperty
{
    private readonly ILogger<LoggerFromPrimaryConstructor> logger;

    public LogWithExplicitProperty(ILogger<LoggerFromPrimaryConstructor> logger)
    {
        this.logger = logger;
    }

    [LoggerMessage(Level = LogLevel.Debug, Message = "This is a test")]
    public partial void LogTest();
}

Actual behavior

Compilation fails with error SYSLIB1019: Couldn't find a field of type Microsoft.Extensions.Logging.ILogger in class LoggerFromPrimaryConstructor.

I also got error CS1525: Invalid expression term '<' in another project where the source generator did find the logger for some reason, but generated incorrect code that referenced the logger by the symbol <logger>P. I assume this is the generated field name? I could not reproduce it however.

Regression?

This is not a regression, as primary constructors are not GA.

Known Workarounds

Use a regular constructor instead of a primary constructor.

Configuration

I am using .NET 8 preview 7 on Windows 10 Enterprise 22H2 x64.

Other information

No response

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

ghost commented Aug 25, 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

The source generator for LoggerMessage does not work if the logger is injected from the primary constructor.

Reproduction Steps

Create the project.

  • dotnet new classlib
  • dotnet add package Microsoft.Extensions.Logging --version 8.0.0-preview.7.23375.6

Add the following c# file to the project.

using Microsoft.Extensions.Logging;

namespace Test;

public partial class LoggerFromPrimaryConstructor(ILogger<LoggerFromPrimaryConstructor> logger)
{
    [LoggerMessage(Level = LogLevel.Debug, Message = "This is a test")]
    public partial void LogTest();
}

Expected behavior

The source generator emits code with similar behaviour as if I had written the following.

public partial class LogWithExplicitProperty
{
    private readonly ILogger<LoggerFromPrimaryConstructor> logger;

    public LogWithExplicitProperty(ILogger<LoggerFromPrimaryConstructor> logger)
    {
        this.logger = logger;
    }

    [LoggerMessage(Level = LogLevel.Debug, Message = "This is a test")]
    public partial void LogTest();
}

Actual behavior

Compilation fails with error SYSLIB1019: Couldn't find a field of type Microsoft.Extensions.Logging.ILogger in class LoggerFromPrimaryConstructor.

I also got error CS1525: Invalid expression term '<' in another project where the source generator did find the logger for some reason, but generated incorrect code that referenced the logger by the symbol <logger>P. I assume this is the generated field name? I could not reproduce it however.

Regression?

This is not a regression, as primary constructors are not GA.

Known Workarounds

Use a regular constructor instead of a primary constructor.

Configuration

I am using .NET 8 preview 7 on Windows 10 Enterprise 22H2 x64.

Other information

No response

Author: stmimon
Assignees: -
Labels:

area-Extensions-Logging

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented Aug 25, 2023

Yes, the primary constructors feature is a new compiler feature and is not supported yet with the logging source gen. This is something we can consider supporting in future releases. Thanks for your report.

CC @geeknoid

@tarekgh tarekgh added this to the 9.0.0 milestone Aug 25, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 25, 2023
@sloncho
Copy link

sloncho commented Jan 29, 2024

@tarekgh 8.0 is now GA and long-term support. Fixing this for 9.0 will leave more than 2 years with unsupported code.

@tarekgh
Copy link
Member

tarekgh commented Jan 29, 2024

@sloncho we usually get issues after facts which we address it in the next releases. If we find demands on specific issue, we consider porting the fix back to the service release.

@stmimon
Copy link
Author

stmimon commented Jan 29, 2024

@sloncho If you want to use a primary constructor, another workaround is to explicitly pass the logger instance to the logging methods.

[LoggerMessage(Level = LogLevel.Debug, Message = "This is a test")]
public partial void LogTest(ILogger logger);

@timia2109
Copy link

Or you could do something like this:

public partial class Foo(ILogger<Foo> logger) {
    private readonly ILogger _logger = logger;
}

@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-Logging in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants