Avoid incorrectly providing diagnostics for chained calls (#16639)
* Avoid incorrectly providing diagnostics for chained calls Fixes https://github.com/aspnet/AspNetCore/issues/15203 * Update src/Analyzers/Analyzers/src/UseAuthorizationAnalyzer.cs
This commit is contained in:
parent
0540ec259d
commit
e79b144062
|
|
@ -2,6 +2,7 @@
|
||||||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||||
|
|
||||||
using System.Diagnostics;
|
using System.Diagnostics;
|
||||||
|
using System.Linq;
|
||||||
using Microsoft.CodeAnalysis;
|
using Microsoft.CodeAnalysis;
|
||||||
using Microsoft.CodeAnalysis.Diagnostics;
|
using Microsoft.CodeAnalysis.Diagnostics;
|
||||||
|
|
||||||
|
|
@ -27,6 +28,7 @@ namespace Microsoft.AspNetCore.Analyzers
|
||||||
{
|
{
|
||||||
MiddlewareItem? useAuthorizationItem = default;
|
MiddlewareItem? useAuthorizationItem = default;
|
||||||
MiddlewareItem? useRoutingItem = default;
|
MiddlewareItem? useRoutingItem = default;
|
||||||
|
MiddlewareItem? useEndpoint = default;
|
||||||
|
|
||||||
var length = middlewareAnalysis.Middleware.Length;
|
var length = middlewareAnalysis.Middleware.Length;
|
||||||
for (var i = length - 1; i >= 0; i-- )
|
for (var i = length - 1; i >= 0; i-- )
|
||||||
|
|
@ -70,9 +72,24 @@ namespace Microsoft.AspNetCore.Analyzers
|
||||||
useAuthorizationItem.Operation.Syntax.GetLocation(),
|
useAuthorizationItem.Operation.Syntax.GetLocation(),
|
||||||
middlewareItem.UseMethod.Name));
|
middlewareItem.UseMethod.Name));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
useEndpoint = middlewareItem;
|
||||||
}
|
}
|
||||||
else if (middleware == "UseRouting")
|
else if (middleware == "UseRouting")
|
||||||
{
|
{
|
||||||
|
if (useEndpoint is null)
|
||||||
|
{
|
||||||
|
// We're likely here because the middleware uses an expression chain e.g.
|
||||||
|
// app.UseRouting()
|
||||||
|
// .UseAuthorization()
|
||||||
|
// .UseEndpoints(..));
|
||||||
|
// This analyzer expects MiddlewareItem instances to appear in the order in which they appear in source
|
||||||
|
// which unfortunately isn't true for chained calls (the operations appear in reverse order).
|
||||||
|
// We'll avoid doing any analysis in this event and rely on the runtime guardrails.
|
||||||
|
// We'll use https://github.com/aspnet/AspNetCore/issues/16648 to track addressing this in a future milestone
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
useRoutingItem = middlewareItem;
|
useRoutingItem = middlewareItem;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -244,6 +244,22 @@ namespace Microsoft.AspNetCore.Analyzers
|
||||||
Assert.Empty(diagnostics);
|
Assert.Empty(diagnostics);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task StartupAnalyzer_UseAuthorizationConfiguredAsAChain_ReportsNoDiagnostics()
|
||||||
|
{
|
||||||
|
// Regression test for https://github.com/aspnet/AspNetCore/issues/15203
|
||||||
|
// Arrange
|
||||||
|
var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthConfiguredCorrectlyChained));
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
var middlewareAnalysis = Assert.Single(Analyses.OfType<MiddlewareAnalysis>());
|
||||||
|
Assert.NotEmpty(middlewareAnalysis.Middleware);
|
||||||
|
Assert.Empty(diagnostics);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task StartupAnalyzer_UseAuthorizationInvokedMultipleTimesInEndpointRoutingBlock_ReportsNoDiagnostics()
|
public async Task StartupAnalyzer_UseAuthorizationInvokedMultipleTimesInEndpointRoutingBlock_ReportsNoDiagnostics()
|
||||||
{
|
{
|
||||||
|
|
@ -279,6 +295,23 @@ namespace Microsoft.AspNetCore.Analyzers
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task StartupAnalyzer_UseAuthorizationConfiguredBeforeUseRoutingChained_ReportsDiagnostics()
|
||||||
|
{
|
||||||
|
// This one asserts a false negative for https://github.com/aspnet/AspNetCore/issues/15203.
|
||||||
|
// We don't correctly identify chained calls, this test verifies the behavior.
|
||||||
|
// Arrange
|
||||||
|
var source = Read(nameof(TestFiles.StartupAnalyzerTest.UseAuthBeforeUseRoutingChained));
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
var middlewareAnalysis = Assert.Single(Analyses.OfType<MiddlewareAnalysis>());
|
||||||
|
Assert.NotEmpty(middlewareAnalysis.Middleware);
|
||||||
|
Assert.Empty(diagnostics);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task StartupAnalyzer_UseAuthorizationConfiguredAfterUseEndpoints_ReportsDiagnostics()
|
public async Task StartupAnalyzer_UseAuthorizationConfiguredAfterUseEndpoints_ReportsDiagnostics()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,17 @@
|
||||||
|
// Copyright (c) .NET Foundation. All rights reserved.
|
||||||
|
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||||
|
|
||||||
|
using Microsoft.AspNetCore.Builder;
|
||||||
|
|
||||||
|
namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest {
|
||||||
|
public class UseAuthBeforeUseRoutingChained
|
||||||
|
{
|
||||||
|
public void Configure(IApplicationBuilder app)
|
||||||
|
{
|
||||||
|
app.UseFileServer()
|
||||||
|
.UseAuthorization()
|
||||||
|
.UseRouting()
|
||||||
|
.UseEndpoints(r => { });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
// Copyright (c) .NET Foundation. All rights reserved.
|
||||||
|
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||||
|
|
||||||
|
using Microsoft.AspNetCore.Builder;
|
||||||
|
|
||||||
|
namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest {
|
||||||
|
public class UseAuthConfiguredCorrectlyChained
|
||||||
|
{
|
||||||
|
public void Configure(IApplicationBuilder app)
|
||||||
|
{
|
||||||
|
app.UseRouting()
|
||||||
|
.UseAuthorization()
|
||||||
|
.UseEndpoints(r => { });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue