Skip to content

Commit 1a18528

Browse files
maryamariyanmaryamariyan
authored
Logging Generator Parser - Code cleanup PR feedback (#52018)
1 parent 3245528 commit 1a18528

File tree

2 files changed

+58
-174
lines changed

2 files changed

+58
-174
lines changed

‎src/libraries/Microsoft.Extensions.Logging/gen/LoggerMessageGenerator.Parser.cs

Lines changed: 58 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.Diagnostics;
56
using System.Collections.Generic;
67
using System.Linq;
78
using System.Threading;
@@ -61,19 +62,8 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
6162
return Array.Empty<LoggerClass>();
6263
}
6364

64-
INamedTypeSymbol enumerableSymbol = _compilation.GetTypeByMetadataName("System.Collections.IEnumerable");
65-
if (enumerableSymbol == null)
66-
{
67-
Diag(DiagnosticDescriptors.MissingRequiredType, null, "System.Collections.IEnumerable");
68-
return Array.Empty<LoggerClass>();
69-
}
70-
71-
INamedTypeSymbol stringSymbol = _compilation.GetTypeByMetadataName("System.String");
72-
if (stringSymbol == null)
73-
{
74-
Diag(DiagnosticDescriptors.MissingRequiredType, null, "System.String");
75-
return Array.Empty<LoggerClass>();
76-
}
65+
INamedTypeSymbol enumerableSymbol = _compilation.GetSpecialType(SpecialType.System_Collections_IEnumerable);
66+
INamedTypeSymbol stringSymbol = _compilation.GetSpecialType(SpecialType.System_String);
7767

7868
var results = new List<LoggerClass>();
7969
var ids = new HashSet<int>();
@@ -102,29 +92,29 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
10292
continue;
10393
}
10494

95+
sm ??= _compilation.GetSemanticModel(classDec.SyntaxTree);
96+
10597
foreach (AttributeListSyntax mal in method.AttributeLists)
10698
{
10799
foreach (AttributeSyntax ma in mal.Attributes)
108100
{
109-
sm ??= _compilation.GetSemanticModel(classDec.SyntaxTree);
110-
111101
IMethodSymbol attrCtorSymbol = sm.GetSymbolInfo(ma, _cancellationToken).Symbol as IMethodSymbol;
112102
if (attrCtorSymbol == null || !loggerMessageAttribute.Equals(attrCtorSymbol.ContainingType, SymbolEqualityComparer.Default))
113103
{
114104
// badly formed attribute definition, or not the right attribute
115105
continue;
116106
}
117107

118-
(int eventId, int? level, string? message, string? eventName) = ExtractAttributeValues(ma.ArgumentList!, sm);
108+
(int eventId, int? level, string message, string? eventName) = ExtractAttributeValues(ma.ArgumentList!, sm);
119109

120110
IMethodSymbol? methodSymbol = sm.GetDeclaredSymbol(method, _cancellationToken);
121111
if (methodSymbol != null)
122112
{
123113
var lm = new LoggerMethod
124114
{
125-
Name = method.Identifier.ToString(),
115+
Name = methodSymbol.Name,
126116
Level = level,
127-
Message = message ?? string.Empty,
117+
Message = message,
128118
EventId = eventId,
129119
EventName = eventName,
130120
IsExtensionMethod = methodSymbol.IsExtensionMethod,
@@ -142,7 +132,7 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
142132
keepMethod = false;
143133
}
144134

145-
if (sm.GetTypeInfo(method.ReturnType!).Type!.SpecialType != SpecialType.System_Void)
135+
if (!methodSymbol.ReturnsVoid)
146136
{
147137
// logging methods must return void
148138
Diag(DiagnosticDescriptors.LoggingMethodMustReturnVoid, method.ReturnType.GetLocation());
@@ -208,38 +198,36 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
208198
bool foundLogger = false;
209199
bool foundException = false;
210200
bool foundLogLevel = level != null;
211-
foreach (ParameterSyntax p in method.ParameterList.Parameters)
201+
foreach (IParameterSymbol paramSymbol in methodSymbol.Parameters)
212202
{
213-
string paramName = p.Identifier.ToString();
203+
string paramName = paramSymbol.Name;
214204
if (string.IsNullOrWhiteSpace(paramName))
215205
{
216206
// semantic problem, just bail quietly
217207
keepMethod = false;
218208
break;
219209
}
220210

221-
IParameterSymbol? declSymbol = sm.GetDeclaredSymbol(p);
222-
ITypeSymbol paramSymbol = declSymbol!.Type;
223-
if (paramSymbol is IErrorTypeSymbol)
211+
ITypeSymbol paramTypeSymbol = paramSymbol!.Type;
212+
if (paramTypeSymbol is IErrorTypeSymbol)
224213
{
225214
// semantic problem, just bail quietly
226215
keepMethod = false;
227216
break;
228217
}
229218

230-
IParameterSymbol declaredType = sm.GetDeclaredSymbol(p);
231-
string typeName = declaredType!.Type.ToDisplayString(
219+
string typeName = paramTypeSymbol.ToDisplayString(
232220
SymbolDisplayFormat.FullyQualifiedFormat.WithMiscellaneousOptions(
233221
SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier));
234222

235223
var lp = new LoggerParameter
236224
{
237225
Name = paramName,
238226
Type = typeName,
239-
IsLogger = !foundLogger && IsBaseOrIdentity(paramSymbol!, loggerSymbol),
240-
IsException = !foundException && IsBaseOrIdentity(paramSymbol!, exceptionSymbol),
241-
IsLogLevel = !foundLogLevel && IsBaseOrIdentity(paramSymbol!, logLevelSymbol),
242-
IsEnumerable = IsBaseOrIdentity(paramSymbol!, enumerableSymbol) && !IsBaseOrIdentity(paramSymbol!, stringSymbol),
227+
IsLogger = !foundLogger && IsBaseOrIdentity(paramTypeSymbol!, loggerSymbol),
228+
IsException = !foundException && IsBaseOrIdentity(paramTypeSymbol!, exceptionSymbol),
229+
IsLogLevel = !foundLogLevel && IsBaseOrIdentity(paramTypeSymbol!, logLevelSymbol),
230+
IsEnumerable = IsBaseOrIdentity(paramTypeSymbol!, enumerableSymbol) && !IsBaseOrIdentity(paramTypeSymbol!, stringSymbol),
243231
};
244232

245233
foundLogger |= lp.IsLogger;
@@ -248,30 +236,30 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
248236

249237
if (lp.IsLogger && lm.TemplateMap.ContainsKey(paramName))
250238
{
251-
Diag(DiagnosticDescriptors.ShouldntMentionLoggerInMessage, p.Identifier.GetLocation(), paramName);
239+
Diag(DiagnosticDescriptors.ShouldntMentionLoggerInMessage, paramSymbol.Locations[0], paramName);
252240
}
253241
else if (lp.IsException && lm.TemplateMap.ContainsKey(paramName))
254242
{
255-
Diag(DiagnosticDescriptors.ShouldntMentionExceptionInMessage, p.Identifier.GetLocation(), paramName);
243+
Diag(DiagnosticDescriptors.ShouldntMentionExceptionInMessage, paramSymbol.Locations[0], paramName);
256244
}
257245
else if (lp.IsLogLevel && lm.TemplateMap.ContainsKey(paramName))
258246
{
259-
Diag(DiagnosticDescriptors.ShouldntMentionLogLevelInMessage, p.Identifier.GetLocation(), paramName);
247+
Diag(DiagnosticDescriptors.ShouldntMentionLogLevelInMessage, paramSymbol.Locations[0], paramName);
260248
}
261249
else if (lp.IsLogLevel && level != null && !lm.TemplateMap.ContainsKey(paramName))
262250
{
263-
Diag(DiagnosticDescriptors.ArgumentHasNoCorrespondingTemplate, p.Identifier.GetLocation(), paramName);
251+
Diag(DiagnosticDescriptors.ArgumentHasNoCorrespondingTemplate, paramSymbol.Locations[0], paramName);
264252
}
265253
else if (lp.IsTemplateParameter && !lm.TemplateMap.ContainsKey(paramName))
266254
{
267-
Diag(DiagnosticDescriptors.ArgumentHasNoCorrespondingTemplate, p.Identifier.GetLocation(), paramName);
255+
Diag(DiagnosticDescriptors.ArgumentHasNoCorrespondingTemplate, paramSymbol.Locations[0], paramName);
268256
}
269257

270258
if (paramName[0] == '_')
271259
{
272260
// can't have logging method parameter names that start with _ since that can lead to conflicting symbol names
273261
// because all generated symbols start with _
274-
Diag(DiagnosticDescriptors.InvalidLoggingMethodParameterName, p.Identifier.GetLocation());
262+
Diag(DiagnosticDescriptors.InvalidLoggingMethodParameterName, paramSymbol.Locations[0]);
275263
}
276264

277265
lm.AllParameters.Add(lp);
@@ -427,88 +415,32 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
427415
return (loggerField, false);
428416
}
429417

430-
private (int eventId, int? level, string? message, string? eventName) ExtractAttributeValues(AttributeArgumentListSyntax args, SemanticModel sm)
418+
private (int eventId, int? level, string message, string? eventName) ExtractAttributeValues(AttributeArgumentListSyntax args, SemanticModel sm)
431419
{
432-
// two constructor arg shapes:
433-
//
434-
// (eventId, level, message)
435-
// (eventId, message)
436-
437420
int eventId = 0;
438421
int? level = null;
439422
string? eventName = null;
440-
string? message = null;
441-
int numPositional = 0;
423+
string message = string.Empty;
442424
foreach (AttributeArgumentSyntax a in args.Arguments)
443425
{
444-
if (a.NameEquals != null)
445-
{
446-
switch (a.NameEquals.Name.ToString())
447-
{
448-
case "EventId":
449-
eventId = (int)sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
450-
break;
451-
case "EventName":
452-
eventName = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
453-
break;
454-
case "Level":
455-
level = (int)sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
456-
break;
457-
case "Message":
458-
message = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
459-
break;
460-
}
461-
}
462-
else if (a.NameColon != null)
426+
// argument syntax takes parameters. e.g. EventId = 0
427+
Debug.Assert(a.NameEquals != null);
428+
switch (a.NameEquals.Name.ToString())
463429
{
464-
switch (a.NameColon.Name.ToString())
465-
{
466-
case "eventId":
467-
eventId = (int)sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
468-
break;
469-
470-
case "level":
471-
level = (int)sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
472-
break;
473-
474-
case "message":
475-
message = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
476-
break;
477-
}
478-
}
479-
else
480-
{
481-
switch (numPositional)
482-
{
483-
// event id
484-
case 0:
485-
eventId = (int)sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
486-
break;
487-
488-
// log level or message
489-
case 1:
490-
object o = sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
491-
if (o is int l)
492-
{
493-
level = l;
494-
}
495-
else
496-
{
497-
message = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
498-
}
499-
500-
break;
501-
502-
// message
503-
case 2:
504-
message = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
505-
break;
506-
}
507-
508-
numPositional++;
430+
case "EventId":
431+
eventId = (int)sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
432+
break;
433+
case "EventName":
434+
eventName = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
435+
break;
436+
case "Level":
437+
level = (int)sm.GetConstantValue(a.Expression, _cancellationToken).Value!;
438+
break;
439+
case "Message":
440+
message = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
441+
break;
509442
}
510443
}
511-
512444
return (eventId, level, message, eventName);
513445
}
514446

@@ -528,7 +460,7 @@ private bool IsBaseOrIdentity(ITypeSymbol source, ITypeSymbol dest)
528460
/// <summary>
529461
/// Finds the template arguments contained in the message string.
530462
/// </summary>
531-
private static void ExtractTemplates(string? message, IDictionary<string, string> templateMap, IList<string> templateList)
463+
private static void ExtractTemplates(string? message, IDictionary<string, string> templateMap, ICollection<string> templateList)
532464
{
533465
if (string.IsNullOrEmpty(message))
534466
{
@@ -613,6 +545,21 @@ private static int FindIndexOfAny(string message, char[] chars, int startIndex,
613545
int findIndex = message.IndexOfAny(chars, startIndex, endIndex - startIndex);
614546
return findIndex == -1 ? endIndex : findIndex;
615547
}
548+
549+
private string GetStringExpression(SemanticModel sm, SyntaxNode expr)
550+
{
551+
Optional<object?> optional = sm.GetConstantValue(expr, _cancellationToken);
552+
if (optional.HasValue)
553+
{
554+
object o = optional.Value;
555+
if (o != null)
556+
{
557+
return o.ToString();
558+
}
559+
}
560+
561+
return string.Empty;
562+
}
616563
}
617564

618565
/// <summary>
@@ -642,7 +589,7 @@ internal class LoggerMethod
642589
public string? EventName;
643590
public bool IsExtensionMethod;
644591
public string Modifiers = string.Empty;
645-
public string LoggerField;
592+
public string LoggerField = string.Empty;
646593
}
647594

648595
/// <summary>

0 commit comments

Comments
 (0)