Add analyzer and codefix that suggests removing unnecessary invalid model state validity checks

Fixes #8146
This commit is contained in:
Pranav K 2018-07-27 15:08:54 -07:00
parent 367717760b
commit d346255db6
No known key found for this signature in database
GPG Key ID: 1963DA6D96C3057A
30 changed files with 780 additions and 544 deletions

View File

@ -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<MemberAccessExpressionSyntax>())
{
var ancestorIfStatement = memberAccessSyntax.FirstAncestorOrSelf<IfStatementSyntax>();
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;
}
}
}

View File

@ -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<string> 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<Document> 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();
}
}
}
}

View File

@ -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<DiagnosticDescriptor> 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;
}
}
}

View File

@ -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<string> 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<Document> 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();
}
}
}
}

View File

@ -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;
}
}
}

View File

@ -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; }

View File

@ -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> declaredApiResponseMetadata, int statusCode)
{
if (declaredApiResponseMetadata.Count == 0)

View File

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

View File

@ -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";

View File

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

View File

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

View File

@ -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_");
}
}
}

View File

@ -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<Compilation> 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();

View File

@ -22,5 +22,10 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure
{
return GetDiagnosticsAsync(sources: new[] { source }, Analyzer, Array.Empty<string>());
}
public Task<Diagnostic[]> GetDiagnosticsAsync(Project project)
{
return GetDiagnosticsAsync(new[] { project }, Analyzer, Array.Empty<string>());
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}

View File

@ -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();
}
}
}