Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Meziantou.Analyzer.Vsix/source.extension.vsixmanifest
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
<Metadata>
<Identity Id="Meziantou.Analyzer.dd3c9fea-c73f-4c59-a7e0-cd45a3a44d24" Version="1.0.42" Language="en-US" Publisher="Meziantou"/>
<Identity Id="Meziantou.Analyzer.dd3c9fea-c73f-4c59-a7e0-cd45a3a44d24" Version="1.0.43" Language="en-US" Publisher="Meziantou"/>
<DisplayName>Meziantou.Analyzer</DisplayName>
<Description xml:space="preserve">A Roslyn analyzer to enforce some good pratices in C#</Description>
</Metadata>
Expand Down
8 changes: 8 additions & 0 deletions src/Meziantou.Analyzer/Internals/TypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ public static bool IsString(this ITypeSymbol symbol)
return symbol.SpecialType == SpecialType.System_String;
}

public static bool IsInt32(this ITypeSymbol symbol)
{
if (symbol == null)
return false;

return symbol.SpecialType == SpecialType.System_Int32;
}

public static bool IsBoolean(this ITypeSymbol symbol)
{
if (symbol == null)
Expand Down
3 changes: 2 additions & 1 deletion src/Meziantou.Analyzer/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ internal static class RuleIdentifiers
public const string DoNotRemoveOriginalExceptionFromThrowStatement = "MA0027";
public const string OptimizeStringBuilderUsage = "MA0028";
public const string OptimizeLinqUsage = "MA0029";
public const string DuplicateOrderBy = "MA0030";
public const string DuplicateEnumerable_OrderBy = "MA0030";
public const string OptimizeEnumerable_Count = "MA0031";

public static string GetHelpUri(string idenfifier)
{
Expand Down
238 changes: 224 additions & 14 deletions src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using static System.FormattableString;

namespace Meziantou.Analyzer.Rules
{
Expand Down Expand Up @@ -32,16 +32,26 @@ public class OptimizeLinqUsageAnalyzer : DiagnosticAnalyzer
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.OptimizeLinqUsage));

private static readonly DiagnosticDescriptor s_duplicateOrderByMethodsRule = new DiagnosticDescriptor(
RuleIdentifiers.DuplicateOrderBy,
RuleIdentifiers.DuplicateEnumerable_OrderBy,
title: "Optimize LINQ usage",
messageFormat: "Remove the first '{0}' method or use '{1}'",
RuleCategories.Usage,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: "",
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.DuplicateOrderBy));
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.DuplicateEnumerable_OrderBy));

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(s_listMethodsRule, s_combineLinqMethodsRule, s_duplicateOrderByMethodsRule);
private static readonly DiagnosticDescriptor s_optimizeCountRule = new DiagnosticDescriptor(
RuleIdentifiers.OptimizeEnumerable_Count,
title: "Optimize Enumerable.Count usage",
messageFormat: "{0}",
RuleCategories.Usage,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: "",
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.OptimizeEnumerable_Count));

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(s_listMethodsRule, s_combineLinqMethodsRule, s_duplicateOrderByMethodsRule, s_optimizeCountRule);

public override void Initialize(AnalysisContext context)
{
Expand Down Expand Up @@ -70,15 +80,7 @@ private void Analyze(OperationAnalysisContext context)
UseIndexerInsteadOfElementAt(context, operation);
CombineWhereWithNextMethod(context, operation, enumerableSymbol);
RemoveTwoConsecutiveOrderBy(context, operation, enumerableSymbol);

// TODO Count() < 0 => false
// TODO Count() <= 0 => !Any()
// TODO Count() == 0 => !Any()
// TODO Count() > 0 => Any()
// TODO Count() < 10 => .Any()
// TODO Count() <= 10 => .Any()
// TODO Count() >= 10 => .Skip(9).Any()
// TODO Count() > 10 => .Skip(10).Any()
OptimizeCountUsage(context, operation);
}

private void UseCountPropertyInsteadOfMethod(OperationAnalysisContext context, IInvocationOperation operation)
Expand Down Expand Up @@ -197,7 +199,9 @@ private void CombineWhereWithNextMethod(OperationAnalysisContext context, IInvoc
private void RemoveTwoConsecutiveOrderBy(OperationAnalysisContext context, IInvocationOperation operation, ITypeSymbol enumerableSymbol)
{
if (string.Equals(operation.TargetMethod.Name, nameof(Enumerable.OrderBy), StringComparison.Ordinal) ||
string.Equals(operation.TargetMethod.Name, nameof(Enumerable.OrderByDescending), StringComparison.Ordinal))
string.Equals(operation.TargetMethod.Name, nameof(Enumerable.OrderByDescending), StringComparison.Ordinal) ||
string.Equals(operation.TargetMethod.Name, nameof(Enumerable.ThenBy), StringComparison.Ordinal) ||
string.Equals(operation.TargetMethod.Name, nameof(Enumerable.ThenByDescending), StringComparison.Ordinal))
{
var parent = GetParentLinqOperation(operation);
if (parent != null && parent.TargetMethod.ContainingType.IsEqualsTo(enumerableSymbol))
Expand All @@ -211,6 +215,193 @@ private void RemoveTwoConsecutiveOrderBy(OperationAnalysisContext context, IInvo
}
}

private void OptimizeCountUsage(OperationAnalysisContext context, IInvocationOperation operation)
{
if (!string.Equals(operation.TargetMethod.Name, nameof(Enumerable.Count), StringComparison.Ordinal))
return;

var binaryOperation = GetParentBinaryOperation(operation, out var countOperand);
if (binaryOperation == null)
return;

if (!IsSupportedOperator(binaryOperation.OperatorKind))
return;

if (!binaryOperation.LeftOperand.Type.IsInt32() || !binaryOperation.RightOperand.Type.IsInt32())
return;

var opKind = NormalizeOperator();
var otherOperand = binaryOperation.LeftOperand == countOperand ? binaryOperation.RightOperand : binaryOperation.LeftOperand;
if (otherOperand == null)
return;

if (otherOperand.ConstantValue.HasValue && otherOperand.ConstantValue.Value is int value)
{
string message = null;
switch (opKind)
{
case BinaryOperatorKind.Equals:
if (value < 0)
{
// expr.Count() == -1
message = "Expression is always false";
}
else if (value == 0)
{
// expr.Count() == 0
message = "Replace 'Count() == 0' with 'Any() == false'";
}
else if (value == 1)
{
// expr.Count() == 1
message = "Replace 'Count() == 1' with 'Any()'";
}
else
{
// expr.Count() == 10 => expr.Skip(9).Any()
message = Invariant($"Replace 'Count() == {value}' with 'Skip({value - 1}).Any()'");
}

break;

case BinaryOperatorKind.NotEquals:
if (value < 0)
{
// expr.Count() != -1 is always true
message = "Expression is always true";
}
else if (value == 0)
{
// expr.Count() != 0
message = "Replace 'Count() != 0' with 'Any()'";
}

break;

case BinaryOperatorKind.LessThan:
if (value <= 0)
{
// expr.Count() < 0
message = "Expression is always false";
}
else if (value == 1)
{
// expr.Count() < 1 ==> expr.Count() == 0
message = "Replace 'Count() < 1' with 'Any() == false'";
}
else
{
// expr.Count() < 10
message = Invariant($"Replace 'Count() < {value}' with 'Skip({value - 1}).Any() == false'");
}

break;

case BinaryOperatorKind.LessThanOrEqual:
if (value < 0)
{
// expr.Count() <= -1
message = "Expression is always false";
}
else if (value == 0)
{
// expr.Count() <= 0
message = "Replace 'Count() <= 0' with 'Any() == false'";
}
else
{
// expr.Count() < 10
message = Invariant($"Replace 'Count() <= {value}' with 'Skip({value}).Any() == false'");
}

break;

case BinaryOperatorKind.GreaterThan:
if (value < 0)
{
// expr.Count() > -1
message = "Expression is always true";
}
else if (value == 0)
{
// expr.Count() > 0
message = "Replace 'Count() > 0' with 'Any()'";
}
else
{
// expr.Count() > 1
message = Invariant($"Replace 'Count() > {value}' with 'Skip({value}).Any()'");
}

break;

case BinaryOperatorKind.GreaterThanOrEqual:
if (value <= 0)
{
// expr.Count() >= 0
message = "Expression is always true";
}
else if (value == 1)
{
// expr.Count() >= 1
message = "Replace 'Count() >= 1' with 'Any()'";
}
else
{
// expr.Count() >= 2
message = Invariant($"Replace 'Count() >= {value}' with 'Skip({value - 1}).Any()'");
}

break;

}

if (message != null)
{
context.ReportDiagnostic(Diagnostic.Create(s_optimizeCountRule, binaryOperation.Syntax.GetLocation(), message));
}

// TODO detect non constant values
}

bool IsSupportedOperator(BinaryOperatorKind operatorKind)
{
switch (operatorKind)
{
case BinaryOperatorKind.Equals:
case BinaryOperatorKind.NotEquals:
case BinaryOperatorKind.LessThan:
case BinaryOperatorKind.LessThanOrEqual:
case BinaryOperatorKind.GreaterThanOrEqual:
case BinaryOperatorKind.GreaterThan:
return true;
default:
return false;
}
}

BinaryOperatorKind NormalizeOperator()
{
bool isCountLeftOperand = binaryOperation.LeftOperand == countOperand;
switch (binaryOperation.OperatorKind)
{
case BinaryOperatorKind.LessThan:
return isCountLeftOperand ? BinaryOperatorKind.LessThan : BinaryOperatorKind.GreaterThan;
case BinaryOperatorKind.LessThanOrEqual:
return isCountLeftOperand ? BinaryOperatorKind.LessThanOrEqual : BinaryOperatorKind.GreaterThanOrEqual;

case BinaryOperatorKind.GreaterThanOrEqual:
return isCountLeftOperand ? BinaryOperatorKind.GreaterThanOrEqual : BinaryOperatorKind.LessThanOrEqual;

case BinaryOperatorKind.GreaterThan:
return isCountLeftOperand ? BinaryOperatorKind.GreaterThan : BinaryOperatorKind.LessThan;

default:
return binaryOperation.OperatorKind;
}
}
}

private static ITypeSymbol GetActualType(IArgumentOperation argument)
{
var value = argument.Value;
Expand Down Expand Up @@ -240,5 +431,24 @@ private static IInvocationOperation GetParentLinqOperation(IOperation op)

return null;
}

private static IBinaryOperation GetParentBinaryOperation(IOperation op, out IOperation operand)
{
var parent = op.Parent;
if (parent is IConversionOperation)
{
op = parent;
parent = parent.Parent;
}

if (parent is IBinaryOperation binaryOperation)
{
operand = op;
return binaryOperation;
}

operand = null;
return null;
}
}
}
Loading