[LSG] Logging Source Generator fails to detect event name uniqueness within a class scope

[maryamariyan]

As per design logging source generator provides a warning diagnostic when event ID is not unique within the scope of a class between log methods using LoggerMessageAttribute. It should also be able to set diagnostic message and detect when event name is being reused in the same scope.

Proposal

The proposed diagnostic descriptor would be:

public static DiagnosticDescriptor ShouldntReuseEventNames { get; } = new DiagnosticDescriptor(
    id: "SYSLIB1025",
    title: new LocalizableResourceString(nameof(SR.ShouldntReuseEventNamesTitle), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
    messageFormat: new LocalizableResourceString(nameof(SR.ShouldntReuseEventNamesMessage), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
    category: "LoggingGenerator",
    DiagnosticSeverity.Warning,
    isEnabledByDefault: true);

With the following title:

Multiple logging methods should not use the same event name within a class

And the following message format:

Multiple logging methods are using event name {0} in class {1}

Code Sample

IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
    partial class MyClass
    {
        [LoggerMessage(EventId = 0, EventName = ""MyEvent"", Level = LogLevel.Debug, Message = ""M1"")]
        static partial void M1(ILogger logger);

        [LoggerMessage(EventId = 1, EventName = ""MyEvent"", Level = LogLevel.Debug, Message = ""M1"")]
        static partial void M2(ILogger logger);
    }
");

[ghost]

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

Issue Details

---

As per design logging source generator provides a warning diagnostic when event ID is not unique within the scope of a class between log methods using LoggerMessageAttribute. It should also be able to set diagnostic message and detect when event name is being reused in the same scope.

Author:	maryamariyan
Assignees:	-
Labels:	untriaged, area-Extensions-Logging
Milestone:	-

[allantargino]

Hi @maryamariyan/@tarekgh, I'd like to take this one :)

[allantargino]

Proposal

The proposed diagnostic descriptor would be:

public static DiagnosticDescriptor ShouldntReuseEventNames { get; } = new DiagnosticDescriptor(
    id: "SYSLIB1025",
    title: new LocalizableResourceString(nameof(SR.ShouldntReuseEventNamesTitle), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
    messageFormat: new LocalizableResourceString(nameof(SR.ShouldntReuseEventNamesMessage), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
    category: "LoggingGenerator",
    DiagnosticSeverity.Warning,
    isEnabledByDefault: true);

With the following title:

Multiple logging methods should not use the same event name within a class

And the following message format:

Multiple logging methods are using event name {0} in class {1}

Code Sample

IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
    partial class MyClass
    {
        [LoggerMessage(EventId = 0, EventName = ""MyEvent"", Level = LogLevel.Debug, Message = ""M1"")]
        static partial void M1(ILogger logger);

        [LoggerMessage(EventId = 1, EventName = ""MyEvent"", Level = LogLevel.Debug, Message = ""M1"")]
        static partial void M2(ILogger logger);
    }
");

[tarekgh]

@allantargino thanks for the proposal. I'll update the issue subject with it and we can take it to the design review.

[allantargino]

Thanks @tarekgh. Again, sorry - I did a mistake when copying the code sample from the tests. Would you please update it again? I really appreciated!

[terrajobst]

Video

Looks good as proposed