Skip to content

Commit d07a583

Browse files
author
Carlos dos Santos
committed
Merge pull request #568 from giggio/gb/checkfornull
Dont raise diag if checked for null in UseInvokeMethodToFireEventAnalyzer
2 parents 26e9ec6 + 1aeb0c2 commit d07a583

3 files changed

Lines changed: 165 additions & 3 deletions

File tree

‎src/CSharp/CodeCracker/Design/UseInvokeMethodToFireEventAnalyzer.cs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Microsoft.CodeAnalysis.Diagnostics;
55
using System;
66
using System.Collections.Immutable;
7+
using System.Linq;
78

89
namespace CodeCracker.CSharp.Design
910
{
@@ -49,7 +50,42 @@ private static void Analyzer(SyntaxNodeAnalysisContext context)
4950
if (invokedMethodSymbol == null) return;
5051
if (!invokedMethodSymbol.ReturnsVoid && !invokedMethodSymbol.ReturnType.IsReferenceType) return;
5152

53+
if (HasCheckForNull(invocation, context.SemanticModel, symbol)) return;
5254
context.ReportDiagnostic(Diagnostic.Create(Rule, invocation.GetLocation(), identifier.Identifier.Text));
5355
}
56+
57+
private static bool HasCheckForNull(InvocationExpressionSyntax invocation, SemanticModel semanticModel, ISymbol symbol)
58+
{
59+
var method = invocation.FirstAncestorOfKind(SyntaxKind.MethodDeclaration) as MethodDeclarationSyntax;
60+
if (method != null)
61+
{
62+
var ifs = method.Body.Statements.OfKind(SyntaxKind.IfStatement);
63+
foreach (IfStatementSyntax @if in ifs)
64+
{
65+
if (!@if.Condition?.IsKind(SyntaxKind.EqualsExpression) ?? true) continue;
66+
var equals = (BinaryExpressionSyntax)@if.Condition;
67+
if (equals.Left == null || equals.Right == null) continue;
68+
if (@if.GetLocation().SourceSpan.Start > invocation.GetLocation().SourceSpan.Start) return false;
69+
ISymbol identifierSymbol;
70+
if (equals.Right.IsKind(SyntaxKind.NullLiteralExpression) && equals.Left.IsKind(SyntaxKind.IdentifierName))
71+
identifierSymbol = semanticModel.GetSymbolInfo(equals.Left).Symbol;
72+
else if (equals.Left.IsKind(SyntaxKind.NullLiteralExpression) && equals.Right.IsKind(SyntaxKind.IdentifierName))
73+
identifierSymbol = semanticModel.GetSymbolInfo(equals.Right).Symbol;
74+
else continue;
75+
if (!symbol.Equals(identifierSymbol)) continue;
76+
if (@if.Statement == null) continue;
77+
if (@if.Statement.IsKind(SyntaxKind.Block))
78+
{
79+
var ifBlock = (BlockSyntax)@if.Statement;
80+
if (ifBlock.Statements.OfKind(SyntaxKind.ThrowStatement, SyntaxKind.ReturnStatement).Any()) return true;
81+
}
82+
else
83+
{
84+
if (@if.Statement.IsAnyKind(SyntaxKind.ThrowStatement, SyntaxKind.ReturnStatement)) return true;
85+
}
86+
}
87+
}
88+
return false;
89+
}
5490
}
5591
}

‎src/CSharp/CodeCracker/Extensions/CSharpAnalyzerExtensions.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,5 +435,19 @@ public static SyntaxNode FirstAncestorOfKind(this SyntaxNode node, params Syntax
435435
}
436436
return null;
437437
}
438+
439+
public static IEnumerable<TNode> OfKind<TNode>(this IEnumerable<TNode> nodes, SyntaxKind kind) where TNode : SyntaxNode
440+
{
441+
foreach (var node in nodes)
442+
if (node.IsKind(kind))
443+
yield return node;
444+
}
445+
446+
public static IEnumerable<TNode> OfKind<TNode>(this IEnumerable<TNode> nodes, params SyntaxKind[] kinds) where TNode : SyntaxNode
447+
{
448+
foreach (var node in nodes)
449+
if (node.IsAnyKind(kinds))
450+
yield return node;
451+
}
438452
}
439453
}

‎test/CSharp/CodeCracker.Test/Design/UseInvokeMethodToFireEventTests.cs‎

Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,126 @@ public async void NotWarningIfEventIsFiredWithInvokeMethod()
100100
public class MyClass
101101
{
102102
public event System.EventHandler MyEvent;
103-
104103
public void Execute()
105104
{
106105
MyEvent?.Invoke(this, System.EventArgs.Empty);
107106
}
108107
}";
108+
await VerifyCSharpHasNoDiagnosticsAsync(test);
109+
}
110+
111+
[Fact]
112+
public async void RaiseDiagnosticEvenWhenVerifiedForNullAndNotReturnedOrThrown()
113+
{
114+
const string test = @"
115+
public class MyClass
116+
{
117+
public static void Execute(System.Action action)
118+
{
119+
if (action == null)
120+
{
121+
var a = 1;
122+
}
123+
action();
124+
}
125+
}";
126+
var expected = new DiagnosticResult
127+
{
128+
Id = DiagnosticId.UseInvokeMethodToFireEvent.ToDiagnosticId(),
129+
Message = string.Format(UseInvokeMethodToFireEventAnalyzer.MessageFormat.ToString(), "action"),
130+
Severity = DiagnosticSeverity.Warning,
131+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 10, 25) }
132+
};
133+
await VerifyCSharpDiagnosticAsync(test, expected);
134+
}
135+
136+
[Fact]
137+
public async void RaiseDiagnosticEvenWhenVerifiedForNullAndNotReturnedOrThrownWithBlocklessIf()
138+
{
139+
const string test = @"
140+
public class MyClass
141+
{
142+
public static void Execute(System.Action action)
143+
{
144+
if (action == null)
145+
System.Console.WriteLine();
146+
action();
147+
}
148+
}";
149+
var expected = new DiagnosticResult
150+
{
151+
Id = DiagnosticId.UseInvokeMethodToFireEvent.ToDiagnosticId(),
152+
Message = string.Format(UseInvokeMethodToFireEventAnalyzer.MessageFormat.ToString(), "action"),
153+
Severity = DiagnosticSeverity.Warning,
154+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 25) }
155+
};
156+
await VerifyCSharpDiagnosticAsync(test, expected);
157+
}
158+
159+
[Fact]
160+
public async void RaiseDiagnosticIfNullCheckIsAfterInvocation()
161+
{
162+
const string test = @"
163+
public class MyClass
164+
{
165+
public static void Execute(System.Action action)
166+
{
167+
action();
168+
if (action == null) throw new Exception();
169+
}
170+
}";
171+
var expected = new DiagnosticResult
172+
{
173+
Id = DiagnosticId.UseInvokeMethodToFireEvent.ToDiagnosticId(),
174+
Message = string.Format(UseInvokeMethodToFireEventAnalyzer.MessageFormat.ToString(), "action"),
175+
Severity = DiagnosticSeverity.Warning,
176+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 6, 25) }
177+
};
178+
await VerifyCSharpDiagnosticAsync(test, expected);
179+
}
109180

181+
[Fact]
182+
public async void IgnoreIfAlreadyVerifiedForNullWithThrow()
183+
{
184+
const string test = @"
185+
public class MyClass
186+
{
187+
public static void Execute(System.Action action)
188+
{
189+
if (action == null) throw new Exception();
190+
action();
191+
}
192+
}";
193+
await VerifyCSharpHasNoDiagnosticsAsync(test);
194+
}
195+
196+
[Fact]
197+
public async void IgnoreIfAlreadyVerifiedForNullInverted()
198+
{
199+
const string test = @"
200+
public class MyClass
201+
{
202+
public static void Execute(System.Action action)
203+
{
204+
if (null == action) throw new Exception();
205+
action();
206+
}
207+
}";
208+
await VerifyCSharpHasNoDiagnosticsAsync(test);
209+
}
210+
211+
[Fact]
212+
public async void IgnoreIfAlreadyVerifiedForNullWithReturn()
213+
{
214+
const string test = @"
215+
public class MyClass
216+
{
217+
public static void Execute(System.Action action)
218+
{
219+
if (action == null) return;
220+
action();
221+
}
222+
}";
110223
await VerifyCSharpHasNoDiagnosticsAsync(test);
111224
}
112225

@@ -200,15 +313,14 @@ public async void ReportOnParametersWhenReturnTypeIsAReferenceType()
200313
var test = @"
201314
public static TReturn Method<T, TReturn>(System.Func<T, TReturn> getter) where T : System.Attribute where TReturn : class
202315
{
203-
if (getter == null) return default(TReturn);
204316
return getter(default(T));
205317
}".WrapInCSharpClass();
206318
var expected = new DiagnosticResult
207319
{
208320
Id = DiagnosticId.UseInvokeMethodToFireEvent.ToDiagnosticId(),
209321
Message = string.Format(UseInvokeMethodToFireEventAnalyzer.MessageFormat.ToString(), "getter"),
210322
Severity = DiagnosticSeverity.Warning,
211-
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 12, 12) }
323+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 11, 12) }
212324
};
213325
await VerifyCSharpDiagnosticAsync(test, expected);
214326
}

0 commit comments

Comments
 (0)