diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs index 078dcea30cf9..d9b830b01e3b 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs @@ -8,15 +8,6 @@ namespace Microsoft.AspNetCore.Analyzers; [System.Diagnostics.CodeAnalysis.SuppressMessage("MicrosoftCodeAnalysisReleaseTracking", "RS2008:Enable analyzer release tracking")] internal static class DiagnosticDescriptors { - internal static readonly DiagnosticDescriptor RoutePatternIssue = new( - "RP0001", - new LocalizableResourceString(nameof(Resources.Invalid_Route_pattern), Resources.ResourceManager, typeof(Resources)), - new LocalizableResourceString(nameof(Resources.Route_issue_0), Resources.ResourceManager, typeof(Resources)), - "Style", - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - helpLinkUri: "https://aka.ms/aspnet/analyzers"); - internal static readonly DiagnosticDescriptor DoNotUseModelBindingAttributesOnRouteHandlerParameters = new( "ASP0003", "Do not use model binding attributes with route handlers", @@ -127,8 +118,8 @@ internal static class DiagnosticDescriptors internal static readonly DiagnosticDescriptor UseHeaderDictionaryPropertiesInsteadOfIndexer = new( "ASP0015", - "Suggest using IHeaderDictionary properties", - "The header '{0}' can be accessed using the {1} property", + new LocalizableResourceString(nameof(Resources.Analyzer_HeaderDictionaryIndexer_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.Analyzer_HeaderDictionaryIndexer_Message), Resources.ResourceManager, typeof(Resources)), "Usage", DiagnosticSeverity.Info, isEnabledByDefault: true, @@ -136,18 +127,27 @@ internal static class DiagnosticDescriptors internal static readonly DiagnosticDescriptor DoNotReturnValueFromRequestDelegate = new( "ASP0016", - "Do not return a value from RequestDelegate", - "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.", + new LocalizableResourceString(nameof(Resources.Analyzer_RequestDelegateReturnValue_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.Analyzer_RequestDelegateReturnValue_Message), Resources.ResourceManager, typeof(Resources)), + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); + + internal static readonly DiagnosticDescriptor RoutePatternIssue = new( + "ASP0017", + new LocalizableResourceString(nameof(Resources.Analyzer_RouteIssue_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.Analyzer_RouteIssue_Message), Resources.ResourceManager, typeof(Resources)), "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/aspnet/analyzers"); internal static readonly DiagnosticDescriptor RoutePatternUnusedParameter = new( - "RP0002", - new LocalizableResourceString(nameof(Resources.Unused_Route_parameter), Resources.ResourceManager, typeof(Resources)), - new LocalizableResourceString(nameof(Resources.Unused_Route_parameter_0), Resources.ResourceManager, typeof(Resources)), - "Style", + "ASP0018", + new LocalizableResourceString(nameof(Resources.Analyzer_UnusedParameter_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.Analyzer_UnusedParameter_Message), Resources.ResourceManager, typeof(Resources)), + "Usage", DiagnosticSeverity.Info, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/aspnet/analyzers"); diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 21b77df9b158..09b7e852e242 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -165,12 +165,6 @@ An error occurred while creating the route with name '{0}' and template '{1}'. - - Invalid route pattern - - - Route issue: {0} - An empty replacement token ('[]') is not allowed. @@ -183,10 +177,28 @@ An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape. - - Unused route parameter + + The header '{0}' can be accessed using the {1} property + + + Suggest using IHeaderDictionary properties + + + The method used to create a RequestDelegate returns Task<{0}>. 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. - + + Do not return a value from RequestDelegate + + + Route issue: {0} + + + Invalid route pattern + + Unused route parameter '{0}' + + Unused route parameter + \ No newline at end of file diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/RoutePatternUsageDetector.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/RoutePatternUsageDetector.cs index ed95362a1e8c..d31f681062ea 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/RoutePatternUsageDetector.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/RoutePatternUsageDetector.cs @@ -162,8 +162,9 @@ public static RoutePatternUsageContext BuildContext(SyntaxToken token, SemanticM return null; } + // Method has a delegate parameter. Could be Delegate or something that inherits from it, e.g. RequestDelegate. var delegateSymbol = semanticModel.Compilation.GetSpecialType(SpecialType.System_Delegate); - var delegateParameter = method.Parameters.FirstOrDefault(p => SymbolEqualityComparer.Default.Equals(delegateSymbol, p.Type)); + var delegateParameter = method.Parameters.FirstOrDefault(p => delegateSymbol.IsAssignableFrom(p.Type)); if (delegateParameter == null) { return null; diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryIndexerAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryIndexerAnalyzerTests.cs index 26bd95259cf7..51f1ee94e9ff 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryIndexerAnalyzerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryIndexerAnalyzerTests.cs @@ -31,7 +31,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.UseHeaderDictionaryPropertiesInsteadOfIndexer) .WithLocation(0) - .WithMessage("The header 'content-type' can be accessed using the ContentType property")); + .WithMessage(Resources.FormatAnalyzer_HeaderDictionaryIndexer_Message("content-type", "ContentType"))); } [Fact] @@ -52,7 +52,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.UseHeaderDictionaryPropertiesInsteadOfIndexer) .WithLocation(0) - .WithMessage("The header 'content-type' can be accessed using the ContentType property")); + .WithMessage(Resources.FormatAnalyzer_HeaderDictionaryIndexer_Message("content-type", "ContentType"))); } [Fact] @@ -150,7 +150,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.UseHeaderDictionaryPropertiesInsteadOfIndexer) .WithLocation(0) - .WithMessage("The header 'Content-Type' can be accessed using the ContentType property")); + .WithMessage(Resources.FormatAnalyzer_HeaderDictionaryIndexer_Message("Content-Type", "ContentType"))); } [Fact] diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/RequestDelegateReturnTypeAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/RequestDelegateReturnTypeAnalyzerTests.cs index fb8b62e7cb5f..2a1d9bb3933f 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/RequestDelegateReturnTypeAnalyzerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/RequestDelegateReturnTypeAnalyzerTests.cs @@ -10,9 +10,6 @@ namespace Microsoft.AspNetCore.Analyzers.Http; public class RequestDelegateReturnTypeAnalyzerTests { - private string GetMessage(string type) => - $"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."; - [Fact] public async Task AnonymousDelegate_RequestDelegate_ReturnType_EndpointCtor_ReportDiagnostics() { @@ -31,7 +28,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("System.DateTime"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("System.DateTime"))); } [Fact] @@ -50,7 +47,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("object?"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("object?"))); } [Fact] @@ -73,7 +70,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("string"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string"))); } [Fact] @@ -89,7 +86,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("string"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string"))); } [Fact] @@ -109,7 +106,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("string"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string"))); } [Fact] @@ -130,7 +127,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("string"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string"))); } [Fact] @@ -151,7 +148,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("string"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string"))); } [Fact] @@ -179,7 +176,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("string"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string"))); } [Fact] @@ -207,7 +204,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("int"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("int"))); } [Fact] @@ -299,7 +296,7 @@ await VerifyCS.VerifyAnalyzerAsync(@" ", new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate) .WithLocation(0) - .WithMessage(GetMessage("string"))); + .WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string"))); } [Fact] diff --git a/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RouteParameterUnusedParameterFixerTest.cs b/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RouteParameterUnusedParameterFixerTest.cs index ba3b655d1e81..413c7a2a4d36 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RouteParameterUnusedParameterFixerTest.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RouteParameterUnusedParameterFixerTest.cs @@ -389,6 +389,51 @@ static void Main() await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource, expectedIterations: 1); } + [Fact] + public async Task MapGet_UnusedParameter_AddToRequestDelegateLambda() + { + // Arrange + var source = @" +using System; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; + +class Program +{ + static void Main() + { + EndpointRouteBuilderExtensions.MapGet(null, @""{|#0:{id}|}"", (HttpContext context) => Task.CompletedTask); + } +} +"; + + var fixedSource = @" +using System; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; + +class Program +{ + static void Main() + { + EndpointRouteBuilderExtensions.MapGet(null, @""{id}"", (string id, HttpContext context) => Task.CompletedTask); + } +} +"; + + var expectedDiagnostics = new[] + { + new DiagnosticResult(DiagnosticDescriptors.RoutePatternUnusedParameter).WithArguments("id").WithLocation(0) + }; + + // Act & Assert + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource, expectedIterations: 1); + } + [Fact] public async Task MapGet_UnusedParameter_IntPolicy_AddIntToLambda() { diff --git a/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RoutePatternAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RoutePatternAnalyzerTests.cs index f2bed057ce16..7734ee2a6078 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RoutePatternAnalyzerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RoutePatternAnalyzerTests.cs @@ -40,7 +40,7 @@ class HttpGet : Attribute var diagnostic = Assert.Single(diagnostics); Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture)); } [Fact] @@ -73,7 +73,7 @@ public HttpGet([StringSyntax(""Route"")] string pattern) var diagnostic = Assert.Single(diagnostics); Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture)); } [Fact] @@ -101,7 +101,7 @@ static void Main() var diagnostic = Assert.Single(diagnostics); Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture)); } [Fact] @@ -129,7 +129,7 @@ static void Main() var diagnostic = Assert.Single(diagnostics); Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture)); } [Fact] @@ -158,7 +158,7 @@ static void M([StringSyntax(""Route"")] string p) var diagnostic = Assert.Single(diagnostics); Assert.Same(DiagnosticDescriptors.RoutePatternIssue, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), diagnostic.GetMessage(CultureInfo.InvariantCulture)); } [Fact] @@ -189,12 +189,12 @@ static void M([StringSyntax(""Route"")] string p) d => { Assert.Same(DiagnosticDescriptors.RoutePatternIssue, d.Descriptor); - Assert.Equal($"Route issue: {Resources.FormatTemplateRoute_InvalidLiteral("~hi?")}", d.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.FormatTemplateRoute_InvalidLiteral("~hi?")), d.GetMessage(CultureInfo.InvariantCulture)); }, d => { Assert.Same(DiagnosticDescriptors.RoutePatternIssue, d.Descriptor); - Assert.Equal($"Route issue: {Resources.TemplateRoute_InvalidRouteTemplate}", d.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.TemplateRoute_InvalidRouteTemplate), d.GetMessage(CultureInfo.InvariantCulture)); }); } @@ -256,7 +256,7 @@ public void TestAction() d => { Assert.Same(DiagnosticDescriptors.RoutePatternIssue, d.Descriptor); - Assert.Equal($"Route issue: {Resources.AttributeRoute_TokenReplacement_UnclosedToken}", d.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_RouteIssue_Message(Resources.AttributeRoute_TokenReplacement_UnclosedToken), d.GetMessage(CultureInfo.InvariantCulture)); }); } @@ -296,7 +296,7 @@ public object TestAction() d => { Assert.Same(DiagnosticDescriptors.RoutePatternUnusedParameter, d.Descriptor); - Assert.Equal("Unused route parameter 'id'", d.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_UnusedParameter_Message("id"), d.GetMessage(CultureInfo.InvariantCulture)); }); } @@ -376,7 +376,7 @@ public class PageData d => { Assert.Same(DiagnosticDescriptors.RoutePatternUnusedParameter, d.Descriptor); - Assert.Equal("Unused route parameter 'id'", d.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal(Resources.FormatAnalyzer_UnusedParameter_Message("id"), d.GetMessage(CultureInfo.InvariantCulture)); }); }