diff --git a/src/Components/Analyzers/src/StateHasChangedAnalyzer.cs b/src/Components/Analyzers/src/StateHasChangedAnalyzer.cs index 2764b2e3298b..e7e4d4fb380d 100644 --- a/src/Components/Analyzers/src/StateHasChangedAnalyzer.cs +++ b/src/Components/Analyzers/src/StateHasChangedAnalyzer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.Text; #nullable enable @@ -103,7 +104,24 @@ expressionBody.Expression is InvocationExpressionSyntax expressionBodyInvocation return; } - var awaitExpressions = body.DescendantNodes(static node => !IsNestedFunctionLike(node)).OfType().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() .Where(invocation => IsStateHasChangedCall(syntaxContext.SemanticModel, invocation)) .OrderBy(invocation => invocation.SpanStart) @@ -115,9 +133,9 @@ expressionBody.Expression is InvocationExpressionSyntax expressionBodyInvocation } var callLocations = new Dictionary(); - 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); @@ -125,11 +143,9 @@ expressionBody.Expression is InvocationExpressionSyntax expressionBodyInvocation } 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); @@ -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) diff --git a/src/Components/Analyzers/test/StateHasChangedAnalyzerTest.cs b/src/Components/Analyzers/test/StateHasChangedAnalyzerTest.cs index 56d40789c7ac..ab2f8d5bd405 100644 --- a/src/Components/Analyzers/test/StateHasChangedAnalyzerTest.cs +++ b/src/Components/Analyzers/test/StateHasChangedAnalyzerTest.cs @@ -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 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 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 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() {