Skip to content
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

Ec89 csharp #59

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
125 changes: 125 additions & 0 deletions src/EcoCode.Core/Analyzers/EC89.UnnecessaryAssignment.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
using Microsoft.CodeAnalysis.Text;
using System.Linq;

namespace EcoCode.Analyzers;

/// <summary>EC89 : Unnecessary assignment.</summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UnnecessaryAssignment : DiagnosticAnalyzer
{
/// <summary>The diagnostic descriptor.</summary>
public static DiagnosticDescriptor Descriptor { get; } = Rule.CreateDescriptor(
id: Rule.Ids.EC89_UnnecessaryAssignment,
title: "Unnecessary assignment",
message: "Variable is assigned unnecessarily. Return it instead",
category: Rule.Categories.Usage,
severity: DiagnosticSeverity.Warning,
description: "Detects variables assigned in both branches of an if-else statement and returned immediately after.");

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => _supportedDiagnostics;
private static readonly ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics = [Descriptor];

/// <inheritdoc/>
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSyntaxNodeAction(AnalyzeIfStatement, SyntaxKind.IfStatement);
}

private static void AnalyzeIfStatement(SyntaxNodeAnalysisContext context)
{
var ifStatement = (IfStatementSyntax)context.Node;

if (ifStatement.Parent is not BlockSyntax blockSyntax) return;
if (blockSyntax.Kind() is not SyntaxKind.ElseClause && ifStatement.Else is null) return;
if (blockSyntax.Kind() is not SyntaxKind.Block) return;

if (ContainsPolymorphism(blockSyntax, context.SemanticModel)) return;

if (ContainsSameVariableAssignment(blockSyntax, context.SemanticModel))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation()));
}
}

internal static bool ContainsPolymorphism(BlockSyntax block, SemanticModel model)
{
var assignments = block.DescendantNodes().OfType<AssignmentExpressionSyntax>()
.Where(a => a.Kind() == SyntaxKind.SimpleAssignmentExpression)
.ToList();

foreach (var assignment in assignments)
{
var leftSymbol = model.GetSymbolInfo(assignment.Left).Symbol as ILocalSymbol;
if (leftSymbol != null)
{
SymbolEqualityComparer comparer = SymbolEqualityComparer.Default;
var rightType = model.GetTypeInfo(assignment.Right).Type;
if (rightType != null)
{
var baseType = rightType.BaseType;
var typesAssigned = assignments
.Select(a => model.GetTypeInfo(a.Right).Type)
.Where(t => t != null && t.BaseType != null && SymbolEqualityComparer.Default.Equals(t.BaseType, baseType))
.Distinct(SymbolEqualityComparer.Default);

if (typesAssigned.Count() > 1)
{
return true;
}
}
}
}

return false;
}
internal static bool ContainsSameVariableAssignment(BlockSyntax block, SemanticModel model)
{
var ifStatements = block.DescendantNodes().OfType<IfStatementSyntax>();

foreach (var ifStatement in ifStatements)
{
var assignedVariables = ifStatement.Statement.DescendantNodes().OfType<AssignmentExpressionSyntax>()
.Select(a => model.GetSymbolInfo(a.Left).Symbol as ILocalSymbol)
.Where(symbol => symbol != null)
.ToList();

var elseClause = ifStatement.Else;
if (elseClause != null)
{
var elseAssignedVariables = elseClause.Statement.DescendantNodes().OfType<AssignmentExpressionSyntax>()
.Select(a => model.GetSymbolInfo(a.Left).Symbol as ILocalSymbol)
.Where(symbol => symbol != null)
.ToList();

var commonVariables = assignedVariables?.Intersect(elseAssignedVariables, SymbolEqualityComparer.Default);
if (commonVariables.Any())
{
return true;
}

// Check for nested if statements within else
if (elseClause.Statement is BlockSyntax elseBlock)
{
if (ContainsSameVariableAssignment(elseBlock, model))
{
return true;
}
}
}

// Check for nested if statements within if
if (ifStatement.Statement is BlockSyntax ifBlock)
{
if (ContainsSameVariableAssignment(ifBlock, model))
{
return true;
}
}
}

return false;
}
}
1 change: 1 addition & 0 deletions src/EcoCode.Core/Models/Rule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public static class Ids
public const string EC87_UseCollectionIndexer = "EC87";
public const string EC88_DisposeResourceAsynchronously = "EC88";
public const string EC89_DoNotPassMutableStructAsRefReadonly = "EC89";
public const string EC89_UnnecessaryAssignment = "EC89";
}

/// <summary>Creates a diagnostic descriptor.</summary>
Expand Down
156 changes: 156 additions & 0 deletions src/EcoCode.Tests/Tests/EC89.UnnecessaryAssignment.Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
namespace EcoCode.Tests;

[TestClass]
public sealed class UnnecessaryAssignmentTests
{
private static readonly AnalyzerDlg VerifyAsync = TestRunner.VerifyAsync<UnnecessaryAssignment>;

[TestMethod]
public async Task DoNotWarnWhenEmptyCode() => await VerifyAsync("").ConfigureAwait(false);

[TestMethod]
public Task DoNotWarnOnSimpleIfStatement() => VerifyAsync("""
public class Test
{
int Number()
{
int x = 1;
if (x > 1)
{
x = 2;
}

return x;
}
}
""");

[TestMethod]
public Task DoNotWarnWhenAssignInIfElseStatementVariableHavePolymorphic() => VerifyAsync("""
class A {}
class B {}
class C
{
void M()
{
var fun = (bool flag) =>
{
object x;
if (flag)
{
x = new A();
}
else
{
x = new B();
}

return x;
};
}
}
""");

[TestMethod]
public Task WarnOnIfElseStatementWithUnnecessaryAssignment() => VerifyAsync("""
class C
{
int M()
{
bool f = false;
int x = 1; // x
[|if (f)
{
x = 2;
}
else if (f)
{
x = 3;
}|]

return x;
}
}
""");

[TestMethod]
public Task WarnOnMultipleIfElseUnnecessaryAssignment() => VerifyAsync("""
class C
{
int M()
{
bool f = false;

// x
int x = 1;
[|if (f)
{
x = 2;
}
else if (f)
{
x = 3;
}
else
{
x = 5;
}|]

return x; // 1
}
}
""");

[TestMethod]
public Task WarnOnIfElseStatementWithUnnecessaryAssignmentThrowExceptionAtEnd() => VerifyAsync("""
using System;

class C
{
int M()
{
bool f = false;

int x = 1;
[|if (f)
{
x = 2;
}
else if (f)
{
x = 3;
}
else
{
throw new Exception();
}|]

return x;
}
}
""");

[TestMethod]
public Task UnnecessaryAssignment() => VerifyAsync("""
public class Test
{
int Number()
{
int x = 1;
int y = 3;
if (x > 1)
{
x = 2;
}
else
{
y = 4;
}

return x;
}
}
"""
);

}