From 237a63ea8bbb6b277fb98b0d5763878ce3dd00d1 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 9 Jul 2023 17:49:37 -0400 Subject: [PATCH 01/16] Add diagnostic --- .../Analyzers/src/StartupAnalyzer.Diagnostics.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs index 45cf4ed7afbe..747e439f352f 100644 --- a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs +++ b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs @@ -39,11 +39,21 @@ internal static class Diagnostics isEnabledByDefault: true, helpLinkUri: "https://aka.ms/AA64fv1"); + internal static readonly DiagnosticDescriptor IncorrectlyConfiguredProblemDetailsWriter = new( + "ASP0026", + "Custom IProblemDetailsWriter is incorrectly configured", + "The custom IProblemDetailsWriter must be registered before calling AddControllers, AddControllersWithViews, or AddRazorPages.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); + public static readonly ImmutableArray SupportedDiagnostics = ImmutableArray.Create(new[] { // ASP BuildServiceProviderShouldNotCalledInConfigureServicesMethod, IncorrectlyConfiguredAuthorizationMiddleware, + IncorrectlyConfiguredProblemDetailsWriter, // MVC UnsupportedUseMvcWithEndpointRouting, From 1839ce71a8c5f0c89269fa5611c0c6814ee61dda Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 9 Jul 2023 17:57:58 -0400 Subject: [PATCH 02/16] Include AddMvc in diagnostic message --- src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs index 747e439f352f..af1cdf702828 100644 --- a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs +++ b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs @@ -42,7 +42,7 @@ internal static class Diagnostics internal static readonly DiagnosticDescriptor IncorrectlyConfiguredProblemDetailsWriter = new( "ASP0026", "Custom IProblemDetailsWriter is incorrectly configured", - "The custom IProblemDetailsWriter must be registered before calling AddControllers, AddControllersWithViews, or AddRazorPages.", + "The custom IProblemDetailsWriter must be registered before calling AddControllers, AddControllersWithViews, AddMvc, or AddRazorPages.", "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, From 1beee755570e8a7458e72c0164328540bc00cba2 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 9 Jul 2023 17:58:40 -0400 Subject: [PATCH 03/16] Add symbol names for MvcServiceCollectionExtensions --- src/Analyzers/Analyzers/src/SymbolNames.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Analyzers/Analyzers/src/SymbolNames.cs b/src/Analyzers/Analyzers/src/SymbolNames.cs index af767a9ece1e..9f390933a0bc 100644 --- a/src/Analyzers/Analyzers/src/SymbolNames.cs +++ b/src/Analyzers/Analyzers/src/SymbolNames.cs @@ -48,4 +48,17 @@ public static class MvcOptions public const string EnableEndpointRoutingPropertyName = "EnableEndpointRouting"; } + + public static class MvcServiceCollectionExtensions + { + public const string MetadataName = "Microsoft.Extensions.DependencyInjection.MvcServiceCollectionExtensions"; + + public const string AddControllersMethodName = "AddControllers"; + + public const string AddControllersWithViewsMethodName = "AddControllersWithViews"; + + public const string AddMvcMethodName = "AddMvc"; + + public const string AddRazorPagesMethodName = "AddRazorPages"; + } } From 98503310845059bd4a286316580ea907ef05b21a Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 9 Jul 2023 21:06:30 -0400 Subject: [PATCH 04/16] Add IProblemDetailsWriterAnalyzer --- .../src/ProblemDetailsWriterAnalyzer.cs | 109 ++++++++++++++++ .../Analyzers/src/StartupAnalyzer.cs | 14 +- src/Analyzers/Analyzers/src/SymbolNames.cs | 16 ++- .../Analyzers/test/StartupAnalyzerTest.cs | 120 ++++++++++++++++++ 4 files changed, 250 insertions(+), 9 deletions(-) create mode 100644 src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs diff --git a/src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs b/src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs new file mode 100644 index 000000000000..4b7072299850 --- /dev/null +++ b/src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs @@ -0,0 +1,109 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers; + +internal sealed class ProblemDetailsWriterAnalyzer +{ + private readonly StartupAnalysis _context; + + public ProblemDetailsWriterAnalyzer(StartupAnalysis context) + { + _context = context; + } + + public void AnalyzeSymbol(SymbolAnalysisContext context) + { + Debug.Assert(context.Symbol.Kind == SymbolKind.NamedType); + + var type = (INamedTypeSymbol)context.Symbol; + + var serviceAnalyses = _context.GetRelatedAnalyses(type); + if (serviceAnalyses == null) + { + return; + } + + foreach (var serviceAnalysis in serviceAnalyses) + { + var mvcServiceItems = serviceAnalysis.Services + .Where(IsMvcServiceCollectionExtension) + .ToArray(); + + if (mvcServiceItems.Length == 0) + { + continue; + } + + var problemDetailsWriterServiceItems = serviceAnalysis.Services + .Where(IsProblemDetailsWriterRegistration) + .ToArray(); + + if (problemDetailsWriterServiceItems.Length == 0) + { + continue; + } + + var mvcServiceTextSpans = mvcServiceItems.Select(x => x.Operation.Syntax.Span); + + foreach (var problemDetailsWriterServiceItem in problemDetailsWriterServiceItems) + { + var problemDetailsWriterServiceTextSpan = problemDetailsWriterServiceItem.Operation.Syntax.Span; + + foreach (var mvcServiceTextSpan in mvcServiceTextSpans) + { + // Check if the IProblemDetailsWriter registration is after the MVC registration in the source. + if (problemDetailsWriterServiceTextSpan.CompareTo(mvcServiceTextSpan) > 0) + { + context.ReportDiagnostic(Diagnostic.Create( + StartupAnalyzer.Diagnostics.IncorrectlyConfiguredProblemDetailsWriter, + problemDetailsWriterServiceItem.Operation.Syntax.GetLocation())); + + break; + } + } + } + } + } + + private static bool IsMvcServiceCollectionExtension(ServicesItem middlewareItem) + { + var methodName = middlewareItem.UseMethod.Name; + + if (string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddControllersMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddControllersWithViewsMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddMvcMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddRazorPagesMethodName, StringComparison.Ordinal)) + { + return true; + } + + return false; + } + + private static bool IsProblemDetailsWriterRegistration(ServicesItem servicesItem) + { + var methodName = servicesItem.UseMethod.Name; + + if (string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddTransientMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddScopedMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddSingletonMethodName, StringComparison.Ordinal)) + { + var typeArguments = servicesItem.Operation.TargetMethod.TypeArguments; + + if (typeArguments.Length == 2 + && string.Equals(typeArguments[0].Name, SymbolNames.IProblemDetailsWriter.Name, StringComparison.Ordinal)) + { + return true; + } + } + + return false; + } +} diff --git a/src/Analyzers/Analyzers/src/StartupAnalyzer.cs b/src/Analyzers/Analyzers/src/StartupAnalyzer.cs index 6daf4f6b2720..94bf6f8a4571 100644 --- a/src/Analyzers/Analyzers/src/StartupAnalyzer.cs +++ b/src/Analyzers/Analyzers/src/StartupAnalyzer.cs @@ -5,7 +5,6 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Operations; namespace Microsoft.AspNetCore.Analyzers; @@ -91,12 +90,13 @@ private void OnCompilationStart(CompilationStartAnalysisContext context) // Run after analyses have had a chance to finish to add diagnostics. context.RegisterSymbolEndAction(context => - { - var analysis = builder.Build(); - new UseMvcAnalyzer(analysis).AnalyzeSymbol(context); - new BuildServiceProviderAnalyzer(analysis).AnalyzeSymbol(context); - new UseAuthorizationAnalyzer(analysis).AnalyzeSymbol(context); - }); + { + var analysis = builder.Build(); + new UseMvcAnalyzer(analysis).AnalyzeSymbol(context); + new BuildServiceProviderAnalyzer(analysis).AnalyzeSymbol(context); + new UseAuthorizationAnalyzer(analysis).AnalyzeSymbol(context); + new ProblemDetailsWriterAnalyzer(analysis).AnalyzeSymbol(context); + }); }, SymbolKind.NamedType); } diff --git a/src/Analyzers/Analyzers/src/SymbolNames.cs b/src/Analyzers/Analyzers/src/SymbolNames.cs index 9f390933a0bc..45ce48a0f8fc 100644 --- a/src/Analyzers/Analyzers/src/SymbolNames.cs +++ b/src/Analyzers/Analyzers/src/SymbolNames.cs @@ -51,8 +51,6 @@ public static class MvcOptions public static class MvcServiceCollectionExtensions { - public const string MetadataName = "Microsoft.Extensions.DependencyInjection.MvcServiceCollectionExtensions"; - public const string AddControllersMethodName = "AddControllers"; public const string AddControllersWithViewsMethodName = "AddControllersWithViews"; @@ -61,4 +59,18 @@ public static class MvcServiceCollectionExtensions public const string AddRazorPagesMethodName = "AddRazorPages"; } + + public static class ServiceCollectionServiceExtensions + { + public const string AddTransientMethodName = "AddTransient"; + + public const string AddScopedMethodName = "AddScoped"; + + public const string AddSingletonMethodName = "AddSingleton"; + } + + public static class IProblemDetailsWriter + { + public const string Name = "IProblemDetailsWriter"; + } } diff --git a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs index 188352e795e1..66022d35adcf 100644 --- a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs +++ b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs @@ -651,6 +651,126 @@ public void Configure(IApplicationBuilder app) Assert.NotEmpty(middlewareAnalysis.Middleware); } + [Theory] + [InlineData("AddControllers")] + [InlineData("AddControllersWithViews")] + [InlineData("AddMvc")] + [InlineData("AddRazorPages")] + public async Task StartupAnalyzer_ProblemDetailsWriter_AfterMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.{methodName}(); + + {{|#0:services.AddTransient()|}}; + }} + }} + + {_sampleProblemDetailsWriterSource} +}}"; + + var diagnosticResult = new DiagnosticResult(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(0); + + // Act + Assert + await VerifyAnalyzerAsync(source, diagnosticResult); + } + + [Theory] + [InlineData("AddScoped")] + [InlineData("AddSingleton")] + public async Task StartupAnalyzer_ProblemDetailsWriter_OtherLifetimes_AfterMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.AddControllers(); + + {{|#0:services.{methodName}()|}}; + }} + }} + + {_sampleProblemDetailsWriterSource} +}}"; + + var diagnosticResult = new DiagnosticResult(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(0); + + // Act + Assert + await VerifyAnalyzerAsync(source, diagnosticResult); + } + + [Theory] + [InlineData("AddControllers")] + [InlineData("AddControllersWithViews")] + [InlineData("AddMvc")] + [InlineData("AddRazorPages")] + public async Task StartupAnalyzer_ProblemDetailsWriter_BeforeMvcServiceCollectionsExtension_ReportsNoDiagnostics(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.AddTransient(); + + services.{methodName}(); + }} + }} + + {_sampleProblemDetailsWriterSource} +}}"; + + // Act + await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); + + // Assert + var middlewareAnalysis = Analyses.OfType().First(); + Assert.NotEmpty(middlewareAnalysis.Services); + } + + private const string _sampleProblemDetailsWriterSource = @" +public class SampleProblemDetailsWriter : IProblemDetailsWriter +{ + public bool CanWrite(ProblemDetailsContext context) + => context.HttpContext.Response.StatusCode == 400; + + public ValueTask WriteAsync(ProblemDetailsContext context) + { + var response = context.HttpContext.Response; + return new ValueTask(response.WriteAsJsonAsync(context.ProblemDetails)); + } +}"; + private Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) { var test = new StartupCSharpAnalyzerTest(StartupAnalyzer, TestReferences.MetadataReferences) From 6f4a4b914ec4f2421bf2d969d1b3cf2135479872 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 5 May 2024 12:58:24 -0400 Subject: [PATCH 05/16] Move ProblemDetailsWriterAnalyzer, StartupAnalyzer infrastructure to Framework/AspNetCoreAnalyzers --- Directory.Build.props | 1 + src/Analyzers/Analyzers/src/StartupSymbols.cs | 6 +- .../src/Analyzers/DiagnosticDescriptors.cs | 9 + .../Microsoft.AspNetCore.App.Analyzers.csproj | 13 ++ .../src/Analyzers/Resources.resx | 10 +- .../Analyzers/Startup/MiddlewareAnalyzer.cs | 47 ++++++ .../src/Analyzers/Startup/OptionsAnalyzer.cs | 45 +++++ .../Startup/ProblemDetailsWriterAnalyzer.cs | 109 ++++++++++++ .../src/Analyzers/Startup/ServicesAnalyzer.cs | 42 +++++ .../Startup/StartupAnalysisBuilder.cs | 94 +++++++++++ .../Startup/StartupAnalyzer.Events.cs | 47 ++++++ .../src/Analyzers/Startup/StartupAnalyzer.cs | 102 ++++++++++++ .../src/Analyzers/Startup/SymbolNames.cs | 37 +++++ .../src/CodeFixes/CodeFixes.sln | 25 +++ ...osoft.AspNetCore.App.Analyzers.Test.csproj | 5 + .../test/Startup/StartupAnalyzerTests.cs | 157 ++++++++++++++++++ .../test/Startup/StartupCSharpAnalyzerTest.cs | 24 +++ 17 files changed, 768 insertions(+), 5 deletions(-) create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalysisBuilder.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.Events.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/CodeFixes.sln create mode 100644 src/Framework/AspNetCoreAnalyzers/test/Startup/StartupAnalyzerTests.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs diff --git a/Directory.Build.props b/Directory.Build.props index a727412840a4..49cf8a2406d2 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -140,6 +140,7 @@ $(MSBuildThisFileDirectory)src\Shared\ + $(MSBuildThisFileDirectory)src\Analyzers\Analyzers\src\ $(RepoRoot)src\submodules\googletest\ diff --git a/src/Analyzers/Analyzers/src/StartupSymbols.cs b/src/Analyzers/Analyzers/src/StartupSymbols.cs index 943527586267..b801a1c1ebf1 100644 --- a/src/Analyzers/Analyzers/src/StartupSymbols.cs +++ b/src/Analyzers/Analyzers/src/StartupSymbols.cs @@ -9,9 +9,9 @@ internal sealed class StartupSymbols { public StartupSymbols(Compilation compilation) { - IApplicationBuilder = compilation.GetTypeByMetadataName(SymbolNames.IApplicationBuilder.MetadataName); - IServiceCollection = compilation.GetTypeByMetadataName(SymbolNames.IServiceCollection.MetadataName); - MvcOptions = compilation.GetTypeByMetadataName(SymbolNames.MvcOptions.MetadataName); + IApplicationBuilder = compilation.GetTypeByMetadataName(SymbolNames.IApplicationBuilder.MetadataName)!; + IServiceCollection = compilation.GetTypeByMetadataName(SymbolNames.IServiceCollection.MetadataName)!; + MvcOptions = compilation.GetTypeByMetadataName(SymbolNames.MvcOptions.MetadataName)!; } public bool HasRequiredSymbols => IApplicationBuilder != null && IServiceCollection != null; diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs index 85bc320937b6..01d9be55e2bc 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs @@ -214,4 +214,13 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Info, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/aspnet/analyzers"); + + internal static readonly DiagnosticDescriptor IncorrectlyConfiguredProblemDetailsWriter = new( + "ASP0026", + new LocalizableResourceString(nameof(Resources.Analyzer_IncorrectlyConfiguredProblemDetailsWriter_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.Analyzer_IncorrectlyConfiguredProblemDetailsWriter_Message), Resources.ResourceManager, typeof(Resources)), + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj index 97439d9a3d20..88cebb986417 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj @@ -32,4 +32,17 @@ + + + + + + + + + + + + + diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 9c20abc6939c..c30c983d3855 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -309,10 +309,16 @@ Route '{0}' conflicts with another action route. An HTTP request that matches multiple routes results in an ambiguous match error. Fix the conflict by changing the route's pattern, HTTP method, or route constraints. + + Use AddAuthorizationBuilder + Use AddAuthorizationBuilder to register authorization services and construct policies - - Use AddAuthorizationBuilder + + Custom IProblemDetailsWriter is incorrectly configured + + + The custom IProblemDetailsWriter must be registered before calling AddControllers, AddControllersWithViews, AddMvc, or AddRazorPages \ No newline at end of file diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs new file mode 100644 index 000000000000..bb8b099aa6cb --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +internal sealed class MiddlewareAnalyzer +{ + private readonly StartupAnalysisBuilder _context; + + public MiddlewareAnalyzer(StartupAnalysisBuilder context) + { + _context = context; + } + + public void AnalyzeConfigureMethod(OperationBlockStartAnalysisContext context) + { + var configureMethod = (IMethodSymbol)context.OwningSymbol; + var middleware = ImmutableArray.CreateBuilder(); + + // Note: this is a simple source-order implementation. We don't attempt perform data flow + // analysis in order to determine the actual order in which middleware are ordered. + // + // This can currently be confused by things like Map(...) + context.RegisterOperationAction(context => + { + // We're looking for usage of extension methods, so we need to look at the 'this' parameter + // rather than invocation.Instance. + if (context.Operation is IInvocationOperation invocation && + invocation.Instance == null && + invocation.Arguments.Length >= 1 && + SymbolEqualityComparer.Default.Equals(invocation.Arguments[0].Parameter?.Type, _context.StartupSymbols.IApplicationBuilder)) + { + middleware.Add(new MiddlewareItem(invocation)); + } + }, OperationKind.Invocation); + + context.RegisterOperationBlockEndAction(context => + { + _context.ReportAnalysis(new MiddlewareAnalysis(configureMethod, middleware.ToImmutable())); + }); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs new file mode 100644 index 000000000000..17d3ab51f8d6 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +internal sealed class OptionsAnalyzer +{ + private readonly StartupAnalysisBuilder _context; + + public OptionsAnalyzer(StartupAnalysisBuilder context) + { + _context = context; + } + + public void AnalyzeConfigureServices(OperationBlockStartAnalysisContext context) + { + var configureServicesMethod = (IMethodSymbol)context.OwningSymbol; + var options = ImmutableArray.CreateBuilder(); + context.RegisterOperationAction(context => + { + if (context.Operation is ISimpleAssignmentOperation operation && + operation.Value.ConstantValue.HasValue && + // For nullable types, it's possible for Value to be null when HasValue is true. + operation.Value.ConstantValue.Value != null && + operation.Target is IPropertyReferenceOperation property && + property.Property?.ContainingType?.Name != null && + property.Property.ContainingType.Name.EndsWith("Options", StringComparison.Ordinal)) + { + options.Add(new OptionsItem(property.Property, operation.Value.ConstantValue.Value)); + } + + }, OperationKind.SimpleAssignment); + + context.RegisterOperationBlockEndAction(context => + { + _context.ReportAnalysis(new OptionsAnalysis(configureServicesMethod, options.ToImmutable())); + }); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs new file mode 100644 index 000000000000..837a6bbdb60d --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs @@ -0,0 +1,109 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +internal sealed class ProblemDetailsWriterAnalyzer +{ + private readonly StartupAnalysis _context; + + public ProblemDetailsWriterAnalyzer(StartupAnalysis context) + { + _context = context; + } + + public void AnalyzeSymbol(SymbolAnalysisContext context) + { + Debug.Assert(context.Symbol.Kind == SymbolKind.NamedType); + + var type = (INamedTypeSymbol)context.Symbol; + + var serviceAnalyses = _context.GetRelatedAnalyses(type); + if (serviceAnalyses == null) + { + return; + } + + foreach (var serviceAnalysis in serviceAnalyses) + { + var mvcServiceItems = serviceAnalysis.Services + .Where(IsMvcServiceCollectionExtension) + .ToArray(); + + if (mvcServiceItems.Length == 0) + { + continue; + } + + var problemDetailsWriterServiceItems = serviceAnalysis.Services + .Where(IsProblemDetailsWriterRegistration) + .ToArray(); + + if (problemDetailsWriterServiceItems.Length == 0) + { + continue; + } + + var mvcServiceTextSpans = mvcServiceItems.Select(x => x.Operation.Syntax.Span); + + foreach (var problemDetailsWriterServiceItem in problemDetailsWriterServiceItems) + { + var problemDetailsWriterServiceTextSpan = problemDetailsWriterServiceItem.Operation.Syntax.Span; + + foreach (var mvcServiceTextSpan in mvcServiceTextSpans) + { + // Check if the IProblemDetailsWriter registration is after the MVC registration in the source. + if (problemDetailsWriterServiceTextSpan.CompareTo(mvcServiceTextSpan) > 0) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter, + problemDetailsWriterServiceItem.Operation.Syntax.GetLocation())); + + break; + } + } + } + } + } + + private static bool IsMvcServiceCollectionExtension(ServicesItem middlewareItem) + { + var methodName = middlewareItem.UseMethod.Name; + + if (string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddControllersMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddControllersWithViewsMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddMvcMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddRazorPagesMethodName, StringComparison.Ordinal)) + { + return true; + } + + return false; + } + + private static bool IsProblemDetailsWriterRegistration(ServicesItem servicesItem) + { + var methodName = servicesItem.UseMethod.Name; + + if (string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddTransientMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddScopedMethodName, StringComparison.Ordinal) + || string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddSingletonMethodName, StringComparison.Ordinal)) + { + var typeArguments = servicesItem.Operation.TargetMethod.TypeArguments; + + if (typeArguments.Length == 2 + && string.Equals(typeArguments[0].Name, SymbolNames.IProblemDetailsWriter.Name, StringComparison.Ordinal)) + { + return true; + } + } + + return false; + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs new file mode 100644 index 000000000000..41718b8ab7d5 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +internal sealed class ServicesAnalyzer +{ + private readonly StartupAnalysisBuilder _context; + + public ServicesAnalyzer(StartupAnalysisBuilder context) + { + _context = context; + } + + public void AnalyzeConfigureServices(OperationBlockStartAnalysisContext context) + { + var configureServicesMethod = (IMethodSymbol)context.OwningSymbol; + var services = ImmutableArray.CreateBuilder(); + context.RegisterOperationAction(context => + { + // We're looking for usage of extension methods, so we need to look at the 'this' parameter + // rather than invocation.Instance. + if (context.Operation is IInvocationOperation invocation && + invocation.Instance == null && + invocation.Arguments.Length >= 1 && + SymbolEqualityComparer.Default.Equals(invocation.Arguments[0].Parameter?.Type, _context.StartupSymbols.IServiceCollection)) + { + services.Add(new ServicesItem(invocation)); + } + }, OperationKind.Invocation); + + context.RegisterOperationBlockEndAction(context => + { + _context.ReportAnalysis(new ServicesAnalysis(configureServicesMethod, services.ToImmutable())); + }); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalysisBuilder.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalysisBuilder.cs new file mode 100644 index 000000000000..e59b1cfe352c --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalysisBuilder.cs @@ -0,0 +1,94 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +internal sealed class StartupAnalysisBuilder +{ + private readonly Dictionary> _analysesByType; + private readonly StartupAnalyzer _analyzer; + private readonly object _lock; + + public StartupAnalysisBuilder(StartupAnalyzer analyzer, StartupSymbols startupSymbols) + { + _analyzer = analyzer; + StartupSymbols = startupSymbols; + +#pragma warning disable RS1024 // Compare symbols correctly + _analysesByType = new Dictionary>(SymbolEqualityComparer.Default); +#pragma warning restore RS1024 // Compare symbols correctly + _lock = new object(); + } + + public StartupSymbols StartupSymbols { get; } + + public StartupAnalysis Build() + { + lock (_lock) + { + return new StartupAnalysis( + StartupSymbols, + _analysesByType.ToImmutableDictionary( + k => k.Key, + v => v.Value.ToImmutableArray(), + // Custom equality comparer as SymbolEqualityComparer.Default will upcast to ISymbol. + new NamedTypeSymbolEqualityComparer(SymbolEqualityComparer.Default))); + } + } + + public void ReportAnalysis(ServicesAnalysis analysis) + { + ReportAnalysisCore(analysis.StartupType, analysis); + _analyzer.OnServicesAnalysisCompleted(analysis); + } + + public void ReportAnalysis(OptionsAnalysis analysis) + { + ReportAnalysisCore(analysis.StartupType, analysis); + _analyzer.OnOptionsAnalysisCompleted(analysis); + } + + public void ReportAnalysis(MiddlewareAnalysis analysis) + { + ReportAnalysisCore(analysis.StartupType, analysis); + _analyzer.OnMiddlewareAnalysisCompleted(analysis); + } + + private void ReportAnalysisCore(INamedTypeSymbol type, object analysis) + { + lock (_lock) + { + if (!_analysesByType.TryGetValue(type, out var list)) + { + list = new List(); + _analysesByType.Add(type, list); + } + + list.Add(analysis); + } + } + + private class NamedTypeSymbolEqualityComparer : IEqualityComparer + { + private readonly SymbolEqualityComparer _baseComparer; + + public NamedTypeSymbolEqualityComparer(SymbolEqualityComparer baseComparer) + { + _baseComparer = baseComparer; + } + + public bool Equals(INamedTypeSymbol x, INamedTypeSymbol y) + { + return _baseComparer.Equals(x, y); + } + + public int GetHashCode(INamedTypeSymbol obj) + { + return _baseComparer.GetHashCode(obj); + } + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.Events.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.Events.cs new file mode 100644 index 000000000000..f03fc8b1ac0d --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.Events.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +// Events for testability. Allows us to unit test the data we gather from analysis. +public partial class StartupAnalyzer : DiagnosticAnalyzer +{ + internal event EventHandler? ConfigureServicesMethodFound; + + internal void OnConfigureServicesMethodFound(IMethodSymbol method) + { + ConfigureServicesMethodFound?.Invoke(this, method); + } + + internal event EventHandler? ServicesAnalysisCompleted; + + internal void OnServicesAnalysisCompleted(ServicesAnalysis analysis) + { + ServicesAnalysisCompleted?.Invoke(this, analysis); + } + + internal event EventHandler? OptionsAnalysisCompleted; + + internal void OnOptionsAnalysisCompleted(OptionsAnalysis analysis) + { + OptionsAnalysisCompleted?.Invoke(this, analysis); + } + + internal event EventHandler? ConfigureMethodFound; + + internal void OnConfigureMethodFound(IMethodSymbol method) + { + ConfigureMethodFound?.Invoke(this, method); + } + + internal event EventHandler? MiddlewareAnalysisCompleted; + + internal void OnMiddlewareAnalysisCompleted(MiddlewareAnalysis analysis) + { + MiddlewareAnalysisCompleted?.Invoke(this, analysis); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.cs new file mode 100644 index 000000000000..4ab547323371 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/StartupAnalyzer.cs @@ -0,0 +1,102 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public partial class StartupAnalyzer : DiagnosticAnalyzer +{ + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create( + DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter + ); + + public override void Initialize(AnalysisContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private void OnCompilationStart(CompilationStartAnalysisContext context) + { + var symbols = new StartupSymbols(context.Compilation); + + // Don't run analyzer if ASP.NET Core types cannot be found + if (!symbols.HasRequiredSymbols) + { + return; + } + + var entryPoint = context.Compilation.GetEntryPoint(context.CancellationToken); + + context.RegisterSymbolStartAction(context => + { + var type = (INamedTypeSymbol)context.Symbol; + if (!StartupFacts.IsStartupClass(symbols, type) && !SymbolEqualityComparer.Default.Equals(entryPoint?.ContainingType, type)) + { + // Not a startup class, nothing to do. + return; + } + + // This analyzer fans out a bunch of jobs. The context will capture the results of doing analysis + // on the startup code, so that other analyzers that run later can examine them. + var builder = new StartupAnalysisBuilder(this, symbols); + + var services = new ServicesAnalyzer(builder); + var options = new OptionsAnalyzer(builder); + var middleware = new MiddlewareAnalyzer(builder); + + context.RegisterOperationBlockStartAction(context => + { + if (context.OwningSymbol.Kind != SymbolKind.Method) + { + return; + } + + var method = (IMethodSymbol)context.OwningSymbol; + var isConfigureServices = StartupFacts.IsConfigureServices(symbols, method); + if (isConfigureServices) + { + OnConfigureServicesMethodFound(method); + } + + // In the future we can consider looking at more methods, but for now limit to Main, implicit Main, and Configure* methods + var isMain = SymbolEqualityComparer.Default.Equals(entryPoint, context.OwningSymbol); + + if (isConfigureServices || isMain) + { + services.AnalyzeConfigureServices(context); + options.AnalyzeConfigureServices(context); + } + + var isConfigure = StartupFacts.IsConfigure(symbols, method); + if (isConfigure) + { + OnConfigureMethodFound(method); + } + if (isConfigure || isMain) + { + middleware.AnalyzeConfigureMethod(context); + } + }); + + // Run after analyses have had a chance to finish to add diagnostics. + context.RegisterSymbolEndAction(context => + { + var analysis = builder.Build(); + new ProblemDetailsWriterAnalyzer(analysis).AnalyzeSymbol(context); + }); + + }, SymbolKind.NamedType); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs new file mode 100644 index 000000000000..741759d88f4c --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs @@ -0,0 +1,37 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +internal static class SymbolNames +{ + public static class MvcServiceCollectionExtensions + { + public const string AddControllersMethodName = "AddControllers"; + + public const string AddControllersWithViewsMethodName = "AddControllersWithViews"; + + public const string AddMvcMethodName = "AddMvc"; + + public const string AddRazorPagesMethodName = "AddRazorPages"; + } + + public static class ServiceCollectionServiceExtensions + { + public const string AddTransientMethodName = "AddTransient"; + + public const string AddScopedMethodName = "AddScoped"; + + public const string AddSingletonMethodName = "AddSingleton"; + } + + public static class IProblemDetailsWriter + { + public const string Name = "IProblemDetailsWriter"; + } +} \ No newline at end of file diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/CodeFixes.sln b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/CodeFixes.sln new file mode 100644 index 000000000000..f781fa9305fb --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/CodeFixes.sln @@ -0,0 +1,25 @@ + +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio Version 17 +VisualStudioVersion = 17.5.002.0 +MinimumVisualStudioVersion = 10.0.40219.1 +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.App.CodeFixes", "Microsoft.AspNetCore.App.CodeFixes.csproj", "{F932DB57-A17C-4F2C-BC27-12AFEDBF09B4}" +EndProject +Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution + Debug|Any CPU = Debug|Any CPU + Release|Any CPU = Release|Any CPU + EndGlobalSection + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {F932DB57-A17C-4F2C-BC27-12AFEDBF09B4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {F932DB57-A17C-4F2C-BC27-12AFEDBF09B4}.Debug|Any CPU.Build.0 = Debug|Any CPU + {F932DB57-A17C-4F2C-BC27-12AFEDBF09B4}.Release|Any CPU.ActiveCfg = Release|Any CPU + {F932DB57-A17C-4F2C-BC27-12AFEDBF09B4}.Release|Any CPU.Build.0 = Release|Any CPU + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE + EndGlobalSection + GlobalSection(ExtensibilityGlobals) = postSolution + SolutionGuid = {D1B1613F-5749-4FB4-B736-E00351198B1E} + EndGlobalSection +EndGlobal diff --git a/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj index 8070e80c0b51..4257e9a7aae5 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj +++ b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj @@ -26,6 +26,11 @@ + + + + + diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupAnalyzerTests.cs new file mode 100644 index 000000000000..23ecb81825f6 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupAnalyzerTests.cs @@ -0,0 +1,157 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Concurrent; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +public sealed class StartupAnalyzerTests +{ + public StartupAnalyzerTests() + { + StartupAnalyzer = new StartupAnalyzer(); + + Analyses = new ConcurrentBag(); + ConfigureServicesMethods = new ConcurrentBag(); + ConfigureMethods = new ConcurrentBag(); + StartupAnalyzer.ServicesAnalysisCompleted += (sender, analysis) => Analyses.Add(analysis); + StartupAnalyzer.OptionsAnalysisCompleted += (sender, analysis) => Analyses.Add(analysis); + StartupAnalyzer.MiddlewareAnalysisCompleted += (sender, analysis) => Analyses.Add(analysis); + StartupAnalyzer.ConfigureServicesMethodFound += (sender, method) => ConfigureServicesMethods.Add(method); + StartupAnalyzer.ConfigureMethodFound += (sender, method) => ConfigureMethods.Add(method); + } + + internal StartupAnalyzer StartupAnalyzer { get; } + + internal ConcurrentBag Analyses { get; } + + internal ConcurrentBag ConfigureServicesMethods { get; } + + internal ConcurrentBag ConfigureMethods { get; } + + [Theory] + [InlineData("AddControllers")] + [InlineData("AddControllersWithViews")] + [InlineData("AddMvc")] + [InlineData("AddRazorPages")] + public async Task StartupAnalyzer_ProblemDetailsWriter_AfterMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.{methodName}(); + {{|#0:services.AddTransient()|}}; + }} + }} + {_sampleProblemDetailsWriterSource} +}}"; + + var diagnosticResult = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(0); + + // Act + Assert + await VerifyAnalyzerAsync(source, diagnosticResult); + } + + [Theory] + [InlineData("AddScoped")] + [InlineData("AddSingleton")] + public async Task StartupAnalyzer_ProblemDetailsWriter_OtherLifetimes_AfterMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.AddControllers(); + {{|#0:services.{methodName}()|}}; + }} + }} + {_sampleProblemDetailsWriterSource} +}}"; + + var diagnosticResult = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(0); + + // Act + Assert + await VerifyAnalyzerAsync(source, diagnosticResult); + } + + [Theory] + [InlineData("AddControllers")] + [InlineData("AddControllersWithViews")] + [InlineData("AddMvc")] + [InlineData("AddRazorPages")] + public async Task StartupAnalyzer_ProblemDetailsWriter_BeforeMvcServiceCollectionsExtension_ReportsNoDiagnostics(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.AddTransient(); + services.{methodName}(); + }} + }} + {_sampleProblemDetailsWriterSource} +}}"; + + // Act + await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); + + // Assert + var middlewareAnalysis = Analyses.OfType().First(); + Assert.NotEmpty(middlewareAnalysis.Services); + } + + private const string _sampleProblemDetailsWriterSource = @" +public class SampleProblemDetailsWriter : IProblemDetailsWriter +{ + public bool CanWrite(ProblemDetailsContext context) + => context.HttpContext.Response.StatusCode == 400; + public ValueTask WriteAsync(ProblemDetailsContext context) + { + var response = context.HttpContext.Response; + return new ValueTask(response.WriteAsJsonAsync(context.ProblemDetails)); + } +}"; + + private Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new StartupCSharpAnalyzerTest(StartupAnalyzer, TestReferences.MetadataReferences) + { + TestCode = source, + ReferenceAssemblies = TestReferences.EmptyReferenceAssemblies, + }; + // Tests are just the Configure/ConfigureServices methods, no Main, so we need to mark the output as not console + test.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; + + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } +} \ No newline at end of file diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs new file mode 100644 index 000000000000..1a9e6e9e8fc1 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing.Verifiers; + +namespace Microsoft.AspNetCore.Analyzers.Startup; + +internal sealed class StartupCSharpAnalyzerTest : CSharpAnalyzerTest +{ + public StartupCSharpAnalyzerTest(StartupAnalyzer analyzer, ImmutableArray metadataReferences) + { + StartupAnalyzer = analyzer; + TestState.OutputKind = OutputKind.WindowsApplication; + TestState.AdditionalReferences.AddRange(metadataReferences); + } + + public StartupAnalyzer StartupAnalyzer { get; } + + protected override IEnumerable GetDiagnosticAnalyzers() => new[] { StartupAnalyzer }; +} From 3243e6c3534c82cd3d68096a7e1799fa944a3f08 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 5 May 2024 13:02:34 -0400 Subject: [PATCH 06/16] Revert changes to legacy Analyzers project --- .../src/ProblemDetailsWriterAnalyzer.cs | 109 ---------------- .../src/StartupAnalyzer.Diagnostics.cs | 10 -- .../Analyzers/src/StartupAnalyzer.cs | 14 +- src/Analyzers/Analyzers/src/SymbolNames.cs | 25 ---- .../Analyzers/test/StartupAnalyzerTest.cs | 120 ------------------ 5 files changed, 7 insertions(+), 271 deletions(-) delete mode 100644 src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs diff --git a/src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs b/src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs deleted file mode 100644 index 4b7072299850..000000000000 --- a/src/Analyzers/Analyzers/src/ProblemDetailsWriterAnalyzer.cs +++ /dev/null @@ -1,109 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Diagnostics; -using System.Linq; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; - -namespace Microsoft.AspNetCore.Analyzers; - -internal sealed class ProblemDetailsWriterAnalyzer -{ - private readonly StartupAnalysis _context; - - public ProblemDetailsWriterAnalyzer(StartupAnalysis context) - { - _context = context; - } - - public void AnalyzeSymbol(SymbolAnalysisContext context) - { - Debug.Assert(context.Symbol.Kind == SymbolKind.NamedType); - - var type = (INamedTypeSymbol)context.Symbol; - - var serviceAnalyses = _context.GetRelatedAnalyses(type); - if (serviceAnalyses == null) - { - return; - } - - foreach (var serviceAnalysis in serviceAnalyses) - { - var mvcServiceItems = serviceAnalysis.Services - .Where(IsMvcServiceCollectionExtension) - .ToArray(); - - if (mvcServiceItems.Length == 0) - { - continue; - } - - var problemDetailsWriterServiceItems = serviceAnalysis.Services - .Where(IsProblemDetailsWriterRegistration) - .ToArray(); - - if (problemDetailsWriterServiceItems.Length == 0) - { - continue; - } - - var mvcServiceTextSpans = mvcServiceItems.Select(x => x.Operation.Syntax.Span); - - foreach (var problemDetailsWriterServiceItem in problemDetailsWriterServiceItems) - { - var problemDetailsWriterServiceTextSpan = problemDetailsWriterServiceItem.Operation.Syntax.Span; - - foreach (var mvcServiceTextSpan in mvcServiceTextSpans) - { - // Check if the IProblemDetailsWriter registration is after the MVC registration in the source. - if (problemDetailsWriterServiceTextSpan.CompareTo(mvcServiceTextSpan) > 0) - { - context.ReportDiagnostic(Diagnostic.Create( - StartupAnalyzer.Diagnostics.IncorrectlyConfiguredProblemDetailsWriter, - problemDetailsWriterServiceItem.Operation.Syntax.GetLocation())); - - break; - } - } - } - } - } - - private static bool IsMvcServiceCollectionExtension(ServicesItem middlewareItem) - { - var methodName = middlewareItem.UseMethod.Name; - - if (string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddControllersMethodName, StringComparison.Ordinal) - || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddControllersWithViewsMethodName, StringComparison.Ordinal) - || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddMvcMethodName, StringComparison.Ordinal) - || string.Equals(methodName, SymbolNames.MvcServiceCollectionExtensions.AddRazorPagesMethodName, StringComparison.Ordinal)) - { - return true; - } - - return false; - } - - private static bool IsProblemDetailsWriterRegistration(ServicesItem servicesItem) - { - var methodName = servicesItem.UseMethod.Name; - - if (string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddTransientMethodName, StringComparison.Ordinal) - || string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddScopedMethodName, StringComparison.Ordinal) - || string.Equals(methodName, SymbolNames.ServiceCollectionServiceExtensions.AddSingletonMethodName, StringComparison.Ordinal)) - { - var typeArguments = servicesItem.Operation.TargetMethod.TypeArguments; - - if (typeArguments.Length == 2 - && string.Equals(typeArguments[0].Name, SymbolNames.IProblemDetailsWriter.Name, StringComparison.Ordinal)) - { - return true; - } - } - - return false; - } -} diff --git a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs index af1cdf702828..45cf4ed7afbe 100644 --- a/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs +++ b/src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs @@ -39,21 +39,11 @@ internal static class Diagnostics isEnabledByDefault: true, helpLinkUri: "https://aka.ms/AA64fv1"); - internal static readonly DiagnosticDescriptor IncorrectlyConfiguredProblemDetailsWriter = new( - "ASP0026", - "Custom IProblemDetailsWriter is incorrectly configured", - "The custom IProblemDetailsWriter must be registered before calling AddControllers, AddControllersWithViews, AddMvc, or AddRazorPages.", - "Usage", - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - helpLinkUri: "https://aka.ms/aspnet/analyzers"); - public static readonly ImmutableArray SupportedDiagnostics = ImmutableArray.Create(new[] { // ASP BuildServiceProviderShouldNotCalledInConfigureServicesMethod, IncorrectlyConfiguredAuthorizationMiddleware, - IncorrectlyConfiguredProblemDetailsWriter, // MVC UnsupportedUseMvcWithEndpointRouting, diff --git a/src/Analyzers/Analyzers/src/StartupAnalyzer.cs b/src/Analyzers/Analyzers/src/StartupAnalyzer.cs index 94bf6f8a4571..6daf4f6b2720 100644 --- a/src/Analyzers/Analyzers/src/StartupAnalyzer.cs +++ b/src/Analyzers/Analyzers/src/StartupAnalyzer.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; namespace Microsoft.AspNetCore.Analyzers; @@ -90,13 +91,12 @@ private void OnCompilationStart(CompilationStartAnalysisContext context) // Run after analyses have had a chance to finish to add diagnostics. context.RegisterSymbolEndAction(context => - { - var analysis = builder.Build(); - new UseMvcAnalyzer(analysis).AnalyzeSymbol(context); - new BuildServiceProviderAnalyzer(analysis).AnalyzeSymbol(context); - new UseAuthorizationAnalyzer(analysis).AnalyzeSymbol(context); - new ProblemDetailsWriterAnalyzer(analysis).AnalyzeSymbol(context); - }); + { + var analysis = builder.Build(); + new UseMvcAnalyzer(analysis).AnalyzeSymbol(context); + new BuildServiceProviderAnalyzer(analysis).AnalyzeSymbol(context); + new UseAuthorizationAnalyzer(analysis).AnalyzeSymbol(context); + }); }, SymbolKind.NamedType); } diff --git a/src/Analyzers/Analyzers/src/SymbolNames.cs b/src/Analyzers/Analyzers/src/SymbolNames.cs index 45ce48a0f8fc..af767a9ece1e 100644 --- a/src/Analyzers/Analyzers/src/SymbolNames.cs +++ b/src/Analyzers/Analyzers/src/SymbolNames.cs @@ -48,29 +48,4 @@ public static class MvcOptions public const string EnableEndpointRoutingPropertyName = "EnableEndpointRouting"; } - - public static class MvcServiceCollectionExtensions - { - public const string AddControllersMethodName = "AddControllers"; - - public const string AddControllersWithViewsMethodName = "AddControllersWithViews"; - - public const string AddMvcMethodName = "AddMvc"; - - public const string AddRazorPagesMethodName = "AddRazorPages"; - } - - public static class ServiceCollectionServiceExtensions - { - public const string AddTransientMethodName = "AddTransient"; - - public const string AddScopedMethodName = "AddScoped"; - - public const string AddSingletonMethodName = "AddSingleton"; - } - - public static class IProblemDetailsWriter - { - public const string Name = "IProblemDetailsWriter"; - } } diff --git a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs index 66022d35adcf..188352e795e1 100644 --- a/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs +++ b/src/Analyzers/Analyzers/test/StartupAnalyzerTest.cs @@ -651,126 +651,6 @@ public void Configure(IApplicationBuilder app) Assert.NotEmpty(middlewareAnalysis.Middleware); } - [Theory] - [InlineData("AddControllers")] - [InlineData("AddControllersWithViews")] - [InlineData("AddMvc")] - [InlineData("AddRazorPages")] - public async Task StartupAnalyzer_ProblemDetailsWriter_AfterMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) - { - // Arrange - var source = $@" -using System.Threading.Tasks; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; - -namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest -{{ - public class ProblemDetailsWriterRegistration - {{ - public void ConfigureServices(IServiceCollection services) - {{ - services.{methodName}(); - - {{|#0:services.AddTransient()|}}; - }} - }} - - {_sampleProblemDetailsWriterSource} -}}"; - - var diagnosticResult = new DiagnosticResult(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredProblemDetailsWriter) - .WithLocation(0); - - // Act + Assert - await VerifyAnalyzerAsync(source, diagnosticResult); - } - - [Theory] - [InlineData("AddScoped")] - [InlineData("AddSingleton")] - public async Task StartupAnalyzer_ProblemDetailsWriter_OtherLifetimes_AfterMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) - { - // Arrange - var source = $@" -using System.Threading.Tasks; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; - -namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest -{{ - public class ProblemDetailsWriterRegistration - {{ - public void ConfigureServices(IServiceCollection services) - {{ - services.AddControllers(); - - {{|#0:services.{methodName}()|}}; - }} - }} - - {_sampleProblemDetailsWriterSource} -}}"; - - var diagnosticResult = new DiagnosticResult(StartupAnalyzer.Diagnostics.IncorrectlyConfiguredProblemDetailsWriter) - .WithLocation(0); - - // Act + Assert - await VerifyAnalyzerAsync(source, diagnosticResult); - } - - [Theory] - [InlineData("AddControllers")] - [InlineData("AddControllersWithViews")] - [InlineData("AddMvc")] - [InlineData("AddRazorPages")] - public async Task StartupAnalyzer_ProblemDetailsWriter_BeforeMvcServiceCollectionsExtension_ReportsNoDiagnostics(string methodName) - { - // Arrange - var source = $@" -using System.Threading.Tasks; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; - -namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest -{{ - public class ProblemDetailsWriterRegistration - {{ - public void ConfigureServices(IServiceCollection services) - {{ - services.AddTransient(); - - services.{methodName}(); - }} - }} - - {_sampleProblemDetailsWriterSource} -}}"; - - // Act - await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); - - // Assert - var middlewareAnalysis = Analyses.OfType().First(); - Assert.NotEmpty(middlewareAnalysis.Services); - } - - private const string _sampleProblemDetailsWriterSource = @" -public class SampleProblemDetailsWriter : IProblemDetailsWriter -{ - public bool CanWrite(ProblemDetailsContext context) - => context.HttpContext.Response.StatusCode == 400; - - public ValueTask WriteAsync(ProblemDetailsContext context) - { - var response = context.HttpContext.Response; - return new ValueTask(response.WriteAsJsonAsync(context.ProblemDetails)); - } -}"; - private Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) { var test = new StartupCSharpAnalyzerTest(StartupAnalyzer, TestReferences.MetadataReferences) From 5919473d589757d884eed8776e2a264a4f05e443 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 5 May 2024 19:40:38 -0400 Subject: [PATCH 07/16] Formatting, clean up --- .../src/Analyzers/Startup/MiddlewareAnalyzer.cs | 6 +++--- .../src/Analyzers/Startup/OptionsAnalyzer.cs | 1 + .../src/Analyzers/Startup/ServicesAnalyzer.cs | 7 ++++--- .../src/Analyzers/Startup/SymbolNames.cs | 7 +------ 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs index bb8b099aa6cb..64a9065d26cd 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/MiddlewareAnalyzer.cs @@ -31,9 +31,9 @@ public void AnalyzeConfigureMethod(OperationBlockStartAnalysisContext context) // We're looking for usage of extension methods, so we need to look at the 'this' parameter // rather than invocation.Instance. if (context.Operation is IInvocationOperation invocation && - invocation.Instance == null && - invocation.Arguments.Length >= 1 && - SymbolEqualityComparer.Default.Equals(invocation.Arguments[0].Parameter?.Type, _context.StartupSymbols.IApplicationBuilder)) + invocation.Instance == null && + invocation.Arguments.Length >= 1 && + SymbolEqualityComparer.Default.Equals(invocation.Arguments[0].Parameter?.Type, _context.StartupSymbols.IApplicationBuilder)) { middleware.Add(new MiddlewareItem(invocation)); } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs index 17d3ab51f8d6..fd442c8676e0 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/OptionsAnalyzer.cs @@ -22,6 +22,7 @@ public void AnalyzeConfigureServices(OperationBlockStartAnalysisContext context) { var configureServicesMethod = (IMethodSymbol)context.OwningSymbol; var options = ImmutableArray.CreateBuilder(); + context.RegisterOperationAction(context => { if (context.Operation is ISimpleAssignmentOperation operation && diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs index 41718b8ab7d5..debab455e34c 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ServicesAnalyzer.cs @@ -21,14 +21,15 @@ public void AnalyzeConfigureServices(OperationBlockStartAnalysisContext context) { var configureServicesMethod = (IMethodSymbol)context.OwningSymbol; var services = ImmutableArray.CreateBuilder(); + context.RegisterOperationAction(context => { // We're looking for usage of extension methods, so we need to look at the 'this' parameter // rather than invocation.Instance. if (context.Operation is IInvocationOperation invocation && - invocation.Instance == null && - invocation.Arguments.Length >= 1 && - SymbolEqualityComparer.Default.Equals(invocation.Arguments[0].Parameter?.Type, _context.StartupSymbols.IServiceCollection)) + invocation.Instance == null && + invocation.Arguments.Length >= 1 && + SymbolEqualityComparer.Default.Equals(invocation.Arguments[0].Parameter?.Type, _context.StartupSymbols.IServiceCollection)) { services.Add(new ServicesItem(invocation)); } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs index 741759d88f4c..9631965745c9 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/SymbolNames.cs @@ -1,11 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Immutable; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; - namespace Microsoft.AspNetCore.Analyzers.Startup; internal static class SymbolNames @@ -34,4 +29,4 @@ public static class IProblemDetailsWriter { public const string Name = "IProblemDetailsWriter"; } -} \ No newline at end of file +} From e483d13640da247c4fffaf0ad624dba6ab32e0a8 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sat, 8 Jun 2024 15:44:18 -0400 Subject: [PATCH 08/16] Add IncorrectlyConfiguredProblemDetailsWriterFixer --- .../Startup/ProblemDetailsWriterAnalyzer.cs | 10 +- ...ctlyConfiguredProblemDetailsWriterFixer.cs | 98 +++++++++++++++++++ ...rTests.cs => ProblemDetailsWriterTests.cs} | 77 ++++++++++++--- .../test/Startup/StartupCSharpAnalyzerTest.cs | 4 +- 4 files changed, 170 insertions(+), 19 deletions(-) create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs rename src/Framework/AspNetCoreAnalyzers/test/Startup/{StartupAnalyzerTests.cs => ProblemDetailsWriterTests.cs} (68%) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs index 837a6bbdb60d..83f2413cd4cd 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Startup/ProblemDetailsWriterAnalyzer.cs @@ -50,7 +50,7 @@ public void AnalyzeSymbol(SymbolAnalysisContext context) continue; } - var mvcServiceTextSpans = mvcServiceItems.Select(x => x.Operation.Syntax.Span); + var mvcServiceTextSpans = mvcServiceItems.ToDictionary(x => x, x => x.Operation.Syntax.Span); foreach (var problemDetailsWriterServiceItem in problemDetailsWriterServiceItems) { @@ -58,12 +58,16 @@ public void AnalyzeSymbol(SymbolAnalysisContext context) foreach (var mvcServiceTextSpan in mvcServiceTextSpans) { + var mvcService = mvcServiceTextSpan.Key; + var textSpan = mvcServiceTextSpan.Value; + // Check if the IProblemDetailsWriter registration is after the MVC registration in the source. - if (problemDetailsWriterServiceTextSpan.CompareTo(mvcServiceTextSpan) > 0) + if (problemDetailsWriterServiceTextSpan.CompareTo(textSpan) > 0) { context.ReportDiagnostic(Diagnostic.Create( DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter, - problemDetailsWriterServiceItem.Operation.Syntax.GetLocation())); + problemDetailsWriterServiceItem.Operation.Syntax.GetLocation(), + additionalLocations: [mvcService.Operation.Syntax.GetLocation()])); break; } diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs new file mode 100644 index 000000000000..781df4f0f014 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs @@ -0,0 +1,98 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using System.Composition; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Microsoft.AspNetCore.Analyzers.Startup.Fixers; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public sealed class IncorrectlyConfiguredProblemDetailsWriterFixer : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds { get; } = [DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter.Id]; + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root == null) + { + return; + } + + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (semanticModel == null) + { + return; + } + + foreach (var diagnostic in context.Diagnostics) + { + if (CanFixOrderOfProblemDetailsWriter(diagnostic, root, out var registerProblemDetailsWriterInvocation, out var mvcServiceInvocation)) + { + const string title = "Fix order of ProblemDetailsWriter registration"; + context.RegisterCodeFix( + CodeAction.Create( + title, + cancellationToken => FixOrderOfProblemDetailsWriter(root, context.Document, registerProblemDetailsWriterInvocation, mvcServiceInvocation), + equivalenceKey: DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter.Id), + diagnostic); + } + } + } + + private static bool CanFixOrderOfProblemDetailsWriter( + Diagnostic diagnostic, + SyntaxNode root, + [NotNullWhen(true)] out InvocationExpressionSyntax? registerProblemDetailsWriterInvocation, + [NotNullWhen(true)] out InvocationExpressionSyntax? mvcServiceInvocation) + { + registerProblemDetailsWriterInvocation = null; + mvcServiceInvocation = null; + + var registerProblemDetailsWriterInvocationTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); + if (registerProblemDetailsWriterInvocationTarget is not InvocationExpressionSyntax registerProblemDetailsWriterInvocationSyntax) + { + return false; + } + + registerProblemDetailsWriterInvocation = registerProblemDetailsWriterInvocationSyntax; + + var mvcServiceInvocationLocation = diagnostic.AdditionalLocations.FirstOrDefault(); + if (mvcServiceInvocationLocation is null) + { + return false; + } + + var mvcServiceInvocationTarget = root.FindNode(mvcServiceInvocationLocation.SourceSpan, getInnermostNodeForTie: true); + if (mvcServiceInvocationTarget is not InvocationExpressionSyntax mvcServiceInvocationSyntax) + { + return false; + } + + mvcServiceInvocation = mvcServiceInvocationSyntax; + + return true; + } + + private static Task FixOrderOfProblemDetailsWriter( + SyntaxNode root, + Document document, + InvocationExpressionSyntax registerProblemDetailsWriterInvocation, + InvocationExpressionSyntax mvcServiceInvocation) + { + var newRoot = root.ReplaceNodes( + new[] { registerProblemDetailsWriterInvocation, mvcServiceInvocation }, + (original, _) => original == registerProblemDetailsWriterInvocation ? mvcServiceInvocation : registerProblemDetailsWriterInvocation); + + return Task.FromResult(document.WithSyntaxRoot(newRoot)); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs similarity index 68% rename from src/Framework/AspNetCoreAnalyzers/test/Startup/StartupAnalyzerTests.cs rename to src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs index 23ecb81825f6..ea11def8b026 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupAnalyzerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs @@ -2,14 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Concurrent; +using Microsoft.AspNetCore.Analyzers.Startup.Fixers; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; namespace Microsoft.AspNetCore.Analyzers.Startup; -public sealed class StartupAnalyzerTests +public sealed class ProblemDetailsWriterTests { - public StartupAnalyzerTests() + public ProblemDetailsWriterTests() { StartupAnalyzer = new StartupAnalyzer(); @@ -50,18 +51,37 @@ public class ProblemDetailsWriterRegistration {{ public void ConfigureServices(IServiceCollection services) {{ - services.{methodName}(); + {{|#1:services.{methodName}()|}}; {{|#0:services.AddTransient()|}}; }} }} {_sampleProblemDetailsWriterSource} }}"; - var diagnosticResult = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) - .WithLocation(0); + var diagnostic = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(0) + .WithLocation(1); + + var fixedSource = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.AddTransient(); + services.{methodName}(); + }} + }} + {_sampleProblemDetailsWriterSource} +}}"; // Act + Assert - await VerifyAnalyzerAsync(source, diagnosticResult); + await VerifyCodeFix(source, fixedSource, diagnostic); } [Theory] @@ -81,18 +101,37 @@ public class ProblemDetailsWriterRegistration {{ public void ConfigureServices(IServiceCollection services) {{ - services.AddControllers(); + {{|#1:services.AddControllers()|}}; {{|#0:services.{methodName}()|}}; }} }} {_sampleProblemDetailsWriterSource} }}"; - var diagnosticResult = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) - .WithLocation(0); + var diagnostic = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(0) + .WithLocation(1); + + var fixedSource = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.{methodName}(); + services.AddControllers(); + }} + }} + {_sampleProblemDetailsWriterSource} +}}"; // Act + Assert - await VerifyAnalyzerAsync(source, diagnosticResult); + await VerifyCodeFix(source, fixedSource, diagnostic); } [Theory] @@ -122,7 +161,7 @@ public void ConfigureServices(IServiceCollection services) }}"; // Act - await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); + await VerifyNoCodeFix(source); // Assert var middlewareAnalysis = Analyses.OfType().First(); @@ -141,17 +180,25 @@ public ValueTask WriteAsync(ProblemDetailsContext context) } }"; - private Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + private Task VerifyNoCodeFix(string source) { - var test = new StartupCSharpAnalyzerTest(StartupAnalyzer, TestReferences.MetadataReferences) + return VerifyCodeFix(source, source, Array.Empty()); + } + + private async Task VerifyCodeFix(string source, string fixedSource, params DiagnosticResult[] expected) + { + var test = new StartupCSharpAnalyzerTest(StartupAnalyzer, TestReferences.MetadataReferences) { TestCode = source, + FixedCode = fixedSource, ReferenceAssemblies = TestReferences.EmptyReferenceAssemblies, }; + // Tests are just the Configure/ConfigureServices methods, no Main, so we need to mark the output as not console test.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; test.ExpectedDiagnostics.AddRange(expected); - return test.RunAsync(); + + await test.RunAsync(); } -} \ No newline at end of file +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs index 1a9e6e9e8fc1..0b3ee1fb096b 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/StartupCSharpAnalyzerTest.cs @@ -3,13 +3,15 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing.Verifiers; namespace Microsoft.AspNetCore.Analyzers.Startup; -internal sealed class StartupCSharpAnalyzerTest : CSharpAnalyzerTest +internal sealed class StartupCSharpAnalyzerTest : CSharpCodeFixTest + where TCodeFix : CodeFixProvider, new() { public StartupCSharpAnalyzerTest(StartupAnalyzer analyzer, ImmutableArray metadataReferences) { From 814364b0fad13de69b21b4e0a544b7908963718a Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 9 Jun 2024 01:53:24 -0400 Subject: [PATCH 09/16] Implement code fix --- ...ctlyConfiguredProblemDetailsWriterFixer.cs | 162 ++++++++++++++---- .../test/Startup/ProblemDetailsWriterTests.cs | 150 ++++++++++++---- 2 files changed, 245 insertions(+), 67 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs index 781df4f0f014..ab1c3b2b0412 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs @@ -1,15 +1,19 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; namespace Microsoft.AspNetCore.Analyzers.Startup.Fixers; @@ -18,7 +22,13 @@ public sealed class IncorrectlyConfiguredProblemDetailsWriterFixer : CodeFixProv { public override ImmutableArray FixableDiagnosticIds { get; } = [DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter.Id]; - public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + public sealed override FixAllProvider? GetFixAllProvider() + { + return FixAllProvider.Create(async (context, document, diagnostics) => + { + return await CanFixOrderOfAllProblemDetailsWriters(document, diagnostics, context.CancellationToken).ConfigureAwait(false); + }); + } public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) { @@ -28,23 +38,25 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } - var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); - if (semanticModel == null) - { - return; - } - foreach (var diagnostic in context.Diagnostics) { - if (CanFixOrderOfProblemDetailsWriter(diagnostic, root, out var registerProblemDetailsWriterInvocation, out var mvcServiceInvocation)) + if (CanFixOrderOfProblemDetailsWriter(diagnostic, root, out var problemDetailsWriter, out var mvcServiceCollectionExtension)) { const string title = "Fix order of ProblemDetailsWriter registration"; - context.RegisterCodeFix( - CodeAction.Create( - title, - cancellationToken => FixOrderOfProblemDetailsWriter(root, context.Document, registerProblemDetailsWriterInvocation, mvcServiceInvocation), - equivalenceKey: DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter.Id), - diagnostic); + + async Task CreateChangedDocument(CancellationToken cancellationToken) => + await CanFixOrderOfProblemDetailsWriter( + context.Document, + problemDetailsWriter, + mvcServiceCollectionExtension, + cancellationToken).ConfigureAwait(false); + + var codeAction = CodeAction.Create( + title, + CreateChangedDocument, + equivalenceKey: DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter.Id); + + context.RegisterCodeFix(codeAction, diagnostic); } } } @@ -52,47 +64,121 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) private static bool CanFixOrderOfProblemDetailsWriter( Diagnostic diagnostic, SyntaxNode root, - [NotNullWhen(true)] out InvocationExpressionSyntax? registerProblemDetailsWriterInvocation, - [NotNullWhen(true)] out InvocationExpressionSyntax? mvcServiceInvocation) + [NotNullWhen(true)] out ExpressionStatementSyntax? problemDetailsWriterStatement, + [NotNullWhen(true)] out ExpressionStatementSyntax? mvcServiceCollectionExtensionStatement) { - registerProblemDetailsWriterInvocation = null; - mvcServiceInvocation = null; + problemDetailsWriterStatement = null; + mvcServiceCollectionExtensionStatement = null; + + Debug.Assert(diagnostic.AdditionalLocations.Count == 1, + "Expected exactly one additional location for the MvcServiceCollectionExtension."); + + return diagnostic.AdditionalLocations.Count == 1 && + // Ensure that the ProblemDetailsWriter registration appears after the MvcServiceCollectionExtension in source. + diagnostic.Location.SourceSpan.CompareTo(diagnostic.AdditionalLocations[0].SourceSpan) > 0 && + TryGetInvocationExpressionStatement(diagnostic.Location, root, out problemDetailsWriterStatement) && + TryGetInvocationExpressionStatement(diagnostic.AdditionalLocations[0], root, out mvcServiceCollectionExtensionStatement); + } - var registerProblemDetailsWriterInvocationTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); - if (registerProblemDetailsWriterInvocationTarget is not InvocationExpressionSyntax registerProblemDetailsWriterInvocationSyntax) + private static async Task CanFixOrderOfProblemDetailsWriter( + Document document, + ExpressionStatementSyntax problemDetailsWriterExpression, + ExpressionStatementSyntax mvcServiceCollectionExtensionExpression, + CancellationToken cancellationToken) + { + var groupedStatements = new Dictionary> + { + [mvcServiceCollectionExtensionExpression] = [problemDetailsWriterExpression], + }; + + return await MoveProblemDetailsWritersBeforeMvcExtensions(document, groupedStatements, cancellationToken).ConfigureAwait(false); + } + + private static async Task CanFixOrderOfAllProblemDetailsWriters( + Document document, + ImmutableArray diagnostics, + CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root == null) { - return false; + return document; } - registerProblemDetailsWriterInvocation = registerProblemDetailsWriterInvocationSyntax; + var groupedStatements = new Dictionary>(); - var mvcServiceInvocationLocation = diagnostic.AdditionalLocations.FirstOrDefault(); - if (mvcServiceInvocationLocation is null) + foreach (var diagnostic in diagnostics) { - return false; + if (!CanFixOrderOfProblemDetailsWriter(diagnostic, root, out var problemDetailsWriterStatement, out var mvcServiceCollectionExtensionStatement)) + { + continue; + } + + if (groupedStatements.TryGetValue(mvcServiceCollectionExtensionStatement, out var problemDetailsWriterStatements)) + { + problemDetailsWriterStatements.Add(problemDetailsWriterStatement); + } + else + { + groupedStatements[mvcServiceCollectionExtensionStatement] = [problemDetailsWriterStatement]; + } } - var mvcServiceInvocationTarget = root.FindNode(mvcServiceInvocationLocation.SourceSpan, getInnermostNodeForTie: true); - if (mvcServiceInvocationTarget is not InvocationExpressionSyntax mvcServiceInvocationSyntax) + if (groupedStatements.Count == 0) { - return false; + return document; } - mvcServiceInvocation = mvcServiceInvocationSyntax; + // Maintain the relative source order of the ProblemDetailsWriter registrations. + var comparer = Comparer.Create((a, b) => a.Span.CompareTo(b.Span)); + + foreach (var group in groupedStatements) + { + groupedStatements[group.Key] = [.. group.Value.OrderBy(x => x, comparer)]; + } - return true; + return await MoveProblemDetailsWritersBeforeMvcExtensions(document, groupedStatements, cancellationToken).ConfigureAwait(false); } - private static Task FixOrderOfProblemDetailsWriter( - SyntaxNode root, + private static async Task MoveProblemDetailsWritersBeforeMvcExtensions( Document document, - InvocationExpressionSyntax registerProblemDetailsWriterInvocation, - InvocationExpressionSyntax mvcServiceInvocation) + IDictionary> groupedStatements, + CancellationToken cancellationToken) + { + var documentEditor = await DocumentEditor.CreateAsync(document, cancellationToken); + + foreach (var group in groupedStatements) + { + var mvcServiceExtensionExpression = group.Key; + var registerProblemDetailsWriterExpressions = group.Value; + + foreach (var registerProblemDetailsWriterExpression in registerProblemDetailsWriterExpressions) + { + documentEditor.RemoveNode(registerProblemDetailsWriterExpression, SyntaxRemoveOptions.KeepNoTrivia); + } + + documentEditor.InsertBefore(mvcServiceExtensionExpression, registerProblemDetailsWriterExpressions); + } + + return documentEditor.GetChangedDocument(); + } + + private static bool TryGetInvocationExpressionStatement( + Location location, + SyntaxNode root, + [NotNullWhen(true)] out ExpressionStatementSyntax? expressionStatement) { - var newRoot = root.ReplaceNodes( - new[] { registerProblemDetailsWriterInvocation, mvcServiceInvocation }, - (original, _) => original == registerProblemDetailsWriterInvocation ? mvcServiceInvocation : registerProblemDetailsWriterInvocation); + expressionStatement = null; + + var node = root.FindNode(location.SourceSpan, getInnermostNodeForTie: true); + + if (node is InvocationExpressionSyntax invocationExpression && + invocationExpression.Parent is ExpressionStatementSyntax invocationExpressionStatement) + { + expressionStatement = invocationExpressionStatement; + return true; + } - return Task.FromResult(document.WithSyntaxRoot(newRoot)); + return false; } } diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs index ea11def8b026..ce2423d96592 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs @@ -51,16 +51,16 @@ public class ProblemDetailsWriterRegistration {{ public void ConfigureServices(IServiceCollection services) {{ - {{|#1:services.{methodName}()|}}; - {{|#0:services.AddTransient()|}}; + {{|#0:services.{methodName}()|}}; + {{|#1:services.AddTransient()|}}; }} }} - {_sampleProblemDetailsWriterSource} + {GetSampleProblemDetailsWriterSource()} }}"; var diagnostic = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) - .WithLocation(0) - .WithLocation(1); + .WithLocation(1) + .WithLocation(0); var fixedSource = $@" using System.Threading.Tasks; @@ -77,11 +77,80 @@ public void ConfigureServices(IServiceCollection services) services.{methodName}(); }} }} - {_sampleProblemDetailsWriterSource} + {GetSampleProblemDetailsWriterSource()} }}"; // Act + Assert - await VerifyCodeFix(source, fixedSource, diagnostic); + await VerifyCodeFix(source, [diagnostic], fixedSource); + } + + [Theory] + [InlineData("AddControllers")] + [InlineData("AddControllersWithViews")] + [InlineData("AddMvc")] + [InlineData("AddRazorPages")] + public async Task StartupAnalyzer_MultipleProblemDetailsWriters_AfterMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + {{|#0:services.{methodName}()|}}; + {{|#1:services.AddTransient()|}}; + {{|#2:services.AddTransient()|}}; + {{|#3:services.AddTransient()|}}; + }} + }} + {GetSampleProblemDetailsWriterSource("A")} + {GetSampleProblemDetailsWriterSource("B")} + {GetSampleProblemDetailsWriterSource("C")} +}}"; + + var diagnostics = new[] + { + new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(1) + .WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(2) + .WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(3) + .WithLocation(0) + }; + + var fixedSource = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.AddTransient(); + services.AddTransient(); + services.AddTransient(); + services.{methodName}(); + }} + }} + {GetSampleProblemDetailsWriterSource("A")} + {GetSampleProblemDetailsWriterSource("B")} + {GetSampleProblemDetailsWriterSource("C")} +}}"; + + // Act + Assert + await VerifyCodeFix(source, diagnostics, fixedSource); } [Theory] @@ -101,16 +170,16 @@ public class ProblemDetailsWriterRegistration {{ public void ConfigureServices(IServiceCollection services) {{ - {{|#1:services.AddControllers()|}}; - {{|#0:services.{methodName}()|}}; + {{|#0:services.AddControllers()|}}; + {{|#1:services.{methodName}()|}}; }} }} - {_sampleProblemDetailsWriterSource} + {GetSampleProblemDetailsWriterSource()} }}"; var diagnostic = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) - .WithLocation(0) - .WithLocation(1); + .WithLocation(1) + .WithLocation(0); var fixedSource = $@" using System.Threading.Tasks; @@ -127,11 +196,11 @@ public void ConfigureServices(IServiceCollection services) services.AddControllers(); }} }} - {_sampleProblemDetailsWriterSource} + {GetSampleProblemDetailsWriterSource()} }}"; // Act + Assert - await VerifyCodeFix(source, fixedSource, diagnostic); + await VerifyCodeFix(source, [diagnostic], fixedSource); } [Theory] @@ -157,7 +226,7 @@ public void ConfigureServices(IServiceCollection services) services.{methodName}(); }} }} - {_sampleProblemDetailsWriterSource} + {GetSampleProblemDetailsWriterSource()} }}"; // Act @@ -168,37 +237,60 @@ public void ConfigureServices(IServiceCollection services) Assert.NotEmpty(middlewareAnalysis.Services); } - private const string _sampleProblemDetailsWriterSource = @" -public class SampleProblemDetailsWriter : IProblemDetailsWriter -{ + private static string GetSampleProblemDetailsWriterSource(string suffix = null) + { + return $@" +public class SampleProblemDetailsWriter{suffix} : IProblemDetailsWriter +{{ public bool CanWrite(ProblemDetailsContext context) => context.HttpContext.Response.StatusCode == 400; + public ValueTask WriteAsync(ProblemDetailsContext context) - { + {{ var response = context.HttpContext.Response; return new ValueTask(response.WriteAsJsonAsync(context.ProblemDetails)); + }} +}}"; + } + + private async Task VerifyNoCodeFix(string source) + { + await VerifyCodeFix(source, [], source); + } + + private async Task VerifyCodeFix(string source, DiagnosticResult[] diagnostics, string fixedSource) + { + var test = CreateAnalyzerTest(); + + test.TestCode = source; + test.FixedCode = fixedSource; + test.ExpectedDiagnostics.AddRange(diagnostics); + + await test.RunAsync(); } -}"; - private Task VerifyNoCodeFix(string source) + private async Task VerifyCodeFixAll(string source, DiagnosticResult[] diagnostics, string fixedSource) { - return VerifyCodeFix(source, source, Array.Empty()); + var test = CreateAnalyzerTest(); + + test.TestCode = source; + test.FixedCode = fixedSource; + test.ExpectedDiagnostics.AddRange(diagnostics); + test.NumberOfFixAllIterations = 1; + + await test.RunAsync(); } - private async Task VerifyCodeFix(string source, string fixedSource, params DiagnosticResult[] expected) + private StartupCSharpAnalyzerTest CreateAnalyzerTest() { var test = new StartupCSharpAnalyzerTest(StartupAnalyzer, TestReferences.MetadataReferences) { - TestCode = source, - FixedCode = fixedSource, - ReferenceAssemblies = TestReferences.EmptyReferenceAssemblies, + ReferenceAssemblies = TestReferences.EmptyReferenceAssemblies }; // Tests are just the Configure/ConfigureServices methods, no Main, so we need to mark the output as not console test.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; - test.ExpectedDiagnostics.AddRange(expected); - - await test.RunAsync(); + return test; } } From 2e1c3f4a445424a9bbcb89cbcc1f4cf0e568965f Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 9 Jun 2024 01:57:22 -0400 Subject: [PATCH 10/16] Fix method names --- .../IncorrectlyConfiguredProblemDetailsWriterFixer.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs index ab1c3b2b0412..08d6b91e0370 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs @@ -26,7 +26,7 @@ public sealed class IncorrectlyConfiguredProblemDetailsWriterFixer : CodeFixProv { return FixAllProvider.Create(async (context, document, diagnostics) => { - return await CanFixOrderOfAllProblemDetailsWriters(document, diagnostics, context.CancellationToken).ConfigureAwait(false); + return await FixOrderOfAllProblemDetailsWriter(document, diagnostics, context.CancellationToken).ConfigureAwait(false); }); } @@ -45,7 +45,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) const string title = "Fix order of ProblemDetailsWriter registration"; async Task CreateChangedDocument(CancellationToken cancellationToken) => - await CanFixOrderOfProblemDetailsWriter( + await FixOrderOfProblemDetailsWriter( context.Document, problemDetailsWriter, mvcServiceCollectionExtension, @@ -80,7 +80,7 @@ private static bool CanFixOrderOfProblemDetailsWriter( TryGetInvocationExpressionStatement(diagnostic.AdditionalLocations[0], root, out mvcServiceCollectionExtensionStatement); } - private static async Task CanFixOrderOfProblemDetailsWriter( + private static async Task FixOrderOfProblemDetailsWriter( Document document, ExpressionStatementSyntax problemDetailsWriterExpression, ExpressionStatementSyntax mvcServiceCollectionExtensionExpression, @@ -94,7 +94,7 @@ private static async Task CanFixOrderOfProblemDetailsWriter( return await MoveProblemDetailsWritersBeforeMvcExtensions(document, groupedStatements, cancellationToken).ConfigureAwait(false); } - private static async Task CanFixOrderOfAllProblemDetailsWriters( + private static async Task FixOrderOfAllProblemDetailsWriter( Document document, ImmutableArray diagnostics, CancellationToken cancellationToken) From 68c6e92650ebc2128ee2274a22fb9c32a92ff6a3 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sun, 9 Jun 2024 17:08:47 -0400 Subject: [PATCH 11/16] Don't fix IProblemDetailsWriter registrations in chained invocations --- ...ctlyConfiguredProblemDetailsWriterFixer.cs | 15 +++- .../test/Startup/ProblemDetailsWriterTests.cs | 85 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs index 08d6b91e0370..a783e4d6f8e6 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs @@ -173,7 +173,9 @@ private static bool TryGetInvocationExpressionStatement( var node = root.FindNode(location.SourceSpan, getInnermostNodeForTie: true); if (node is InvocationExpressionSyntax invocationExpression && - invocationExpression.Parent is ExpressionStatementSyntax invocationExpressionStatement) + invocationExpression.Parent is ExpressionStatementSyntax invocationExpressionStatement && + // Exclude expressions that may be part of an invocation chain to avoid moving unrelated code. + !IsPotentiallyPartOfInvocationChain(invocationExpression)) { expressionStatement = invocationExpressionStatement; return true; @@ -181,4 +183,15 @@ private static bool TryGetInvocationExpressionStatement( return false; } + + private static bool IsPotentiallyPartOfInvocationChain(InvocationExpressionSyntax invocationExpression) + { + if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression && + memberAccessExpression.Expression is IdentifierNameSyntax) + { + return false; + } + + return true; + } } diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs index ce2423d96592..5ae4faa406f5 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs @@ -153,6 +153,91 @@ public void ConfigureServices(IServiceCollection services) await VerifyCodeFix(source, diagnostics, fixedSource); } + [Theory] + [InlineData("AddControllers")] + [InlineData("AddControllersWithViews")] + [InlineData("AddMvc")] + [InlineData("AddRazorPages")] + public async Task StartupAnalyzer_ProblemDetailsWriterRegistrationChained_AfterMvcServiceCollectionsExtension_ReportsDiagnosticButDoesNotFix(string methodName) + { + // Arrange + var source = $@" +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + {{|#0:services.{methodName}()|}}; + + {{|#1:services.AddTransient()|}} + .AddTransient(provider => Array.Empty()); + + {{|#2:services.AddTransient(provider => Array.Empty()) + .AddTransient()|}}; + + {{|#3:services.AddTransient(provider => Array.Empty()) + .AddTransient()|}} + .AddTransient(provider => Array.Empty()); + }} + }} + {GetSampleProblemDetailsWriterSource("A")} + {GetSampleProblemDetailsWriterSource("B")} + {GetSampleProblemDetailsWriterSource("C")} +}}"; + + var diagnostics = new[] + { + new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(1) + .WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(2) + .WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(3) + .WithLocation(0) + }; + + var fixedSource = $@" +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + {{|#0:services.{methodName}()|}}; + + {{|#1:services.AddTransient()|}} + .AddTransient(provider => Array.Empty()); + + {{|#2:services.AddTransient(provider => Array.Empty()) + .AddTransient()|}}; + + {{|#3:services.AddTransient(provider => Array.Empty()) + .AddTransient()|}} + .AddTransient(provider => Array.Empty()); + }} + }} + {GetSampleProblemDetailsWriterSource("A")} + {GetSampleProblemDetailsWriterSource("B")} + {GetSampleProblemDetailsWriterSource("C")} +}}"; + + // Act + Assert + await VerifyCodeFix(source, diagnostics, fixedSource); + } + [Theory] [InlineData("AddScoped")] [InlineData("AddSingleton")] From 69bc174d95250109d8eac81a5a953e711194e688 Mon Sep 17 00:00:00 2001 From: David Acker Date: Fri, 9 Aug 2024 20:47:53 -0400 Subject: [PATCH 12/16] Exclude chained expressions from code fix --- ...ncorrectlyConfiguredProblemDetailsWriterFixer.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs index a783e4d6f8e6..06e9b63fa7ac 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs @@ -77,6 +77,8 @@ private static bool CanFixOrderOfProblemDetailsWriter( // Ensure that the ProblemDetailsWriter registration appears after the MvcServiceCollectionExtension in source. diagnostic.Location.SourceSpan.CompareTo(diagnostic.AdditionalLocations[0].SourceSpan) > 0 && TryGetInvocationExpressionStatement(diagnostic.Location, root, out problemDetailsWriterStatement) && + // Exclude ProblemDetailsWriter registrations that may be part of an invocation chain to avoid moving unrelated code. + !IsPotentiallyPartOfInvocationChain(problemDetailsWriterStatement) && TryGetInvocationExpressionStatement(diagnostic.AdditionalLocations[0], root, out mvcServiceCollectionExtensionStatement); } @@ -145,7 +147,7 @@ private static async Task MoveProblemDetailsWritersBeforeMvcExtensions IDictionary> groupedStatements, CancellationToken cancellationToken) { - var documentEditor = await DocumentEditor.CreateAsync(document, cancellationToken); + var documentEditor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); foreach (var group in groupedStatements) { @@ -173,9 +175,7 @@ private static bool TryGetInvocationExpressionStatement( var node = root.FindNode(location.SourceSpan, getInnermostNodeForTie: true); if (node is InvocationExpressionSyntax invocationExpression && - invocationExpression.Parent is ExpressionStatementSyntax invocationExpressionStatement && - // Exclude expressions that may be part of an invocation chain to avoid moving unrelated code. - !IsPotentiallyPartOfInvocationChain(invocationExpression)) + invocationExpression.Parent is ExpressionStatementSyntax invocationExpressionStatement) { expressionStatement = invocationExpressionStatement; return true; @@ -184,9 +184,10 @@ invocationExpression.Parent is ExpressionStatementSyntax invocationExpressionSta return false; } - private static bool IsPotentiallyPartOfInvocationChain(InvocationExpressionSyntax invocationExpression) + private static bool IsPotentiallyPartOfInvocationChain(ExpressionStatementSyntax expressionStatement) { - if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression && + if (expressionStatement.Expression is InvocationExpressionSyntax invocationExpressionSyntax && + invocationExpressionSyntax.Expression is MemberAccessExpressionSyntax memberAccessExpression && memberAccessExpression.Expression is IdentifierNameSyntax) { return false; From c6bcbdc4ce5222d61a45a9b37841ca9d8e0e9f58 Mon Sep 17 00:00:00 2001 From: David Acker Date: Fri, 9 Aug 2024 20:48:40 -0400 Subject: [PATCH 13/16] Assert FixAll behavior in multiple PDWs test --- .../test/Startup/ProblemDetailsWriterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs index 5ae4faa406f5..d237a6036c34 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs @@ -150,7 +150,7 @@ public void ConfigureServices(IServiceCollection services) }}"; // Act + Assert - await VerifyCodeFix(source, diagnostics, fixedSource); + await VerifyCodeFixAll(source, diagnostics, fixedSource); } [Theory] From a9c7114bffd60a36609a73033f41e9acacb4833a Mon Sep 17 00:00:00 2001 From: David Acker Date: Fri, 9 Aug 2024 21:22:54 -0400 Subject: [PATCH 14/16] Get parent expression statement for chained invocations --- ...ctlyConfiguredProblemDetailsWriterFixer.cs | 25 +++++++-- .../test/Startup/ProblemDetailsWriterTests.cs | 55 +++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs index 06e9b63fa7ac..c8cfcb4562a5 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Startup/IncorrectlyConfiguredProblemDetailsWriterFixer.cs @@ -174,11 +174,28 @@ private static bool TryGetInvocationExpressionStatement( var node = root.FindNode(location.SourceSpan, getInnermostNodeForTie: true); - if (node is InvocationExpressionSyntax invocationExpression && - invocationExpression.Parent is ExpressionStatementSyntax invocationExpressionStatement) + if (node is not InvocationExpressionSyntax) { - expressionStatement = invocationExpressionStatement; - return true; + return false; + } + + var parentNode = node.Parent; + + while (parentNode != null) + { + if (parentNode is ExpressionStatementSyntax expressionStatementSyntax) + { + expressionStatement = expressionStatementSyntax; + return true; + } + + if (parentNode is not MemberAccessExpressionSyntax + && parentNode is not InvocationExpressionSyntax) + { + break; + } + + parentNode = parentNode.Parent; } return false; diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs index d237a6036c34..9eeada166a1a 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs @@ -32,6 +32,61 @@ public ProblemDetailsWriterTests() internal ConcurrentBag ConfigureMethods { get; } + [Theory] + [InlineData("AddControllers")] + [InlineData("AddControllersWithViews")] + [InlineData("AddMvc")] + [InlineData("AddRazorPages")] + public async Task StartupAnalyzer_ProblemDetailsWriter_AfterChainedMvcServiceCollectionsExtension_ReportsDiagnostic(string methodName) + { + // Arrange + var source = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + {{|#0:services.{methodName}()|}} + .AddViewLocalization(); + {{|#1:services.AddTransient()|}}; + }} + }} + {GetSampleProblemDetailsWriterSource()} +}}"; + + var diagnostic = new DiagnosticResult(DiagnosticDescriptors.IncorrectlyConfiguredProblemDetailsWriter) + .WithLocation(1) + .WithLocation(0); + + var fixedSource = $@" +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{{ + public class ProblemDetailsWriterRegistration + {{ + public void ConfigureServices(IServiceCollection services) + {{ + services.AddTransient(); + services.{methodName}() + .AddViewLocalization(); + }} + }} + {GetSampleProblemDetailsWriterSource()} +}}"; + + // Act + Assert + await VerifyCodeFix(source, [diagnostic], fixedSource); + } + + [Theory] [InlineData("AddControllers")] [InlineData("AddControllersWithViews")] From 00dd60da938686bb6f95c499909c736deab5c01d Mon Sep 17 00:00:00 2001 From: David Acker Date: Fri, 23 Aug 2024 16:20:35 -0400 Subject: [PATCH 15/16] Remove extra line, unused properties in test --- .../test/Startup/ProblemDetailsWriterTests.cs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs index 9eeada166a1a..3c95d1630629 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Startup/ProblemDetailsWriterTests.cs @@ -15,23 +15,15 @@ public ProblemDetailsWriterTests() StartupAnalyzer = new StartupAnalyzer(); Analyses = new ConcurrentBag(); - ConfigureServicesMethods = new ConcurrentBag(); - ConfigureMethods = new ConcurrentBag(); StartupAnalyzer.ServicesAnalysisCompleted += (sender, analysis) => Analyses.Add(analysis); StartupAnalyzer.OptionsAnalysisCompleted += (sender, analysis) => Analyses.Add(analysis); StartupAnalyzer.MiddlewareAnalysisCompleted += (sender, analysis) => Analyses.Add(analysis); - StartupAnalyzer.ConfigureServicesMethodFound += (sender, method) => ConfigureServicesMethods.Add(method); - StartupAnalyzer.ConfigureMethodFound += (sender, method) => ConfigureMethods.Add(method); } internal StartupAnalyzer StartupAnalyzer { get; } internal ConcurrentBag Analyses { get; } - internal ConcurrentBag ConfigureServicesMethods { get; } - - internal ConcurrentBag ConfigureMethods { get; } - [Theory] [InlineData("AddControllers")] [InlineData("AddControllersWithViews")] @@ -86,7 +78,6 @@ public void ConfigureServices(IServiceCollection services) await VerifyCodeFix(source, [diagnostic], fixedSource); } - [Theory] [InlineData("AddControllers")] [InlineData("AddControllersWithViews")] @@ -428,7 +419,7 @@ private StartupCSharpAnalyzerTest Date: Thu, 29 Aug 2024 20:51:03 -0400 Subject: [PATCH 16/16] Revert unintended changes to resource file --- .../AspNetCoreAnalyzers/src/Analyzers/Resources.resx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 7e2fdd927e41..b89a405ab354 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -309,12 +309,12 @@ Route '{0}' conflicts with another action route. An HTTP request that matches multiple routes results in an ambiguous match error. Fix the conflict by changing the route's pattern, HTTP method, or route constraints. - - Use AddAuthorizationBuilder - Use AddAuthorizationBuilder to register authorization services and construct policies + + Use AddAuthorizationBuilder + This [Authorize] attribute is overridden by an [AllowAnonymous] attribute from farther away on '{0}'. See https://aka.ms/aspnetcore-warnings/ASP0026 for more details.