Skip to content

Commit

Permalink
Fixes bad log method generation in certain cases.
Browse files Browse the repository at this point in the history
In certain cases when developer by mistake places ILogger, Exception, or LogLevel in the message template,  the code generator will produce the expected warning and makes sure the code will indeed compile and run correctly.

Prior to this fix, the code generator would fail to compile with when either of ILogger, Exception or LogLevel were placed in message template incorrectly.

Fixes dotnet#64310
  • Loading branch information
maryamariyan committed Jan 26, 2022
1 parent 9a50493 commit 39c972e
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -339,17 +339,21 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
foundException |= lp.IsException;
foundLogLevel |= lp.IsLogLevel;

bool forceAsTemplateParams = false;
if (lp.IsLogger && lm.TemplateMap.ContainsKey(paramName))
{
Diag(DiagnosticDescriptors.ShouldntMentionLoggerInMessage, paramSymbol.Locations[0], paramName);
forceAsTemplateParams = true;
}
else if (lp.IsException && lm.TemplateMap.ContainsKey(paramName))
{
Diag(DiagnosticDescriptors.ShouldntMentionExceptionInMessage, paramSymbol.Locations[0], paramName);
forceAsTemplateParams = true;
}
else if (lp.IsLogLevel && lm.TemplateMap.ContainsKey(paramName))
{
Diag(DiagnosticDescriptors.ShouldntMentionLogLevelInMessage, paramSymbol.Locations[0], paramName);
forceAsTemplateParams = true;
}
else if (lp.IsLogLevel && level != null && !lm.TemplateMap.ContainsKey(paramName))
{
Expand All @@ -368,7 +372,7 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
}

lm.AllParameters.Add(lp);
if (lp.IsTemplateParameter)
if (lp.IsTemplateParameter || forceAsTemplateParams)
{
lm.TemplateParameters.Add(lp);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,20 @@ public void LevelTests()
Assert.Equal(LogLevel.Warning, logger.LastLogLevel);
Assert.Equal(1, logger.CallCount);
Assert.Equal(11, logger.LastEventId.Id);

logger.Reset();
LevelTestExtensions.M12(logger, LogLevel.Trace);
Assert.Null(logger.LastException);
Assert.Equal("M12 Trace", logger.LastFormattedString);
Assert.Equal(LogLevel.Trace, logger.LastLogLevel);
Assert.Equal(1, logger.CallCount);

logger.Reset();
LevelTestExtensions.M13(logger, LogLevel.Trace);
Assert.Null(logger.LastException);
Assert.Equal("M13 Microsoft.Extensions.Logging.Generators.Tests.MockLogger", logger.LastFormattedString);
Assert.Equal(LogLevel.Trace, logger.LastLogLevel);
Assert.Equal(1, logger.CallCount);
}

[Fact]
Expand All @@ -361,6 +375,13 @@ public void ExceptionTests()
Assert.Equal("M1 System.ArgumentException: Bar", logger.LastFormattedString);
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
Assert.Equal(1, logger.CallCount);

logger.Reset();
ExceptionTestExtensions.M2(logger, "One", new ArgumentException("Foo"));
Assert.Equal("Foo", logger.LastException!.Message);
Assert.Equal("M2 One: System.ArgumentException: Foo", logger.LastFormattedString);
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
Assert.Equal(1, logger.CallCount);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public async Task TestEmitter()
new[] { typeof(ILogger).Assembly, typeof(LoggerMessageAttribute).Assembly },
new[] { testSourceCode }).ConfigureAwait(false);

Assert.Empty(d);
Assert.True(src.Contains("WithDiagnostics") ? !d.IsEmpty : d.IsEmpty);
Assert.Single(r);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,15 @@ partial class C
{
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1 {ex} {ex2}"")]
static partial void M1(ILogger logger, System.Exception ex, System.Exception ex2);
[LoggerMessage(EventId = 2, Level = LogLevel.Debug, Message = ""M2 {arg1}: {ex}"")]
static partial void M2(ILogger logger, string arg1, System.Exception ex);
}
");

Assert.Single(diagnostics);
Assert.Equal(2, diagnostics.Count);
Assert.Equal(DiagnosticDescriptors.ShouldntMentionExceptionInMessage.Id, diagnostics[0].Id);
Assert.Equal(DiagnosticDescriptors.ShouldntMentionExceptionInMessage.Id, diagnostics[1].Id);
}

[Fact]
Expand All @@ -234,11 +238,15 @@ partial class C
{
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1 {logger}"")]
static partial void M1(ILogger logger);
[LoggerMessage(EventId = 2, Message = ""M2 {logger}"")]
static partial void M2(ILogger logger, LogLevel level);
}
");

Assert.Single(diagnostics);
Assert.Equal(2, diagnostics.Count);
Assert.Equal(DiagnosticDescriptors.ShouldntMentionLoggerInMessage.Id, diagnostics[0].Id);
Assert.Equal(DiagnosticDescriptors.ShouldntMentionLoggerInMessage.Id, diagnostics[1].Id);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
{
internal static partial class ExceptionTestExtensions
{
#pragma warning disable SYSLIB1013
[LoggerMessage(EventId = 2, Level = LogLevel.Debug, Message = "M2 {arg1}: {ex}")]
public static partial void M2(ILogger logger, string arg1, Exception ex);
#pragma warning disable SYSLIB1013
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
{
internal static partial class LevelTestExtensions
{
#pragma warning disable SYSLIB1002
[LoggerMessage(EventId = 12, Message = "M12 {level}")]
public static partial void M12(ILogger logger, LogLevel level);
#pragma warning restore SYSLIB1002

#pragma warning disable SYSLIB1018
[LoggerMessage(EventId = 13, Message = "M13 {logger}")]
public static partial void M13(ILogger logger, LogLevel level);
#pragma warning restore SYSLIB1018
}
}

0 comments on commit 39c972e

Please sign in to comment.