Skip to content

Commit 9788b03

Browse files
RalfKobanRalfKoban
committed
Added new rule MiKo_5018 to call value comparisons before reference comparisons
(resolves #1090)
1 parent df5aad6 commit 9788b03

8 files changed

+496
-1
lines changed

‎MiKo.Analyzer.Shared/MiKo.Analyzer.Shared.CodeFixes.projitems

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@
321321
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5014_CodeFixProvider.cs" />
322322
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5015_CodeFixProvider.cs" />
323323
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5017_CodeFixProvider.cs" />
324+
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5018_CodeFixProvider.cs" />
324325
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\PerformanceCodeFixProvider.cs" />
325326
<Compile Include="$(MSBuildThisFileDirectory)Rules\Spacing\MiKo_6001_CodeFixProvider.cs" />
326327
<Compile Include="$(MSBuildThisFileDirectory)Rules\Spacing\MiKo_6002_CodeFixProvider.cs" />

‎MiKo.Analyzer.Shared/MiKo.Analyzer.Shared.projitems

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@
483483
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5015_StringLiteralGetsInternedAnalyzer.cs" />
484484
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5016_RemoveAllUsesContainsOfHashSetAnalyzer.cs" />
485485
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5017_StringLiteralVariableAssignmentIsConstantAnalyzer.cs" />
486+
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\MiKo_5018_LogicalValueComparisonsComeFirstAnalyzer.cs" />
486487
<Compile Include="$(MSBuildThisFileDirectory)Rules\Performance\PerformanceAnalyzer.cs" />
487488
<Compile Include="$(MSBuildThisFileDirectory)Rules\Spacing\CallSurroundedByBlankLinesAnalyzer.cs" />
488489
<Compile Include="$(MSBuildThisFileDirectory)Rules\Spacing\MiKo_6001_LogStatementSurroundedByBlankLinesAnalyzer.cs" />

‎MiKo.Analyzer.Shared/Resources.Designer.cs

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎MiKo.Analyzer.Shared/Resources.resx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4964,6 +4964,18 @@ When used e.g. on a list, that 'Contains' call has to loop over that list again
49644964
<data name="MiKo_5017_Title" xml:space="preserve">
49654965
<value>Fields or variables assigned with string literals should be constant</value>
49664966
</data>
4967+
<data name="MiKo_5018_CodeFixTitle" xml:space="preserve">
4968+
<value>Move value comparison before reference comparison</value>
4969+
</data>
4970+
<data name="MiKo_5018_Description" xml:space="preserve">
4971+
<value>Comparisons on simple values are faster than comparisons on complex references such as collections. Hence, the value comparisons should be placed before the reference comparisons.</value>
4972+
</data>
4973+
<data name="MiKo_5018_MessageFormat" xml:space="preserve">
4974+
<value>Place value comparison before reference comparison</value>
4975+
</data>
4976+
<data name="MiKo_5018_Title" xml:space="preserve">
4977+
<value>Value comparisons should be performed before reference comparisons</value>
4978+
</data>
49674979
<data name="MiKo_6001_CodeFixTitle" xml:space="preserve">
49684980
<value>Surround with blank lines</value>
49694981
</data>

‎MiKo.Analyzer.Shared/Rules/Performance/MiKo_5018_CodeFixProvider.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
using System.Collections.Generic;
2+
using System.Composition;
3+
using System.Linq;
4+
5+
using Microsoft.CodeAnalysis;
6+
using Microsoft.CodeAnalysis.CodeFixes;
7+
using Microsoft.CodeAnalysis.CSharp;
8+
using Microsoft.CodeAnalysis.CSharp.Syntax;
9+
10+
namespace MiKoSolutions.Analyzers.Rules.Performance
11+
{
12+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MiKo_5018_CodeFixProvider)), Shared]
13+
public sealed class MiKo_5018_CodeFixProvider : PerformanceCodeFixProvider
14+
{
15+
private static readonly SyntaxKind[] LogicalConditions = { SyntaxKind.LogicalAndExpression, SyntaxKind.LogicalOrExpression };
16+
17+
public override string FixableDiagnosticId => "MiKo_5018";
18+
19+
protected override SyntaxNode GetSyntax(IEnumerable<SyntaxNode> syntaxNodes) => syntaxNodes.OfType<BinaryExpressionSyntax>().FirstOrDefault();
20+
21+
protected override SyntaxNode GetUpdatedSyntax(Document document, SyntaxNode syntax, Diagnostic issue)
22+
{
23+
if (syntax is BinaryExpressionSyntax binary)
24+
{
25+
var left = FindProblematicNode(binary.Left);
26+
var right = binary.Right;
27+
28+
return binary.ReplaceNodes(new[] { left, right }, (original, rewritten) => ReferenceEquals(original, left) ? right.WithTrailingSpace() : left.WithoutTrivia());
29+
}
30+
31+
return syntax;
32+
}
33+
34+
private static ExpressionSyntax FindProblematicNode(ExpressionSyntax expression)
35+
{
36+
while (true)
37+
{
38+
if (expression is BinaryExpressionSyntax binary && binary.IsAnyKind(LogicalConditions))
39+
{
40+
expression = binary.Right;
41+
}
42+
else
43+
{
44+
return expression;
45+
}
46+
}
47+
}
48+
}
49+
}

‎MiKo.Analyzer.Shared/Rules/Performance/MiKo_5018_LogicalValueComparisonsComeFirstAnalyzer.cs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
using Microsoft.CodeAnalysis;
2+
using Microsoft.CodeAnalysis.CSharp;
3+
using Microsoft.CodeAnalysis.CSharp.Syntax;
4+
using Microsoft.CodeAnalysis.Diagnostics;
5+
6+
namespace MiKoSolutions.Analyzers.Rules.Performance
7+
{
8+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
9+
public sealed class MiKo_5018_LogicalValueComparisonsComeFirstAnalyzer : PerformanceAnalyzer
10+
{
11+
public const string Id = "MiKo_5018";
12+
13+
private static readonly SyntaxKind[] LogicalConditions = { SyntaxKind.LogicalAndExpression, SyntaxKind.LogicalOrExpression };
14+
15+
public MiKo_5018_LogicalValueComparisonsComeFirstAnalyzer() : base(Id)
16+
{
17+
}
18+
19+
protected override void InitializeCore(CompilationStartAnalysisContext context) => context.RegisterSyntaxNodeAction(AnalyzeLogicalCondition, LogicalConditions);
20+
21+
private static bool IsNullCheck(ExpressionSyntax expression)
22+
{
23+
switch (expression.WithoutParenthesis())
24+
{
25+
case BinaryExpressionSyntax binary when binary.Left.IsKind(SyntaxKind.NullLiteralExpression) || binary.Right.IsKind(SyntaxKind.NullLiteralExpression):
26+
return true;
27+
28+
case IsPatternExpressionSyntax pattern when pattern.Pattern is UnaryPatternSyntax unary && unary.Pattern is ConstantPatternSyntax constant && constant.Expression.IsKind(SyntaxKind.NullLiteralExpression):
29+
return true;
30+
}
31+
32+
return false;
33+
}
34+
35+
private static bool HasIssue(BinaryExpressionSyntax binary, SyntaxNodeAnalysisContext context)
36+
{
37+
if (binary.Right.WithoutParenthesis() is BinaryExpressionSyntax right && right.IsKind(SyntaxKind.EqualsExpression))
38+
{
39+
var semanticModel = context.SemanticModel;
40+
41+
if (right.Left.GetTypeSymbol(semanticModel)?.IsValueType is true && right.Right.GetTypeSymbol(semanticModel)?.IsValueType is true)
42+
{
43+
if (binary.Left.WithoutParenthesis() is ExpressionSyntax left)
44+
{
45+
if (IsNullCheck(left))
46+
{
47+
// do not report null checks
48+
return false;
49+
}
50+
51+
if (left is BinaryExpressionSyntax nested && nested.IsAnyKind(LogicalConditions) is false)
52+
{
53+
if (nested.Left.GetTypeSymbol(semanticModel)?.IsValueType is true && nested.Right.GetTypeSymbol(semanticModel)?.IsValueType is true)
54+
{
55+
// do not report checks on value types
56+
return false;
57+
}
58+
}
59+
}
60+
61+
return true;
62+
}
63+
}
64+
65+
return false;
66+
}
67+
68+
private void AnalyzeLogicalCondition(SyntaxNodeAnalysisContext context)
69+
{
70+
if (context.Node is BinaryExpressionSyntax expression)
71+
{
72+
if (HasIssue(expression, context))
73+
{
74+
context.ReportDiagnostic(Issue(expression));
75+
}
76+
}
77+
}
78+
}
79+
}

0 commit comments

Comments
 (0)