Skip to content

Commit 526417e

Browse files
authored
Fix ambiguous route analyzer false positive with switch (#51881) (#52035)
1 parent 296daad commit 526417e

File tree

2 files changed

+104
-6
lines changed

2 files changed

+104
-6
lines changed

src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DetectAmbiguousRoutes.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ private static void DetectAmbiguousRoutes(in OperationBlockAnalysisContext conte
3131
.Where(u => u.ResolvedOperation != null && !u.MapOperation.RouteUsageModel.UsageContext.HttpMethods.IsDefault)
3232
.GroupBy(u => new MapOperationGroupKey(u.MapOperation.Builder, u.ResolvedOperation!, u.MapOperation.RouteUsageModel.RoutePattern, u.MapOperation.RouteUsageModel.UsageContext.HttpMethods));
3333

34-
foreach (var ambigiousGroup in groupedByParent.Where(g => g.Count() >= 2))
34+
foreach (var ambiguousGroup in groupedByParent.Where(g => g.Count() >= 2))
3535
{
36-
foreach (var ambigiousMapOperation in ambigiousGroup)
36+
foreach (var ambiguousMapOperation in ambiguousGroup)
3737
{
38-
var model = ambigiousMapOperation.MapOperation.RouteUsageModel;
38+
var model = ambiguousMapOperation.MapOperation.RouteUsageModel;
3939

4040
context.ReportDiagnostic(Diagnostic.Create(
4141
DiagnosticDescriptors.AmbiguousRouteHandlerRoute,
@@ -67,15 +67,16 @@ private static void DetectAmbiguousRoutes(in OperationBlockAnalysisContext conte
6767

6868
while (current != null)
6969
{
70-
if (current.Parent is IBlockOperation blockOperation)
70+
if (current.Parent is IBlockOperation or ISwitchCaseOperation)
7171
{
72-
return blockOperation;
72+
return current.Parent;
7373
}
7474
else if (current.Parent is IConditionalOperation or
7575
ICoalesceOperation or
7676
IAssignmentOperation or
7777
IArgumentOperation or
78-
IInvocationOperation)
78+
IInvocationOperation or
79+
ISwitchExpressionArmOperation)
7980
{
8081
return current;
8182
}

src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/DetectAmbiguousMappedRoutesTest.cs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,79 @@ void Hello() { }
9797
await VerifyCS.VerifyAnalyzerAsync(source);
9898
}
9999

100+
[Fact]
101+
public async Task DuplicateRoutes_SwitchStatement_NoDiagnostics()
102+
{
103+
// Arrange
104+
var source = @"
105+
using System;
106+
using Microsoft.AspNetCore.Builder;
107+
var app = WebApplication.Create();
108+
switch (Random.Shared.Next())
109+
{
110+
case 0:
111+
app.MapGet(""/"", () => Hello());
112+
return;
113+
case 1:
114+
app.MapGet(""/"", () => Hello());
115+
return;
116+
}
117+
void Hello() { }
118+
";
119+
120+
// Act & Assert
121+
await VerifyCS.VerifyAnalyzerAsync(source);
122+
}
123+
124+
[Fact]
125+
public async Task DuplicateRoutes_InsideSwitchStatement_HasDiagnostics()
126+
{
127+
// Arrange
128+
var source = @"
129+
using System;
130+
using Microsoft.AspNetCore.Builder;
131+
var app = WebApplication.Create();
132+
switch (Random.Shared.Next())
133+
{
134+
case 0:
135+
app.MapGet({|#0:""/""|}, () => Hello());
136+
app.MapGet({|#1:""/""|}, () => Hello());
137+
return;
138+
139+
}
140+
void Hello() { }
141+
";
142+
143+
var expectedDiagnostics = new[] {
144+
new DiagnosticResult(DiagnosticDescriptors.AmbiguousRouteHandlerRoute).WithArguments("/").WithLocation(0),
145+
new DiagnosticResult(DiagnosticDescriptors.AmbiguousRouteHandlerRoute).WithArguments("/").WithLocation(1)
146+
};
147+
148+
// Act & Assert
149+
await VerifyCS.VerifyAnalyzerAsync(source, expectedDiagnostics);
150+
}
151+
152+
[Fact]
153+
public async Task DuplicateRoutes_SwitchExpression_NoDiagnostics()
154+
{
155+
// Arrange
156+
var source = @"
157+
using System;
158+
using Microsoft.AspNetCore.Builder;
159+
var app = WebApplication.Create();
160+
_ = Random.Shared.Next() switch
161+
{
162+
0 => app.MapGet(""/"", () => Hello()),
163+
1 => app.MapGet(""/"", () => Hello()),
164+
_ => throw new Exception()
165+
};
166+
void Hello() { }
167+
";
168+
169+
// Act & Assert
170+
await VerifyCS.VerifyAnalyzerAsync(source);
171+
}
172+
100173
[Fact]
101174
public async Task DuplicateRoutes_NullCoalescing_NoDiagnostics()
102175
{
@@ -166,6 +239,30 @@ void Hello() { }
166239
await VerifyCS.VerifyAnalyzerAsync(source);
167240
}
168241

242+
[Fact]
243+
public async Task DuplicateMapGetRoutes_DuplicatesInsideConditional_NoDiagnostics()
244+
{
245+
// Arrange
246+
var source = @"
247+
using Microsoft.AspNetCore.Builder;
248+
var app = WebApplication.Create();
249+
if (true)
250+
{
251+
app.MapGet({|#0:""/""|}, () => Hello());
252+
app.MapGet({|#1:""/""|}, () => Hello());
253+
}
254+
void Hello() { }
255+
";
256+
257+
var expectedDiagnostics = new[] {
258+
new DiagnosticResult(DiagnosticDescriptors.AmbiguousRouteHandlerRoute).WithArguments("/").WithLocation(0),
259+
new DiagnosticResult(DiagnosticDescriptors.AmbiguousRouteHandlerRoute).WithArguments("/").WithLocation(1)
260+
};
261+
262+
// Act & Assert
263+
await VerifyCS.VerifyAnalyzerAsync(source, expectedDiagnostics);
264+
}
265+
169266
[Fact]
170267
public async Task DuplicateRoutes_UnknownUsageOfEndConventionBuilderExtension_NoDiagnostics()
171268
{

0 commit comments

Comments
 (0)