Skip to content

Fix BL0012 false positive on await foreach and await using #68720

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.

Sign up for GitHub

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

Open
AzeemullahRg wants to merge 1 commit into
base: main
Choose a base branch
from
+225 −6
Open

Fix BL0012 false positive on await foreach and await using #68720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 55 additions & 6 deletions src/Components/Analyzers/src/StateHasChangedAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.Text;

#nullable enable

Expand Down Expand Up @@ -103,7 +104,24 @@ expressionBody.Expression is InvocationExpressionSyntax expressionBodyInvocation
return;
}

var awaitExpressions = body.DescendantNodes(static node => !IsNestedFunctionLike(node)).OfType<AwaitExpressionSyntax>().OrderBy(n => n.SpanStart).ToList();
var suspensionStart = int.MaxValue;
var suspensionEnd = int.MinValue;
foreach (var node in body.DescendantNodes(static node => !IsNestedFunctionLike(node)))
{
if (TryGetSuspensionSpan(node, out var suspension))
{
if (suspension.Start < suspensionStart)
{
suspensionStart = suspension.Start;
}

if (suspension.End > suspensionEnd)
{
suspensionEnd = suspension.End;
}
}
}

var stateCalls = body.DescendantNodes(static node => !IsNestedFunctionLike(node)).OfType<InvocationExpressionSyntax>()
.Where(invocation => IsStateHasChangedCall(syntaxContext.SemanticModel, invocation))
.OrderBy(invocation => invocation.SpanStart)
Expand All @@ -115,21 +133,19 @@ expressionBody.Expression is InvocationExpressionSyntax expressionBodyInvocation
}

var callLocations = new Dictionary<int, Location>();
if (awaitExpressions.Count == 0)
if (suspensionStart > suspensionEnd)
{
// no await expressions, all calls are potentially redundant
// no awaits, all calls are potentially redundant
foreach (var stateCall in stateCalls)
{
AddCallLocation(callLocations, stateCall);
}
}
else
{
var firstAwaitStart = awaitExpressions[0].SpanStart;
var lastAwaitStart = awaitExpressions[awaitExpressions.Count - 1].SpanStart;
foreach (var stateCall in stateCalls)
{
if (stateCall.SpanStart < firstAwaitStart || stateCall.SpanStart > lastAwaitStart)
if (stateCall.SpanStart < suspensionStart || stateCall.SpanStart > suspensionEnd)
{
// any calls before the first await or after the last one are redundant, because ComponentBase calls StateHasChanged afterwards.
AddCallLocation(callLocations, stateCall);
Expand Down Expand Up @@ -215,6 +231,39 @@ private static bool IsNestedFunctionLike(SyntaxNode node)
return node is LocalFunctionStatementSyntax or AnonymousFunctionExpressionSyntax;
}

private static bool TryGetSuspensionSpan(SyntaxNode node, out TextSpan span)
{
switch (node)
{
case AwaitExpressionSyntax awaitExpression:
span = awaitExpression.Span;
return true;

case CommonForEachStatementSyntax forEachStatement when IsAwaitKeyword(forEachStatement.AwaitKeyword):
span = forEachStatement.Span;
return true;

case UsingStatementSyntax usingStatement when IsAwaitKeyword(usingStatement.AwaitKeyword):
span = usingStatement.Span;
return true;

case LocalDeclarationStatementSyntax declaration when IsAwaitKeyword(declaration.AwaitKeyword):
span = TextSpan.FromBounds(
declaration.SpanStart,
declaration.Parent is BlockSyntax enclosingBlock ? enclosingBlock.Span.End : declaration.Span.End);
return true;

default:
span = default;
return false;
}
}

private static bool IsAwaitKeyword(SyntaxToken token)
{
return token.IsKind(Microsoft.CodeAnalysis.CSharp.SyntaxKind.AwaitKeyword);
}

private static IMethodSymbol? TryGetMethodFromOperation(IOperation operation)
{
switch (operation)
Expand Down
170 changes: 170 additions & 0 deletions src/Components/Analyzers/test/StateHasChangedAnalyzerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,176 @@ protected override async Task OnInitializedAsync()
VerifyCSharpDiagnostic(test);
}

[Fact]
public void StateHasChangedInsideAwaitForeach_DoesNotReportDiagnostic()
{
var test = @"
namespace ConsoleApplication1
{
using Microsoft.AspNetCore.Components;
using System.Collections.Generic;
using System.Threading.Tasks;

class TestComponent : ComponentBase
{
protected override async Task OnInitializedAsync()
{
await foreach (var item in StreamAsync())
{
StateHasChanged();
}
}

private static async IAsyncEnumerable<int> StreamAsync()
{
await Task.Delay(1);
yield return 1;
}
}
}" + ComponentDeclarations;

VerifyCSharpDiagnostic(test);
}

[Fact]
public void StateHasChangedBeforeAwaitForeach_ReportsDiagnostic()
{
var test = @"
namespace ConsoleApplication1
{
using Microsoft.AspNetCore.Components;
using System.Collections.Generic;
using System.Threading.Tasks;

class TestComponent : ComponentBase
{
protected override async Task OnInitializedAsync()
{
StateHasChanged();

await foreach (var item in StreamAsync())
{
}
}

private static async IAsyncEnumerable<int> StreamAsync()
{
await Task.Delay(1);
yield return 1;
}
}
}" + ComponentDeclarations;

VerifyCSharpDiagnostic(
test,
new DiagnosticResult
{
Id = DiagnosticDescriptors.UnnecessaryStateHasChangedCall.Id,
Message = "StateHasChanged is unnecessary in method 'OnInitializedAsync' and can be removed.",
Severity = DiagnosticSeverity.Warning,
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 12, 17) }
});
}

[Fact]
public void StateHasChangedAfterAwaitForeach_ReportsDiagnostic()
{
var test = @"
namespace ConsoleApplication1
{
using Microsoft.AspNetCore.Components;
using System.Collections.Generic;
using System.Threading.Tasks;

class TestComponent : ComponentBase
{
protected override async Task OnInitializedAsync()
{
await foreach (var item in StreamAsync())
{
}

StateHasChanged();
}

private static async IAsyncEnumerable<int> StreamAsync()
{
await Task.Delay(1);
yield return 1;
}
}
}" + ComponentDeclarations;

VerifyCSharpDiagnostic(
test,
new DiagnosticResult
{
Id = DiagnosticDescriptors.UnnecessaryStateHasChangedCall.Id,
Message = "StateHasChanged is unnecessary in method 'OnInitializedAsync' and can be removed.",
Severity = DiagnosticSeverity.Warning,
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 16, 17) }
});
}

[Fact]
public void StateHasChangedInsideAwaitUsingBlock_DoesNotReportDiagnostic()
{
var test = @"
namespace ConsoleApplication1
{
using Microsoft.AspNetCore.Components;
using System;
using System.Threading.Tasks;

class TestComponent : ComponentBase
{
protected override async Task OnInitializedAsync()
{
await using (var scope = new Scope())
{
StateHasChanged();
}
}

private sealed class Scope : IAsyncDisposable
{
public ValueTask DisposeAsync() => default;
}
}
}" + ComponentDeclarations;

VerifyCSharpDiagnostic(test);
}

[Fact]
public void StateHasChangedAfterAwaitUsingDeclaration_DoesNotReportDiagnostic()
{
var test = @"
namespace ConsoleApplication1
{
using Microsoft.AspNetCore.Components;
using System;
using System.Threading.Tasks;

class TestComponent : ComponentBase
{
protected override async Task OnInitializedAsync()
{
await using var scope = new Scope();

StateHasChanged();
}

private sealed class Scope : IAsyncDisposable
{
public ValueTask DisposeAsync() => default;
}
}
}" + ComponentDeclarations;

VerifyCSharpDiagnostic(test);
}

[Fact]
public void NonComponentBaseClassWithOnInitialized_DoesNotReportDiagnostic()
{
Expand Down
Loading