Skip to content

Commit bbffdc3

Browse files
SergeyTeplyakovCopilot
SergeyTeplyakov
and
Copilot
authored
Add analyzer to warn on returning null for Task-like types (#302)
* Add analyzer to warn on returning null for Task-like types Introduces DoNotReturnNullForTaskLikeAnalyzer (EPC31) to warn when methods returning Task, Task<T>, or similar types return null or default, which can cause runtime exceptions. Includes analyzer implementation, tests, diagnostic descriptor, and release notes update. * Update src/ErrorProne.NET.CoreAnalyzers/DiagnosticDescriptors.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent d4d68e3 commit bbffdc3

5 files changed

Lines changed: 374 additions & 0 deletions

File tree

‎src/ErrorProne.NET.CoreAnalyzers.Tests/AsyncAnalyzers/DoNotReturnNullForTaskLikeAnalyzerTests.cs‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
using System.Threading.Tasks;
2+
using NUnit.Framework;
3+
using VerifyCS = ErrorProne.NET.TestHelpers.CSharpCodeFixVerifier<
4+
ErrorProne.NET.AsyncAnalyzers.DoNotReturnNullForTaskLikeAnalyzer,
5+
Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;
6+
7+
namespace ErrorProne.NET.CoreAnalyzers.Tests.AsyncAnalyzers
8+
{
9+
[TestFixture]
10+
public class DoNotReturnNullForTaskLikeAnalyzerTests
11+
{
12+
[Test]
13+
public async Task Warn_When_Null_Is_Returned_For_Task()
14+
{
15+
string code = @"
16+
using System.Threading.Tasks;
17+
public class MyClass
18+
{
19+
public Task Foo()
20+
{
21+
[|return null;|]
22+
}
23+
}";
24+
await VerifyCS.VerifyAsync(code);
25+
}
26+
27+
[Test]
28+
public async Task Warn_When_Null_Is_Returned_For_Task_Of_T()
29+
{
30+
string code = @"
31+
using System.Threading.Tasks;
32+
public class MyClass
33+
{
34+
public Task<int> Foo()
35+
{
36+
[|return null;|]
37+
}
38+
}";
39+
await VerifyCS.VerifyAsync(code);
40+
}
41+
42+
[Test]
43+
public async Task Warn_When_Null_Is_Part_Of_Switch()
44+
{
45+
string code = @"
46+
using System.Threading.Tasks;
47+
public class MyClass
48+
{
49+
Task FooBar(int x) => [|x switch { 1 => null, _ => Task.CompletedTask }|];
50+
}";
51+
await VerifyCS.VerifyAsync(code);
52+
}
53+
54+
[Test]
55+
public async Task Warn_When_Null_Is_Returned_For_Task_Of_T_Expression_Body()
56+
{
57+
string code = @"
58+
using System.Threading.Tasks;
59+
public class MyClass
60+
{
61+
Task FooBar(int x) => [|x switch { 1 => null, _ => Task.CompletedTask }|];
62+
}";
63+
await VerifyCS.VerifyAsync(code);
64+
}
65+
66+
[Test]
67+
public async Task Warn_When_Null_Is_Returned_For_Task_In_Local_Function()
68+
{
69+
string code = @"
70+
using System.Threading.Tasks;
71+
public class MyClass
72+
{
73+
public void Foo()
74+
{
75+
Task Bar(bool f)
76+
{
77+
if (f)
78+
[|return null;|]
79+
80+
return Task.CompletedTask;
81+
}
82+
}
83+
}";
84+
await VerifyCS.VerifyAsync(code);
85+
}
86+
87+
[Test]
88+
public async Task Warn_When_Null_Is_Returned_For_Task_In_Lambda()
89+
{
90+
string code = @"
91+
using System;
92+
using System.Threading.Tasks;
93+
public class MyClass
94+
{
95+
public void Foo()
96+
{
97+
Func<Task> f = () => { [|return null;|] };
98+
}
99+
}";
100+
await VerifyCS.VerifyAsync(code);
101+
}
102+
103+
[Test]
104+
public async Task No_Warn_For_Return_In_Async_Task()
105+
{
106+
string code = @"
107+
using System.Threading.Tasks;
108+
public class MyClass
109+
{
110+
public async Task Foo()
111+
{
112+
await Task.Yield();
113+
return;
114+
}
115+
}";
116+
await VerifyCS.VerifyAsync(code);
117+
}
118+
119+
[Test]
120+
public async Task No_Warn_When_Nullability_Is_On()
121+
{
122+
string code = @"
123+
using System.Threading.Tasks;
124+
#nullable enable
125+
public class MyClass
126+
{
127+
public Task Foo()
128+
{
129+
return null;
130+
}
131+
}";
132+
await VerifyCS.VerifyAsync(code);
133+
}
134+
135+
[Test]
136+
public async Task No_Warn_When_Default_Is_Returned_For_ValueTask()
137+
{
138+
string code = @"
139+
using System.Threading.Tasks;
140+
public class MyClass
141+
{
142+
public ValueTask Foo()
143+
{
144+
return default;
145+
}
146+
}";
147+
await VerifyCS.VerifyAsync(code);
148+
}
149+
150+
[Test]
151+
public async Task No_Warn_When_Default_Is_Returned_For_ValueTask_Of_T()
152+
{
153+
string code = @"
154+
using System.Threading.Tasks;
155+
public class MyClass
156+
{
157+
public ValueTask<int> Foo()
158+
{
159+
return default;
160+
}
161+
}";
162+
await VerifyCS.VerifyAsync(code);
163+
}
164+
165+
[Test]
166+
public async Task Warn_When_Default_Is_Returned_For_Task()
167+
{
168+
string code = @"
169+
using System.Threading.Tasks;
170+
public class MyClass
171+
{
172+
public Task Foo()
173+
{
174+
[|return default;|]
175+
}
176+
}";
177+
await VerifyCS.VerifyAsync(code);
178+
}
179+
180+
[Test]
181+
public async Task Warn_When_Default_Is_Returned_For_Task_Of_T()
182+
{
183+
string code = @"
184+
using System.Threading.Tasks;
185+
public class MyClass
186+
{
187+
public Task<int> Foo()
188+
{
189+
[|return default;|]
190+
}
191+
}";
192+
await VerifyCS.VerifyAsync(code);
193+
}
194+
}
195+
}

‎src/ErrorProne.NET.CoreAnalyzers/AnalyzerReleases.Unshipped.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ EPC27 | CodeSmell | Warning | AsyncVoidMethodAnalyzer
2222
EPC28 | CodeSmell | Warning | ExcludeFromCodeCoverageOnPartialClassAnalyzer
2323
EPC29 | CodeSmell | Warning | ExcludeFromCodeCoverageMessageAnalyzer: Warn when ExcludeFromCodeCoverageAttribute is used without a message argument.
2424
EPC30 | CodeSmell | Warning | RecursiveCallAnalyzer: Warns when a method calls itself recursively (conditionally or unconditionally).
25+
EPC31 | CodeSmell | Warning | DoNotReturnNullForTaskLikeAnalyzer
2526
ERP021 | CodeSmell | Warning | ThrowExAnalyzer
2627
ERP022 | CodeSmell | Warning | SwallowAllExceptionsAnalyzer
2728
ERP031 | Concurrency | Warning | ConcurrentCollectionAnalyzer

‎src/ErrorProne.NET.CoreAnalyzers/AsyncAnalyzers/DoNotReturnNullForTaskLikeAnalyzer.cs‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
using Microsoft.CodeAnalysis;
2+
using Microsoft.CodeAnalysis.Diagnostics;
3+
using ErrorProne.NET.Core;
4+
using Microsoft.CodeAnalysis.Operations;
5+
using ErrorProne.NET.CoreAnalyzers;
6+
7+
namespace ErrorProne.NET.AsyncAnalyzers
8+
{
9+
/// <summary>
10+
/// Warns when a method that returns a Task-like type (Task, Task&lt;T&gt;, ValueTask, etc.) returns null.
11+
/// The analyzer is useful only when non-nullable reference types are not enabled!
12+
/// If they are enabled, the compiler will already warn about returning null for a non-nullable return type.
13+
/// </summary>
14+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
15+
public sealed class DoNotReturnNullForTaskLikeAnalyzer : DiagnosticAnalyzerBase
16+
{
17+
private static readonly DiagnosticDescriptor Rule = DiagnosticDescriptors.EPC31;
18+
19+
/// <nodoc />
20+
public DoNotReturnNullForTaskLikeAnalyzer()
21+
: base(Rule)
22+
{
23+
}
24+
25+
/// <inheritdoc />
26+
protected override void InitializeCore(AnalysisContext context)
27+
{
28+
context.EnableConcurrentExecution();
29+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
30+
31+
context.RegisterOperationAction(AnalyzeReturnOperation, OperationKind.Return);
32+
}
33+
34+
private void AnalyzeReturnOperation(OperationAnalysisContext context)
35+
{
36+
var returnOperation = (IReturnOperation)context.Operation;
37+
38+
// In case of 'return;'
39+
if (returnOperation.ReturnedValue == null)
40+
{
41+
return;
42+
}
43+
44+
//var semanticModel = context.Compilation.GetSemanticModel(returnOperation.Syntax.SyntaxTree, false);
45+
46+
47+
48+
// We don't need to analyze the method (which might be expensive), but we can just check the type of the return value.
49+
// And do nothing if it's not a Task-like type.
50+
var returnType = returnOperation.ReturnedValue?.Type;
51+
if (returnType == null || returnType.NullableAnnotation == NullableAnnotation.Annotated)
52+
{
53+
// Not running this analysis when nullable types are on.
54+
// In this case, the compiler will emit a warning.
55+
return;
56+
}
57+
58+
if (returnType.IsTaskLike(context.Compilation) != true)
59+
{
60+
return;
61+
}
62+
63+
var returnedValueOperation = returnOperation.ReturnedValue;
64+
IMethodSymbol? methodSymbol = findParentLocalOrLambdaSymbol(returnedValueOperation) ?? context.ContainingSymbol as IMethodSymbol;
65+
if (methodSymbol is not null && !methodSymbol.IsAsync)
66+
{
67+
if (isReturningNull(returnedValueOperation, context.Compilation) && methodSymbol.ReturnType.IsTaskLike(context.Compilation))
68+
{
69+
context.ReportDiagnostic(Diagnostic.Create(Rule, returnOperation.Syntax.GetLocation(), methodSymbol.Name));
70+
}
71+
}
72+
73+
static bool isReturningNull(IOperation? operation, Compilation compilation)
74+
{
75+
if (operation == null)
76+
{
77+
return false;
78+
}
79+
80+
if (operation is IReturnOperation returnedValueOperation)
81+
{
82+
// Case A: Check if the returned value is a constant null.
83+
if (returnedValueOperation.ConstantValue.HasValue && returnedValueOperation.ConstantValue.Value == null)
84+
{
85+
return true;
86+
}
87+
88+
89+
}
90+
91+
// Case B: Check for the 'default' keyword used with a reference type.
92+
// This handles 'return default;' where the method's return type is a class, interface, delegate, or array.
93+
if (operation is IDefaultValueOperation &&
94+
operation.Type?.IsReferenceType == true)
95+
{
96+
return true;
97+
}
98+
99+
if (operation.ConstantValue.HasValue && operation.ConstantValue.Value is null)
100+
{
101+
return true;
102+
}
103+
104+
if (operation is IConversionOperation conversion)
105+
{
106+
if (conversion.Type.IsTaskLike(compilation))
107+
{
108+
return false;
109+
}
110+
111+
return isReturningNull(conversion.Operand, compilation);
112+
}
113+
114+
if (operation is IConditionalAccessOperation conditionalAccess)
115+
{
116+
return isReturningNull(conditionalAccess.Operation, compilation) || isReturningNull(conditionalAccess.WhenNotNull, compilation);
117+
}
118+
else if (operation is IConditionalOperation conditional)
119+
{
120+
return isReturningNull(conditional.WhenTrue, compilation) || isReturningNull(conditional.WhenFalse, compilation);
121+
}
122+
else if (operation is ISwitchExpressionOperation switchExpression)
123+
{
124+
foreach (var arm in switchExpression.Arms)
125+
{
126+
if (isReturningNull(arm.Value, compilation))
127+
{
128+
return true;
129+
}
130+
}
131+
}
132+
133+
return false;
134+
135+
}
136+
137+
static IMethodSymbol? findParentLocalOrLambdaSymbol(IOperation operation)
138+
{
139+
foreach (var parent in operation.EnumerateParentOperations())
140+
{
141+
if (parent is ILocalFunctionOperation lf)
142+
{
143+
return lf.Symbol;
144+
}
145+
146+
if (parent is IAnonymousFunctionOperation f)
147+
{
148+
return f.Symbol;
149+
}
150+
}
151+
152+
return null;
153+
}
154+
}
155+
}
156+
}

‎src/ErrorProne.NET.CoreAnalyzers/CompilationExtensions.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,18 @@ public static IEnumerable<IOperation> EnumerateChildOperations(this IOperation?
108108

109109
return argumentOperation?.Type;
110110
}
111+
112+
public static IEnumerable<IOperation> EnumerateParentOperations(this IOperation? operation)
113+
{
114+
while (operation != null)
115+
{
116+
operation = operation.Parent;
117+
if (operation != null)
118+
{
119+
yield return operation;
120+
}
121+
}
122+
}
111123
}
112124

113125

‎src/ErrorProne.NET.CoreAnalyzers/DiagnosticDescriptors.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,16 @@ internal static class DiagnosticDescriptors
184184
isEnabledByDefault: true,
185185
description: "Detects when a method calls itself recursively, either conditionally or unconditionally.");
186186

187+
/// <nodoc />
188+
public static readonly DiagnosticDescriptor EPC31 = new DiagnosticDescriptor(
189+
nameof(EPC31),
190+
title: "Do not return null for Task-like types",
191+
messageFormat: "Do not return null for Task-like type from method '{0}'",
192+
category: CodeSmellCategory,
193+
defaultSeverity: DiagnosticSeverity.Warning,
194+
isEnabledByDefault: true,
195+
description: "Returning null for a Task-like type may lead to NullReferenceException when the task is awaited. Return Task.CompletedTask instead.");
196+
187197
/// <nodoc />
188198
public static readonly DiagnosticDescriptor ERP031 = new DiagnosticDescriptor(
189199
nameof(ERP031),

0 commit comments

Comments
 (0)