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

Logging Source Generator fails to compile due to CS0246 and CS0265 errors if type for generic constraint is in a different namespace #58550

Closed
Tracked by #64015
martincostello opened this issue Sep 2, 2021 · 11 comments · Fixed by #64593
Assignees
Milestone

Comments

@martincostello
Copy link
Member

martincostello commented Sep 2, 2021

Description

If a containing class for a class used to provide the partial methods for the Logging Source Generator uses as class as a type constraint which is defined in another namespace, the project fails to compile with errors similar to the below.

Repro\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(6,47): error CS0246: The type or namespace name 'Message' could not be found (are you missing a using directive or an assembly reference?) [Repro.csproj]
Repro\MessagePrinter.cs(6,26): error CS0265: Partial declarations of 'MessagePrinter<T>' have inconsistent constraints for type parameter 'T' [Repro.csproj]

If the type which is used for the type constraint of the class is in the same namespace, then project compiles and runs as expected.

The compilation errors can be worked around by using global using statements, such as:

<ItemGroup>
  <Using Include="Repro.AnotherNamespace" />
</ItemGroup>

Minimal repro

Repro.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <OutputType>Exe</OutputType>
    <RootNamespace>Repro</RootNamespace>
    <TargetFramework>net6.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.0-rc.1.21424.15" />
    <PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0-rc.1.21424.15" />
    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="6.0.0-rc.1.21424.15" />
  </ItemGroup>
</Project>

Program.cs

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Repro;

using var serviceProvider = new ServiceCollection()
    .AddLogging(builder => builder.AddConsole())
    .BuildServiceProvider();

var loggerFactory = serviceProvider.GetRequiredService<ILoggerFactory>();
var logger = loggerFactory.CreateLogger<MessagePrinter<Message>>();

var printer = new MessagePrinter<Message>(logger);
printer.Print(new() { Text = "Hello" });

Message.cs

namespace Repro.AnotherNamespace
{
    public class Message
    {
        public string? Text { get; set; }
    }
}

MessagePrinter.cs

using Microsoft.Extensions.Logging;
using Repro.AnotherNamespace;

namespace Repro
{
    public partial class MessagePrinter<T>
        where T : Message
    {
        private readonly ILogger _logger;

        public MessagePrinter(ILogger<MessagePrinter<T>> logger)
        {
            _logger = logger;
        }

        public void Print(T message)
        {
            Console.WriteLine(message);
            Log.Message(_logger, message.Text);
        }

        internal static partial class Log
        {
            [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "The message is {Text}.")]
            internal static partial void Message(ILogger logger, string? text);
        }
    }
}

Expected Output

Repro> dotnet run
Repro.Message
info: Repro.MessagePrinter[1]
      The message is Hello.

Configuration

.NET SDK 6.0.100-rc.1.21424.38
Microsoft.Extensions.Logging 6.0.0-rc.1.21424.15

Regression?

No.

@dotnet-issue-labeler dotnet-issue-labeler bot added area-Extensions-Logging untriaged New issue has not been triaged by the area owner labels Sep 2, 2021
@ghost
Copy link

ghost commented Sep 2, 2021

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

If a containing class for a class used to provide the partial methods for the Logging Source Generator uses as class as a type constraint which is defined in another namespace, the project fails to compile with errors similar to the below.

Repro\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(6,47): error CS0246: The type or namespace name 'Message' could not be found (are you missing a using directive or an assembly reference?) [Repro.csproj]
Repro\MessagePrinter.cs(6,26): error CS0265: Partial declarations of 'MessagePrinter<T>' have inconsistent constraints for type parameter 'T' [Repro.csproj]

If the type which is used for the type constraint of the class is in the same namespace, then project compiles and runs as expected.

Minimal repro

Repro.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <OutputType>Exe</OutputType>
    <RootNamespace>Repro</RootNamespace>
    <TargetFramework>net6.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.0-rc.1.21424.15" />
    <PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0-rc.1.21424.15" />
    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="6.0.0-rc.1.21424.15" />
  </ItemGroup>
</Project>

Program.cs

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Repro;

using var serviceProvider = new ServiceCollection()
    .AddLogging(builder => builder.AddConsole())
    .BuildServiceProvider();

var loggerFactory = serviceProvider.GetRequiredService<ILoggerFactory>();
var logger = loggerFactory.CreateLogger<MessagePrinter<Message>>();

var printer = new MessagePrinter<Message>(logger);
printer.Print(new() { Text = "Hello" });

Message.cs

namespace Repro.AnotherNamespace
{
    public class Message
    {
        public string? Text { get; set; }
    }
}

MessagePrinter.cs

using Microsoft.Extensions.Logging;
using Repro.AnotherNamespace;

namespace Repro
{
    public partial class MessagePrinter<T>
        where T : Message
    {
        private readonly ILogger _logger;

        public MessagePrinter(ILogger<MessagePrinter<T>> logger)
        {
            _logger = logger;
        }

        public void Print(T message)
        {
            Console.WriteLine(message);
            Log.Message(_logger, message.Text);
        }

        internal static partial class Log
        {
            [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "The message is {Text}.")]
            internal static partial void Message(ILogger logger, string? text);
        }
    }
}

Expected Output

Repro> dotnet run
Repro.Message
info: Repro.MessagePrinter[1]
      The message is Hello.

Configuration

.NET SDK 6.0.100-rc.1.21424.38
Microsoft.Extensions.Logging 6.0.0-rc.1.21424.15

Regression?

No.

Author: martincostello
Assignees: -
Labels:

untriaged, area-Extensions-Logging

Milestone: -

@ghost ghost added this to Untriaged in ML, Extensions, Globalization, etc, POD. Sep 2, 2021
@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Sep 2, 2021
@maryamariyan maryamariyan added this to the 6.0.0 milestone Sep 2, 2021
@ghost ghost moved this from Untriaged to 6.0.0 in ML, Extensions, Globalization, etc, POD. Sep 2, 2021
@maryamariyan maryamariyan modified the milestones: 6.0.0, 7.0.0 Sep 2, 2021
@ghost ghost moved this from 6.0.0 to Untriaged in ML, Extensions, Globalization, etc, POD. Sep 2, 2021
@ghost ghost moved this from Untriaged to 7.0.0 in ML, Extensions, Globalization, etc, POD. Sep 2, 2021
@Tornhoof
Copy link
Contributor

Tornhoof commented Sep 3, 2021

Do you really want to punt that to 7.0? Most, if not all, source gen errors for the JSON source generator were classified as blocking.
My reasoning:
This problem also occurs if MessagePrinter is an abstract base class with constraints, I looked through my code where a class gets the non-generic ILogger interface in the constructor (a cursory glance verifies that these are abstract), i.e. an injected logger is used in a shared base class somewhere and ~25% of those types use some kind of generic constraint. That's a few dozen types as that specific project is using MediatR quite heavily.

Many of the source gen compiler errors are sufficiently obscure that without looking at the g.cs, they are not easy to understand.

Having said that, if you can't fix it for 6.0 you want to make sure that you put that bug fairly prominently into the release notes/documentation including the workaround by @martincostello

@martincostello
Copy link
Member Author

For what it's worth, I found this in a library code base converting the logging which was using the nested private Log class pattern to the source generators and hit this on three classes, two of which were abstract.

I went with non-abstract for writing up the repro as it was easier to follow.

@N0D4N
Copy link
Contributor

N0D4N commented Nov 12, 2021

Another workaround is to use full qualified name of class in another namespace, that is being used as constraint, e.g.

// code
public partial class MessagePrinter<T> where T : Repro.AnotherNamespace.Message
{}
// code

@dasMulli
Copy link
Contributor

dasMulli commented Dec 8, 2021

Running into this as well trying to convert lots of my code to use this.. happens in 5 files for my MediatR extensions.

Example:

using MediatR;
using Microsoft.Extensions.Logging;

namespace LoggingGeneratorErrorExample;

internal partial class LoggingNotificationHandlerWrapper<TInner, TNotification> : INotificationHandler<TNotification>
    where TNotification : INotification
    where TInner : INotificationHandler<TNotification>
{
    private readonly ILogger<TInner> logger = null!;

    public Task Handle(TNotification notification, CancellationToken cancellationToken)
        => Task.CompletedTask;

    [LoggerMessage(Level = LogLevel.Debug, Message = "Test Message")]
    private partial void LogNotification();
}
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <EmitCompilerGeneratedFiles>True</EmitCompilerGeneratedFiles>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="MediatR" Version="9.0.0" />
    <PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0" />
  </ItemGroup>
</Project>

Fails with:

C:\demos\LoggingGeneratorErrorExample\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(6,98): error CS0246: The type or namespace name 'INotification' could not be found (are you missing a using directive or an assembly reference?) [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]
C:\demos\LoggingGeneratorErrorExample\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(7,20): error CS0246: The type or namespace name 'INotificationHandler<>' could not be found (are you missing a using directive or an assembly reference?) [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]
C:\demos\LoggingGeneratorErrorExample\LoggingNotificationHandlerWrapper.cs(6,24): error CS0265: Partial declarations of 'LoggingNotificationHandlerWrapper<TInner, TNotification>' have inconsistent constraints for type parameter 'TInner' [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]
C:\demos\LoggingGeneratorErrorExample\LoggingNotificationHandlerWrapper.cs(6,24): error CS0265: Partial declarations of 'LoggingNotificationHandlerWrapper<TInner, TNotification>' have inconsistent constraints for type parameter 'TNotification' [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]

Build FAILED.

C:\demos\LoggingGeneratorErrorExample\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(6,98): error CS0246: The type or namespace name 'INotification' could not be found (are you missing a using directive or an assembly reference?) [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]
C:\demos\LoggingGeneratorErrorExample\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(7,20): error CS0246: The type or namespace name 'INotificationHandler<>' could not be found (are you missing a using directive or an assembly reference?) [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]
C:\demos\LoggingGeneratorErrorExample\LoggingNotificationHandlerWrapper.cs(6,24): error CS0265: Partial declarations of 'LoggingNotificationHandlerWrapper<TInner, TNotification>' have inconsistent constraints for type parameter 'TInner' [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]  
C:\demos\LoggingGeneratorErrorExample\LoggingNotificationHandlerWrapper.cs(6,24): error CS0265: Partial declarations of 'LoggingNotificationHandlerWrapper<TInner, TNotification>' have inconsistent constraints for type parameter 'TNotification' [C:\demos\LoggingGeneratorErrorExample\LoggingGeneratorErrorExample.csproj]
    0 Warning(s)
    4 Error(s)

Generated code emitted by the generator:

// <auto-generated/>
#nullable enable

namespace LoggingGeneratorErrorExample
{
    partial class LoggingNotificationHandlerWrapper<TInner, TNotification> where TNotification : INotification
    where TInner : INotificationHandler<TNotification>
    {
        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.5.2210")]
        private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, global::System.Exception?> __LogNotificationCallback =
            global::Microsoft.Extensions.Logging.LoggerMessage.Define(global::Microsoft.Extensions.Logging.LogLevel.Debug, new global::Microsoft.Extensions.Logging.EventId(-1, nameof(LogNotification)), "Test Message", new global::Microsoft.Extensions.Logging.LogDefineOptions() { SkipEnabledCheck = true }); 

        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.5.2210")]
        private partial void LogNotification()
        {
            if (logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug))
            {
                __LogNotificationCallback(logger, null);
            }
        }
    }
}

@Tornhoof
Copy link
Contributor

Tornhoof commented Dec 8, 2021

Fwiw, I moved the conversion of my code base to the logger src gen to 7.0. I had way too many build errors for complex examples (mostly this case) that I stopped my attempts.

@dasMulli
Copy link
Contributor

dasMulli commented Dec 8, 2021

I was just hoping this can be fixed sooner not just to get the feature working in non-trivial setups but also b/c the in-box generators are a good source of reference (and documentation.. especially for the newer Roslyn 4 incremental generator APIs) for other people writing generators

@Tornhoof
Copy link
Contributor

Tornhoof commented Dec 8, 2021

I was just hoping this can be fixed sooner not just to get the feature working in non-trivial

Yeah I second that, but as there was no feedback from MS when I specifically asked if they want to punt that. There are quite a few logger src gen issues, which either point to missing test coverage, or more likely, that the developer experience is still bad (as the json src gen had similar problems).

@maryamariyan
Copy link
Member

maryamariyan commented Dec 22, 2021

Yeah I second that, but as there was no feedback from MS when I specifically asked if they want to punt that. There are quite a few logger src gen issues, which either point to missing test coverage, or more likely, that the developer experience is still bad (as the json src gen had similar problems).

Thanks for the feedback. Will take a look and make improvements here.

@maryamariyan maryamariyan self-assigned this Dec 22, 2021
@ericstj
Copy link
Member

ericstj commented Jan 10, 2022

A couple small observations around possible fixes. I don't think partial files need to restate all the derived types and generic parameter constraints, so it may be possible to just omit things in the generated file. Another approach we've often taken in generated files is to fully qualify all type references, so if we aren't doing that already that might be another thing to consider.

cc @layomia @stephentoub @elinor-fung who've built other generators and may have an opinion here or wish to address a similar bug in other generators.

@elinor-fung
Copy link
Member

+1 to both @ericstj's suggestions.

For this issue, I think the simplest and correct thing to do would be to just remove the constraints when emitting code for the parent classes and logger class.

parentClasses.Add($"partial {parent.Keyword} {parent.Name} {parent.Constraints}");

{nestedIndentation}partial {lc.Keyword} {lc.Name} {lc.Constraints}

For the p/invoke generator, we don't have this specific problem with generic constraints (since DllImports can't be in generic types), but in general we try to both avoid unnecessarily re-specifying things on partial class/method definitions and use fully qualified names in the generated code.

maryamariyan added a commit to maryamariyan/runtime that referenced this issue Feb 1, 2022
- Supports usage of "in" modifier
- Improves support for generic constraints

Fixes dotnet#58550, dotnet#62644
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 1, 2022
ML, Extensions, Globalization, etc, POD. automation moved this from 7.0.0 to Done Feb 2, 2022
maryamariyan added a commit that referenced this issue Feb 2, 2022
… / when dealing with constraints (#64593)

* Fixes some logging source gen bugs:

- Supports usage of "in" modifier
- Improves support for generic constraints

Fixes #58550, #62644

* Apply PR feedback

* Add another test
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 2, 2022
github-actions bot pushed a commit that referenced this issue Feb 2, 2022
- Supports usage of "in" modifier
- Improves support for generic constraints

Fixes #58550, #62644
maryamariyan added a commit to maryamariyan/runtime that referenced this issue Feb 4, 2022
… / when dealing with constraints (dotnet#64593)

* Fixes some logging source gen bugs:

- Supports usage of "in" modifier
- Improves support for generic constraints

Fixes dotnet#58550, dotnet#62644

* Apply PR feedback

* Add another test
@ghost ghost locked as resolved and limited conversation to collaborators Mar 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.