Skip to content

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

Merged
merged 12 commits into from
Sep 28, 2024
Merged
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- https://github.com/dotnet/roslyn/blob/main/docs/wiki/NuGet-packages.md
- https://learn.microsoft.com/en-us/visualstudio/productinfo/vs-servicing
-->
<PackageVersion Include="Microsoft.Bcl.HashCode" Version="1.1.1" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.3.1" />
</ItemGroup>
<ItemGroup>
Expand Down
4 changes: 3 additions & 1 deletion effectivecsharpanalyzers.sln.DotSettings
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
<s:Boolean x:Key="/Default/Environment/Filtering/ExcludeCoverageFilters/=EffectiveCSharp_002EAnalyzers_002EBenchmarks_003B_002A_003B_002A_003B_002A/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Formattable/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=stringly/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
<s:Boolean x:Key="/Default/UserDictionary/Words/=stringly/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Ecs/@EntryIndexedValue">True</s:Boolean>
</wpf:ResourceDictionary>
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
ECS1400 | Initialization | Info | MinimizeDuplicateInitializationLogicAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/4f481355aaf72846c9ff201c6ccd8551bf7874f5/docs/rules/ECS1400.md)
1 change: 1 addition & 0 deletions src/EffectiveCSharp.Analyzers/Common/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ internal static class DiagnosticIds
internal const string UseSpanInstead = "ECS1000";
internal const string PreferDeclarationInitializersToAssignmentStatement = "ECS1200";
internal const string StaticClassMemberInitialization = "ECS1300";
internal const string MinimizeDuplicateInitializationLogic = "ECS1400";
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Bcl.HashCode" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="all" />
</ItemGroup>

Expand Down
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

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

/// <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,
})
.Where(c => c.Declaration is { Initializer: null })
.ToList();

if (constructors.Count < 2)
{
return;
}

SyntaxTree? syntaxTree = constructors[0].Declaration?.SyntaxTree;
if (syntaxTree == null)
{
return;
}

#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;
}
}

// 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;
}
}
}

#pragma warning disable MA0051
private static List<InitializationStatement> GetInitializationStatements(

Check failure on line 110 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L110

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.

Check failure on line 110 in src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L110

The Cyclomatic Complexity of this method is 13 which is greater than 10 authorized.
ConstructorDeclarationSyntax? constructor,
SemanticModel semanticModel,
CancellationToken cancellationToken)
{
List<InitializationStatement> statements = [];

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

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L124

Add a 'default' clause to this 'switch' statement.
{
// 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

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L163

Refactor this code to not nest more than 3 control flow statements.
{
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

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L172

Refactor this code to not nest more than 3 control flow statements.
{
statements.Add(
new InitializationStatement(
kind: InitializationKind.VariableDeclaration,
leftSymbol: symbol,
rightSymbol: initializerType));
}
}

break;
}
}
}

return statements;
}
#pragma warning restore MA0051

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];

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

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L215

Implement 'IEquatable<InitializationStatement>'.
{
public InitializationStatement(InitializationKind kind, ISymbol leftSymbol, ISymbol rightSymbol)
: this(kind)
{
LeftSymbol = leftSymbol;
RightSymbol = rightSymbol;
}

public InitializationStatement(InitializationKind kind, IMethodSymbol methodSymbol)
: this(kind)
{
MethodSymbol = methodSymbol;
}

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.IncludeNullability;

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,
};
}

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

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/MinimizeDuplicateInitializationLogicAnalyzer.cs#L274

Add a 'default' clause to this 'switch' statement.
{
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;
}

return hashCode.ToHashCode();
}
}
}
Loading