Skip to content

Commit

Permalink
Fix #5188 (#5191)
Browse files Browse the repository at this point in the history
The logging code generator wasn't aware of the
NoDataClassification attribute and treated the
associated data as having in fact a data classification.
This led to an incorrect warning.

In addition, what should have been a warning was
treated like an error and aborted compilation. Now,
if the legit problem occurs, it will be reported
as warning and not stop compilation.

Co-authored-by: Martin Taillefer <[email protected]>
  • Loading branch information
geeknoid and Martin Taillefer authored Jun 3, 2024
1 parent 300c3ca commit 4bafacb
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
id: DiagnosticIds.LoggerMessage.LOGGEN035,
title: Resources.RecordTypeSensitiveArgumentIsInTemplateTitle,
messageFormat: Resources.RecordTypeSensitiveArgumentIsInTemplateMessage,
category: Category);
category: Category,
DiagnosticSeverity.Warning);

public static DiagnosticDescriptor DefaultToString { get; } = Make(
id: DiagnosticIds.LoggerMessage.LOGGEN036,
Expand Down
1 change: 1 addition & 0 deletions src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ private static List<INamedTypeSymbol> GetDataClassificationAttributes(ISymbol sy
.GetAttributes()
.Select(static attribute => attribute.AttributeClass)
.Where(x => x is not null && symbols.DataClassificationAttribute is not null && ParserUtilities.IsBaseOrIdentity(x, symbols.DataClassificationAttribute, symbols.Compilation))
.Where(x => !SymbolEqualityComparer.Default.Equals(x, symbols.NoDataClassificationAttribute))
.Select(static x => x!)
.ToList();

Expand Down
3 changes: 2 additions & 1 deletion src/Generators/Microsoft.Gen.Logging/Parsing/SymbolHolder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ internal sealed record class SymbolHolder(
INamedTypeSymbol EnumerableSymbol,
INamedTypeSymbol FormatProviderSymbol,
INamedTypeSymbol? SpanFormattableSymbol,
INamedTypeSymbol? DataClassificationAttribute);
INamedTypeSymbol? DataClassificationAttribute,
INamedTypeSymbol? NoDataClassificationAttribute);
5 changes: 4 additions & 1 deletion src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ internal static class SymbolLoader
internal const string LogLevelType = "Microsoft.Extensions.Logging.LogLevel";
internal const string ExceptionType = "System.Exception";
internal const string DataClassificationAttribute = "Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute";
internal const string NoDataClassificationAttribute = "Microsoft.Extensions.Compliance.Classification.NoDataClassificationAttribute";
internal const string IEnrichmentPropertyBag = "Microsoft.Extensions.Diagnostics.Enrichment.IEnrichmentPropertyBag";
internal const string IFormatProviderType = "System.IFormatProvider";
internal const string ISpanFormattableType = "System.ISpanFormattable";
Expand Down Expand Up @@ -60,6 +61,7 @@ internal static class SymbolLoader
var tagCollectorSymbol = compilation.GetTypeByMetadataName(ITagCollectorType);
var logPropertyIgnoreAttributeSymbol = compilation.GetTypeByMetadataName(LogPropertyIgnoreAttribute);
var dataClassificationAttribute = compilation.GetTypeByMetadataName(DataClassificationAttribute);
var noDataClassificationAttribute = compilation.GetTypeByMetadataName(NoDataClassificationAttribute);

#pragma warning disable S1067 // Expressions should not be too complex
if (loggerSymbol == null
Expand Down Expand Up @@ -113,6 +115,7 @@ internal static class SymbolLoader
enumerableSymbol,
formatProviderSymbol,
spanFormattableSymbol,
dataClassificationAttribute);
dataClassificationAttribute,
noDataClassificationAttribute);
}
}
19 changes: 19 additions & 0 deletions test/Generators/Microsoft.Gen.Logging/TestClasses/Bug5188.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Microsoft.Extensions.Compliance.Classification;
using Microsoft.Extensions.Logging;

public static partial class Bug5188
{
internal record User()
{
public int Id { get; set; }

[NoDataClassification]
public string? Email { get; set; }
}

internal static partial class Logging
{
[LoggerMessage(6001, LogLevel.Information, "Logging User: {User}")]
public static partial void LogUser(ILogger logger, [LogProperties] User user);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public void RecordHasSensitivePublicMembers_ShouldReturnFalse_WhenNoDataClasses(
null!,
null!,
null!,
null!,
null!);

var diagMock = new Mock<Action<Diagnostic>>();
Expand Down Expand Up @@ -112,7 +113,8 @@ public void RecordHasSensitivePublicMembers_ShouldNotThrow_WhenNoMembersOnType(b
null!,
null!,
null!,
Mock.Of<INamedTypeSymbol>());
Mock.Of<INamedTypeSymbol>(),
null!);

var diagMock = new Mock<Action<Diagnostic>>();
var parser = new Parser(null!, diagMock.Object, CancellationToken.None);
Expand Down Expand Up @@ -141,6 +143,7 @@ public void ProcessLogPropertiesForParameter_ShouldNotThrow_WhenNoMembersOnType(
null!,
null!,
null!,
null!,
null!);

const string ParamType = "param type";
Expand Down

0 comments on commit 4bafacb

Please sign in to comment.