Skip to content

Commit f50cd52

Browse files
JamesNKJamesNK
authored
Analyzer clean up (#44393)
1 parent f6ab2a7 commit f50cd52

File tree

7 files changed

+108
-53
lines changed

7 files changed

+108
-53
lines changed

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,6 @@ namespace Microsoft.AspNetCore.Analyzers;
88
[System.Diagnostics.CodeAnalysis.SuppressMessage("MicrosoftCodeAnalysisReleaseTracking", "RS2008:Enable analyzer release tracking")]
99
internal static class DiagnosticDescriptors
1010
{
11-
internal static readonly DiagnosticDescriptor RoutePatternIssue = new(
12-
"RP0001",
13-
new LocalizableResourceString(nameof(Resources.Invalid_Route_pattern), Resources.ResourceManager, typeof(Resources)),
14-
new LocalizableResourceString(nameof(Resources.Route_issue_0), Resources.ResourceManager, typeof(Resources)),
15-
"Style",
16-
DiagnosticSeverity.Warning,
17-
isEnabledByDefault: true,
18-
helpLinkUri: "https://aka.ms/aspnet/analyzers");
19-
2011
internal static readonly DiagnosticDescriptor DoNotUseModelBindingAttributesOnRouteHandlerParameters = new(
2112
"ASP0003",
2213
"Do not use model binding attributes with route handlers",
@@ -127,27 +118,36 @@ internal static class DiagnosticDescriptors
127118

128119
internal static readonly DiagnosticDescriptor UseHeaderDictionaryPropertiesInsteadOfIndexer = new(
129120
"ASP0015",
130-
"Suggest using IHeaderDictionary properties",
131-
"The header '{0}' can be accessed using the {1} property",
121+
new LocalizableResourceString(nameof(Resources.Analyzer_HeaderDictionaryIndexer_Title), Resources.ResourceManager, typeof(Resources)),
122+
new LocalizableResourceString(nameof(Resources.Analyzer_HeaderDictionaryIndexer_Message), Resources.ResourceManager, typeof(Resources)),
132123
"Usage",
133124
DiagnosticSeverity.Info,
134125
isEnabledByDefault: true,
135126
helpLinkUri: "https://aka.ms/aspnet/analyzers");
136127

137128
internal static readonly DiagnosticDescriptor DoNotReturnValueFromRequestDelegate = new(
138129
"ASP0016",
139-
"Do not return a value from RequestDelegate",
140-
"The method used to create a RequestDelegate returns Task<{0}>. RequestDelegate discards this value. If this isn't intended then don't return a value or change the method signature to not match RequestDelegate.",
130+
new LocalizableResourceString(nameof(Resources.Analyzer_RequestDelegateReturnValue_Title), Resources.ResourceManager, typeof(Resources)),
131+
new LocalizableResourceString(nameof(Resources.Analyzer_RequestDelegateReturnValue_Message), Resources.ResourceManager, typeof(Resources)),
132+
"Usage",
133+
DiagnosticSeverity.Warning,
134+
isEnabledByDefault: true,
135+
helpLinkUri: "https://aka.ms/aspnet/analyzers");
136+
137+
internal static readonly DiagnosticDescriptor RoutePatternIssue = new(
138+
"ASP0017",
139+
new LocalizableResourceString(nameof(Resources.Analyzer_RouteIssue_Title), Resources.ResourceManager, typeof(Resources)),
140+
new LocalizableResourceString(nameof(Resources.Analyzer_RouteIssue_Message), Resources.ResourceManager, typeof(Resources)),
141141
"Usage",
142142
DiagnosticSeverity.Warning,
143143
isEnabledByDefault: true,
144144
helpLinkUri: "https://aka.ms/aspnet/analyzers");
145145

146146
internal static readonly DiagnosticDescriptor RoutePatternUnusedParameter = new(
147-
"RP0002",
148-
new LocalizableResourceString(nameof(Resources.Unused_Route_parameter), Resources.ResourceManager, typeof(Resources)),
149-
new LocalizableResourceString(nameof(Resources.Unused_Route_parameter_0), Resources.ResourceManager, typeof(Resources)),
150-
"Style",
147+
"ASP0018",
148+
new LocalizableResourceString(nameof(Resources.Analyzer_UnusedParameter_Title), Resources.ResourceManager, typeof(Resources)),
149+
new LocalizableResourceString(nameof(Resources.Analyzer_UnusedParameter_Message), Resources.ResourceManager, typeof(Resources)),
150+
"Usage",
151151
DiagnosticSeverity.Info,
152152
isEnabledByDefault: true,
153153
helpLinkUri: "https://aka.ms/aspnet/analyzers");

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,6 @@
165165
<data name="TemplateRoute_Exception" xml:space="preserve">
166166
<value>An error occurred while creating the route with name '{0}' and template '{1}'.</value>
167167
</data>
168-
<data name="Invalid_Route_pattern" xml:space="preserve">
169-
<value>Invalid route pattern</value>
170-
</data>
171-
<data name="Route_issue_0" xml:space="preserve">
172-
<value>Route issue: {0}</value>
173-
</data>
174168
<data name="AttributeRoute_TokenReplacement_EmptyTokenNotAllowed" xml:space="preserve">
175169
<value>An empty replacement token ('[]') is not allowed.</value>
176170
</data>
@@ -183,10 +177,28 @@
183177
<data name="AttributeRoute_TokenReplacement_UnescapedBraceInToken" xml:space="preserve">
184178
<value>An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape.</value>
185179
</data>
186-
<data name="Unused_Route_parameter" xml:space="preserve">
187-
<value>Unused route parameter</value>
180+
<data name="Analyzer_HeaderDictionaryIndexer_Message" xml:space="preserve">
181+
<value>The header '{0}' can be accessed using the {1} property</value>
182+
</data>
183+
<data name="Analyzer_HeaderDictionaryIndexer_Title" xml:space="preserve">
184+
<value>Suggest using IHeaderDictionary properties</value>
185+
</data>
186+
<data name="Analyzer_RequestDelegateReturnValue_Message" xml:space="preserve">
187+
<value>The method used to create a RequestDelegate returns Task&lt;{0}&gt;. RequestDelegate discards this value. If this isn't intended then change the return type to non-generic Task or, if the delegate is a route handler, cast it to Delegate so the return value is written to the response.</value>
188188
</data>
189-
<data name="Unused_Route_parameter_0" xml:space="preserve">
189+
<data name="Analyzer_RequestDelegateReturnValue_Title" xml:space="preserve">
190+
<value>Do not return a value from RequestDelegate</value>
191+
</data>
192+
<data name="Analyzer_RouteIssue_Message" xml:space="preserve">
193+
<value>Route issue: {0}</value>
194+
</data>
195+
<data name="Analyzer_RouteIssue_Title" xml:space="preserve">
196+
<value>Invalid route pattern</value>
197+
</data>
198+
<data name="Analyzer_UnusedParameter_Message" xml:space="preserve">
190199
<value>Unused route parameter '{0}'</value>
191200
</data>
201+
<data name="Analyzer_UnusedParameter_Title" xml:space="preserve">
202+
<value>Unused route parameter</value>
203+
</data>
192204
</root>

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/RoutePatternUsageDetector.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,9 @@ public static RoutePatternUsageContext BuildContext(SyntaxToken token, SemanticM
162162
return null;
163163
}
164164

165+
// Method has a delegate parameter. Could be Delegate or something that inherits from it, e.g. RequestDelegate.
165166
var delegateSymbol = semanticModel.Compilation.GetSpecialType(SpecialType.System_Delegate);
166-
var delegateParameter = method.Parameters.FirstOrDefault(p => SymbolEqualityComparer.Default.Equals(delegateSymbol, p.Type));
167+
var delegateParameter = method.Parameters.FirstOrDefault(p => delegateSymbol.IsAssignableFrom(p.Type));
167168
if (delegateParameter == null)
168169
{
169170
return null;

‎src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryIndexerAnalyzerTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
3131
",
3232
new DiagnosticResult(DiagnosticDescriptors.UseHeaderDictionaryPropertiesInsteadOfIndexer)
3333
.WithLocation(0)
34-
.WithMessage("The header 'content-type' can be accessed using the ContentType property"));
34+
.WithMessage(Resources.FormatAnalyzer_HeaderDictionaryIndexer_Message("content-type", "ContentType")));
3535
}
3636

3737
[Fact]
@@ -52,7 +52,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
5252
",
5353
new DiagnosticResult(DiagnosticDescriptors.UseHeaderDictionaryPropertiesInsteadOfIndexer)
5454
.WithLocation(0)
55-
.WithMessage("The header 'content-type' can be accessed using the ContentType property"));
55+
.WithMessage(Resources.FormatAnalyzer_HeaderDictionaryIndexer_Message("content-type", "ContentType")));
5656
}
5757

5858
[Fact]
@@ -150,7 +150,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
150150
",
151151
new DiagnosticResult(DiagnosticDescriptors.UseHeaderDictionaryPropertiesInsteadOfIndexer)
152152
.WithLocation(0)
153-
.WithMessage("The header 'Content-Type' can be accessed using the ContentType property"));
153+
.WithMessage(Resources.FormatAnalyzer_HeaderDictionaryIndexer_Message("Content-Type", "ContentType")));
154154
}
155155

156156
[Fact]

‎src/Framework/AspNetCoreAnalyzers/test/Http/RequestDelegateReturnTypeAnalyzerTests.cs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ namespace Microsoft.AspNetCore.Analyzers.Http;
1010

1111
public class RequestDelegateReturnTypeAnalyzerTests
1212
{
13-
private string GetMessage(string type) =>
14-
$"The method used to create a RequestDelegate returns Task<{type}>. RequestDelegate discards this value. If this isn't intended then don't return a value or change the method signature to not match RequestDelegate.";
15-
1613
[Fact]
1714
public async Task AnonymousDelegate_RequestDelegate_ReturnType_EndpointCtor_ReportDiagnostics()
1815
{
@@ -31,7 +28,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
3128
",
3229
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
3330
.WithLocation(0)
34-
.WithMessage(GetMessage("System.DateTime")));
31+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("System.DateTime")));
3532
}
3633

3734
[Fact]
@@ -50,7 +47,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
5047
",
5148
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
5249
.WithLocation(0)
53-
.WithMessage(GetMessage("object?")));
50+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("object?")));
5451
}
5552

5653
[Fact]
@@ -73,7 +70,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
7370
",
7471
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
7572
.WithLocation(0)
76-
.WithMessage(GetMessage("string")));
73+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
7774
}
7875

7976
[Fact]
@@ -89,7 +86,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
8986
",
9087
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
9188
.WithLocation(0)
92-
.WithMessage(GetMessage("string")));
89+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
9390
}
9491

9592
[Fact]
@@ -109,7 +106,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
109106
",
110107
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
111108
.WithLocation(0)
112-
.WithMessage(GetMessage("string")));
109+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
113110
}
114111

115112
[Fact]
@@ -130,7 +127,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
130127
",
131128
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
132129
.WithLocation(0)
133-
.WithMessage(GetMessage("string")));
130+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
134131
}
135132

136133
[Fact]
@@ -151,7 +148,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
151148
",
152149
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
153150
.WithLocation(0)
154-
.WithMessage(GetMessage("string")));
151+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
155152
}
156153

157154
[Fact]
@@ -179,7 +176,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
179176
",
180177
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
181178
.WithLocation(0)
182-
.WithMessage(GetMessage("string")));
179+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
183180
}
184181

185182
[Fact]
@@ -207,7 +204,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
207204
",
208205
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
209206
.WithLocation(0)
210-
.WithMessage(GetMessage("int")));
207+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("int")));
211208
}
212209

213210
[Fact]
@@ -299,7 +296,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
299296
",
300297
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
301298
.WithLocation(0)
302-
.WithMessage(GetMessage("string")));
299+
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
303300
}
304301

305302
[Fact]

‎src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RouteParameterUnusedParameterFixerTest.cs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,51 @@ static void Main()
389389
await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource, expectedIterations: 1);
390390
}
391391

392+
[Fact]
393+
public async Task MapGet_UnusedParameter_AddToRequestDelegateLambda()
394+
{
395+
// Arrange
396+
var source = @"
397+
using System;
398+
using System.Diagnostics.CodeAnalysis;
399+
using System.Threading.Tasks;
400+
using Microsoft.AspNetCore.Builder;
401+
using Microsoft.AspNetCore.Http;
402+
403+
class Program
404+
{
405+
static void Main()
406+
{
407+
EndpointRouteBuilderExtensions.MapGet(null, @""{|#0:{id}|}"", (HttpContext context) => Task.CompletedTask);
408+
}
409+
}
410+
";
411+
412+
var fixedSource = @"
413+
using System;
414+
using System.Diagnostics.CodeAnalysis;
415+
using System.Threading.Tasks;
416+
using Microsoft.AspNetCore.Builder;
417+
using Microsoft.AspNetCore.Http;
418+
419+
class Program
420+
{
421+
static void Main()
422+
{
423+
EndpointRouteBuilderExtensions.MapGet(null, @""{id}"", (string id, HttpContext context) => Task.CompletedTask);
424+
}
425+
}
426+
";
427+
428+
var expectedDiagnostics = new[]
429+
{
430+
new DiagnosticResult(DiagnosticDescriptors.RoutePatternUnusedParameter).WithArguments("id").WithLocation(0)
431+
};
432+
433+
// Act & Assert
434+
await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource, expectedIterations: 1);
435+
}
436+
392437
[Fact]
393438
public async Task MapGet_UnusedParameter_IntPolicy_AddIntToLambda()
394439
{

‎src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RoutePatternAnalyzerTests.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class HttpGet : Attribute
4040
var diagnostic = Assert.Single(diagnostics);
4141
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor);
4242
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
43-
Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture));
43+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture));
4444
}
4545

4646
[Fact]
@@ -73,7 +73,7 @@ public HttpGet([StringSyntax(""Route"")] string pattern)
7373
var diagnostic = Assert.Single(diagnostics);
7474
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor);
7575
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
76-
Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture));
76+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture));
7777
}
7878

7979
[Fact]
@@ -101,7 +101,7 @@ static void Main()
101101
var diagnostic = Assert.Single(diagnostics);
102102
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor);
103103
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
104-
Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture));
104+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture));
105105
}
106106

107107
[Fact]
@@ -129,7 +129,7 @@ static void Main()
129129
var diagnostic = Assert.Single(diagnostics);
130130
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor);
131131
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
132-
Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture));
132+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture));
133133
}
134134

135135
[Fact]
@@ -158,7 +158,7 @@ static void M([StringSyntax(""Route"")] string p)
158158
var diagnostic = Assert.Single(diagnostics);
159159
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor);
160160
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
161-
Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture));
161+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture));
162162
}
163163

164164
[Fact]
@@ -189,12 +189,12 @@ static void M([StringSyntax(""Route"")] string p)
189189
d =>
190190
{
191191
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, d.Descriptor);
192-
Assert.Equal($"Route issue: {Resources.FormatTemplateRoute_InvalidLiteral("~hi?")}", d.GetMessage(CultureInfo.InvariantCulture));
192+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.FormatTemplateRoute_InvalidLiteral("~hi?")), d.GetMessage(CultureInfo.InvariantCulture));
193193
},
194194
d =>
195195
{
196196
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, d.Descriptor);
197-
Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", d.GetMessage(CultureInfo.InvariantCulture));
197+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), d.GetMessage(CultureInfo.InvariantCulture));
198198
});
199199
}
200200

@@ -256,7 +256,7 @@ public void TestAction()
256256
d =>
257257
{
258258
Assert.Same(DiagnosticDescriptors.RoutePatternIssue, d.Descriptor);
259-
Assert.Equal($"Route issue: {Resources.AttributeRoute_TokenReplacement_UnclosedToken}", d.GetMessage(CultureInfo.InvariantCulture));
259+
Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.AttributeRoute_TokenReplacement_UnclosedToken), d.GetMessage(CultureInfo.InvariantCulture));
260260
});
261261
}
262262

@@ -296,7 +296,7 @@ public object TestAction()
296296
d =>
297297
{
298298
Assert.Same(DiagnosticDescriptors.RoutePatternUnusedParameter, d.Descriptor);
299-
Assert.Equal("Unused route parameter 'id'", d.GetMessage(CultureInfo.InvariantCulture));
299+
Assert.Equal(Resources.FormatAnalyzer_UnusedParameter_Message("id"), d.GetMessage(CultureInfo.InvariantCulture));
300300
});
301301
}
302302

@@ -376,7 +376,7 @@ public class PageData
376376
d =>
377377
{
378378
Assert.Same(DiagnosticDescriptors.RoutePatternUnusedParameter, d.Descriptor);
379-
Assert.Equal("Unused route parameter 'id'", d.GetMessage(CultureInfo.InvariantCulture));
379+
Assert.Equal(Resources.FormatAnalyzer_UnusedParameter_Message("id"), d.GetMessage(CultureInfo.InvariantCulture));
380380
});
381381
}
382382

0 commit comments

Comments
 (0)