Skip to content

Commit 74f3952

Browse files
committed
ASP001 handle multiple parameters.
1 parent 3ff564b commit 74f3952

4 files changed

Lines changed: 317 additions & 23 deletions

File tree

AspNetCoreAnalyzers.Tests/ASP001ParameterNameTests/CodeFix.cs

Lines changed: 226 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public class CodeFix
1212
private static readonly CodeFixProvider RenameParameterFix = new RenameParameterFix();
1313

1414
[Test]
15-
public void RenameParameterImplicit()
15+
public void ImplicitSingleParameter()
1616
{
1717
var order = @"
1818
namespace ValidCode
@@ -98,7 +98,231 @@ public async Task<IActionResult> GetOrder(int id)
9898
}
9999

100100
[Test]
101-
public void RenameParameterExplicit()
101+
public void ImplicitFirstParameter()
102+
{
103+
var orderItem = @"
104+
namespace ValidCode
105+
{
106+
public class OrderItem
107+
{
108+
public int Id { get; set; }
109+
}
110+
}";
111+
var order = @"
112+
namespace ValidCode
113+
{
114+
using System.Collections.Generic;
115+
116+
public class Order
117+
{
118+
public int Id { get; set; }
119+
120+
public IEnumerable<OrderItem> Items { get; set; }
121+
}
122+
}";
123+
124+
var db = @"
125+
namespace ValidCode
126+
{
127+
using Microsoft.EntityFrameworkCore;
128+
129+
public class Db : DbContext
130+
{
131+
public DbSet<Order> Orders { get; set; }
132+
}
133+
}";
134+
var before = @"
135+
namespace ValidCode
136+
{
137+
using System.Linq;
138+
using System.Threading.Tasks;
139+
using Microsoft.AspNetCore.Mvc;
140+
using Microsoft.EntityFrameworkCore;
141+
142+
[ApiController]
143+
public class OrdersController : Controller
144+
{
145+
private readonly Db db;
146+
147+
public OrdersController(Db db)
148+
{
149+
this.db = db;
150+
}
151+
152+
[HttpGet(""api/orders/{orderId}/items/{itemId}"")]
153+
public async Task<IActionResult> GetOrder(int ↓wrong, int itemId)
154+
{
155+
var order = await this.db.Orders.FirstOrDefaultAsync(x => x.Id == wrong);
156+
if (order == null)
157+
{
158+
return this.NotFound();
159+
}
160+
161+
var match = order.Items.FirstOrDefault(x => x.Id == itemId);
162+
if (match == null)
163+
{
164+
return this.NotFound();
165+
}
166+
167+
return this.Ok(match);
168+
}
169+
}
170+
}";
171+
172+
var after = @"
173+
namespace ValidCode
174+
{
175+
using System.Linq;
176+
using System.Threading.Tasks;
177+
using Microsoft.AspNetCore.Mvc;
178+
using Microsoft.EntityFrameworkCore;
179+
180+
[ApiController]
181+
public class OrdersController : Controller
182+
{
183+
private readonly Db db;
184+
185+
public OrdersController(Db db)
186+
{
187+
this.db = db;
188+
}
189+
190+
[HttpGet(""api/orders/{orderId}/items/{itemId}"")]
191+
public async Task<IActionResult> GetOrder(int orderId, int itemId)
192+
{
193+
var order = await this.db.Orders.FirstOrDefaultAsync(x => x.Id == orderId);
194+
if (order == null)
195+
{
196+
return this.NotFound();
197+
}
198+
199+
var match = order.Items.FirstOrDefault(x => x.Id == itemId);
200+
if (match == null)
201+
{
202+
return this.NotFound();
203+
}
204+
205+
return this.Ok(match);
206+
}
207+
}
208+
}";
209+
AnalyzerAssert.CodeFix(Analyzer, RenameParameterFix, ExpectedDiagnostic, new[] { orderItem, order, db, before }, after);
210+
}
211+
212+
[Test]
213+
public void ImplicitLastParameter()
214+
{
215+
var orderItem = @"
216+
namespace ValidCode
217+
{
218+
public class OrderItem
219+
{
220+
public int Id { get; set; }
221+
}
222+
}";
223+
var order = @"
224+
namespace ValidCode
225+
{
226+
using System.Collections.Generic;
227+
228+
public class Order
229+
{
230+
public int Id { get; set; }
231+
232+
public IEnumerable<OrderItem> Items { get; set; }
233+
}
234+
}";
235+
236+
var db = @"
237+
namespace ValidCode
238+
{
239+
using Microsoft.EntityFrameworkCore;
240+
241+
public class Db : DbContext
242+
{
243+
public DbSet<Order> Orders { get; set; }
244+
}
245+
}";
246+
var before = @"
247+
namespace ValidCode
248+
{
249+
using System.Linq;
250+
using System.Threading.Tasks;
251+
using Microsoft.AspNetCore.Mvc;
252+
using Microsoft.EntityFrameworkCore;
253+
254+
[ApiController]
255+
public class OrdersController : Controller
256+
{
257+
private readonly Db db;
258+
259+
public OrdersController(Db db)
260+
{
261+
this.db = db;
262+
}
263+
264+
[HttpGet(""api/orders/{orderId}/items/{itemId}"")]
265+
public async Task<IActionResult> GetOrder(int orderId, int ↓wrong)
266+
{
267+
var order = await this.db.Orders.FirstOrDefaultAsync(x => x.Id == orderId);
268+
if (order == null)
269+
{
270+
return this.NotFound();
271+
}
272+
273+
var match = order.Items.FirstOrDefault(x => x.Id == wrong);
274+
if (match == null)
275+
{
276+
return this.NotFound();
277+
}
278+
279+
return this.Ok(match);
280+
}
281+
}
282+
}";
283+
284+
var after = @"
285+
namespace ValidCode
286+
{
287+
using System.Linq;
288+
using System.Threading.Tasks;
289+
using Microsoft.AspNetCore.Mvc;
290+
using Microsoft.EntityFrameworkCore;
291+
292+
[ApiController]
293+
public class OrdersController : Controller
294+
{
295+
private readonly Db db;
296+
297+
public OrdersController(Db db)
298+
{
299+
this.db = db;
300+
}
301+
302+
[HttpGet(""api/orders/{orderId}/items/{itemId}"")]
303+
public async Task<IActionResult> GetOrder(int orderId, int itemId)
304+
{
305+
var order = await this.db.Orders.FirstOrDefaultAsync(x => x.Id == orderId);
306+
if (order == null)
307+
{
308+
return this.NotFound();
309+
}
310+
311+
var match = order.Items.FirstOrDefault(x => x.Id == itemId);
312+
if (match == null)
313+
{
314+
return this.NotFound();
315+
}
316+
317+
return this.Ok(match);
318+
}
319+
}
320+
}";
321+
AnalyzerAssert.CodeFix(Analyzer, RenameParameterFix, ExpectedDiagnostic, new[] { orderItem, order, db, before }, after);
322+
}
323+
324+
[Test]
325+
public void ExplicitAttributeSingleParameter()
102326
{
103327
var order = @"
104328
namespace ValidCode

AspNetCoreAnalyzers/Analyzers/AttributeAnalyzer.cs

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,25 @@ context.ContainingSymbol is IMethodSymbol method &&
2828
attribute.TryFirstAncestor(out MethodDeclarationSyntax methodDeclaration) &&
2929
TryGetTemplate(attribute, context, out var template))
3030
{
31-
foreach (var component in template.Path)
31+
using (var pairs = GetPairs(template, method))
3232
{
33-
if (component.Parameter is TemplateParameter parameter)
33+
if (pairs.TrySingle(x => x.Template == null, out var withMethodParameter) &&
34+
pairs.TrySingle(x => x.Method == null, out var withTemplateParameter))
3435
{
35-
if (method.Parameters.TryFirst(x => IsFromRoute(x) && x.Name != parameter.Name.Text, out var single))
36-
{
37-
context.ReportDiagnostic(
38-
Diagnostic.Create(
39-
ASP001ParameterName.Descriptor,
40-
single.Locations.Single(),
41-
ImmutableDictionary<string, string>.Empty.Add(nameof(NameSyntax), parameter.Name.Text)));
42-
}
36+
context.ReportDiagnostic(
37+
Diagnostic.Create(
38+
ASP001ParameterName.Descriptor,
39+
withMethodParameter.Method.Locations.Single(),
40+
ImmutableDictionary<string, string>.Empty.Add(nameof(NameSyntax), withTemplateParameter.Template.Value.Name.Text)));
41+
}
4342

44-
if (!method.Parameters.TryFirst(x => IsFromRoute(x), out _))
45-
{
46-
context.ReportDiagnostic(
47-
Diagnostic.Create(
48-
ASP002MissingParameter.Descriptor,
49-
methodDeclaration.ParameterList.GetLocation()));
50-
}
43+
if (pairs.TrySingle(x => x.Template != null, out _) &&
44+
pairs.TrySingle(x => x.Method == null, out _))
45+
{
46+
context.ReportDiagnostic(
47+
Diagnostic.Create(
48+
ASP002MissingParameter.Descriptor,
49+
methodDeclaration.ParameterList.GetLocation()));
5150
}
5251
}
5352
}
@@ -59,10 +58,11 @@ private static bool TryGetTemplate(AttributeSyntax attribute, SyntaxNodeAnalysis
5958
argumentList.Arguments.TrySingle(out var argument) &&
6059
argument.Expression is LiteralExpressionSyntax literal &&
6160
literal.IsKind(SyntaxKind.StringLiteralExpression) &&
62-
(Attribute.IsType(attribute, KnownSymbol.HttpGetAttribute, context.SemanticModel, context.CancellationToken) ||
61+
(Attribute.IsType(attribute, KnownSymbol.HttpDeleteAttribute, context.SemanticModel, context.CancellationToken) ||
62+
Attribute.IsType(attribute, KnownSymbol.HttpGetAttribute, context.SemanticModel, context.CancellationToken) ||
63+
Attribute.IsType(attribute, KnownSymbol.HttpPatchAttribute, context.SemanticModel, context.CancellationToken) ||
6364
Attribute.IsType(attribute, KnownSymbol.HttpPostAttribute, context.SemanticModel, context.CancellationToken) ||
64-
Attribute.IsType(attribute, KnownSymbol.HttpPutAttribute, context.SemanticModel, context.CancellationToken) ||
65-
Attribute.IsType(attribute, KnownSymbol.HttpDeleteAttribute, context.SemanticModel, context.CancellationToken)) &&
65+
Attribute.IsType(attribute, KnownSymbol.HttpPutAttribute, context.SemanticModel, context.CancellationToken)) &&
6666
UrlTemplate.TryParse(literal, out template))
6767
{
6868
return true;
@@ -86,5 +86,30 @@ private static bool IsFromRoute(IParameterSymbol p)
8686

8787
return true;
8888
}
89+
90+
private static PooledList<ParameterPair> GetPairs(UrlTemplate template, IMethodSymbol method)
91+
{
92+
var list = PooledList<ParameterPair>.Borrow();
93+
foreach (var parameter in method.Parameters)
94+
{
95+
if (IsFromRoute(parameter))
96+
{
97+
list.Add(template.Path.TrySingle(x => x.Parameter?.Name.Text == parameter.Name, out var templateParameter)
98+
? new ParameterPair(templateParameter.Parameter, parameter)
99+
: new ParameterPair(null, parameter));
100+
}
101+
}
102+
103+
foreach (var component in template.Path)
104+
{
105+
if (component.Parameter is TemplateParameter templateParameter &&
106+
list.All(x => x.Template != templateParameter))
107+
{
108+
list.Add(new ParameterPair(templateParameter, null));
109+
}
110+
}
111+
112+
return list;
113+
}
89114
}
90115
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
namespace AspNetCoreAnalyzers
2+
{
3+
using Microsoft.CodeAnalysis;
4+
5+
public struct ParameterPair
6+
{
7+
public ParameterPair(TemplateParameter? template, IParameterSymbol method)
8+
{
9+
this.Template = template;
10+
this.Method = method;
11+
}
12+
13+
public TemplateParameter? Template { get; }
14+
15+
public IParameterSymbol Method { get; }
16+
}
17+
}

AspNetCoreAnalyzers/Helpers/TemplateParameter.cs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
namespace AspNetCoreAnalyzers
22
{
3-
public struct TemplateParameter
3+
using System;
4+
5+
public struct TemplateParameter : IEquatable<TemplateParameter>
46
{
57
public TemplateParameter(TextAndLocation name, bool isOptional, TextAndLocation? type)
68
{
@@ -15,6 +17,17 @@ public TemplateParameter(TextAndLocation name, bool isOptional, TextAndLocation?
1517

1618
public TextAndLocation? Type { get; }
1719

20+
21+
public static bool operator ==(TemplateParameter left, TemplateParameter right)
22+
{
23+
return left.Equals(right);
24+
}
25+
26+
public static bool operator !=(TemplateParameter left, TemplateParameter right)
27+
{
28+
return !left.Equals(right);
29+
}
30+
1831
public static bool TryParse(TextAndLocation textAndLocation, out TemplateParameter result)
1932
{
2033
var text = textAndLocation.Text;
@@ -82,5 +95,20 @@ public static bool TryParse(TextAndLocation textAndLocation, out TemplateParamet
8295
return textAndLocation.Substring(typeStart, typeEnd - typeStart);
8396
}
8497
}
98+
99+
public bool Equals(TemplateParameter other)
100+
{
101+
return this.Name.Equals(other.Name);
102+
}
103+
104+
public override bool Equals(object obj)
105+
{
106+
return obj is TemplateParameter other && this.Equals(other);
107+
}
108+
109+
public override int GetHashCode()
110+
{
111+
return this.Name.GetHashCode();
112+
}
85113
}
86114
}

0 commit comments

Comments
 (0)