From 6018c9dc784cea267bf0120ae8943c7c18d9c0b0 Mon Sep 17 00:00:00 2001 From: Martin Taillefer Date: Thu, 3 Aug 2023 06:25:30 -0700 Subject: [PATCH] Address PR feedback --- .../Emission/Emitter.Method.cs | 35 +++++++++---------- .../Microsoft.Gen.Logging/Emission/Emitter.cs | 2 +- .../Model/LoggingMethodParameter.cs | 1 + .../Model/LoggingProperty.cs | 1 + 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs index f934417ecc8..f03621a4c38 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs @@ -207,7 +207,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc { if (NeedsASlot(p)) { - if (p.ClassificationAttributeType != null) + if (p.HasDataClassification) { numClassifiedProps++; } @@ -221,7 +221,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc { p.TraverseParameterPropertiesTransitively((_, member) => { - if (member.ClassificationAttributeType != null) + if (member.HasDataClassification) { numClassifiedProps++; } @@ -248,26 +248,25 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc int count = numReservedUnclassifiedProps; foreach (var p in lm.Parameters) { - if (NeedsASlot(p) && p.ClassificationAttributeType == null) + if (NeedsASlot(p) && !p.HasDataClassification) { var key = $"\"{lm.GetParameterNameInTemplate(p)}\""; + string value; if (p.IsEnumerable) { - var value = p.PotentiallyNull + value = p.PotentiallyNull ? $"{p.NameWithAt} != null ? {LoggerMessageHelperType}.Stringify({p.NameWithAt}) : null" : $"{LoggerMessageHelperType}.Stringify({p.NameWithAt})"; - - OutLn($"{stateName}.PropertyArray[{--count}] = new({key}, {value});"); } else { - var value = ShouldStringify(p.Type) + value = ShouldStringify(p.Type) ? ConvertToString(p, p.NameWithAt) : p.NameWithAt; - - OutLn($"{stateName}.PropertyArray[{--count}] = new({key}, {value});"); } + + OutLn($"{stateName}.PropertyArray[{--count}] = new({key}, {value});"); } } @@ -277,7 +276,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc { p.TraverseParameterPropertiesTransitively((propertyChain, member) => { - if (member.ClassificationAttributeType == null) + if (!member.HasDataClassification) { var propName = PropertyChainToString(propertyChain, member, "_", omitParameterName: p.OmitParameterName); var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: "."); @@ -309,10 +308,10 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc int count = numReservedClassifiedProps; foreach (var p in lm.Parameters) { - if (NeedsASlot(p) && p.ClassificationAttributeType != null) + if (NeedsASlot(p) && p.HasDataClassification) { var key = $"\"{lm.GetParameterNameInTemplate(p)}\""; - var classification = $"_{EncodeTypeName(p.ClassificationAttributeType)}_Classification"; + var classification = $"_{EncodeTypeName(p.ClassificationAttributeType!)}_Classification"; var value = ShouldStringify(p.Type) ? ConvertToString(p, p.NameWithAt) @@ -328,13 +327,13 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc { p.TraverseParameterPropertiesTransitively((propertyChain, member) => { - if (member.ClassificationAttributeType != null) + if (member.HasDataClassification) { var propName = PropertyChainToString(propertyChain, member, "_", omitParameterName: p.OmitParameterName); var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: "."); var value = ConvertPropertyToString(member, accessExpression); - var classification = $"_{EncodeTypeName(member.ClassificationAttributeType)}_Classification"; + var classification = $"_{EncodeTypeName(member.ClassificationAttributeType!)}_Classification"; OutLn($"{stateName}.ClassifiedPropertyArray[{--count}] = new(\"{propName}\", {value}, {classification});"); } @@ -349,7 +348,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc { p.TraverseParameterPropertiesTransitively((propertyChain, member) => { - if (member.ClassificationAttributeType == null) + if (!member.HasDataClassification) { var propName = PropertyChainToString(propertyChain, member, "_", omitParameterName: p.OmitParameterName); var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: "."); @@ -372,7 +371,7 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc var accessExpression = PropertyChainToString(propertyChain, member, "?.", nonNullSeparator: "."); var value = ConvertPropertyToString(member, accessExpression); - var classification = $"_{EncodeTypeName(member.ClassificationAttributeType)}_Classification"; + var classification = $"_{EncodeTypeName(member.ClassificationAttributeType!)}_Classification"; var skipNull = member.PotentiallyNull ? $"if ({value} != null) " @@ -406,7 +405,7 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes int index = numReservedUnclassifiedProps - 1; foreach (var p in lm.Parameters) { - if (NeedsASlot(p) && p.ClassificationAttributeType == null) + if (NeedsASlot(p) && !p.HasDataClassification) { if (p.UsedAsTemplate) { @@ -433,7 +432,7 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes index = numReservedClassifiedProps - 1; foreach (var p in lm.Parameters) { - if (NeedsASlot(p) && p.ClassificationAttributeType != null) + if (NeedsASlot(p) && p.HasDataClassification) { if (p.UsedAsTemplate) { diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs index 34b4f947ad1..5d6bdecd92d 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs @@ -65,7 +65,7 @@ private void GenType(LoggingType lt) OutOpenBrace(); var isRedactionRequired = - lt.Methods.SelectMany(static lm => lm.Parameters).Any(static lp => lp.ClassificationAttributeType != null) + lt.Methods.SelectMany(static lm => lm.Parameters).Any(static lp => lp.HasDataClassification) || lt.Methods.SelectMany(static lm => GetLogPropertiesAttributes(lm)).Any(); if (isRedactionRequired) diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs index 23068085dde..6eff65be9db 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs @@ -43,6 +43,7 @@ public string PotentiallyNullableType // but instead is supposed to be taken as a normal parameter. public bool IsNormalParameter => !IsLogger && !IsException && !IsLogLevel; + public bool HasDataClassification => ClassificationAttributeType != null; public bool HasProperties => PropertiesToLog.Count > 0; public bool HasPropsProvider => LogPropertiesProvider is not null; public bool PotentiallyNull => (IsReference && !IsLogger) || IsNullable; diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs index 37f40fdb44f..7c675f5b44e 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs @@ -21,6 +21,7 @@ internal sealed record LoggingProperty( bool ImplementsIFormattable, IReadOnlyCollection TransitiveMembers) { + public bool HasDataClassification => ClassificationAttributeType != null; public string NameWithAt => NeedsAtSign ? "@" + Name : Name; public bool PotentiallyNull => IsReference || IsNullable; }