Generator cleanup before updating code gen.

- Remove constraints about not starting log method names or log method parameters with _, the code now dynamically picks non-conflicting symbol names. There's still a potential conflict with the name of redactor instances, but I'll be removing all those in my follow-on PR. - Simplified logic around handling of template parameters. I couldn't figure out what the code was doing, so it needed a bit of cleanup. - Remove some dead code I stumbled on. - Fix broken .resx file handling in the generators, I messed that up when simplifying the Roslyn dependencies last week.
dotnet · Jul 29, 2023 · 43dbb90 · 43dbb90
1 parent 8f5680b
commit 43dbb90
Show file tree

Hide file tree

Showing 25 changed files with 248 additions and 331 deletions.
diff --git a/src/Generators/Microsoft.Gen.AutoClient/Microsoft.Gen.AutoClient.csproj b/src/Generators/Microsoft.Gen.AutoClient/Microsoft.Gen.AutoClient.csproj
@@ -21,6 +21,10 @@
     <Compile Include="..\Shared\*.cs" LinkBase="Shared" />
   </ItemGroup>
 
+  <ItemGroup>
+    <EmbeddedResource Update="Resources.resx" Generator="ResXFileCodeGenerator" LastGenOutput="Resources.Designer.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="all" />
     <PackageReference Include="Microsoft.CodeAnalysis" />

diff --git a/src/Generators/Microsoft.Gen.ContextualOptions/Microsoft.Gen.ContextualOptions.csproj b/src/Generators/Microsoft.Gen.ContextualOptions/Microsoft.Gen.ContextualOptions.csproj
@@ -17,6 +17,10 @@
     <Compile Include="..\Shared\EmitterBase.cs" LinkBase="Shared" />
   </ItemGroup>
 
+  <ItemGroup>
+    <EmbeddedResource Update="Resources.resx" Generator="ResXFileCodeGenerator" LastGenOutput="Resources.Designer.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="all" />
     <PackageReference Include="Microsoft.CodeAnalysis" />

diff --git a/src/Generators/Microsoft.Gen.EnumStrings/Microsoft.Gen.EnumStrings.csproj b/src/Generators/Microsoft.Gen.EnumStrings/Microsoft.Gen.EnumStrings.csproj
@@ -15,6 +15,10 @@
     <Compile Include="..\Shared\*.cs" LinkBase="Shared" />
   </ItemGroup>
 
+  <ItemGroup>
+    <EmbeddedResource Update="Resources.resx" Generator="ResXFileCodeGenerator" LastGenOutput="Resources.Designer.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="all" />
     <PackageReference Include="Microsoft.CodeAnalysis" />

diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs
diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Utils.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Utils.cs
@@ -77,7 +77,7 @@ internal static string EscapeMessageStringForXmlDocumentation(string s)
     internal static IReadOnlyCollection<string> GetLogPropertiesAttributes(LoggingMethod lm)
     {
         var result = new HashSet<string?>();
-        var parametersWithLogProps = lm.AllParameters.Where(x => x.HasProperties && !x.HasPropsProvider);
+        var parametersWithLogProps = lm.Parameters.Where(x => x.HasProperties && !x.HasPropsProvider);
         foreach (var parameter in parametersWithLogProps)
         {
             parameter.TraverseParameterPropertiesTransitively((_, property) => result.Add(property.ClassificationAttributeType));
@@ -96,7 +96,7 @@ internal static string GetLoggerMethodLogLevel(LoggingMethod lm)
 
         if (lm.Level == null)
         {
-            foreach (var p in lm.AllParameters)
+            foreach (var p in lm.Parameters)
             {
                 if (p.IsLogLevel)
                 {
@@ -150,4 +150,30 @@ internal static string GetLoggerMethodLogLevel(LoggingMethod lm)
     }
 
     internal static string EncodeTypeName(string typeName) => typeName.Replace("_", "__").Replace('.', '_');
+
+    internal static string PickUniqueName(string baseName, IEnumerable<string> potentialConflicts)
+    {
+        var name = baseName;
+        while (true)
+        {
+            bool conflict = false;
+            foreach (var potentialConflict in potentialConflicts)
+            {
+                if (name == potentialConflict)
+                {
+                    conflict = true;
+                    break;
+                }
+            }
+
+            if (!conflict)
+            {
+                return name;
+            }
+
+#pragma warning disable S1643 // Strings should not be concatenated using '+' in a loop
+            name += "_";
+#pragma warning restore S1643 // Strings should not be concatenated using '+' in a loop
+        }
+    }
 }
diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs
@@ -18,12 +18,10 @@ internal sealed partial class Emitter : EmitterBase
 
     private readonly StringBuilderPool _sbPool = new();
     private bool _isRedactorProviderInTheInstance;
-    private int _memberCounter;
 
     public string Emit(IEnumerable<LoggingType> logTypes, CancellationToken cancellationToken)
     {
         _isRedactorProviderInTheInstance = false;
-        _memberCounter = 0;
 
         foreach (var lt in logTypes.OrderBy(static lt => lt.Namespace + "." + lt.Name))
         {
@@ -72,7 +70,7 @@ private void GenType(LoggingType lt)
 
         var isRedactionRequired =
             lt.Methods
-                .SelectMany(static lm => lm.AllParameters)
+                .SelectMany(static lm => lm.Parameters)
                 .Any(static lp => lp.ClassificationAttributeType != null)
             || lt.Methods
                 .SelectMany(static lm => GetLogPropertiesAttributes(lm))
@@ -81,7 +79,7 @@ private void GenType(LoggingType lt)
         if (isRedactionRequired)
         {
             _isRedactorProviderInTheInstance = lt.Methods
-                .SelectMany(static lm => lm.AllParameters)
+                .SelectMany(static lm => lm.Parameters)
                 .All(static lp => !lp.IsRedactorProvider);
 
             if (_isRedactorProviderInTheInstance)
@@ -105,7 +103,6 @@ private void GenType(LoggingType lt)
             }
 
             GenLogMethod(lm);
-            _memberCounter++;
         }
 
         OutCloseBrace();
@@ -129,7 +126,7 @@ private void GenAttributeClassifications(LoggingType lt)
 
         var logPropsDataClasses = lt.Methods.SelectMany(lm => GetLogPropertiesAttributes(lm));
         var classificationAttributeTypes = lt.Methods
-            .SelectMany(static lm => lm.AllParameters)
+            .SelectMany(static lm => lm.Parameters)
             .Where(static lp => lp.ClassificationAttributeType is not null)
             .Select(static lp => lp.ClassificationAttributeType!)
             .Concat(logPropsDataClasses)
@@ -151,7 +148,7 @@ private void GenRedactorProperties(LoggingType lt)
 
         var logPropsDataClasses = lt.Methods.SelectMany(lm => GetLogPropertiesAttributes(lm));
         var classificationAttributeTypes = lt.Methods
-            .SelectMany(static lm => lm.AllParameters)
+            .SelectMany(static lm => lm.Parameters)
             .Where(static lp => lp.ClassificationAttributeType is not null)
             .Select(static lp => lp.ClassificationAttributeType!)
             .Concat(logPropsDataClasses)

diff --git a/src/Generators/Microsoft.Gen.Logging/Microsoft.Gen.Logging.csproj b/src/Generators/Microsoft.Gen.Logging/Microsoft.Gen.Logging.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <RootNamespace>Microsoft.Gen.Logging</RootNamespace>
     <Description>Code generator to support Microsoft.Extensions.Telemetry's logging features.</Description>
@@ -11,10 +11,14 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <Compile Update="Resources.Designer.cs" DesignTime="True" AutoGen="True" DependentUpon="Resources.resx" />
+    <Compile Update="Parsing\Resources.Designer.cs" DesignTime="True" AutoGen="True" DependentUpon="Resources.resx" />
     <Compile Include="..\Shared\*.cs" LinkBase="Shared" />
   </ItemGroup>
 
+  <ItemGroup>
+    <EmbeddedResource Update="Parsing\Resources.resx" Generator="ResXFileCodeGenerator" LastGenOutput="Resources.Designer.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="all" />
     <PackageReference Include="Microsoft.CodeAnalysis" />

diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs
@@ -11,9 +11,8 @@ namespace Microsoft.Gen.Logging.Model;
 /// </summary>
 internal sealed class LoggingMethod
 {
-    public readonly List<LoggingMethodParameter> AllParameters = new();
-    public readonly List<LoggingMethodParameter> TemplateParameters = new();
-    public readonly Dictionary<string, string> TemplateMap = new(StringComparer.OrdinalIgnoreCase);
+    public readonly List<LoggingMethodParameter> Parameters = new();
+    public readonly Dictionary<string, string> TemplateToParameterName = new(StringComparer.OrdinalIgnoreCase);
     public string Name = string.Empty;
     public string Message = string.Empty;
     public int? Level;
@@ -30,7 +29,7 @@ internal sealed class LoggingMethod
     public bool HasXmlDocumentation;
 
     public string GetParameterNameInTemplate(LoggingMethodParameter parameter)
-        => TemplateMap.TryGetValue(parameter.Name, out var value)
+        => TemplateToParameterName.TryGetValue(parameter.Name, out var value)
             ? value
             : parameter.Name;
 }
diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs
@@ -25,9 +25,10 @@ internal sealed class LoggingMethodParameter
     public bool IsNullable;
     public bool IsReference;
     public bool ImplementsIConvertible;
-    public bool ImplementsIFormatable;
+    public bool ImplementsIFormattable;
     public bool SkipNullProperties;
     public bool OmitParameterName;
+    public bool UsedAsTemplate;
     public string? ClassificationAttributeType;
     public List<LoggingProperty> PropertiesToLog = new();
     public LoggingPropertyProvider? LogPropertiesProvider;

diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs
@@ -18,7 +18,7 @@ internal sealed record LoggingProperty(
     bool IsReference,
     bool IsEnumerable,
     bool ImplementsIConvertible,
-    bool ImplementsIFormatable,
+    bool ImplementsIFormattable,
     IReadOnlyCollection<LoggingProperty> TransitiveMembers)
 {
     public string NameWithAt => NeedsAtSign ? "@" + Name : Name;

diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs
@@ -10,11 +10,7 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
 {
     private const string Category = "LogMethod";
 
-    public static DiagnosticDescriptor InvalidLoggingMethodName { get; } = Make(
-        id: "R9G000",
-        title: Resources.InvalidLoggingMethodNameTitle,
-        messageFormat: Resources.InvalidLoggingMethodNameMessage,
-        category: Category);
+    // R9G000 retired
 
     public static DiagnosticDescriptor ShouldntMentionLogLevelInMessage { get; } = Make(
         id: "R9G001",
@@ -23,13 +19,7 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
         category: Category,
         DiagnosticSeverity.Warning);
 
-    public static DiagnosticDescriptor InvalidLoggingMethodParameterName { get; } = Make(
-        id: "R9G002",
-        title: Resources.InvalidLoggingMethodParameterNameTitle,
-        messageFormat: Resources.InvalidLoggingMethodParameterNameMessage,
-        category: Category);
-
-    // R9G003 is no longer in use
+    // R9G002,R9G003 retired
 
     public static DiagnosticDescriptor MissingRequiredType { get; } = Make(
         id: "R9G004",
@@ -50,10 +40,10 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
         messageFormat: Resources.LoggingMethodMustReturnVoidMessage,
         category: Category);
 
-    public static DiagnosticDescriptor MissingLoggerArgument { get; } = Make(
+    public static DiagnosticDescriptor MissingLoggerParameter { get; } = Make(
         id: "R9G007",
-        title: Resources.MissingLoggerArgumentTitle,
-        messageFormat: Resources.MissingLoggerArgumentMessage,
+        title: Resources.MissingLoggerParameterTitle,
+        messageFormat: Resources.MissingLoggerParameterMessage,
         category: Category);
 
     public static DiagnosticDescriptor LoggingMethodShouldBeStatic { get; } = Make(
@@ -89,16 +79,16 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
         category: Category,
         DiagnosticSeverity.Warning);
 
-    public static DiagnosticDescriptor TemplateHasNoCorrespondingArgument { get; } = Make(
+    public static DiagnosticDescriptor TemplateHasNoCorrespondingParameter { get; } = Make(
         id: "R9G013",
-        title: Resources.TemplateHasNoCorrespondingArgumentTitle,
-        messageFormat: Resources.TemplateHasNoCorrespondingArgumentMessage,
+        title: Resources.TemplateHasNoCorrespondingParameterTitle,
+        messageFormat: Resources.TemplateHasNoCorrespondingParameterMessage,
         category: Category);
 
-    public static DiagnosticDescriptor ArgumentHasNoCorrespondingTemplate { get; } = Make(
+    public static DiagnosticDescriptor ParameterHasNoCorrespondingTemplate { get; } = Make(
         id: "R9G014",
-        title: Resources.ArgumentHasNoCorrespondingTemplateTitle,
-        messageFormat: Resources.ArgumentHasNoCorrespondingTemplateMessage,
+        title: Resources.ParameterHasNoCorrespondingTemplateTitle,
+        messageFormat: Resources.ParameterHasNoCorrespondingTemplateMessage,
         category: Category,
         DiagnosticSeverity.Info);
 
@@ -139,16 +129,16 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
         messageFormat: Resources.MultipleDataClassificationAttributesMessage,
         category: Category);
 
-    public static DiagnosticDescriptor MissingRedactorProviderArgument { get; } = Make(
+    public static DiagnosticDescriptor MissingRedactorProviderParameter { get; } = Make(
         id: "R9G021",
-        title: Resources.MissingRedactorProviderArgumentTitle,
-        messageFormat: Resources.MissingRedactorProviderArgumentMessage,
+        title: Resources.MissingRedactorProviderParameterTitle,
+        messageFormat: Resources.MissingRedactorProviderParameterMessage,
         category: Category);
 
-    public static DiagnosticDescriptor MissingDataClassificationArgument { get; } = Make(
+    public static DiagnosticDescriptor MissingDataClassificationParameter { get; } = Make(
         id: "R9G022",
-        title: Resources.MissingDataClassificationArgumentTitle,
-        messageFormat: Resources.MissingDataClassificationArgumentMessage,
+        title: Resources.MissingDataClassificationParameterTitle,
+        messageFormat: Resources.MissingDataClassificationParameterMessage,
         category: Category,
         DiagnosticSeverity.Warning);