Skip to content

Analyzer clean up #44393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 6, 2022
Merged

Analyzer clean up #44393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3ae393
Analyzer clean up
JamesNK Oct 6, 2022
7406439
Support RequestDelegate
JamesNK Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -127,27 +118,36 @@ 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,
helpLinkUri: "https://aka.ms/aspnet/analyzers");

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");
Expand Down
30 changes: 21 additions & 9 deletions src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,6 @@
<data name="TemplateRoute_Exception" xml:space="preserve">
<value>An error occurred while creating the route with name '{0}' and template '{1}'.</value>
</data>
<data name="Invalid_Route_pattern" xml:space="preserve">
<value>Invalid route pattern</value>
</data>
<data name="Route_issue_0" xml:space="preserve">
<value>Route issue: {0}</value>
</data>
<data name="AttributeRoute_TokenReplacement_EmptyTokenNotAllowed" xml:space="preserve">
<value>An empty replacement token ('[]') is not allowed.</value>
</data>
Expand All @@ -183,10 +177,28 @@
<data name="AttributeRoute_TokenReplacement_UnescapedBraceInToken" xml:space="preserve">
<value>An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape.</value>
</data>
<data name="Unused_Route_parameter" xml:space="preserve">
<value>Unused route parameter</value>
<data name="Analyzer_HeaderDictionaryIndexer_Message" xml:space="preserve">
<value>The header '{0}' can be accessed using the {1} property</value>
</data>
<data name="Analyzer_HeaderDictionaryIndexer_Title" xml:space="preserve">
<value>Suggest using IHeaderDictionary properties</value>
</data>
<data name="Analyzer_RequestDelegateReturnValue_Message" xml:space="preserve">
<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>
</data>
<data name="Unused_Route_parameter_0" xml:space="preserve">
<data name="Analyzer_RequestDelegateReturnValue_Title" xml:space="preserve">
<value>Do not return a value from RequestDelegate</value>
</data>
<data name="Analyzer_RouteIssue_Message" xml:space="preserve">
<value>Route issue: {0}</value>
</data>
<data name="Analyzer_RouteIssue_Title" xml:space="preserve">
<value>Invalid route pattern</value>
</data>
<data name="Analyzer_UnusedParameter_Message" xml:space="preserve">
<value>Unused route parameter '{0}'</value>
</data>
<data name="Analyzer_UnusedParameter_Title" xml:space="preserve">
<value>Unused route parameter</value>
</data>
</root>
3 changes: 2 additions & 1 deletion ...Analyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/RoutePatternUsageDetector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryIndexerAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
23 changes: 10 additions & 13 deletions src/Framework/AspNetCoreAnalyzers/test/Http/RequestDelegateReturnTypeAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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]
Expand All @@ -50,7 +47,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("object?")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("object?")));
}

[Fact]
Expand All @@ -73,7 +70,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("string")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
}

[Fact]
Expand All @@ -89,7 +86,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("string")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
}

[Fact]
Expand All @@ -109,7 +106,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("string")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
}

[Fact]
Expand All @@ -130,7 +127,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("string")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
}

[Fact]
Expand All @@ -151,7 +148,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("string")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
}

[Fact]
Expand Down Expand Up @@ -179,7 +176,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("string")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
}

[Fact]
Expand Down Expand Up @@ -207,7 +204,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("int")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("int")));
}

[Fact]
Expand Down Expand Up @@ -299,7 +296,7 @@ await VerifyCS.VerifyAnalyzerAsync(@"
",
new DiagnosticResult(DiagnosticDescriptors.DoNotReturnValueFromRequestDelegate)
.WithLocation(0)
.WithMessage(GetMessage("string")));
.WithMessage(Resources.FormatAnalyzer_RequestDelegateReturnValue_Message("string")));
}

[Fact]
Expand Down
45 changes: 45 additions & 0 deletions .../AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RouteParameterUnusedParameterFixerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
20 changes: 10 additions & 10 deletions src/Framework/AspNetCoreAnalyzers/test/RouteEmbeddedLanguage/RoutePatternAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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));
});
}

Expand Down Expand Up @@ -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));
});
}

Expand Down Expand Up @@ -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));
});
}

Expand Down Expand Up @@ -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));
});
}

Expand Down