diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers.Experimental/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers.Experimental/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs deleted file mode 100644 index 1e66b2f087..0000000000 --- a/src/Microsoft.AspNetCore.Mvc.Analyzers.Experimental/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs +++ /dev/null @@ -1,94 +0,0 @@ -// 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.Linq; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Diagnostics; - -namespace Microsoft.AspNetCore.Mvc.Analyzers -{ - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer : ApiControllerAnalyzerBase - { - public ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer() - : base(ExperimentalDiagnosticDescriptors.MVC7001_ApiActionsHaveBadModelStateFilter) - { - } - - protected override void InitializeWorker(ApiControllerAnalyzerContext analyzerContext) - { - analyzerContext.Context.RegisterSyntaxNodeAction(context => - { - var methodSyntax = (MethodDeclarationSyntax)context.Node; - if (methodSyntax.Body == null) - { - // Ignore expression bodied methods. - return; - } - - var method = context.SemanticModel.GetDeclaredSymbol(methodSyntax, context.CancellationToken); - if (!analyzerContext.IsApiAction(method)) - { - return; - } - - if (method.ReturnsVoid || method.ReturnType == analyzerContext.SystemThreadingTaskOfT) - { - // Void or Task returning methods. We don't have to check anything here since we're specifically - // looking for return BadRequest(..); - return; - } - - // Only look for top level statements that look like "if (!ModelState.IsValid)" - foreach (var memberAccessSyntax in methodSyntax.Body.DescendantNodes().OfType()) - { - var ancestorIfStatement = memberAccessSyntax.FirstAncestorOrSelf(); - if (ancestorIfStatement == null) - { - // Node's not in an if statement. - continue; - } - - var symbolInfo = context.SemanticModel.GetSymbolInfo(memberAccessSyntax, context.CancellationToken); - - if (!(symbolInfo.Symbol is IPropertySymbol property) || - (property.ContainingType != analyzerContext.ModelStateDictionary) || - !string.Equals(property.Name, "IsValid", StringComparison.Ordinal) || - !IsFalseExpression(memberAccessSyntax)) - { - continue; - } - - var containingBlock = (SyntaxNode)ancestorIfStatement; - if (containingBlock.Parent.Kind() == SyntaxKind.ElseClause) - { - containingBlock = containingBlock.Parent; - } - context.ReportDiagnostic(Diagnostic.Create(SupportedDiagnostic, containingBlock.GetLocation())); - return; - } - }, SyntaxKind.MethodDeclaration); - } - - private static bool IsFalseExpression(MemberAccessExpressionSyntax memberAccessSyntax) - { - switch (memberAccessSyntax.Parent.Kind()) - { - case SyntaxKind.LogicalNotExpression: - // !ModelState.IsValid - return true; - case SyntaxKind.EqualsExpression: - var binaryExpression = (BinaryExpressionSyntax)memberAccessSyntax.Parent; - // ModelState.IsValid == false - // false == ModelState.IsValid - return binaryExpression.Left.Kind() == SyntaxKind.FalseLiteralExpression || - binaryExpression.Right.Kind() == SyntaxKind.FalseLiteralExpression; - } - - return false; - } - } -} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers.Experimental/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers.Experimental/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider.cs deleted file mode 100644 index b238ad8ece..0000000000 --- a/src/Microsoft.AspNetCore.Mvc.Analyzers.Experimental/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider.cs +++ /dev/null @@ -1,46 +0,0 @@ -// 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.Collections.Immutable; -using System.Composition; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.Editing; - -namespace Microsoft.AspNetCore.Mvc.Analyzers -{ - [ExportCodeFixProvider(LanguageNames.CSharp)] - [Shared] - public class ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider : CodeFixProvider - { - public sealed override ImmutableArray FixableDiagnosticIds => - ImmutableArray.Create(ExperimentalDiagnosticDescriptors.MVC7001_ApiActionsHaveBadModelStateFilter.Id); - - public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; - - public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - const string title = "Remove ModelState.IsValid check"; - var rootNode = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - - context.RegisterCodeFix( - CodeAction.Create( - title, - createChangedDocument: CreateChangedDocumentAsync, - equivalenceKey: title), - context.Diagnostics); - - async Task CreateChangedDocumentAsync(CancellationToken cancellationToken) - { - var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false); - var node = rootNode.FindNode(context.Span); - editor.RemoveNode(node); - - return editor.GetChangedDocument(); - } - } - } -} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs new file mode 100644 index 0000000000..bdd8dbac75 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.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.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer : DiagnosticAnalyzer + { + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create( + DiagnosticDescriptors.MVC1007_ApiActionsDoNotRequireExplicitModelValidationCheck); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(compilationStartAnalysisContext => + { + var symbolCache = new ApiControllerSymbolCache(compilationStartAnalysisContext.Compilation); + if (symbolCache.ApiConventionTypeAttribute == null || symbolCache.ApiConventionTypeAttribute.TypeKind == TypeKind.Error) + { + // No-op if we can't find types we care about. + return; + } + + InitializeWorker(compilationStartAnalysisContext, symbolCache); + }); + } + + private void InitializeWorker(CompilationStartAnalysisContext context, ApiControllerSymbolCache symbolCache) + { + context.RegisterOperationAction(operationAnalysisContext => + { + var ifOperation = (IConditionalOperation)operationAnalysisContext.Operation; + if (!(ifOperation.Syntax is IfStatementSyntax ifStatement)) + { + return; + } + + if (ifOperation.WhenTrue == null || ifOperation.WhenFalse != null) + { + // We only support expressions of the format + // if (!ModelState.IsValid) + // or + // if (ModelState.IsValid == false) + // If the conditional is misisng a true condition or has an else expression, skip this operation. + return; + } + + var parent = ifOperation.Parent; + if (parent?.Kind == OperationKind.Block) + { + parent = parent?.Parent; + } + + if (parent?.Kind != OperationKind.MethodBodyOperation) + { + // Only support top-level ModelState IsValid checks. + return; + } + + var trueStatement = UnwrapSingleStatementBlock(ifOperation.WhenTrue); + if (trueStatement.Kind != OperationKind.Return) + { + // We need to verify that the if statement does a ModelState.IsValid check and that the block inside contains + // a single return statement returning a 400. We'l get to it in just a bit + return; + } + + var methodSyntax = (MethodDeclarationSyntax)parent.Syntax; + var semanticModel = operationAnalysisContext.Compilation.GetSemanticModel(methodSyntax.SyntaxTree); + var methodSymbol = semanticModel.GetDeclaredSymbol(methodSyntax, operationAnalysisContext.CancellationToken); + + if (!ApiControllerFacts.IsApiControllerAction(symbolCache, methodSymbol)) + { + // Not a ApiController. Nothing to do here. + return; + } + + if (!IsModelStateIsValidCheck(symbolCache, ifOperation.Condition)) + { + return; + } + + var returnOperation = (IReturnOperation)trueStatement; + + var returnValue = returnOperation.ReturnedValue; + if (returnValue == null || + !symbolCache.IActionResult.IsAssignableFrom(returnValue.Type)) + { + return; + } + + var returnStatementSyntax = (ReturnStatementSyntax)returnOperation.Syntax; + var actualMetadata = SymbolApiResponseMetadataProvider.InspectReturnStatementSyntax( + symbolCache, + semanticModel, + returnStatementSyntax, + operationAnalysisContext.CancellationToken); + + if (actualMetadata == null || actualMetadata.Value.StatusCode != 400) + { + return; + } + + var additionalLocations = new[] + { + ifStatement.GetLocation(), + returnStatementSyntax.GetLocation(), + }; + + operationAnalysisContext.ReportDiagnostic( + Diagnostic.Create( + DiagnosticDescriptors.MVC1007_ApiActionsDoNotRequireExplicitModelValidationCheck, + ifStatement.GetLocation(), + additionalLocations: additionalLocations)); + }, OperationKind.Conditional); + } + + private bool IsModelStateIsValidCheck(in ApiControllerSymbolCache symbolCache, IOperation condition) + { + switch (condition.Kind) + { + case OperationKind.UnaryOperator: + var operation = ((IUnaryOperation)condition).Operand; + return IsModelStateIsValidPropertyAccessor(symbolCache, operation); + + case OperationKind.BinaryOperator: + var binaryOperation = (IBinaryOperation)condition; + if (binaryOperation.OperatorKind == BinaryOperatorKind.Equals) + { + // (ModelState.IsValid == false) OR (false == ModelState.IsValid) + return EvaluateBinaryOperator(symbolCache, binaryOperation.LeftOperand, binaryOperation.RightOperand, false) || + EvaluateBinaryOperator(symbolCache, binaryOperation.RightOperand, binaryOperation.LeftOperand, false); + } + else if (binaryOperation.OperatorKind == BinaryOperatorKind.NotEquals) + { + // (ModelState.IsValid != true) OR (true != ModelState.IsValid) + return EvaluateBinaryOperator(symbolCache, binaryOperation.LeftOperand, binaryOperation.RightOperand, true) || + EvaluateBinaryOperator(symbolCache, binaryOperation.RightOperand, binaryOperation.LeftOperand, true); + } + return false; + + default: + return false; + } + } + + private bool EvaluateBinaryOperator( + in ApiControllerSymbolCache symbolCache, + IOperation operation, + IOperation otherOperation, + bool expectedConstantValue) + { + if (operation.Kind != OperationKind.Literal) + { + return false; + } + + var constantValue = ((ILiteralOperation)operation).ConstantValue; + if (!constantValue.HasValue || + !(constantValue.Value is bool boolCostantVaue) || + boolCostantVaue != expectedConstantValue) + { + return false; + } + + return IsModelStateIsValidPropertyAccessor(symbolCache, otherOperation); + } + + private static bool IsModelStateIsValidPropertyAccessor(in ApiControllerSymbolCache symbolCache, IOperation operation) + { + if (operation.Kind != OperationKind.PropertyReference) + { + return false; + } + + var propertyReference = (IPropertyReferenceOperation)operation; + if (propertyReference.Property.Name != "IsValid") + { + return false; + } + + if (propertyReference.Member.ContainingType != symbolCache.ModelStateDictionary) + { + return false; + } + + if (propertyReference.Instance?.Kind != OperationKind.PropertyReference) + { + // Verify this is refering to the ModelState property on the current controller instance + return false; + } + + var modelStatePropertyReference = (IPropertyReferenceOperation)propertyReference.Instance; + if (modelStatePropertyReference.Instance?.Kind != OperationKind.InstanceReference) + { + return false; + } + + return true; + } + + private static IOperation UnwrapSingleStatementBlock(IOperation statement) + { + return statement is IBlockOperation block && block.Operations.Length == 1 ? + block.Operations[0] : + statement; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCodeFixProvider.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCodeFixProvider.cs new file mode 100644 index 0000000000..3446de28a4 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCodeFixProvider.cs @@ -0,0 +1,69 @@ +// 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.Collections.Immutable; +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Text; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + [Shared] + public class ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider : CodeFixProvider + { + public sealed override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(DiagnosticDescriptors.MVC1007_ApiActionsDoNotRequireExplicitModelValidationCheck.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) + { + if (context.Diagnostics.Length != 1) + { + return Task.CompletedTask; + } + + var diagnostic = context.Diagnostics[0]; + if (diagnostic.Id != DiagnosticDescriptors.MVC1007_ApiActionsDoNotRequireExplicitModelValidationCheck.Id) + { + return Task.CompletedTask; + } + + context.RegisterCodeFix(new MyCodeAction(context.Document, context.Span), diagnostic); + return Task.CompletedTask; + } + + private class MyCodeAction : CodeAction + { + private readonly Document _document; + private readonly TextSpan _ifBlockSpan; + + public MyCodeAction(Document document, TextSpan ifBlockSpan) + { + _document = document; + _ifBlockSpan = ifBlockSpan; + } + + public override string EquivalenceKey => Title; + + public override string Title => "Remove ModelState.IsValid check"; + + protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) + { + var rootNode = await _document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var editor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false); + + var ifBlockSyntax = rootNode.FindNode(_ifBlockSpan); + editor.RemoveNode(ifBlockSyntax); + + return editor.GetChangedDocument(); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerFacts.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerFacts.cs new file mode 100644 index 0000000000..48d81975ef --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerFacts.cs @@ -0,0 +1,37 @@ +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + internal static class ApiControllerFacts + { + public static bool IsApiControllerAction(ApiControllerSymbolCache symbolCache, IMethodSymbol method) + { + if (method == null) + { + return false; + } + + if (method.ReturnsVoid || method.ReturnType.TypeKind == TypeKind.Error) + { + return false; + } + + if (!MvcFacts.IsController(method.ContainingType, symbolCache.ControllerAttribute, symbolCache.NonControllerAttribute)) + { + return false; + } + + if (!method.ContainingType.HasAttribute(symbolCache.IApiBehaviorMetadata, inherit: true)) + { + return false; + } + + if (!MvcFacts.IsControllerAction(method, symbolCache.NonActionAttribute, symbolCache.IDisposableDispose)) + { + return false; + } + + return true; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerSymbolCache.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerSymbolCache.cs index 74e687e710..a28cee2458 100644 --- a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerSymbolCache.cs +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerSymbolCache.cs @@ -20,6 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers IActionResult = compilation.GetTypeByMetadataName(SymbolNames.IActionResult); IApiBehaviorMetadata = compilation.GetTypeByMetadataName(SymbolNames.IApiBehaviorMetadata); IConvertToActionResult = compilation.GetTypeByMetadataName(SymbolNames.IConvertToActionResult); + ModelStateDictionary = compilation.GetTypeByMetadataName(SymbolNames.ModelStateDictionary); NonActionAttribute = compilation.GetTypeByMetadataName(SymbolNames.NonActionAttribute); NonControllerAttribute = compilation.GetTypeByMetadataName(SymbolNames.NonControllerAttribute); ProducesResponseTypeAttribute = compilation.GetTypeByMetadataName(SymbolNames.ProducesResponseTypeAttribute); @@ -51,6 +52,8 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers public IMethodSymbol IDisposableDispose { get; } + public ITypeSymbol ModelStateDictionary { get; } + public INamedTypeSymbol NonActionAttribute { get; } public INamedTypeSymbol NonControllerAttribute { get; } diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiConventionAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiConventionAnalyzer.cs index 57ce1b1e26..30b4b507d8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiConventionAnalyzer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiConventionAnalyzer.cs @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers var semanticModel = syntaxNodeContext.SemanticModel; var method = semanticModel.GetDeclaredSymbol(methodSyntax, syntaxNodeContext.CancellationToken); - if (!ShouldEvaluateMethod(symbolCache, method)) + if (!ApiControllerFacts.IsApiControllerAction(symbolCache, method)) { return; } @@ -113,36 +113,6 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers return attributes; } - internal static bool ShouldEvaluateMethod(ApiControllerSymbolCache symbolCache, IMethodSymbol method) - { - if (method == null) - { - return false; - } - - if (method.ReturnsVoid || method.ReturnType.TypeKind == TypeKind.Error) - { - return false; - } - - if (!MvcFacts.IsController(method.ContainingType, symbolCache.ControllerAttribute, symbolCache.NonControllerAttribute)) - { - return false; - } - - if (!method.ContainingType.HasAttribute(symbolCache.IApiBehaviorMetadata, inherit: true)) - { - return false; - } - - if (!MvcFacts.IsControllerAction(method, symbolCache.NonActionAttribute, symbolCache.IDisposableDispose)) - { - return false; - } - - return true; - } - internal static bool HasStatusCode(IList declaredApiResponseMetadata, int statusCode) { if (declaredApiResponseMetadata.Count == 0) diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/DiagnosticDescriptors.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/DiagnosticDescriptors.cs index ea5e213385..f8c1b7bb72 100644 --- a/src/Microsoft.AspNetCore.Mvc.Analyzers/DiagnosticDescriptors.cs +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/DiagnosticDescriptors.cs @@ -69,5 +69,16 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers "Usage", DiagnosticSeverity.Info, isEnabledByDefault: false); + + public static readonly DiagnosticDescriptor MVC1007_ApiActionsDoNotRequireExplicitModelValidationCheck = + new DiagnosticDescriptor( + "MVC1007", + "Action methods on ApiController instances do not require explicit model validation check.", + "Action methods on ApiController instances do not require explicit model validation check.", + "Usage", + DiagnosticSeverity.Info, + isEnabledByDefault: true, + customTags: new[] { WellKnownDiagnosticTags.Unnecessary }); + } } diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/SymbolNames.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/SymbolNames.cs index 32b7cf62f3..db0874e74e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Analyzers/SymbolNames.cs +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/SymbolNames.cs @@ -23,6 +23,8 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers public const string DefaultStatusCodeAttribute = "Microsoft.AspNetCore.Mvc.Infrastructure.DefaultStatusCodeAttribute"; + public const string HtmlHelperPartialExtensionsType = "Microsoft.AspNetCore.Mvc.Rendering.HtmlHelperPartialExtensions"; + public const string IApiBehaviorMetadata = "Microsoft.AspNetCore.Mvc.Internal.IApiBehaviorMetadata"; public const string IActionResult = "Microsoft.AspNetCore.Mvc.IActionResult"; @@ -31,12 +33,12 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers public const string IFilterMetadataType = "Microsoft.AspNetCore.Mvc.Filters.IFilterMetadata"; - public const string HtmlHelperPartialExtensionsType = "Microsoft.AspNetCore.Mvc.Rendering.HtmlHelperPartialExtensions"; - public const string IHtmlHelperType = "Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper"; public const string IRouteTemplateProvider = "Microsoft.AspNetCore.Mvc.Routing.IRouteTemplateProvider"; + public const string ModelStateDictionary = "Microsoft.AspNetCore.Mvc.ModelBinding.ModelStateDictionary"; + public const string NonActionAttribute = "Microsoft.AspNetCore.Mvc.NonActionAttribute"; public const string NonControllerAttribute = "Microsoft.AspNetCore.Mvc.NonControllerAttribute"; diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Experimental.Test/ApActionsDoNotRequireExplicitModelValidationCheckFacts.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Experimental.Test/ApActionsDoNotRequireExplicitModelValidationCheckFacts.cs deleted file mode 100644 index 85c805bca9..0000000000 --- a/test/Microsoft.AspNetCore.Mvc.Analyzers.Experimental.Test/ApActionsDoNotRequireExplicitModelValidationCheckFacts.cs +++ /dev/null @@ -1,359 +0,0 @@ -// 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.Threading.Tasks; -using Microsoft.AspNetCore.Analyzer.Testing; -using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.Diagnostics; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.Analyzers -{ - public class ApiActionsDoNotRequireExplicitModelValidationCheckFacts : AnalyzerTestBase - { - private static DiagnosticDescriptor DiagnosticDescriptor = ExperimentalDiagnosticDescriptors.MVC7001_ApiActionsHaveBadModelStateFilter; - - protected override DiagnosticAnalyzer DiagnosticAnalyzer { get; } - = new ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer(); - - protected override CodeFixProvider CodeFixProvider { get; } - = new ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider(); - - [Fact] - public async Task NoDiagnosticsAreReturned_FoEmptyScenarios() - { - // Arrange - var test = @""; - var project = CreateProject(test); - - // Act - var result = await GetDiagnosticAsync(project); - - // Assert - Assert.Empty(result); - } - - [Fact] - public async Task NoDiagnosticsAreReturned_WhenTypeIsNotApiController() - { - // Arrange - var test = -@" -using Microsoft.AspNetCore.Mvc; - -public class HomeController : Controller -{ - public IActionResult Index() => View(); -}"; - var project = CreateProject(test); - - // Act - var result = await GetDiagnosticAsync(project); - - // Assert - Assert.Empty(result); - } - - [Fact] - public async Task NoDiagnosticsAreReturned_WhenActionDoesNotHaveModelStateCheck() - { - // Arrange - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : Controller -{ - public IActionResult GetPetId() - { - return Ok(new object()); - } -}"; - var project = CreateProject(test); - - // Act - var result = await GetDiagnosticAsync(project); - - // Assert - Assert.Empty(result); - } - - [Fact] - public async Task NoDiagnosticsAreReturned_WhenAActionsUseExpressionBodies() - { - // Arrange - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : Controller -{ - public IActionResult GetPetId() => ModelState.IsVald ? OK() : BadResult(); -}"; - var project = CreateProject(test); - - // Act - var result = await GetDiagnosticAsync(project); - - // Assert - Assert.Empty(result); - } - - [Fact] - public async Task NoDiagnosticsAreReturned_ForNonActions() - { - // Arrange - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - private int GetPetIdPrivate() => 0; - protected int GetPetIdProtected() => 0; - public static IActionResult FindPetByStatus(int status) => null; - [NonAction] - public object Reset(int state) => null; -}"; - - var project = CreateProject(test); - - // Act - var result = await GetDiagnosticAsync(project); - - // Assert - Assert.Empty(result); - } - - [Fact] - public Task DiagnosticsAndCodeFixes_WhenActionHasModelStateIsValidCheck() - { - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - if (!ModelState.IsValid) - { - return BadRequest(); - } - - return Ok(); - } -}"; - - // Act & Assert - return VerifyAsync(test); - } - - - [Fact] - public Task DiagnosticsAndCodeFixes_WhenActionHasModelStateIsValidCheck_UsingComparisonToFalse() - { - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - if (ModelState.IsValid == false) - { - return BadRequest(); - } - - return Ok(); - } -}"; - - // Act & Assert - return VerifyAsync(test); - } - - [Fact] - public Task DiagnosticsAndCodeFixes_WhenActionHasModelStateIsValidCheck_WithoutBraces() - { - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - if (!ModelState.IsValid) - return BadRequest(); - - return Ok(); - } -}"; - return VerifyAsync(test); - } - - [Fact] - public async Task DiagnosticsAndCodeFixes_WhenModelStateIsInElseIf() - { - // Arrange - var expectedLocation = new DiagnosticLocation("Test.cs", 13, 9); - - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - if (User == null) - { - return Unauthorized(); - } - else if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - - return Ok(); - } -}"; - var expectedFix = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - if (User == null) - { - return Unauthorized(); - } - - return Ok(); - } -}"; - var project = CreateProject(test); - - // Act & Assert - var actualDiagnostics = await GetDiagnosticAsync(project); - AssertDiagnostic(expectedLocation, actualDiagnostics); - var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); - Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); - } - - [Fact] - public async Task DiagnosticsAndCodeFixes_WhenModelStateIsInNestedBlock() - { - // Arrange - var expectedLocation = new DiagnosticLocation("Test.cs", 15, 13); - - var test = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - if (User == null) - { - return Unauthorized(); - } - else - { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - - Debug.Assert(ModelState.Count == 0); - } - - return Ok(); - } -}"; - var expectedFix = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - if (User == null) - { - return Unauthorized(); - } - else - { - Debug.Assert(ModelState.Count == 0); - } - - return Ok(); - } -}"; - - var project = CreateProject(test); - - // Act & Assert - var actualDiagnostics = await GetDiagnosticAsync(project); - AssertDiagnostic(expectedLocation, actualDiagnostics); - var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); - Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); - } - - private async Task VerifyAsync(string test) - { - // Arrange - var expectedLocation = new DiagnosticLocation("Test.cs", 9, 9); - var expectedFix = -@" -using Microsoft.AspNetCore.Mvc; - -[ApiController] -public class PetController : ControllerBase -{ - public IActionResult GetPetId() - { - return Ok(); - } -}"; - var project = CreateProject(test); - - // Act & Assert - var actualDiagnostics = await GetDiagnosticAsync(project); - AssertDiagnostic(expectedLocation, actualDiagnostics); - var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); - Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); - } - - private void AssertDiagnostic(DiagnosticLocation expectedLocation, Diagnostic[] actualDiagnostics) - { - // Assert - Assert.Collection( - actualDiagnostics, - diagnostic => - { - Assert.Equal(DiagnosticDescriptor.Id, diagnostic.Id); - Assert.Same(DiagnosticDescriptor, diagnostic.Descriptor); - AnalyzerAssert.DiagnosticLocation(expectedLocation, diagnostic.Location); - }); - } - } -} \ No newline at end of file diff --git a/test/Mvc.Analyzers.Test/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest.cs b/test/Mvc.Analyzers.Test/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest.cs new file mode 100644 index 0000000000..d2feb871ff --- /dev/null +++ b/test/Mvc.Analyzers.Test/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest.cs @@ -0,0 +1,86 @@ +// 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.Runtime.CompilerServices; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest + { + private MvcDiagnosticAnalyzerRunner AnalyzerRunner { get; } = new MvcDiagnosticAnalyzerRunner(new ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer()); + + [Fact] + public Task NoDiagnosticsAreReturned_ForNonApiController() + => RunNoDiagnosticsAreReturned(); + + [Fact] + public Task NoDiagnosticsAreReturned_ForRazorPageModels() + => RunNoDiagnosticsAreReturned(); + + [Fact] + public Task NoDiagnosticsAreReturned_ForApiActionsWithoutModelStateChecks() + => RunNoDiagnosticsAreReturned(); + + [Fact] + public Task NoDiagnosticsAreReturned_ForApiActionsReturning400FromNonModelStateIsValidBlocks() + => RunNoDiagnosticsAreReturned(); + + [Fact] + public Task NoDiagnosticsAreReturned_ForApiActionsReturningNot400FromNonModelStateIsValidBlock() + => RunNoDiagnosticsAreReturned(); + + [Fact] + public Task NoDiagnosticsAreReturned_ForApiActionsCheckingAdditionalConditions() + => RunNoDiagnosticsAreReturned(); + + [Fact] + public Task DiagnosticsAreReturned_ForApiActionsWithModelStateChecks() + => RunTest(); + + [Fact] + public Task DiagnosticsAreReturned_ForApiActionsWithModelStateChecksUsingEquality() + => RunTest(); + + [Fact] + public Task DiagnosticsAreReturned_ForApiActionsWithModelStateChecksWithoutBracing() + => RunTest(); + + private async Task RunNoDiagnosticsAreReturned([CallerMemberName] string testMethod = "") + { + // Arrange + var testSource = MvcTestSource.Read(GetType().Name, testMethod); + var expectedLocation = testSource.DefaultMarkerLocation; + + // Act + var result = await AnalyzerRunner.GetDiagnosticsAsync(testSource.Source); + + // Assert + Assert.Empty(result); + } + + private async Task RunTest([CallerMemberName] string testMethod = "") + { + // Arrange + var descriptor = DiagnosticDescriptors.MVC1007_ApiActionsDoNotRequireExplicitModelValidationCheck; + var testSource = MvcTestSource.Read(GetType().Name, testMethod); + var expectedLocation = testSource.DefaultMarkerLocation; + + // Act + var result = await AnalyzerRunner.GetDiagnosticsAsync(testSource.Source); + + // Assert + Assert.Collection( + result, + diagnostic => + { + Assert.Equal(descriptor.Id, diagnostic.Id); + Assert.Same(descriptor, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(expectedLocation, diagnostic.Location); + }); + } + } +} diff --git a/test/Mvc.Analyzers.Test/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest.cs b/test/Mvc.Analyzers.Test/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest.cs new file mode 100644 index 0000000000..d470e22112 --- /dev/null +++ b/test/Mvc.Analyzers.Test/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest.cs @@ -0,0 +1,63 @@ +// 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.Runtime.CompilerServices; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest + { + private MvcDiagnosticAnalyzerRunner AnalyzerRunner { get; } = new MvcDiagnosticAnalyzerRunner(new ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer()); + + private CodeFixRunner CodeFixRunner => CodeFixRunner.Default; + + [Fact] + public Task CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck() + => RunTest(); + + [Fact] + public Task CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck() + => RunTest(); + + [Fact] + public Task CodeFixRemovesIfBlockWithoutBraces() + => RunTest(); + + private async Task RunTest([CallerMemberName] string testMethod = "") + { + // Arrange + var project = GetProject(testMethod); + var controllerDocument = project.DocumentIds[0]; + var expectedOutput = Read(testMethod + ".Output"); + + // Act + var diagnostics = await AnalyzerRunner.GetDiagnosticsAsync(project); + Assert.NotEmpty(diagnostics); + var actualOutput = await CodeFixRunner.ApplyCodeFixAsync( + new ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider(), + project.GetDocument(controllerDocument), + diagnostics[0]); + + Assert.Equal(expectedOutput, actualOutput, ignoreLineEndingDifferences: true); + } + + private Project GetProject(string testMethod) + { + var testSource = Read(testMethod + ".Input"); + return DiagnosticProject.Create(GetType().Assembly, new[] { testSource }); + } + + private string Read(string fileName) + { + return MvcTestSource.Read(GetType().Name, fileName) + .Source + .Replace("_INPUT_", "_TEST_") + .Replace("_OUTPUT_", "_TEST_"); + } + } +} diff --git a/test/Mvc.Analyzers.Test/ApiConventionAnalyzerTest.cs b/test/Mvc.Analyzers.Test/ApiControllerFactsTest.cs similarity index 78% rename from test/Mvc.Analyzers.Test/ApiConventionAnalyzerTest.cs rename to test/Mvc.Analyzers.Test/ApiControllerFactsTest.cs index b1e7578557..a264d33878 100644 --- a/test/Mvc.Analyzers.Test/ApiConventionAnalyzerTest.cs +++ b/test/Mvc.Analyzers.Test/ApiControllerFactsTest.cs @@ -10,10 +10,10 @@ using Xunit; namespace Microsoft.AspNetCore.Mvc.Analyzers { - public class ApiConventionAnalyzerTest + public class ApiControllerFactsTest { [Fact] - public async Task ShouldEvaluateMethod_ReturnsFalse_IfMethodReturnTypeIsInvalid() + public async Task IsApiControllerAction_ReturnsFalse_IfMethodReturnTypeIsInvalid() { // Arrange var source = @" @@ -41,14 +41,14 @@ namespace TestNamespace var method = (IMethodSymbol)compilation.GetTypeByMetadataName("TestNamespace.TestController").GetMembers("Get").First(); // Act - var result = ApiConventionAnalyzer.ShouldEvaluateMethod(symbolCache, method); + var result = ApiControllerFacts.IsApiControllerAction(symbolCache, method); // Assert Assert.False(result); } [Fact] - public async Task ShouldEvaluateMethod_ReturnsFalse_IfContainingTypeIsNotController() + public async Task IsApiControllerAction_ReturnsFalse_IfContainingTypeIsNotController() { // Arrange var compilation = await GetCompilation(); @@ -57,14 +57,14 @@ namespace TestNamespace var method = (IMethodSymbol)type.GetMembers(nameof(ApiConventionAnalyzerTest_IndexModel.OnGet)).First(); // Act - var result = ApiConventionAnalyzer.ShouldEvaluateMethod(symbolCache, method); + var result = ApiControllerFacts.IsApiControllerAction(symbolCache, method); // Assert Assert.False(result); } [Fact] - public async Task ShouldEvaluateMethod_ReturnsFalse_IfContainingTypeIsNotApiController() + public async Task IsApiControllerAction_ReturnsFalse_IfContainingTypeIsNotApiController() { // Arrange var compilation = await GetCompilation(); @@ -73,14 +73,14 @@ namespace TestNamespace var method = (IMethodSymbol)type.GetMembers(nameof(ApiConventionAnalyzerTest_NotApiController.Index)).First(); // Act - var result = ApiConventionAnalyzer.ShouldEvaluateMethod(symbolCache, method); + var result = ApiControllerFacts.IsApiControllerAction(symbolCache, method); // Assert Assert.False(result); } [Fact] - public async Task ShouldEvaluateMethod_ReturnsFalse_IfContainingTypeIsNotAction() + public async Task IsApiControllerAction_ReturnsFalse_IfContainingTypeIsNotAction() { // Arrange var compilation = await GetCompilation(); @@ -89,14 +89,14 @@ namespace TestNamespace var method = (IMethodSymbol)type.GetMembers(nameof(ApiConventionAnalyzerTest_NotAction.Index)).First(); // Act - var result = ApiConventionAnalyzer.ShouldEvaluateMethod(symbolCache, method); + var result = ApiControllerFacts.IsApiControllerAction(symbolCache, method); // Assert Assert.False(result); } [Fact] - public async Task ShouldEvaluateMethod_ReturnsTrue_ForValidActionMethods() + public async Task IsApiControllerAction_ReturnsTrue_ForValidActionMethods() { // Arrange var compilation = await GetCompilation(); @@ -105,7 +105,7 @@ namespace TestNamespace var method = (IMethodSymbol)type.GetMembers(nameof(ApiConventionAnalyzerTest_Valid.Index)).First(); // Act - var result = ApiConventionAnalyzer.ShouldEvaluateMethod(symbolCache, method); + var result = ApiControllerFacts.IsApiControllerAction(symbolCache, method); // Assert Assert.True(result); @@ -113,7 +113,7 @@ namespace TestNamespace private Task GetCompilation() { - var testSource = MvcTestSource.Read(GetType().Name, "ApiConventionAnalyzerTestFile"); + var testSource = MvcTestSource.Read(GetType().Name, "TestFile"); var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source }); return project.GetCompilationAsync(); diff --git a/test/Mvc.Analyzers.Test/Infrastructure/MvcDiagnosticAnalyzerRunner.cs b/test/Mvc.Analyzers.Test/Infrastructure/MvcDiagnosticAnalyzerRunner.cs index d3a8d94b73..52a18845f0 100644 --- a/test/Mvc.Analyzers.Test/Infrastructure/MvcDiagnosticAnalyzerRunner.cs +++ b/test/Mvc.Analyzers.Test/Infrastructure/MvcDiagnosticAnalyzerRunner.cs @@ -22,5 +22,10 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure { return GetDiagnosticsAsync(sources: new[] { source }, Analyzer, Array.Empty()); } + + public Task GetDiagnosticsAsync(Project project) + { + return GetDiagnosticsAsync(new[] { project }, Analyzer, Array.Empty()); + } } } diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecks.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecks.cs new file mode 100644 index 0000000000..1c60c0d9ee --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecks.cs @@ -0,0 +1,22 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + [ApiController] + [Route("/api/[controller]")] + public class DiagnosticsAreReturned_ForApiActionsWithModelStateChecks : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + { + return BadRequest(); + } + + /*MM*/if (!ModelState.IsValid) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecksUsingEquality.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecksUsingEquality.cs new file mode 100644 index 0000000000..b4056af443 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecksUsingEquality.cs @@ -0,0 +1,22 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + [ApiController] + [Route("/api/[controller]")] + public class DiagnosticsAreReturned_ForApiActionsWithModelStateChecksUsingEquality : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 1) + { + return NotFound(); + } + + /*MM*/if (ModelState.IsValid == false) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecksWithoutBracing.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecksWithoutBracing.cs new file mode 100644 index 0000000000..26b8712711 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiActionsWithModelStateChecksWithoutBracing.cs @@ -0,0 +1,18 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + [ApiController] + [Route("/api/[controller]")] + public class DiagnosticsAreReturned_ForApiActionsWithModelStateChecksWithoutBracing : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + return BadRequest(); + + /*MM*/if (!ModelState.IsValid) + return BadRequest(); + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsCheckingAdditionalConditions.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsCheckingAdditionalConditions.cs new file mode 100644 index 0000000000..056ba8a266 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsCheckingAdditionalConditions.cs @@ -0,0 +1,17 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + [ApiController] + [Route("/api/[controller]")] + public class NoDiagnosticsAreReturned_ForApiActionsCheckingAdditionalConditions : ControllerBase + { + public IActionResult Method(int id) + { + if (!ModelState.IsValid && !Request.Query.ContainsKey("skip-validation")) + { + return UnprocessableEntity(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsReturning400FromNonModelStateIsValidBlocks.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsReturning400FromNonModelStateIsValidBlocks.cs new file mode 100644 index 0000000000..32cf061473 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsReturning400FromNonModelStateIsValidBlocks.cs @@ -0,0 +1,17 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + [ApiController] + [Route("/api/[controller]")] + public class NoDiagnosticsAreReturned_ForApiActionsReturning400FromNonModelStateIsValidBlocks : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsReturningNot400FromNonModelStateIsValidBlock.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsReturningNot400FromNonModelStateIsValidBlock.cs new file mode 100644 index 0000000000..e08e3e9489 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsReturningNot400FromNonModelStateIsValidBlock.cs @@ -0,0 +1,17 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + [ApiController] + [Route("/api/[controller]")] + public class NoDiagnosticsAreReturned_ForApiActionsReturningNot400FromNonModelStateIsValidBlock : ControllerBase + { + public IActionResult Method(int id) + { + if (!ModelState.IsValid) + { + return UnprocessableEntity(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsWithoutModelStateChecks.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsWithoutModelStateChecks.cs new file mode 100644 index 0000000000..b60f20b7a7 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiActionsWithoutModelStateChecks.cs @@ -0,0 +1,16 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + [ApiController] + public class NoDiagnosticsAreReturned_ForApiActionsWithoutModelStateChecks : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + { + return NotFound(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForNonApiController.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForNonApiController.cs new file mode 100644 index 0000000000..abc6b48359 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForNonApiController.cs @@ -0,0 +1,15 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + public class NoDiagnosticsAreReturned_ForNonApiController : ControllerBase + { + public IActionResult Method(int id) + { + if (!ModelState.IsValid) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForRazorPageModels.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForRazorPageModels.cs new file mode 100644 index 0000000000..4537050f31 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForRazorPageModels.cs @@ -0,0 +1,17 @@ +using Microsoft.AspNetCore.Mvc.RazorPages; + +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzerIntegrationTest +{ + public class Home : PageModel + { + public IActionResult OnPost(int id) + { + if (!ModelState.IsValid) + { + return BadRequest(); + } + + return Page(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesIfBlockWithoutBraces.Input.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesIfBlockWithoutBraces.Input.cs new file mode 100644 index 0000000000..b3c31d8536 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesIfBlockWithoutBraces.Input.cs @@ -0,0 +1,18 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest._INPUT_ +{ + [ApiController] + [Route("/api/[controller]")] + public class CodeFixRemovesIfBlockWithoutBraces : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + return BadRequest(); + + if (!ModelState.IsValid) + return BadRequest(); + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesIfBlockWithoutBraces.Output.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesIfBlockWithoutBraces.Output.cs new file mode 100644 index 0000000000..1b5bd73c47 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesIfBlockWithoutBraces.Output.cs @@ -0,0 +1,14 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest._OUTPUT_ +{ + [ApiController] + [Route("/api/[controller]")] + public class CodeFixRemovesIfBlockWithoutBraces : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + return BadRequest(); + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck.Input.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck.Input.cs new file mode 100644 index 0000000000..6d1d9db2f5 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck.Input.cs @@ -0,0 +1,22 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest._INPUT_ +{ + [ApiController] + [Route("/api/[controller]")] + public class CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + { + return BadRequest(); + } + + if (ModelState.IsValid == false) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck.Output.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck.Output.cs new file mode 100644 index 0000000000..8d61173949 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck.Output.cs @@ -0,0 +1,17 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest._OUTPUT_ +{ + [ApiController] + [Route("/api/[controller]")] + public class CodeFixRemovesModelStateIsInvalidBlockWithEqualityCheck : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck.Input.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck.Input.cs new file mode 100644 index 0000000000..0a1071c29c --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck.Input.cs @@ -0,0 +1,22 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest._INPUT_ +{ + [ApiController] + [Route("/api/[controller]")] + public class CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + { + return BadRequest(); + } + + if (!ModelState.IsValid) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck.Output.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck.Output.cs new file mode 100644 index 0000000000..1d82b016e9 --- /dev/null +++ b/test/Mvc.Analyzers.Test/TestFiles/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest/CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck.Output.cs @@ -0,0 +1,17 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers.TestFiles.ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProviderTest._OUTPUT_ +{ + [ApiController] + [Route("/api/[controller]")] + public class CodeFixRemovesModelStateIsInvalidBlockWithIfNotCheck : ControllerBase + { + public IActionResult Method(int id) + { + if (id == 0) + { + return BadRequest(); + } + + return Ok(); + } + } +} diff --git a/test/Mvc.Analyzers.Test/TestFiles/ApiConventionAnalyzerTest/ApiConventionAnalyzerTestFile.cs b/test/Mvc.Analyzers.Test/TestFiles/ApiControllerFactsTest/TestFile.cs similarity index 100% rename from test/Mvc.Analyzers.Test/TestFiles/ApiConventionAnalyzerTest/ApiConventionAnalyzerTestFile.cs rename to test/Mvc.Analyzers.Test/TestFiles/ApiControllerFactsTest/TestFile.cs