-
Notifications
You must be signed in to change notification settings - Fork 1
Add Item 14: Minimize initialization logic #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
4f48135
943b55f
9134bea
ab77427
20c67f9
7ac005e
9b18a09
1bfaae6
0e2f467
319e0ec
8d4a66f
e7ed206
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,289 @@ | ||
namespace EffectiveCSharp.Analyzers; | ||
Check failure on line 1 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs
|
||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// <summary> | ||
/// A <see cref="DiagnosticAnalyzer"/> for Effective C# Item #14 - Minimize duplicate initialization logic. | ||
/// </summary> | ||
/// <seealso cref="Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer" /> | ||
[DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
public class MinimizeDuplicateInitializationLogicAnalyzer : DiagnosticAnalyzer | ||
{ | ||
private static readonly LocalizableString Title = "Minimize duplicate initialization logic"; | ||
|
||
private static readonly LocalizableString MessageFormat = | ||
"Constructor '{0}' contains duplicate initialization logic"; | ||
|
||
private static readonly DiagnosticDescriptor Rule = new( | ||
DiagnosticIds.MinimizeDuplicateInitializationLogic, | ||
Title, | ||
MessageFormat, | ||
Categories.Initialization, | ||
DiagnosticSeverity.Info, | ||
isEnabledByDefault: true, | ||
helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.MinimizeDuplicateInitializationLogic}.md"); | ||
|
||
/// <inheritdoc/> | ||
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule); | ||
|
||
/// <inheritdoc/> | ||
public override void Initialize(AnalysisContext context) | ||
{ | ||
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); | ||
context.EnableConcurrentExecution(); | ||
context.RegisterSymbolAction(AnalyzeNamedType, SymbolKind.NamedType); | ||
} | ||
|
||
private static void AnalyzeNamedType(SymbolAnalysisContext context) | ||
{ | ||
INamedTypeSymbol namedTypeSymbol = (INamedTypeSymbol)context.Symbol; | ||
|
||
// Collect constructors that are instance constructors and do not use chaining | ||
var constructors = namedTypeSymbol.Constructors | ||
.Where(c => !c.IsStatic && c.DeclaringSyntaxReferences.Length > 0) | ||
.Select(c => new | ||
{ | ||
ConstructorSymbol = c, | ||
Declaration = c.DeclaringSyntaxReferences[0].GetSyntax(context.CancellationToken) as ConstructorDeclarationSyntax, | ||
}) | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.Where(c => c.Declaration is { Initializer: null }) | ||
.ToList(); | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (constructors.Count < 2) | ||
{ | ||
return; | ||
} | ||
|
||
SyntaxTree? syntaxTree = constructors[0].Declaration?.SyntaxTree; | ||
if (syntaxTree == null) | ||
{ | ||
return; | ||
} | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
#pragma warning disable RS1030 | ||
SemanticModel semanticModel = context.Compilation.GetSemanticModel(syntaxTree); | ||
#pragma warning restore RS1030 | ||
|
||
// Compute initialization statements for all constructors once | ||
Dictionary<IMethodSymbol, List<InitializationStatement>> constructorInitStatements = new(SymbolEqualityComparer.IncludeNullability); | ||
|
||
for (int i = 0; i < constructors.Count; i++) | ||
{ | ||
var ctor = constructors[i]; | ||
List<InitializationStatement> initStatements = GetInitializationStatements( | ||
ctor.Declaration, | ||
semanticModel, | ||
context.CancellationToken); | ||
|
||
if (initStatements.Count > 0) | ||
{ | ||
constructorInitStatements[ctor.ConstructorSymbol] = initStatements; | ||
} | ||
} | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Compare initialization statements between constructors | ||
CompareConstructors(context, constructorInitStatements); | ||
} | ||
|
||
private static void CompareConstructors(SymbolAnalysisContext context, Dictionary<IMethodSymbol, List<InitializationStatement>> constructorInitStatements) | ||
{ | ||
foreach (KeyValuePair<IMethodSymbol, List<InitializationStatement>> ctor in constructorInitStatements) | ||
{ | ||
foreach (KeyValuePair<IMethodSymbol, List<InitializationStatement>> otherCtor in constructorInitStatements) | ||
{ | ||
if (ctor.Key.Equals(otherCtor.Key, SymbolEqualityComparer.Default)) | ||
{ | ||
continue; | ||
} | ||
|
||
if (!InitializationStatementsAreEqual(ctor.Value, otherCtor.Value)) | ||
{ | ||
continue; | ||
} | ||
|
||
Diagnostic diagnostic = ctor.Key.Locations[0].CreateDiagnostic(Rule, ctor.Key.Name); | ||
context.ReportDiagnostic(diagnostic); | ||
break; | ||
} | ||
} | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
#pragma warning disable MA0051 | ||
private static List<InitializationStatement> GetInitializationStatements( | ||
Check failure on line 110 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs
|
||
ConstructorDeclarationSyntax? constructor, | ||
SemanticModel semanticModel, | ||
CancellationToken cancellationToken) | ||
{ | ||
List<InitializationStatement> statements = []; | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (constructor?.Body == null) | ||
{ | ||
return statements; | ||
} | ||
|
||
for (int i = 0; i < constructor.Body.Statements.Count; i++) | ||
{ | ||
switch (constructor.Body.Statements[i]) | ||
Check failure on line 124 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs
|
||
{ | ||
// Handle assignments and method calls | ||
case ExpressionStatementSyntax { Expression: AssignmentExpressionSyntax assignment }: | ||
{ | ||
ISymbol? leftSymbol = semanticModel.GetSymbolInfo(assignment.Left, cancellationToken).Symbol; | ||
ISymbol? rightSymbol = semanticModel.GetSymbolInfo(assignment.Right, cancellationToken).Symbol ?? | ||
semanticModel.GetTypeInfo(assignment.Right, cancellationToken).Type; | ||
|
||
if (leftSymbol != null && rightSymbol != null) | ||
{ | ||
statements.Add( | ||
new InitializationStatement( | ||
kind: InitializationKind.Assignment, | ||
leftSymbol: leftSymbol, | ||
rightSymbol: rightSymbol)); | ||
} | ||
|
||
break; | ||
} | ||
|
||
case ExpressionStatementSyntax exprStmt: | ||
{ | ||
if (exprStmt.Expression is InvocationExpressionSyntax invocation && | ||
semanticModel.GetSymbolInfo(invocation, cancellationToken).Symbol is IMethodSymbol methodSymbol) | ||
{ | ||
statements.Add( | ||
new InitializationStatement( | ||
kind: InitializationKind.MethodCall, | ||
methodSymbol: methodSymbol)); | ||
} | ||
|
||
break; | ||
} | ||
|
||
case LocalDeclarationStatementSyntax localDecl: | ||
{ | ||
foreach (VariableDeclaratorSyntax variable in localDecl.Declaration.Variables) | ||
{ | ||
if (variable.Initializer == null) | ||
Check failure on line 163 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs
|
||
{ | ||
continue; | ||
} | ||
|
||
ISymbol? symbol = semanticModel.GetDeclaredSymbol(variable, cancellationToken); | ||
ITypeSymbol? initializerType = semanticModel.GetTypeInfo( | ||
variable.Initializer.Value, | ||
cancellationToken).Type; | ||
if (symbol != null && initializerType != null) | ||
Check failure on line 172 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs
|
||
{ | ||
statements.Add( | ||
new InitializationStatement( | ||
kind: InitializationKind.VariableDeclaration, | ||
leftSymbol: symbol, | ||
rightSymbol: initializerType)); | ||
} | ||
} | ||
|
||
break; | ||
} | ||
} | ||
} | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return statements; | ||
} | ||
#pragma warning restore MA0051 | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
private static bool InitializationStatementsAreEqual( | ||
List<InitializationStatement> first, | ||
List<InitializationStatement> second) | ||
{ | ||
if (first.Count != second.Count || first.Count == 0) | ||
{ | ||
return false; | ||
} | ||
|
||
HashSet<InitializationStatement> firstSet = [.. first]; | ||
HashSet<InitializationStatement> secondSet = [.. second]; | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return firstSet.SetEquals(secondSet) && firstSet.Count > 0; | ||
} | ||
|
||
#pragma warning disable SA1201 | ||
private enum InitializationKind | ||
#pragma warning restore SA1201 | ||
{ | ||
Assignment, | ||
MethodCall, | ||
VariableDeclaration, | ||
} | ||
|
||
private sealed class InitializationStatement : IEquatable<InitializationStatement> | ||
Check notice on line 215 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs
|
||
{ | ||
public InitializationStatement(InitializationKind kind, ISymbol leftSymbol, ISymbol rightSymbol) | ||
: this(kind) | ||
{ | ||
LeftSymbol = leftSymbol; | ||
RightSymbol = rightSymbol; | ||
} | ||
|
||
public InitializationStatement(InitializationKind kind, IMethodSymbol methodSymbol) | ||
: this(kind) | ||
{ | ||
MethodSymbol = methodSymbol; | ||
} | ||
|
||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private InitializationStatement(InitializationKind kind) | ||
{ | ||
Kind = kind; | ||
} | ||
|
||
public InitializationKind Kind { get; } | ||
|
||
public ISymbol? LeftSymbol { get; } | ||
|
||
public ISymbol? RightSymbol { get; } | ||
|
||
public IMethodSymbol? MethodSymbol { get; } | ||
|
||
public bool Equals(InitializationStatement? other) | ||
{ | ||
if (other == null || Kind != other.Kind) | ||
{ | ||
return false; | ||
} | ||
|
||
SymbolEqualityComparer comparer = SymbolEqualityComparer.Default; | ||
|
||
return Kind switch | ||
{ | ||
InitializationKind.Assignment => comparer.Equals(LeftSymbol, other.LeftSymbol) && | ||
comparer.Equals(RightSymbol, other.RightSymbol), | ||
InitializationKind.MethodCall => comparer.Equals(MethodSymbol, other.MethodSymbol), | ||
InitializationKind.VariableDeclaration => comparer.Equals(LeftSymbol, other.LeftSymbol) && | ||
comparer.Equals(RightSymbol, other.RightSymbol), | ||
_ => false, | ||
}; | ||
} | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
public override bool Equals(object? obj) | ||
{ | ||
return Equals(obj as InitializationStatement); | ||
} | ||
|
||
public override int GetHashCode() | ||
{ | ||
HashCode hashCode = default; | ||
hashCode.Add(Kind); | ||
SymbolEqualityComparer comparer = SymbolEqualityComparer.IncludeNullability; | ||
|
||
switch (Kind) | ||
Check failure on line 274 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs
|
||
{ | ||
case InitializationKind.Assignment: | ||
case InitializationKind.VariableDeclaration: | ||
hashCode.Add(comparer.GetHashCode(LeftSymbol)); | ||
hashCode.Add(comparer.GetHashCode(RightSymbol)); | ||
break; | ||
case InitializationKind.MethodCall: | ||
hashCode.Add(comparer.GetHashCode(MethodSymbol)); | ||
break; | ||
} | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return hashCode.ToHashCode(); | ||
} | ||
rjmurillo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.