diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs index 253ca650594..19137492cdf 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -13,6 +14,8 @@ namespace Microsoft.Gen.Logging.Emission; internal sealed partial class Emitter : EmitterBase { + private const string DataClassificationType = "global::Microsoft.Extensions.Compliance.Classification.DataClassification"; + private const string GlobalNamespacePrefix = "global::"; private const string LoggerMessageHelperType = "global::Microsoft.Extensions.Logging.LoggerMessageHelper"; private readonly StringBuilderPool _sbPool = new(); @@ -112,11 +115,14 @@ private void GenAttributeClassifications(LoggingType lt) _classificationMap.Clear(); foreach (var classificationAttr in classificationAttrs) { - var fieldName = PickUniqueName($"_{EncodeTypeName(classificationAttr)}", lt.AllMembers); + var fieldTypeName = classificationAttr.StartsWith(GlobalNamespacePrefix, StringComparison.Ordinal) + ? classificationAttr.Substring(GlobalNamespacePrefix.Length) + : classificationAttr; + var fieldName = PickUniqueName($"_{EncodeTypeName(fieldTypeName)}", lt.AllMembers); _classificationMap.Add(classificationAttr, fieldName); OutGeneratedCodeAttribute(); - OutLn($"private static readonly Microsoft.Extensions.Compliance.Classification.DataClassification {fieldName} = new {classificationAttr}().Classification;"); + OutLn($"private static readonly {DataClassificationType} {fieldName} = new {classificationAttr}().Classification;"); OutLn(); } } diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs index 23aebcdb00f..aedd5d61ce2 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs @@ -162,16 +162,16 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken var current = type; while (current != null) { - classification.UnionWith(GetDataClassificationAttributes(current, symbols).Select(x => x.ToDisplayString())); + classification.UnionWith(GetDataClassificationAttributes(current, symbols).Select(x => x.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat))); current = current.BaseType; } - classification.UnionWith(GetDataClassificationAttributes(property, symbols).Select(x => x.ToDisplayString())); + classification.UnionWith(GetDataClassificationAttributes(property, symbols).Select(x => x.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat))); // A property might be a sensitive parameter of a constructor: if (sensitivePropsFromCtor.TryGetValue(property.Name, out var dataClassesFromCtor)) { - classification.UnionWith(dataClassesFromCtor.Select(x => x.ToDisplayString())); + classification.UnionWith(dataClassesFromCtor.Select(x => x.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat))); } if (classification.Count > 0) diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs index 6b7426dc782..8ef6d5be3fc 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs @@ -634,7 +634,7 @@ private void CheckTagNamesAreUnique(LoggingMethod lm, Dictionary( GetDataClassificationAttributes(paramSymbol, symbols) .Distinct(SymbolEqualityComparer.Default) - .Select(x => x!.ToDisplayString())); + .Select(x => x!.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat))); var extractedType = paramTypeSymbol; if (paramTypeSymbol.IsNullableOfT()) diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs index 3f957905bec..20d8b3c8161 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs @@ -1,10 +1,13 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Generic; +using System.IO; using System.Reflection; using System.Threading.Tasks; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.Extensions.Compliance.Classification; using Microsoft.Extensions.Compliance.Redaction; using Microsoft.Extensions.Compliance.Testing; @@ -12,6 +15,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Gen.Logging.Parsing; using Microsoft.Gen.Shared; +using VerifyXunit; using Xunit; namespace Microsoft.Gen.Logging.Test; @@ -76,6 +80,86 @@ internal static partial class C Assert.Empty(diagnostics); } + [Fact] + public Task DataClassificationAttributeNamespaceIsGloballyQualified() + { + const string Source = @" + using Microsoft.Extensions.Compliance.Classification; + using Microsoft.Extensions.Logging; + + namespace Bar.Classifications + { + public sealed class SecretAttribute : DataClassificationAttribute + { + public SecretAttribute() + : base(new DataClassification(""Taxonomy"", ""Secret"")) + { + } + } + } + + namespace Foo.Bar + { + using global::Bar.Classifications; + + internal static partial class C + { + [LoggerMessage(0, LogLevel.Debug, ""M {p0}"")] + static partial void M(ILogger logger, [Secret] string p0); + } + } + "; + + var generatedSource = RunGeneratorAndAssertNoErrors(Source); + + return Verifier.Verify(generatedSource) + .AddScrubber(_ => _.Replace(GeneratorUtilities.CurrentVersion, "VERSION")) + .UseDirectory(Path.Combine("..", "Verified")); + } + + [Fact] + public void DataClassificationAttributeNamespaceIsGloballyQualifiedForLogProperties() + { + const string Source = @" + using Microsoft.Extensions.Compliance.Classification; + using Microsoft.Extensions.Logging; + + namespace Bar.Classifications + { + public sealed class SecretAttribute : DataClassificationAttribute + { + public SecretAttribute() + : base(new DataClassification(""Taxonomy"", ""Secret"")) + { + } + } + } + + namespace Foo.Bar + { + using global::Bar.Classifications; + + public class Payload + { + [Secret] + public string? Value { get; set; } + } + + public record class Record([property: Secret] string Value); + + internal static partial class C + { + [LoggerMessage(0, LogLevel.Debug, ""M"")] + static partial void M(ILogger logger, [LogProperties] Payload p0, [LogProperties] Record p1); + } + } + "; + + var generatedSource = RunGeneratorAndAssertNoErrors(Source); + + Assert.Contains("new global::Bar.Classifications.SecretAttribute().Classification", generatedSource, StringComparison.Ordinal); + } + [Fact] public async Task MultipleAttributesOnDifferentTopics() { @@ -222,6 +306,22 @@ partial class C Assert.Equal(DiagDescriptors.CantUseDataClassificationWithLogPropertiesOrTagProvider.Id, diagnostics[0].Id); } + // Runs the generator over a full compilation so that the emitted code is verified to actually compile. + private static string RunGeneratorAndAssertNoErrors(string source) + { + GeneratorDriver driver = CSharpGeneratorDriver.Create(new LoggingGenerator()); + driver = driver.RunGeneratorsAndUpdateCompilation( + CompilationHelper.CreateCompilation(source), + out var outputCompilation, + out var generatorDiagnostics); + + Assert.Empty(generatorDiagnostics); + Assert.DoesNotContain(outputCompilation.GetDiagnostics(), static diagnostic => diagnostic.Severity == DiagnosticSeverity.Error); + + var generatedSource = Assert.Single(driver.GetRunResult().Results[0].GeneratedSources); + return generatedSource.SourceText.ToString(); + } + private static async Task> RunGenerator(string code) { var text = $@" diff --git a/test/Generators/Microsoft.Gen.Logging/Verified/AttributeParserTests.DataClassificationAttributeNamespaceIsGloballyQualified.verified.txt b/test/Generators/Microsoft.Gen.Logging/Verified/AttributeParserTests.DataClassificationAttributeNamespaceIsGloballyQualified.verified.txt new file mode 100644 index 00000000000..41d499829b4 --- /dev/null +++ b/test/Generators/Microsoft.Gen.Logging/Verified/AttributeParserTests.DataClassificationAttributeNamespaceIsGloballyQualified.verified.txt @@ -0,0 +1,54 @@ + +// +#nullable enable + +namespace Foo.Bar +{ + partial class C + { + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Gen.Logging", "VERSION")] + private static readonly global::Microsoft.Extensions.Compliance.Classification.DataClassification _Bar_Classifications_SecretAttribute = new global::Bar.Classifications.SecretAttribute().Classification; + + /// + /// Logs "M {p0}" at "Debug" level. + /// + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Gen.Logging", "VERSION")] + static partial void M(global::Microsoft.Extensions.Logging.ILogger logger, string p0) + { + if (!logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug)) + { + return; + } + + var state = global::Microsoft.Extensions.Logging.LoggerMessageHelper.ThreadLocalState; + try + { + + _ = state.ReserveTagSpace(1); + state.TagArray[0] = new("{OriginalFormat}", "M {p0}"); + + _ = state.ReserveClassifiedTagSpace(1); + state.ClassifiedTagArray[0] = new("p0", p0, new global::Microsoft.Extensions.Compliance.Classification.DataClassificationSet(_Bar_Classifications_SecretAttribute)); + + logger.Log( + global::Microsoft.Extensions.Logging.LogLevel.Debug, + new(0, nameof(M)), + state, + null, + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Gen.Logging", "VERSION")] static string (s, _) => + { + var p0 = s.RedactedTagArray[0].Value ?? "(null)"; + #if NET + return string.Create(global::System.Globalization.CultureInfo.InvariantCulture, $"M {p0}"); + #else + return global::System.FormattableString.Invariant($"M {p0}"); + #endif + }); + } + finally + { + state.Clear(); + } + } + } +}