diff --git a/src/Mvc/Mvc.Analyzers/src/DiagnosticDescriptors.cs b/src/Mvc/Mvc.Analyzers/src/DiagnosticDescriptors.cs index 8b0aa761d1..8dba4a4a07 100644 --- a/src/Mvc/Mvc.Analyzers/src/DiagnosticDescriptors.cs +++ b/src/Mvc/Mvc.Analyzers/src/DiagnosticDescriptors.cs @@ -52,5 +52,8 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers DiagnosticSeverity.Warning, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/AA20pbc"); + + + // MVC1005 reserved for startup } } diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/ConfigureMethodAnalysis.cs b/src/Mvc/Mvc.Analyzers/src/Startup/ConfigureMethodAnalysis.cs new file mode 100644 index 0000000000..394dd23bff --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/ConfigureMethodAnalysis.cs @@ -0,0 +1,18 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal abstract class ConfigureMethodAnalysis : StartupComputedAnalysis + { + protected ConfigureMethodAnalysis(IMethodSymbol configureMethod) + : base(configureMethod.ContainingType) + { + ConfigureMethod = configureMethod; + } + + public IMethodSymbol ConfigureMethod { get; } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/ConfigureServicesMethodAnalysis.cs b/src/Mvc/Mvc.Analyzers/src/Startup/ConfigureServicesMethodAnalysis.cs new file mode 100644 index 0000000000..ebe773474b --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/ConfigureServicesMethodAnalysis.cs @@ -0,0 +1,18 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal abstract class ConfigureServicesMethodAnalysis : StartupComputedAnalysis + { + protected ConfigureServicesMethodAnalysis(IMethodSymbol configureServicesMethod) + : base(configureServicesMethod.ContainingType) + { + ConfigureServicesMethod = configureServicesMethod; + } + + public IMethodSymbol ConfigureServicesMethod { get; } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/MiddlewareAnalysis.cs b/src/Mvc/Mvc.Analyzers/src/Startup/MiddlewareAnalysis.cs new file mode 100644 index 0000000000..874dc30ea6 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/MiddlewareAnalysis.cs @@ -0,0 +1,53 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class MiddlewareAnalysis : ConfigureMethodAnalysis + { + public static MiddlewareAnalysis CreateAndInitialize(StartupAnalysisContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var symbols = context.StartupSymbols; + var analysis = new MiddlewareAnalysis((IMethodSymbol)context.OperationBlockStartAnalysisContext.OwningSymbol); + + var middleware = ImmutableArray.CreateBuilder(); + + context.OperationBlockStartAnalysisContext.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 && + invocation.Arguments[0].Parameter?.Type == symbols.IApplicationBuilder) + { + middleware.Add(new MiddlewareItem(invocation)); + } + }, OperationKind.Invocation); + + context.OperationBlockStartAnalysisContext.RegisterOperationBlockEndAction(context => + { + analysis.Middleware = middleware.ToImmutable(); + }); + + return analysis; + } + + public MiddlewareAnalysis(IMethodSymbol configureMethod) + : base(configureMethod) + { + } + + public ImmutableArray Middleware { get; private set; } = ImmutableArray.Empty; + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/MiddlewareItem.cs b/src/Mvc/Mvc.Analyzers/src/Startup/MiddlewareItem.cs new file mode 100644 index 0000000000..6e3e715ced --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/MiddlewareItem.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class MiddlewareItem + { + public MiddlewareItem(IInvocationOperation operation) + { + Operation = operation; + } + + public IInvocationOperation Operation { get; } + + public IMethodSymbol UseMethod => Operation.TargetMethod; + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/MvcOptionsAnalysis.cs b/src/Mvc/Mvc.Analyzers/src/Startup/MvcOptionsAnalysis.cs new file mode 100644 index 0000000000..d5dd63375b --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/MvcOptionsAnalysis.cs @@ -0,0 +1,45 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNetCore.Mvc.Analyzers; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class MvcOptionsAnalysis : ConfigureServicesMethodAnalysis + { + public static MvcOptionsAnalysis CreateAndInitialize(StartupAnalysisContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var analysis = new MvcOptionsAnalysis((IMethodSymbol)context.OperationBlockStartAnalysisContext.OwningSymbol); + + context.OperationBlockStartAnalysisContext.RegisterOperationAction(context => + { + if (context.Operation is ISimpleAssignmentOperation operation && + operation.Value.ConstantValue.HasValue && + operation.Target is IPropertyReferenceOperation property && + property.Member?.Name == SymbolNames.EnableEndpointRoutingProperty) + { + analysis.EndpointRoutingEnabled = operation.Value.ConstantValue.Value as bool?; + } + + }, OperationKind.SimpleAssignment); + + return analysis; + } + + public MvcOptionsAnalysis(IMethodSymbol configureServicesMethod) + : base(configureServicesMethod) + { + } + + public bool? EndpointRoutingEnabled { get; private set; } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/ServicesAnalysis.cs b/src/Mvc/Mvc.Analyzers/src/Startup/ServicesAnalysis.cs new file mode 100644 index 0000000000..c0499c9bf9 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/ServicesAnalysis.cs @@ -0,0 +1,52 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class ServicesAnalysis : ConfigureServicesMethodAnalysis + { + public static ServicesAnalysis CreateAndInitialize(StartupAnalysisContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var symbols = context.StartupSymbols; + var analysis = new ServicesAnalysis((IMethodSymbol)context.OperationBlockStartAnalysisContext.OwningSymbol); + + var services = ImmutableArray.CreateBuilder(); + context.OperationBlockStartAnalysisContext.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 && + invocation.Arguments[0].Parameter?.Type == symbols.IServiceCollection) + { + services.Add(new ServicesItem(invocation)); + } + }, OperationKind.Invocation); + + context.OperationBlockStartAnalysisContext.RegisterOperationBlockEndAction(context => + { + analysis.Services = services.ToImmutable(); + }); + + return analysis; + } + + public ServicesAnalysis(IMethodSymbol configureServicesMethod) + : base(configureServicesMethod) + { + } + + public ImmutableArray Services { get; private set; } = ImmutableArray.Empty; + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/ServicesItem.cs b/src/Mvc/Mvc.Analyzers/src/Startup/ServicesItem.cs new file mode 100644 index 0000000000..e468e62cc1 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/ServicesItem.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class ServicesItem + { + public ServicesItem(IInvocationOperation operation) + { + Operation = operation; + } + + public IInvocationOperation Operation { get; } + + public IMethodSymbol UseMethod => Operation.TargetMethod; + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalysisContext.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalysisContext.cs new file mode 100644 index 0000000000..9d2ff14ba1 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalysisContext.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class StartupAnalysisContext + { + public StartupAnalysisContext(OperationBlockStartAnalysisContext operationBlockStartAnalysisContext, StartupSymbols startupSymbols) + { + OperationBlockStartAnalysisContext = operationBlockStartAnalysisContext; + StartupSymbols = startupSymbols; + } + + public OperationBlockStartAnalysisContext OperationBlockStartAnalysisContext { get; } + + public StartupSymbols StartupSymbols { get; } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalyzer.Diagnostics.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalyzer.Diagnostics.cs new file mode 100644 index 0000000000..ef9d247498 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalyzer.Diagnostics.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers +{ + public partial class StartupAnalzyer : DiagnosticAnalyzer + { + internal readonly static DiagnosticDescriptor UnsupportedUseMvcWithEndpointRouting = new DiagnosticDescriptor( + "MVC1005", + "Cannot use UseMvc with Endpoint Routing.", + "Using '{0}' to configure MVC is not supported while using Endpoint Routing. To continue using '{0}', please set 'MvcOptions.EnableEndpointRounting = false' inside '{1}'.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/YJggeFn"); + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalyzer.Events.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalyzer.Events.cs new file mode 100644 index 0000000000..91479b291d --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalyzer.Events.cs @@ -0,0 +1,33 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers +{ + public partial class StartupAnalzyer : DiagnosticAnalyzer + { + internal event EventHandler AnalysisStarted; + + private void OnAnalysisStarted(StartupComputedAnalysis analysis) + { + AnalysisStarted?.Invoke(this, analysis); + } + + internal event EventHandler ConfigureServicesMethodFound; + + private void OnConfigureServicesMethodFound(IMethodSymbol method) + { + ConfigureServicesMethodFound?.Invoke(this, method); + } + + internal event EventHandler ConfigureMethodFound; + + private void OnConfigureMethodFound(IMethodSymbol method) + { + ConfigureMethodFound?.Invoke(this, method); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalzyer.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalzyer.cs new file mode 100644 index 0000000000..be2a9ebb88 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupAnalzyer.cs @@ -0,0 +1,148 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Concurrent; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public partial class StartupAnalzyer : DiagnosticAnalyzer + { +#pragma warning disable RS1008 // Avoid storing per-compilation data into the fields of a diagnostic analyzer. + private readonly static Func[] ConfigureServicesMethodAnalysisFactories = new Func[] + { + ServicesAnalysis.CreateAndInitialize, + MvcOptionsAnalysis.CreateAndInitialize, + }; + + private readonly static Func[] ConfigureMethodAnalysisFactories = new Func[] + { + MiddlewareAnalysis.CreateAndInitialize, + }; + + private readonly static Func, StartupDiagnosticValidator>[] DiagnosticValidatorFactories = new Func, StartupDiagnosticValidator>[] + { + UseMvcDiagnosticValidator.CreateAndInitialize, + }; + +#pragma warning restore RS1008 // Avoid storing per-compilation data into the fields of a diagnostic analyzer. + + public StartupAnalzyer() + { + SupportedDiagnostics = ImmutableArray.Create(new[] + { + UnsupportedUseMvcWithEndpointRouting, + }); + + // By default the analyzer will only run for files ending with Startup.cs + // Can be overriden for unit testing other file names + // Analzyer only runs for C# so limiting to *.cs file is fine + StartupFilePredicate = path => path.EndsWith("Startup.cs", StringComparison.OrdinalIgnoreCase); + } + + internal Func StartupFilePredicate { get; set; } + + public override ImmutableArray SupportedDiagnostics { get; } + + public override void Initialize(AnalysisContext context) + { + 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.IServiceCollection == null || symbols.IApplicationBuilder == null) + { + return; + } + + // This analyzer is a general-purpose framework that functions by: + // 1. Discovering Startup methods (ConfigureServices, Configure) + // 2. Launching additional analyses of these Startup methods and collecting results + // 3. Running a final pass to add diagnostics based on computed state + var analyses = new ConcurrentBag(); + + context.RegisterOperationBlockStartAction(context => + { + AnalyzeStartupMethods(context, symbols, analyses); + }); + + // Run after analyses have had a chance to finish to add diagnostics. + context.RegisterCompilationEndAction(analysisContext => + { + RunAnalysis(analysisContext, analyses); + }); + } + + private static void RunAnalysis(CompilationAnalysisContext analysisContext, ConcurrentBag analyses) + { + for (var i = 0; i < DiagnosticValidatorFactories.Length; i++) + { + var validator = DiagnosticValidatorFactories[i].Invoke(analysisContext, analyses); + } + } + + private void AnalyzeStartupMethods(OperationBlockStartAnalysisContext context, StartupSymbols symbols, ConcurrentBag analyses) + { + if (!IsStartupFile(context)) + { + return; + } + + if (context.OwningSymbol.Kind != SymbolKind.Method) + { + return; + } + + var startupAnalysisContext = new StartupAnalysisContext(context, symbols); + + var method = (IMethodSymbol)context.OwningSymbol; + if (StartupFacts.IsConfigureServices(symbols, method)) + { + for (var i = 0; i < ConfigureServicesMethodAnalysisFactories.Length; i++) + { + var analysis = ConfigureServicesMethodAnalysisFactories[i].Invoke(startupAnalysisContext); + analyses.Add(analysis); + + OnAnalysisStarted(analysis); + } + + OnConfigureServicesMethodFound(method); + } + + if (StartupFacts.IsConfigure(symbols, method)) + { + for (var i = 0; i < ConfigureMethodAnalysisFactories.Length; i++) + { + var analysis = ConfigureMethodAnalysisFactories[i].Invoke(startupAnalysisContext); + analyses.Add(analysis); + + OnAnalysisStarted(analysis); + } + + OnConfigureMethodFound(method); + } + } + + private bool IsStartupFile(OperationBlockStartAnalysisContext context) + { + foreach (var location in context.OwningSymbol.Locations) + { + if (location.IsInSource && StartupFilePredicate(location.SourceTree.FilePath)) + { + return true; + } + } + + return false; + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupComputedAnalysis.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupComputedAnalysis.cs new file mode 100644 index 0000000000..0bd9efb687 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupComputedAnalysis.cs @@ -0,0 +1,17 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal abstract class StartupComputedAnalysis + { + protected StartupComputedAnalysis(INamedTypeSymbol enclosingType) + { + EnclosingType = enclosingType; + } + + public INamedTypeSymbol EnclosingType { get; } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupDiagnosticValidator.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupDiagnosticValidator.cs new file mode 100644 index 0000000000..56e6dcda7e --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupDiagnosticValidator.cs @@ -0,0 +1,9 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Analyzers +{ + internal abstract class StartupDiagnosticValidator + { + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupFacts.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupFacts.cs new file mode 100644 index 0000000000..4fc93ad6b6 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupFacts.cs @@ -0,0 +1,71 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNetCore.Mvc.Analyzers; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal static class StartupFacts + { + public static bool IsConfigureServices(StartupSymbols symbols, IMethodSymbol symbol) + { + if (symbol == null) + { + throw new ArgumentNullException(nameof(symbol)); + } + + if (symbol.DeclaredAccessibility != Accessibility.Public) + { + return false; + } + + if (!string.Equals(symbol.Name, SymbolNames.ConfigureServicesMethod, StringComparison.Ordinal)) + { + return false; + } + + if (symbol.Parameters.Length != 1) + { + return false; + } + + if (symbol.Parameters[0].Type != symbols.IServiceCollection) + { + return false; + } + + return true; + } + + public static bool IsConfigure(StartupSymbols symbols, IMethodSymbol symbol) + { + if (symbol == null) + { + throw new ArgumentNullException(nameof(symbol)); + } + + if (symbol.DeclaredAccessibility != Accessibility.Public) + { + return false; + } + + if (symbol.Name == null || !symbol.Name.StartsWith(SymbolNames.ConfigureMethod, StringComparison.Ordinal)) + { + return false; + } + + // IApplicationBuilder can appear in any parameter + for (var i = 0; i < symbol.Parameters.Length; i++) + { + if (symbol.Parameters[i].Type == symbols.IApplicationBuilder) + { + return true; + } + } + + return false; + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/StartupSymbols.cs b/src/Mvc/Mvc.Analyzers/src/Startup/StartupSymbols.cs new file mode 100644 index 0000000000..5595cd1a6d --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/StartupSymbols.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Mvc.Analyzers; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class StartupSymbols + { + public StartupSymbols(Compilation compilation) + { + IApplicationBuilder = compilation.GetTypeByMetadataName(SymbolNames.IApplicationBuilder); + IServiceCollection = compilation.GetTypeByMetadataName(SymbolNames.IServiceCollection); + MvcOptions = compilation.GetTypeByMetadataName(SymbolNames.MvcOptions); + } + + public INamedTypeSymbol IApplicationBuilder { get; } + + public INamedTypeSymbol IServiceCollection { get; } + + public INamedTypeSymbol MvcOptions { get; } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/Startup/UseMvcDiagnosticValidator.cs b/src/Mvc/Mvc.Analyzers/src/Startup/UseMvcDiagnosticValidator.cs new file mode 100644 index 0000000000..96464f274e --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/src/Startup/UseMvcDiagnosticValidator.cs @@ -0,0 +1,51 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Concurrent; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers +{ + internal class UseMvcDiagnosticValidator : StartupDiagnosticValidator + { + public static UseMvcDiagnosticValidator CreateAndInitialize(CompilationAnalysisContext context, ConcurrentBag analyses) + { + if (analyses == null) + { + throw new ArgumentNullException(nameof(analyses)); + } + + var validator = new UseMvcDiagnosticValidator(); + + foreach (var mvcOptionsAnalysis in analyses.OfType()) + { + // Each analysis of the options is one-per-class (in user code). Find the middleware analysis foreach of the Configure methods + // defined by this class and validate. + // + // Note that this doesn't attempt to handle inheritance scenarios. + foreach (var middlewareAnalsysis in analyses.OfType().Where(m => m.EnclosingType == mvcOptionsAnalysis.EnclosingType)) + { + foreach (var middlewareItem in middlewareAnalsysis.Middleware) + { + if ((middlewareItem.UseMethod.Name == "UseMvc" || middlewareItem.UseMethod.Name == "UseMvcWithDefaultRoute") && + + // Report a diagnostic if it's unclear that the user turned off Endpoint Routing. + (mvcOptionsAnalysis.EndpointRoutingEnabled == true || mvcOptionsAnalysis.EndpointRoutingEnabled == null)) + { + context.ReportDiagnostic(Diagnostic.Create( + StartupAnalzyer.UnsupportedUseMvcWithEndpointRouting, + middlewareItem.Operation.Syntax.GetLocation(), + middlewareItem.UseMethod.Name, + mvcOptionsAnalysis.ConfigureServicesMethod.Name)); + } + } + } + } + + return validator; + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/src/SymbolNames.cs b/src/Mvc/Mvc.Analyzers/src/SymbolNames.cs index 3b002d24e6..f8ac74c657 100644 --- a/src/Mvc/Mvc.Analyzers/src/SymbolNames.cs +++ b/src/Mvc/Mvc.Analyzers/src/SymbolNames.cs @@ -60,5 +60,17 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers public const string ProducesResponseTypeAttribute = "Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute"; public const string RenderPartialMethod = "RenderPartial"; + + public const string IApplicationBuilder = "Microsoft.AspNetCore.Builder.IApplicationBuilder"; + + public const string IServiceCollection = "Microsoft.Extensions.DependencyInjection.IServiceCollection"; + + public const string MvcOptions = "Microsoft.AspNetCore.Mvc.MvcOptions"; + + public const string EnableEndpointRoutingProperty = "EnableEndpointRouting"; + + public const string ConfigureServicesMethod = "ConfigureServices"; + + public const string ConfigureMethod = "Configure"; } } diff --git a/src/Mvc/Mvc.Analyzers/test/StartupAnalyzerTest.cs b/src/Mvc/Mvc.Analyzers/test/StartupAnalyzerTest.cs new file mode 100644 index 0000000000..b0a8e5ddca --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/StartupAnalyzerTest.cs @@ -0,0 +1,218 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.AspNetCore.Mvc; +using Microsoft.CodeAnalysis; +using Xunit; + +namespace Microsoft.AspNetCore.Analyzers +{ + public class StartupAnalyzerTest + { + public StartupAnalyzerTest() + { + StartupAnalyzer = new StartupAnalzyer(); + StartupAnalyzer.StartupFilePredicate = path => path.Equals("Test.cs", StringComparison.Ordinal); + + Runner = new MvcDiagnosticAnalyzerRunner(StartupAnalyzer); + + Analyses = new ConcurrentBag(); + ConfigureServicesMethods = new ConcurrentBag(); + ConfigureMethods = new ConcurrentBag(); + StartupAnalyzer.AnalysisStarted += (sender, analysis) => Analyses.Add(analysis); + StartupAnalyzer.ConfigureServicesMethodFound += (sender, method) => ConfigureServicesMethods.Add(method); + StartupAnalyzer.ConfigureMethodFound += (sender, method) => ConfigureMethods.Add(method); + } + + private StartupAnalzyer StartupAnalyzer { get; } + + private MvcDiagnosticAnalyzerRunner Runner { get; } + + private ConcurrentBag Analyses { get; } + + private ConcurrentBag ConfigureServicesMethods { get; } + + private ConcurrentBag ConfigureMethods { get; } + + [Fact] + public async Task StartupAnalyzer_FindsStartupMethods_StartupSignatures_Standard() + { + // Arrange + var source = ReadSource("StartupSignatures_Standard"); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + Assert.Empty(diagnostics); + + Assert.Collection(ConfigureServicesMethods, m => Assert.Equal("ConfigureServices", m.Name)); + Assert.Collection(ConfigureMethods, m => Assert.Equal("Configure", m.Name)); + } + + [Fact] + public async Task StartupAnalyzer_FindsStartupMethods_StartupSignatures_MoreVariety() + { + // Arrange + var source = ReadSource("StartupSignatures_MoreVariety"); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + Assert.Empty(diagnostics); + + Assert.Collection( + ConfigureServicesMethods.OrderBy(m => m.Name), + m => Assert.Equal("ConfigureServices", m.Name)); + + Assert.Collection( + ConfigureMethods.OrderBy(m => m.Name), + m => Assert.Equal("Configure", m.Name), + m => Assert.Equal("ConfigureProduction", m.Name)); + } + + [Fact] + public async Task StartupAnalyzer_MvcOptionsAnalysis_UseMvc_FindsEndpointRoutingDisabled() + { + // Arrange + var source = ReadSource("MvcOptions_UseMvcWithDefaultRouteAndEndpointRoutingDisabled"); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var mvcOptionsAnalysis = Assert.Single(Analyses.OfType()); + Assert.False(mvcOptionsAnalysis.EndpointRoutingEnabled); + + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + var middleware = Assert.Single(middlewareAnalysis.Middleware); + Assert.Equal("UseMvcWithDefaultRoute", middleware.UseMethod.Name); + + Assert.Empty(diagnostics); + } + + [Fact] + public async Task StartupAnalyzer_MvcOptionsAnalysis_AddMvcOptions_FindsEndpointRoutingDisabled() + { + // Arrange + var source = ReadSource("MvcOptions_UseMvcWithDefaultRouteAndAddMvcOptionsEndpointRoutingDisabled"); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var mvcOptionsAnalysis = Assert.Single(Analyses.OfType()); + Assert.False(mvcOptionsAnalysis.EndpointRoutingEnabled); + + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + var middleware = Assert.Single(middlewareAnalysis.Middleware); + Assert.Equal("UseMvcWithDefaultRoute", middleware.UseMethod.Name); + + Assert.Empty(diagnostics); + } + + [Theory] + [InlineData("MvcOptions_UseMvc", "UseMvc")] + [InlineData("MvcOptions_UseMvcAndConfiguredRoutes", "UseMvc")] + [InlineData("MvcOptions_UseMvcWithDefaultRoute", "UseMvcWithDefaultRoute")] + public async Task StartupAnalyzer_MvcOptionsAnalysis_FindsEndpointRoutingEnabled(string sourceFileName, string mvcMiddlewareName) + { + // Arrange + var source = ReadSource(sourceFileName); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var mvcOptionsAnalysis = Assert.Single(Analyses.OfType()); + Assert.Null(mvcOptionsAnalysis.EndpointRoutingEnabled); + + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + var middleware = Assert.Single(middlewareAnalysis.Middleware); + Assert.Equal(mvcMiddlewareName, middleware.UseMethod.Name); + + Assert.Collection( + diagnostics, + diagnostic => + { + Assert.Same(StartupAnalzyer.UnsupportedUseMvcWithEndpointRouting, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + }); + } + + [Fact] + public async Task StartupAnalyzer_MvcOptionsAnalysis_MultipleMiddleware() + { + // Arrange + var source = ReadSource("MvcOptions_UseMvcWithOtherMiddleware"); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var mvcOptionsAnalysis = Assert.Single(Analyses.OfType()); + Assert.Null(mvcOptionsAnalysis.EndpointRoutingEnabled); + + var middlewareAnalysis = Assert.Single(Analyses.OfType()); + + Assert.Collection( + middlewareAnalysis.Middleware, + item => Assert.Equal("UseAuthorization", item.UseMethod.Name), + item => Assert.Equal("UseMiddleware", item.UseMethod.Name), + item => Assert.Equal("UseMvc", item.UseMethod.Name), + item => Assert.Equal("UseRouting", item.UseMethod.Name), + item => Assert.Equal("UseEndpoints", item.UseMethod.Name)); + + Assert.Collection( + diagnostics, + diagnostic => + { + Assert.Same(StartupAnalzyer.UnsupportedUseMvcWithEndpointRouting, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + }); + } + + [Fact] + public async Task StartupAnalyzer_MvcOptionsAnalysis_MultipleUseMvc() + { + // Arrange + var source = ReadSource("MvcOptions_UseMvcMultiple"); + + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var mvcOptionsAnalysis = Assert.Single(Analyses.OfType()); + Assert.Null(mvcOptionsAnalysis.EndpointRoutingEnabled); + + Assert.Collection( + diagnostics, + diagnostic => + { + Assert.Same(StartupAnalzyer.UnsupportedUseMvcWithEndpointRouting, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM1"], diagnostic.Location); + }, + diagnostic => + { + Assert.Same(StartupAnalzyer.UnsupportedUseMvcWithEndpointRouting, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM2"], diagnostic.Location); + }, + diagnostic => + { + Assert.Same(StartupAnalzyer.UnsupportedUseMvcWithEndpointRouting, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM3"], diagnostic.Location); + }); + } + + private TestSource ReadSource(string fileName) + { + return MvcTestSource.Read(nameof(StartupAnalyzerTest), fileName); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvc.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvc.cs new file mode 100644 index 0000000000..771d0e07fe --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvc.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class MvcOptions_UseMvc + { + public void ConfigureServices(IServiceCollection services) + { + services.AddMvc(); + } + + public void Configure(IApplicationBuilder app) + { + /*MM*/app.UseMvc(); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs new file mode 100644 index 0000000000..89ab994d92 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcAndConfiguredRoutes.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class MvcOptions_UseMvcAndConfiguredRoutes + { + public void ConfigureServices(IServiceCollection services) + { + services.AddMvc(); + } + + public void Configure(IApplicationBuilder app) + { + /*MM*/app.UseMvc(routes => + { + routes.MapRoute("Name", "Template"); + }); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs new file mode 100644 index 0000000000..c63cb42813 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcMultiple.cs @@ -0,0 +1,34 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class MvcOptions_UseMvcMultiple + { + public void ConfigureServices(IServiceCollection services) + { + services.AddMvc(); + } + + public void Configure(IApplicationBuilder app) + { + /*MM1*/app.UseMvcWithDefaultRoute(); + + app.UseAuthorization(); + app.UseMiddleware(); + + /*MM2*/app.UseMvc(); + + app.UseRouting(); + app.UseEndpoints(endpoints => + { + }); + + /*MM3*/app.UseMvc(); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRoute.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRoute.cs new file mode 100644 index 0000000000..450f6d7e1b --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRoute.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class MvcOptions_UseMvcWithDefaultRoute + { + public void ConfigureServices(IServiceCollection services) + { + services.AddMvc(); + } + + public void Configure(IApplicationBuilder app) + { + /*MM*/app.UseMvcWithDefaultRoute(); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRouteAndAddMvcOptionsEndpointRoutingDisabled.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRouteAndAddMvcOptionsEndpointRoutingDisabled.cs new file mode 100644 index 0000000000..99c031bea2 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRouteAndAddMvcOptionsEndpointRoutingDisabled.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class MvcOptions_UseMvcWithDefaultRouteAndAddMvcOptionsEndpointRoutingDisabled + { + public void ConfigureServices(IServiceCollection services) + { + services.AddMvc().AddMvcOptions(options => options.EnableEndpointRouting = false); + } + + public void Configure(IApplicationBuilder app) + { + app.UseMvcWithDefaultRoute(); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRouteAndEndpointRoutingDisabled.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRouteAndEndpointRoutingDisabled.cs new file mode 100644 index 0000000000..1d9d5e5ec1 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithDefaultRouteAndEndpointRoutingDisabled.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class MvcOptions_UseMvcWithDefaultRouteAndEndpointRoutingDisabled + { + public void ConfigureServices(IServiceCollection services) + { + services.AddMvc(options => options.EnableEndpointRouting = false); + } + + public void Configure(IApplicationBuilder app) + { + app.UseMvcWithDefaultRoute(); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs new file mode 100644 index 0000000000..58005b87f8 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/MvcOptions_UseMvcWithOtherMiddleware.cs @@ -0,0 +1,30 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class MvcOptions_UseMvcWithOtherMiddleware + { + public void ConfigureServices(IServiceCollection services) + { + services.AddMvc(); + } + + public void Configure(IApplicationBuilder app) + { + app.UseAuthorization(); + app.UseMiddleware(); + + /*MM*/app.UseMvc(); + + app.UseRouting(); + app.UseEndpoints(endpoints => + { + }); + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/StartupSignatures_MoreVariety.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/StartupSignatures_MoreVariety.cs new file mode 100644 index 0000000000..46d3ead7c9 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/StartupSignatures_MoreVariety.cs @@ -0,0 +1,37 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Text; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class StartupSignatures_MoreVariety + { + public void ConfigureServices(IServiceCollection services) + { + } + + public void ConfigureServices(IServiceCollection services, StringBuilder s) // Ignored + { + } + + public void Configure(StringBuilder s) // Ignored, + { + } + + public void Configure(IApplicationBuilder app, IWebHostEnvironment env) + { + } + + public void ConfigureProduction(IWebHostEnvironment env, IApplicationBuilder app) + { + } + + private void Configure(IApplicationBuilder app) // Ignored + { + } + } +} diff --git a/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/StartupSignatures_Standard.cs b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/StartupSignatures_Standard.cs new file mode 100644 index 0000000000..286918ba47 --- /dev/null +++ b/src/Mvc/Mvc.Analyzers/test/TestFiles/StartupAnalyzerTest/StartupSignatures_Standard.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Analyzers.TestFiles.StartupAnalyzerTest +{ + public class StartupSignatures_Standard + { + public void ConfigureServices(IServiceCollection services) + { + } + + public void Configure(IApplicationBuilder app) + { + + } + } +}