diff --git a/benchmarkapps/BasicViews/BasicViews.csproj b/benchmarkapps/BasicViews/BasicViews.csproj index 9db233657d..6e62f39598 100644 --- a/benchmarkapps/BasicViews/BasicViews.csproj +++ b/benchmarkapps/BasicViews/BasicViews.csproj @@ -39,5 +39,6 @@ --> + diff --git a/benchmarkapps/RazorRendering/RazorRendering.csproj b/benchmarkapps/RazorRendering/RazorRendering.csproj index fee2d62006..dc531b3fdf 100644 --- a/benchmarkapps/RazorRendering/RazorRendering.csproj +++ b/benchmarkapps/RazorRendering/RazorRendering.csproj @@ -17,6 +17,7 @@ --> + diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ActualApiResponseMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ActualApiResponseMetadata.cs index 15f790450e..018967cb10 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ActualApiResponseMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ActualApiResponseMetadata.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Microsoft.AspNetCore.Mvc.Api.Analyzers @@ -9,16 +10,18 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { private readonly int? _statusCode; - public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement) + public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, ITypeSymbol returnType) { ReturnStatement = returnStatement; + ReturnType = returnType; _statusCode = null; } - public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int statusCode) + public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int statusCode, ITypeSymbol returnType) { ReturnStatement = returnStatement; _statusCode = statusCode; + ReturnType = returnType; } public ReturnStatementSyntax ReturnStatement { get; } @@ -26,5 +29,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers public int StatusCode => _statusCode.Value; public bool IsDefaultResponse => _statusCode == null; + + public ITypeSymbol ReturnType { get; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ActualApiResponseMetadataFactory.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ActualApiResponseMetadataFactory.cs new file mode 100644 index 0000000000..83e1c78546 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ActualApiResponseMetadataFactory.cs @@ -0,0 +1,300 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + public static class ActualApiResponseMetadataFactory + { + private static readonly Func _shouldDescendIntoChildren = ShouldDescendIntoChildren; + + /// + /// This method looks at individual return statments and attempts to parse the status code and the return type. + /// Given a for an action, this method inspects return statements in the body. + /// If the returned type is not assignable from IActionResult, it assumes that an "object" value is being returned. e.g. return new Person(); + /// For return statements returning an action result, it attempts to infer the status code and return type. Helper methods in controller, + /// values set in initializer and new-ing up an IActionResult instance are supported. + /// + internal static bool TryGetActualResponseMetadata( + in ApiControllerSymbolCache symbolCache, + SemanticModel semanticModel, + MethodDeclarationSyntax methodSyntax, + CancellationToken cancellationToken, + out IList actualResponseMetadata) + { + actualResponseMetadata = new List(); + + var allReturnStatementsReadable = true; + + foreach (var returnStatementSyntax in methodSyntax.DescendantNodes(_shouldDescendIntoChildren).OfType()) + { + if (returnStatementSyntax.IsMissing || returnStatementSyntax.Expression == null || returnStatementSyntax.Expression.IsMissing) + { + // Ignore malformed return statements. + allReturnStatementsReadable = false; + continue; + } + + var responseMetadata = InspectReturnStatementSyntax( + symbolCache, + semanticModel, + returnStatementSyntax, + cancellationToken); + + if (responseMetadata != null) + { + actualResponseMetadata.Add(responseMetadata.Value); + } + else + { + allReturnStatementsReadable = false; + } + } + + return allReturnStatementsReadable; + } + + internal static ActualApiResponseMetadata? InspectReturnStatementSyntax( + in ApiControllerSymbolCache symbolCache, + SemanticModel semanticModel, + ReturnStatementSyntax returnStatementSyntax, + CancellationToken cancellationToken) + { + var returnExpression = returnStatementSyntax.Expression; + var typeInfo = semanticModel.GetTypeInfo(returnExpression, cancellationToken); + if (typeInfo.Type == null || typeInfo.Type.TypeKind == TypeKind.Error) + { + return null; + } + + var statementReturnType = typeInfo.Type; + + if (!symbolCache.IActionResult.IsAssignableFrom(statementReturnType)) + { + // Return expression is not an instance of IActionResult. Must be returning the "model". + return new ActualApiResponseMetadata(returnStatementSyntax, statementReturnType); + } + + var defaultStatusCodeAttribute = statementReturnType + .GetAttributes(symbolCache.DefaultStatusCodeAttribute, inherit: true) + .FirstOrDefault(); + + var statusCode = GetDefaultStatusCode(defaultStatusCodeAttribute); + ITypeSymbol returnType = null; + switch (returnExpression) + { + case InvocationExpressionSyntax invocation: + { + // Covers the 'return StatusCode(200)' case. + var result = InspectMethodArguments(symbolCache, semanticModel, invocation.Expression, invocation.ArgumentList, cancellationToken); + statusCode = result.statusCode ?? statusCode; + returnType = result.returnType; + break; + } + + case ObjectCreationExpressionSyntax creation: + { + // Read values from 'return new StatusCodeResult(200) case. + var result = InspectMethodArguments(symbolCache, semanticModel, creation, creation.ArgumentList, cancellationToken); + statusCode = result.statusCode ?? statusCode; + returnType = result.returnType; + + // Read values from property assignments e.g. 'return new ObjectResult(...) { StatusCode = 200 }'. + // Property assignments override constructor assigned values and defaults. + result = InspectInitializers(symbolCache, semanticModel, creation.Initializer, cancellationToken); + statusCode = result.statusCode ?? statusCode; + returnType = result.returnType ?? returnType; + break; + } + } + + if (statusCode == null) + { + return null; + } + + return new ActualApiResponseMetadata(returnStatementSyntax, statusCode.Value, returnType); + } + + private static (int? statusCode, ITypeSymbol returnType) InspectInitializers( + in ApiControllerSymbolCache symbolCache, + SemanticModel semanticModel, + InitializerExpressionSyntax initializer, + CancellationToken cancellationToken) + { + int? statusCode = null; + ITypeSymbol typeSymbol = null; + + for (var i = 0; initializer != null && i < initializer.Expressions.Count; i++) + { + var expression = initializer.Expressions[i]; + + if (!(expression is AssignmentExpressionSyntax assignment) || + !(assignment.Left is IdentifierNameSyntax identifier)) + { + continue; + } + + var symbolInfo = semanticModel.GetSymbolInfo(identifier, cancellationToken); + if (symbolInfo.Symbol is IPropertySymbol property) + { + if (IsInterfaceImplementation(property, symbolCache.StatusCodeActionResultStatusProperty) && + TryGetExpressionStatusCode(semanticModel, assignment.Right, cancellationToken, out var statusCodeValue)) + { + // Look for assignments to IStatusCodeActionResult.StatusCode + statusCode = statusCodeValue; + } + else if (HasAttributeNamed(property, ApiSymbolNames.ActionResultObjectValueAttribute)) + { + // Look for assignment to a property annotated with [ActionResultObjectValue] + typeSymbol = GetExpressionObjectType(semanticModel, assignment.Right, cancellationToken); + } + } + } + + return (statusCode, typeSymbol); + } + + private static (int? statusCode, ITypeSymbol returnType) InspectMethodArguments( + in ApiControllerSymbolCache symbolCache, + SemanticModel semanticModel, + ExpressionSyntax expression, + BaseArgumentListSyntax argumentList, + CancellationToken cancellationToken) + { + int? statusCode = null; + ITypeSymbol typeSymbol = null; + + var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken); + + if (symbolInfo.Symbol is IMethodSymbol method) + { + for (var i = 0; i < method.Parameters.Length; i++) + { + var parameter = method.Parameters[i]; + if (HasAttributeNamed(parameter, ApiSymbolNames.ActionResultStatusCodeAttribute)) + { + var argument = argumentList.Arguments[parameter.Ordinal]; + if (TryGetExpressionStatusCode(semanticModel, argument.Expression, cancellationToken, out var statusCodeValue)) + { + statusCode = statusCodeValue; + } + } + + if (HasAttributeNamed(parameter, ApiSymbolNames.ActionResultObjectValueAttribute)) + { + var argument = argumentList.Arguments[parameter.Ordinal]; + typeSymbol = GetExpressionObjectType(semanticModel, argument.Expression, cancellationToken); + } + } + } + + return (statusCode, typeSymbol); + } + + private static ITypeSymbol GetExpressionObjectType(SemanticModel semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken) + { + var typeInfo = semanticModel.GetTypeInfo(expression, cancellationToken); + return typeInfo.Type; + } + + private static bool TryGetExpressionStatusCode( + SemanticModel semanticModel, + ExpressionSyntax expression, + CancellationToken cancellationToken, + out int statusCode) + { + if (expression is LiteralExpressionSyntax literal && literal.Token.Value is int literalStatusCode) + { + // Covers the 'return StatusCode(200)' case. + statusCode = literalStatusCode; + return true; + } + + if (expression is IdentifierNameSyntax || expression is MemberAccessExpressionSyntax) + { + var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken); + + if (symbolInfo.Symbol is IFieldSymbol field && field.HasConstantValue && field.ConstantValue is int constantStatusCode) + { + // Covers the 'return StatusCode(StatusCodes.Status200OK)' case. + // It also covers the 'return StatusCode(StatusCode)' case, where 'StatusCode' is a constant field. + statusCode = constantStatusCode; + return true; + } + + if (symbolInfo.Symbol is ILocalSymbol local && local.HasConstantValue && local.ConstantValue is int localStatusCode) + { + // Covers the 'return StatusCode(statusCode)' case, where 'statusCode' is a local constant. + statusCode = localStatusCode; + return true; + } + } + + statusCode = default; + return false; + } + + private static bool ShouldDescendIntoChildren(SyntaxNode syntaxNode) + { + return !syntaxNode.IsKind(SyntaxKind.LocalFunctionStatement) && + !syntaxNode.IsKind(SyntaxKind.ParenthesizedLambdaExpression) && + !syntaxNode.IsKind(SyntaxKind.SimpleLambdaExpression) && + !syntaxNode.IsKind(SyntaxKind.AnonymousMethodExpression); + } + + internal static int? GetDefaultStatusCode(AttributeData attribute) + { + if (attribute != null && + attribute.ConstructorArguments.Length == 1 && + attribute.ConstructorArguments[0].Kind == TypedConstantKind.Primitive && + attribute.ConstructorArguments[0].Value is int statusCode) + { + return statusCode; + } + + return null; + } + + private static bool IsInterfaceImplementation(IPropertySymbol property, IPropertySymbol statusCodeActionResultStatusProperty) + { + if (property.Name != statusCodeActionResultStatusProperty.Name) + { + return false; + } + + for (var i = 0; i < property.ExplicitInterfaceImplementations.Length; i++) + { + if (property.ExplicitInterfaceImplementations[i] == statusCodeActionResultStatusProperty) + { + return true; + } + } + + var implementedProperty = property.ContainingType.FindImplementationForInterfaceMember(statusCodeActionResultStatusProperty); + return implementedProperty == property; + } + + private static bool HasAttributeNamed(ISymbol symbol, string attributeName) + { + var attributes = symbol.GetAttributes(); + var length = attributes.Length; + for (var i = 0; i < length; i++) + { + if (attributes[i].AttributeClass.Name == attributeName) + { + return true; + } + } + + return false; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/AddResponseTypeAttributeCodeFixAction.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/AddResponseTypeAttributeCodeFixAction.cs index 9457429a12..e7601b6b7f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/AddResponseTypeAttributeCodeFixAction.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/AddResponseTypeAttributeCodeFixAction.cs @@ -15,6 +15,17 @@ using Microsoft.CodeAnalysis.Simplification; namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { + /// + /// A that adds one or more ProducesResponseType attributes on the action. + /// 1) It get status codes from ProducesResponseType, ProducesDefaultResponseType, and conventions applied to the action to get the declared metadata. + /// 2) It inspects return statements to get actual metadata. + /// Diffing the two gets us a list of undocumented status codes. + /// We'll attempt to generate a [ProducesResponseType(typeof(SomeModel), 4xx)] if + /// a) the status code is 4xx or later. + /// b) the return statement included a return type. + /// c) the return type wasn't the error type (specified by ProducesErrorResponseType or implicit ProblemDetails) + /// In all other cases, we generate [ProducesResponseType(StatusCode)] + /// internal sealed class AddResponseTypeAttributeCodeFixAction : CodeAction { private readonly Document _document; @@ -32,9 +43,10 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { var context = await CreateCodeActionContext(cancellationToken).ConfigureAwait(false); var declaredResponseMetadata = SymbolApiResponseMetadataProvider.GetDeclaredResponseMetadata(context.SymbolCache, context.Method); + var errorResponseType = SymbolApiResponseMetadataProvider.GetErrorResponseType(context.SymbolCache, context.Method); - var statusCodes = CalculateStatusCodesToApply(context, declaredResponseMetadata); - if (statusCodes.Count == 0) + var results = CalculateStatusCodesToApply(context, declaredResponseMetadata); + if (results.Count == 0) { return _document; } @@ -42,9 +54,22 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers var documentEditor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false); var addUsingDirective = false; - foreach (var statusCode in statusCodes.OrderBy(s => s)) + foreach (var (statusCode, returnType) in results.OrderBy(s => s.statusCode)) { - documentEditor.AddAttribute(context.MethodSyntax, CreateProducesResponseTypeAttribute(context, statusCode, out var addUsing)); + AttributeSyntax attributeSyntax; + bool addUsing; + + if (statusCode >= 400 && returnType != null && returnType != errorResponseType) + { + // If a returnType was discovered and is different from the errorResponseType, use it in the result. + attributeSyntax = CreateProducesResponseTypeAttribute(context, statusCode, returnType, out addUsing); + } + else + { + attributeSyntax = CreateProducesResponseTypeAttribute(context, statusCode, out addUsing); + } + + documentEditor.AddAttribute(context.MethodSyntax, attributeSyntax); addUsingDirective |= addUsing; } @@ -103,7 +128,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers return codeActionContext; } - private static Dictionary GetStatusCodeConstants(INamespaceOrTypeSymbol statusCodesType) + private static Dictionary GetStatusCodeConstants(INamedTypeSymbol statusCodesType) { var statusCodeConstants = new Dictionary(); @@ -125,15 +150,15 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers return statusCodeConstants; } - private ICollection CalculateStatusCodesToApply(CodeActionContext context, IList declaredResponseMetadata) + private ICollection<(int statusCode, ITypeSymbol typeSymbol)> CalculateStatusCodesToApply(in CodeActionContext context, IList declaredResponseMetadata) { - if (!SymbolApiResponseMetadataProvider.TryGetActualResponseMetadata(context.SymbolCache, context.SemanticModel, context.MethodSyntax, context.CancellationToken, out var actualResponseMetadata)) + if (!ActualApiResponseMetadataFactory.TryGetActualResponseMetadata(context.SymbolCache, context.SemanticModel, context.MethodSyntax, context.CancellationToken, out var actualResponseMetadata)) { // If we cannot parse metadata correctly, don't offer fixes. - return Array.Empty(); + return Array.Empty<(int, ITypeSymbol)>(); } - var statusCodes = new HashSet(); + var statusCodes = new Dictionary(); foreach (var metadata in actualResponseMetadata) { if (DeclaredApiResponseMetadata.TryGetDeclaredMetadata(declaredResponseMetadata, metadata, result: out var declaredMetadata) && @@ -143,20 +168,39 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers continue; } - statusCodes.Add(metadata.IsDefaultResponse ? 200 : metadata.StatusCode); + var statusCode = metadata.IsDefaultResponse ? 200 : metadata.StatusCode; + statusCodes.Add(statusCode, (statusCode, metadata.ReturnType)); } - return statusCodes; + return statusCodes.Values; } - private static AttributeSyntax CreateProducesResponseTypeAttribute(CodeActionContext context, int statusCode, out bool addUsingDirective) + private static AttributeSyntax CreateProducesResponseTypeAttribute(in CodeActionContext context, int statusCode, out bool addUsingDirective) { + // [ProducesResponseType(StatusCodes.Status400NotFound)] var statusCodeSyntax = CreateStatusCodeSyntax(context, statusCode, out addUsingDirective); return SyntaxFactory.Attribute( SyntaxFactory.ParseName(ApiSymbolNames.ProducesResponseTypeAttribute) .WithAdditionalAnnotations(Simplifier.Annotation), SyntaxFactory.AttributeArgumentList().AddArguments( + + SyntaxFactory.AttributeArgument(statusCodeSyntax))); + } + + private static AttributeSyntax CreateProducesResponseTypeAttribute(in CodeActionContext context, int statusCode, ITypeSymbol typeSymbol, out bool addUsingDirective) + { + // [ProducesResponseType(typeof(ReturnType), StatusCodes.Status400NotFound)] + var statusCodeSyntax = CreateStatusCodeSyntax(context, statusCode, out addUsingDirective); + var responseTypeAttribute = SyntaxFactory.TypeOfExpression( + SyntaxFactory.ParseTypeName(typeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)) + .WithAdditionalAnnotations(Simplifier.Annotation)); + + return SyntaxFactory.Attribute( + SyntaxFactory.ParseName(ApiSymbolNames.ProducesResponseTypeAttribute) + .WithAdditionalAnnotations(Simplifier.Annotation), + SyntaxFactory.AttributeArgumentList().AddArguments( + SyntaxFactory.AttributeArgument(responseTypeAttribute), SyntaxFactory.AttributeArgument(statusCodeSyntax))); } diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs index 9928708f73..9a8dc1c714 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs @@ -103,7 +103,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers } var returnStatementSyntax = (ReturnStatementSyntax)returnOperation.Syntax; - var actualMetadata = SymbolApiResponseMetadataProvider.InspectReturnStatementSyntax( + var actualMetadata = ActualApiResponseMetadataFactory.InspectReturnStatementSyntax( symbolCache, semanticModel, returnStatementSyntax, diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiControllerSymbolCache.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiControllerSymbolCache.cs index 7cbec7eb36..37f6b26291 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiControllerSymbolCache.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiControllerSymbolCache.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; using Microsoft.CodeAnalysis; namespace Microsoft.AspNetCore.Mvc.Api.Analyzers @@ -22,11 +21,11 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers ModelStateDictionary = compilation.GetTypeByMetadataName(ApiSymbolNames.ModelStateDictionary); NonActionAttribute = compilation.GetTypeByMetadataName(ApiSymbolNames.NonActionAttribute); NonControllerAttribute = compilation.GetTypeByMetadataName(ApiSymbolNames.NonControllerAttribute); + ProblemDetails = compilation.GetTypeByMetadataName(ApiSymbolNames.ProblemDetails); ProducesDefaultResponseTypeAttribute = compilation.GetTypeByMetadataName(ApiSymbolNames.ProducesDefaultResponseTypeAttribute); + ProducesErrorResponseTypeAttribute = compilation.GetTypeByMetadataName(ApiSymbolNames.ProducesErrorResponseTypeAttribute); ProducesResponseTypeAttribute = compilation.GetTypeByMetadataName(ApiSymbolNames.ProducesResponseTypeAttribute); - StatusCodeValueAttribute = compilation.GetTypeByMetadataName(ApiSymbolNames.StatusCodeValueAttribute); - var statusCodeActionResult = compilation.GetTypeByMetadataName(ApiSymbolNames.IStatusCodeActionResult); StatusCodeActionResultStatusProperty = (IPropertySymbol)statusCodeActionResult?.GetMembers("StatusCode")[0]; @@ -61,10 +60,12 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers public INamedTypeSymbol NonControllerAttribute { get; } + public INamedTypeSymbol ProblemDetails { get; } + public INamedTypeSymbol ProducesDefaultResponseTypeAttribute { get; } public INamedTypeSymbol ProducesResponseTypeAttribute { get; } - public INamedTypeSymbol StatusCodeValueAttribute { get; } + public INamedTypeSymbol ProducesErrorResponseTypeAttribute { get; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiConventionAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiConventionAnalyzer.cs index 27e5439330..6b92ee4c8f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiConventionAnalyzer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiConventionAnalyzer.cs @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers } var declaredResponseMetadata = SymbolApiResponseMetadataProvider.GetDeclaredResponseMetadata(symbolCache, method); - var hasUnreadableStatusCodes = !SymbolApiResponseMetadataProvider.TryGetActualResponseMetadata(symbolCache, semanticModel, methodSyntax, cancellationToken, out var actualResponseMetadata); + var hasUnreadableStatusCodes = !ActualApiResponseMetadataFactory.TryGetActualResponseMetadata(symbolCache, semanticModel, methodSyntax, cancellationToken, out var actualResponseMetadata); var hasUndocumentedStatusCodes = false; foreach (var actualMetadata in actualResponseMetadata) diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiSymbolNames.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiSymbolNames.cs index 135d742feb..849d07fa0b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiSymbolNames.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/ApiSymbolNames.cs @@ -5,6 +5,10 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { internal static class ApiSymbolNames { + public const string ActionResultStatusCodeAttribute = "ActionResultStatusCodeAttribute"; + + public const string ActionResultObjectValueAttribute = "ActionResultObjectValueAttribute"; + public const string AllowAnonymousAttribute = "Microsoft.AspNetCore.Authorization.AllowAnonymousAttribute"; public const string ApiConventionMethodAttribute = "Microsoft.AspNetCore.Mvc.ApiConventionMethodAttribute"; @@ -31,12 +35,14 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers public const string NonControllerAttribute = "Microsoft.AspNetCore.Mvc.NonControllerAttribute"; + public const string ProblemDetails = "Microsoft.AspNetCore.Mvc.ProblemDetails"; + public const string ProducesDefaultResponseTypeAttribute = "Microsoft.AspNetCore.Mvc.ProducesDefaultResponseTypeAttribute"; + public const string ProducesErrorResponseTypeAttribute = "Microsoft.AspNetCore.Mvc.ProducesErrorResponseTypeAttribute"; + public const string ProducesResponseTypeAttribute = "Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute"; public const string HttpStatusCodes = "Microsoft.AspNetCore.Http.StatusCodes"; - - public const string StatusCodeValueAttribute = "Microsoft.AspNetCore.Mvc.Infrastructure.StatusCodeValueAttribute"; } } diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/SymbolApiResponseMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/SymbolApiResponseMetadataProvider.cs index 7e3744757e..2be586958d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/SymbolApiResponseMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/SymbolApiResponseMetadataProvider.cs @@ -4,10 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { @@ -15,14 +12,13 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { private const string StatusCodeProperty = "StatusCode"; private const string StatusCodeConstructorParameter = "statusCode"; - private static readonly Func _shouldDescendIntoChildren = ShouldDescendIntoChildren; private static readonly IList DefaultResponseMetadatas = new[] { DeclaredApiResponseMetadata.ImplicitResponse, }; - internal static IList GetDeclaredResponseMetadata( - ApiControllerSymbolCache symbolCache, + public static IList GetDeclaredResponseMetadata( + in ApiControllerSymbolCache symbolCache, IMethodSymbol method) { var metadataItems = GetResponseMetadataFromMethodAttributes(symbolCache, method); @@ -44,8 +40,29 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers return metadataItems; } + public static ITypeSymbol GetErrorResponseType( + in ApiControllerSymbolCache symbolCache, + IMethodSymbol method) + { + var errorTypeAttribute = + method.GetAttributes(symbolCache.ProducesErrorResponseTypeAttribute).FirstOrDefault() ?? + method.ContainingType.GetAttributes(symbolCache.ProducesErrorResponseTypeAttribute).FirstOrDefault() ?? + method.ContainingAssembly.GetAttributes(symbolCache.ProducesErrorResponseTypeAttribute).FirstOrDefault(); + + ITypeSymbol errorType = symbolCache.ProblemDetails; + if (errorTypeAttribute != null && + errorTypeAttribute.ConstructorArguments.Length == 1 && + errorTypeAttribute.ConstructorArguments[0].Kind == TypedConstantKind.Type && + errorTypeAttribute.ConstructorArguments[0].Value is ITypeSymbol typeSymbol) + { + errorType = typeSymbol; + } + + return errorType; + } + private static IList GetResponseMetadataFromConventions( - ApiControllerSymbolCache symbolCache, + in ApiControllerSymbolCache symbolCache, IMethodSymbol method, IReadOnlyList conventionTypes) { @@ -63,7 +80,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers return Array.Empty(); } - private static IMethodSymbol GetMethodFromConventionMethodAttribute(ApiControllerSymbolCache symbolCache, IMethodSymbol method) + private static IMethodSymbol GetMethodFromConventionMethodAttribute(in ApiControllerSymbolCache symbolCache, IMethodSymbol method) { var attribute = method.GetAttributes(symbolCache.ApiConventionMethodAttribute, inherit: true) .FirstOrDefault(); @@ -97,7 +114,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers } private static IMethodSymbol MatchConventionMethod( - ApiControllerSymbolCache symbolCache, + in ApiControllerSymbolCache symbolCache, IMethodSymbol method, IReadOnlyList conventionTypes) { @@ -120,7 +137,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers return null; } - private static IList GetResponseMetadataFromMethodAttributes(ApiControllerSymbolCache symbolCache, IMethodSymbol methodSymbol) + private static IList GetResponseMetadataFromMethodAttributes(in ApiControllerSymbolCache symbolCache, IMethodSymbol methodSymbol) { var metadataItems = new List(); var responseMetadataAttributes = methodSymbol.GetAttributes(symbolCache.ProducesResponseTypeAttribute, inherit: true); @@ -141,7 +158,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers return metadataItems; } - internal static IReadOnlyList GetConventionTypes(ApiControllerSymbolCache symbolCache, IMethodSymbol method) + internal static IReadOnlyList GetConventionTypes(in ApiControllerSymbolCache symbolCache, IMethodSymbol method) { var attributes = method.ContainingType.GetAttributes(symbolCache.ApiConventionTypeAttribute).ToArray(); if (attributes.Length == 0) @@ -209,255 +226,5 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers return DefaultStatusCode; } - - internal static bool TryGetActualResponseMetadata( - in ApiControllerSymbolCache symbolCache, - SemanticModel semanticModel, - MethodDeclarationSyntax methodSyntax, - CancellationToken cancellationToken, - out IList actualResponseMetadata) - { - actualResponseMetadata = new List(); - - var allReturnStatementsReadable = true; - - foreach (var returnStatementSyntax in methodSyntax.DescendantNodes(_shouldDescendIntoChildren).OfType()) - { - if (returnStatementSyntax.IsMissing || returnStatementSyntax.Expression.IsMissing) - { - // Ignore malformed return statements. - continue; - } - - var responseMetadata = InspectReturnStatementSyntax( - symbolCache, - semanticModel, - returnStatementSyntax, - cancellationToken); - - if (responseMetadata != null) - { - actualResponseMetadata.Add(responseMetadata.Value); - } - else - { - allReturnStatementsReadable = false; - } - } - - return allReturnStatementsReadable; - } - - internal static ActualApiResponseMetadata? InspectReturnStatementSyntax( - in ApiControllerSymbolCache symbolCache, - SemanticModel semanticModel, - ReturnStatementSyntax returnStatementSyntax, - CancellationToken cancellationToken) - { - var returnExpression = returnStatementSyntax.Expression; - var typeInfo = semanticModel.GetTypeInfo(returnExpression, cancellationToken); - if (typeInfo.Type.TypeKind == TypeKind.Error) - { - return null; - } - - var statementReturnType = typeInfo.Type; - - var defaultStatusCodeAttribute = statementReturnType - .GetAttributes(symbolCache.DefaultStatusCodeAttribute, inherit: true) - .FirstOrDefault(); - - if (defaultStatusCodeAttribute != null) - { - var defaultStatusCode = GetDefaultStatusCode(defaultStatusCodeAttribute); - if (defaultStatusCode == null) - { - // Unable to read the status code even though the attribute exists. - return null; - } - - return new ActualApiResponseMetadata(returnStatementSyntax, defaultStatusCode.Value); - } - - if (!symbolCache.IActionResult.IsAssignableFrom(statementReturnType)) - { - // Return expression does not have a DefaultStatusCodeAttribute and it is not - // an instance of IActionResult. Must be returning the "model". - return new ActualApiResponseMetadata(returnStatementSyntax); - } - - int statusCode; - switch (returnExpression) - { - case InvocationExpressionSyntax invocation: - // Covers the 'return StatusCode(200)' case. - if (TryGetParameterStatusCode(symbolCache, semanticModel, invocation.Expression, invocation.ArgumentList, cancellationToken, out statusCode)) - { - return new ActualApiResponseMetadata(returnStatementSyntax, statusCode); - } - break; - - case ObjectCreationExpressionSyntax creation: - // Covers the 'return new ObjectResult(...) { StatusCode = 200 }' case. - if (TryGetInitializerStatusCode(symbolCache, semanticModel, creation.Initializer, cancellationToken, out statusCode)) - { - return new ActualApiResponseMetadata(returnStatementSyntax, statusCode); - } - - // Covers the 'return new StatusCodeResult(200) case. - if (TryGetParameterStatusCode(symbolCache, semanticModel, creation, creation.ArgumentList, cancellationToken, out statusCode)) - { - return new ActualApiResponseMetadata(returnStatementSyntax, statusCode); - } - break; - } - - return null; - } - - private static bool TryGetInitializerStatusCode( - in ApiControllerSymbolCache symbolCache, - SemanticModel semanticModel, - InitializerExpressionSyntax initializer, - CancellationToken cancellationToken, - out int statusCode) - { - if (initializer == null) - { - statusCode = default; - return false; - } - - for (var i = 0; i < initializer.Expressions.Count; i++) - { - if (!(initializer.Expressions[i] is AssignmentExpressionSyntax assignment)) - { - continue; - } - - if (assignment.Left is IdentifierNameSyntax identifier) - { - var symbolInfo = semanticModel.GetSymbolInfo(identifier, cancellationToken); - - if (symbolInfo.Symbol is IPropertySymbol property && IsInterfaceImplementation(property, symbolCache.StatusCodeActionResultStatusProperty)) - { - return TryGetExpressionStatusCode(semanticModel, assignment.Right, cancellationToken, out statusCode); - } - } - } - - statusCode = default; - return false; - } - - private static bool IsInterfaceImplementation(IPropertySymbol property, IPropertySymbol statusCodeActionResultStatusProperty) - { - if (property.Name != statusCodeActionResultStatusProperty.Name) - { - return false; - } - - for (var i = 0; i < property.ExplicitInterfaceImplementations.Length; i++) - { - if (property.ExplicitInterfaceImplementations[i] == statusCodeActionResultStatusProperty) - { - return true; - } - } - - var implementedProperty = property.ContainingType.FindImplementationForInterfaceMember(statusCodeActionResultStatusProperty); - return implementedProperty == property; - } - - private static bool TryGetParameterStatusCode( - in ApiControllerSymbolCache symbolCache, - SemanticModel semanticModel, - ExpressionSyntax expression, - BaseArgumentListSyntax argumentList, - CancellationToken cancellationToken, - out int statusCode) - { - var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken); - - if (!(symbolInfo.Symbol is IMethodSymbol method)) - { - statusCode = default; - return false; - } - - for (var i = 0; i < method.Parameters.Length; i++) - { - var parameter = method.Parameters[i]; - if (!parameter.HasAttribute(symbolCache.StatusCodeValueAttribute)) - { - continue; - } - - - var argument = argumentList.Arguments[parameter.Ordinal]; - return TryGetExpressionStatusCode(semanticModel, argument.Expression, cancellationToken, out statusCode); - } - - statusCode = default; - return false; - } - - private static bool TryGetExpressionStatusCode( - SemanticModel semanticModel, - ExpressionSyntax expression, - CancellationToken cancellationToken, - out int statusCode) - { - if (expression is LiteralExpressionSyntax literal && literal.Token.Value is int literalStatusCode) - { - // Covers the 'return StatusCode(200)' case. - statusCode = literalStatusCode; - return true; - } - - if (expression is IdentifierNameSyntax || expression is MemberAccessExpressionSyntax) - { - var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken); - - if (symbolInfo.Symbol is IFieldSymbol field && field.HasConstantValue && field.ConstantValue is int constantStatusCode) - { - // Covers the 'return StatusCode(StatusCodes.Status200OK)' case. - // It also covers the 'return StatusCode(StatusCode)' case, where 'StatusCode' is a constant field. - statusCode = constantStatusCode; - return true; - } - - if (symbolInfo.Symbol is ILocalSymbol local && local.HasConstantValue && local.ConstantValue is int localStatusCode) - { - // Covers the 'return StatusCode(statusCode)' case, where 'statusCode' is a local constant. - statusCode = localStatusCode; - return true; - } - } - - statusCode = default; - return false; - } - - private static bool ShouldDescendIntoChildren(SyntaxNode syntaxNode) - { - return !syntaxNode.IsKind(SyntaxKind.LocalFunctionStatement) && - !syntaxNode.IsKind(SyntaxKind.ParenthesizedLambdaExpression) && - !syntaxNode.IsKind(SyntaxKind.SimpleLambdaExpression) && - !syntaxNode.IsKind(SyntaxKind.AnonymousMethodExpression); - } - - internal static int? GetDefaultStatusCode(AttributeData attribute) - { - if (attribute != null && - attribute.ConstructorArguments.Length == 1 && - attribute.ConstructorArguments[0].Kind == TypedConstantKind.Primitive && - attribute.ConstructorArguments[0].Value is int statusCode) - { - return statusCode; - } - - return null; - } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtActionResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtActionResult.cs index 291e25930a..3c392d3abe 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtActionResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtActionResult.cs @@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Mvc string actionName, string controllerName, object routeValues, - object value) + [ActionResultObjectValue] object value) : base(value) { ActionName = actionName; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtRouteResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtRouteResult.cs index 2fb29b505c..e571afe475 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtRouteResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/AcceptedAtRouteResult.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The route data to use for generating the URL. /// The value to format in the entity body. - public AcceptedAtRouteResult(object routeValues, object value) + public AcceptedAtRouteResult(object routeValues, [ActionResultObjectValue] object value) : this(routeName: null, routeValues: routeValues, value: value) { } @@ -41,7 +41,7 @@ namespace Microsoft.AspNetCore.Mvc public AcceptedAtRouteResult( string routeName, object routeValues, - object value) + [ActionResultObjectValue] object value) : base(value) { RouteName = routeName; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/AcceptedResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/AcceptedResult.cs index cf0d383fd3..0279285695 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/AcceptedResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/AcceptedResult.cs @@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The location at which the status of requested content can be monitored. /// The value to format in the entity body. - public AcceptedResult(string location, object value) + public AcceptedResult(string location, [ActionResultObjectValue] object value) : base(value) { Location = location; @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Mvc /// The location at which the status of requested content can be monitored /// It is an optional parameter and may be null /// The value to format in the entity body. - public AcceptedResult(Uri locationUri, object value) + public AcceptedResult(Uri locationUri, [ActionResultObjectValue] object value) : base(value) { if (locationUri == null) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/BadRequestObjectResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/BadRequestObjectResult.cs index 9af96ff7cc..cf3627fb31 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/BadRequestObjectResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/BadRequestObjectResult.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc /// Creates a new instance. /// /// Contains the errors to be returned to the client. - public BadRequestObjectResult(object error) + public BadRequestObjectResult([ActionResultObjectValue] object error) : base(error) { StatusCode = DefaultStatusCode; @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Mvc /// Creates a new instance. /// /// containing the validation errors. - public BadRequestObjectResult(ModelStateDictionary modelState) + public BadRequestObjectResult([ActionResultObjectValue] ModelStateDictionary modelState) : base(new SerializableError(modelState)) { if (modelState == null) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ConflictObjectResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/ConflictObjectResult.cs index 93927d78f9..696357783d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ConflictObjectResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ConflictObjectResult.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc /// Creates a new instance. /// /// Contains the errors to be returned to the client. - public ConflictObjectResult(object error) + public ConflictObjectResult([ActionResultObjectValue] object error) : base(error) { StatusCode = DefaultStatusCode; @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Mvc /// Creates a new instance. /// /// containing the validation errors. - public ConflictObjectResult(ModelStateDictionary modelState) + public ConflictObjectResult([ActionResultObjectValue] ModelStateDictionary modelState) : base(new SerializableError(modelState)) { if (modelState == null) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs index 520f8a5fbe..3cc780746a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs @@ -201,7 +201,7 @@ namespace Microsoft.AspNetCore.Mvc /// The status code to set on the response. /// The created object for the response. [NonAction] - public virtual StatusCodeResult StatusCode([StatusCodeValue] int statusCode) + public virtual StatusCodeResult StatusCode([ActionResultStatusCode] int statusCode) => new StatusCodeResult(statusCode); /// @@ -211,7 +211,7 @@ namespace Microsoft.AspNetCore.Mvc /// The value to set on the . /// The created object for the response. [NonAction] - public virtual ObjectResult StatusCode([StatusCodeValue] int statusCode, object value) + public virtual ObjectResult StatusCode([ActionResultStatusCode] int statusCode, [ActionResultObjectValue] object value) { var result = new ObjectResult(value) { @@ -304,9 +304,10 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual OkObjectResult Ok(object value) + public virtual OkObjectResult Ok([ActionResultObjectValue] object value) => new OkObjectResult(value); + #region RedirectResult variants /// /// Creates a object that redirects () /// to the specified . @@ -1077,7 +1078,9 @@ namespace Microsoft.AspNetCore.Mvc preserveMethod: true, fragment: fragment); } + #endregion + #region FileResult variants /// /// Returns a file with the specified as content (), /// and the specified as the Content-Type. @@ -1698,6 +1701,7 @@ namespace Microsoft.AspNetCore.Mvc EnableRangeProcessing = enableRangeProcessing, }; } + #endregion /// /// Creates an that produces an response. @@ -1712,7 +1716,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The created for the response. [NonAction] - public virtual UnauthorizedObjectResult Unauthorized(object value) + public virtual UnauthorizedObjectResult Unauthorized([ActionResultObjectValue] object value) => new UnauthorizedObjectResult(value); /// @@ -1728,7 +1732,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The created for the response. [NonAction] - public virtual NotFoundObjectResult NotFound(object value) + public virtual NotFoundObjectResult NotFound([ActionResultObjectValue] object value) => new NotFoundObjectResult(value); /// @@ -1745,7 +1749,7 @@ namespace Microsoft.AspNetCore.Mvc /// An error object to be returned to the client. /// The created for the response. [NonAction] - public virtual BadRequestObjectResult BadRequest(object error) + public virtual BadRequestObjectResult BadRequest([ActionResultObjectValue] object error) => new BadRequestObjectResult(error); /// @@ -1754,7 +1758,7 @@ namespace Microsoft.AspNetCore.Mvc /// The containing errors to be returned to the client. /// The created for the response. [NonAction] - public virtual BadRequestObjectResult BadRequest(ModelStateDictionary modelState) + public virtual BadRequestObjectResult BadRequest([ActionResultObjectValue] ModelStateDictionary modelState) { if (modelState == null) { @@ -1780,7 +1784,7 @@ namespace Microsoft.AspNetCore.Mvc /// An error object to be returned to the client. /// The created for the response. [NonAction] - public virtual UnprocessableEntityObjectResult UnprocessableEntity(object error) + public virtual UnprocessableEntityObjectResult UnprocessableEntity([ActionResultObjectValue] object error) { return new UnprocessableEntityObjectResult(error); } @@ -1791,7 +1795,7 @@ namespace Microsoft.AspNetCore.Mvc /// The containing errors to be returned to the client. /// The created for the response. [NonAction] - public virtual UnprocessableEntityObjectResult UnprocessableEntity(ModelStateDictionary modelState) + public virtual UnprocessableEntityObjectResult UnprocessableEntity([ActionResultObjectValue] ModelStateDictionary modelState) { if (modelState == null) { @@ -1815,7 +1819,7 @@ namespace Microsoft.AspNetCore.Mvc /// Contains errors to be returned to the client. /// The created for the response. [NonAction] - public virtual ConflictObjectResult Conflict(object error) + public virtual ConflictObjectResult Conflict([ActionResultObjectValue] object error) => new ConflictObjectResult(error); /// @@ -1824,7 +1828,7 @@ namespace Microsoft.AspNetCore.Mvc /// The containing errors to be returned to the client. /// The created for the response. [NonAction] - public virtual ConflictObjectResult Conflict(ModelStateDictionary modelState) + public virtual ConflictObjectResult Conflict([ActionResultObjectValue] ModelStateDictionary modelState) => new ConflictObjectResult(modelState); /// @@ -1832,7 +1836,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The created for the response. [NonAction] - public virtual ActionResult ValidationProblem(ValidationProblemDetails descriptor) + public virtual ActionResult ValidationProblem([ActionResultObjectValue] ValidationProblemDetails descriptor) { if (descriptor == null) { @@ -1847,7 +1851,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The created for the response. [NonAction] - public virtual ActionResult ValidationProblem(ModelStateDictionary modelStateDictionary) + public virtual ActionResult ValidationProblem([ActionResultObjectValue] ModelStateDictionary modelStateDictionary) { if (modelStateDictionary == null) { @@ -1877,7 +1881,7 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual CreatedResult Created(string uri, object value) + public virtual CreatedResult Created(string uri, [ActionResultObjectValue] object value) { if (uri == null) { @@ -1894,7 +1898,7 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual CreatedResult Created(Uri uri, object value) + public virtual CreatedResult Created(Uri uri, [ActionResultObjectValue] object value) { if (uri == null) { @@ -1911,7 +1915,7 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual CreatedAtActionResult CreatedAtAction(string actionName, object value) + public virtual CreatedAtActionResult CreatedAtAction(string actionName, [ActionResultObjectValue] object value) => CreatedAtAction(actionName, routeValues: null, value: value); /// @@ -1922,7 +1926,7 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual CreatedAtActionResult CreatedAtAction(string actionName, object routeValues, object value) + public virtual CreatedAtActionResult CreatedAtAction(string actionName, object routeValues, [ActionResultObjectValue] object value) => CreatedAtAction(actionName, controllerName: null, routeValues: routeValues, value: value); /// @@ -1938,7 +1942,7 @@ namespace Microsoft.AspNetCore.Mvc string actionName, string controllerName, object routeValues, - object value) + [ActionResultObjectValue] object value) => new CreatedAtActionResult(actionName, controllerName, routeValues, value); /// @@ -1948,7 +1952,7 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual CreatedAtRouteResult CreatedAtRoute(string routeName, object value) + public virtual CreatedAtRouteResult CreatedAtRoute(string routeName, [ActionResultObjectValue] object value) => CreatedAtRoute(routeName, routeValues: null, value: value); /// @@ -1958,7 +1962,7 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual CreatedAtRouteResult CreatedAtRoute(object routeValues, object value) + public virtual CreatedAtRouteResult CreatedAtRoute(object routeValues, [ActionResultObjectValue] object value) => CreatedAtRoute(routeName: null, routeValues: routeValues, value: value); /// @@ -1969,7 +1973,7 @@ namespace Microsoft.AspNetCore.Mvc /// The content value to format in the entity body. /// The created for the response. [NonAction] - public virtual CreatedAtRouteResult CreatedAtRoute(string routeName, object routeValues, object value) + public virtual CreatedAtRouteResult CreatedAtRoute(string routeName, object routeValues, [ActionResultObjectValue] object value) => new CreatedAtRouteResult(routeName, routeValues, value); /// @@ -1986,7 +1990,7 @@ namespace Microsoft.AspNetCore.Mvc /// The optional content value to format in the entity body; may be null. /// The created for the response. [NonAction] - public virtual AcceptedResult Accepted(object value) + public virtual AcceptedResult Accepted([ActionResultObjectValue] object value) => new AcceptedResult(location: null, value: value); /// @@ -2023,7 +2027,7 @@ namespace Microsoft.AspNetCore.Mvc /// The optional content value to format in the entity body; may be null. /// The created for the response. [NonAction] - public virtual AcceptedResult Accepted(string uri, object value) + public virtual AcceptedResult Accepted(string uri, [ActionResultObjectValue] object value) => new AcceptedResult(uri, value); /// @@ -2033,7 +2037,7 @@ namespace Microsoft.AspNetCore.Mvc /// The optional content value to format in the entity body; may be null. /// The created for the response. [NonAction] - public virtual AcceptedResult Accepted(Uri uri, object value) + public virtual AcceptedResult Accepted(Uri uri, [ActionResultObjectValue] object value) { if (uri == null) { @@ -2069,7 +2073,7 @@ namespace Microsoft.AspNetCore.Mvc /// The optional content value to format in the entity body; may be null. /// The created for the response. [NonAction] - public virtual AcceptedAtActionResult AcceptedAtAction(string actionName, object value) + public virtual AcceptedAtActionResult AcceptedAtAction(string actionName, [ActionResultObjectValue] object value) => AcceptedAtAction(actionName, routeValues: null, value: value); /// @@ -2080,7 +2084,7 @@ namespace Microsoft.AspNetCore.Mvc /// The route data to use for generating the URL. /// The created for the response. [NonAction] - public virtual AcceptedAtActionResult AcceptedAtAction(string actionName, string controllerName, object routeValues) + public virtual AcceptedAtActionResult AcceptedAtAction(string actionName, string controllerName, [ActionResultObjectValue] object routeValues) => AcceptedAtAction(actionName, controllerName, routeValues, value: null); /// @@ -2091,7 +2095,7 @@ namespace Microsoft.AspNetCore.Mvc /// The optional content value to format in the entity body; may be null. /// The created for the response. [NonAction] - public virtual AcceptedAtActionResult AcceptedAtAction(string actionName, object routeValues, object value) + public virtual AcceptedAtActionResult AcceptedAtAction(string actionName, object routeValues, [ActionResultObjectValue] object value) => AcceptedAtAction(actionName, controllerName: null, routeValues: routeValues, value: value); /// @@ -2107,7 +2111,7 @@ namespace Microsoft.AspNetCore.Mvc string actionName, string controllerName, object routeValues, - object value) + [ActionResultObjectValue] object value) => new AcceptedAtActionResult(actionName, controllerName, routeValues, value); /// @@ -2116,7 +2120,7 @@ namespace Microsoft.AspNetCore.Mvc /// The route data to use for generating the URL. /// The created for the response. [NonAction] - public virtual AcceptedAtRouteResult AcceptedAtRoute(object routeValues) + public virtual AcceptedAtRouteResult AcceptedAtRoute([ActionResultObjectValue] object routeValues) => AcceptedAtRoute(routeName: null, routeValues: routeValues, value: null); /// @@ -2145,7 +2149,7 @@ namespace Microsoft.AspNetCore.Mvc /// The optional content value to format in the entity body; may be null. /// The created for the response. [NonAction] - public virtual AcceptedAtRouteResult AcceptedAtRoute(object routeValues, object value) + public virtual AcceptedAtRouteResult AcceptedAtRoute(object routeValues, [ActionResultObjectValue] object value) => AcceptedAtRoute(routeName: null, routeValues: routeValues, value: value); /// @@ -2156,7 +2160,7 @@ namespace Microsoft.AspNetCore.Mvc /// The optional content value to format in the entity body; may be null. /// The created for the response. [NonAction] - public virtual AcceptedAtRouteResult AcceptedAtRoute(string routeName, object routeValues, object value) + public virtual AcceptedAtRouteResult AcceptedAtRoute(string routeName, object routeValues, [ActionResultObjectValue] object value) => new AcceptedAtRouteResult(routeName, routeValues, value); /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtActionResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtActionResult.cs index f173c2ba9d..a43516bde6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtActionResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtActionResult.cs @@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Mvc string actionName, string controllerName, object routeValues, - object value) + [ActionResultObjectValue] object value) : base(value) { ActionName = actionName; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtRouteResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtRouteResult.cs index ad05b6e4a9..324f872692 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtRouteResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/CreatedAtRouteResult.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The route data to use for generating the URL. /// The value to format in the entity body. - public CreatedAtRouteResult(object routeValues, object value) + public CreatedAtRouteResult(object routeValues, [ActionResultObjectValue] object value) : this(routeName: null, routeValues: routeValues, value: value) { } @@ -41,7 +41,7 @@ namespace Microsoft.AspNetCore.Mvc public CreatedAtRouteResult( string routeName, object routeValues, - object value) + [ActionResultObjectValue] object value) : base(value) { RouteName = routeName; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionResultObjectValueAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionResultObjectValueAttribute.cs new file mode 100644 index 0000000000..8a87c33b2b --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionResultObjectValueAttribute.cs @@ -0,0 +1,27 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// Attribute annoted on ActionResult constructor, helper method parameters, and properties to indicate + /// that the parameter or property is used to set the "value" for ActionResult. + /// + /// Analyzers match this parameter by type name. This allows users to annotate custom results \ custom helpers + /// with a user defined attribute without having to expose this type. + /// + /// + /// This attribute is intentionally marked Inherited=false since the analyzer does not walk the inheritance graph. + /// + /// + /// + /// BadObjectResult([ActionResultObjectValueAttribute] object value) + /// ObjectResult { [ActionResultObjectValueAttribute] public object Value { get; set; } } + /// + [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property, AllowMultiple = false, Inherited = false)] + internal sealed class ActionResultObjectValueAttribute : Attribute + { + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionResultStatusCodeAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionResultStatusCodeAttribute.cs new file mode 100644 index 0000000000..2b4d1d4119 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionResultStatusCodeAttribute.cs @@ -0,0 +1,26 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// Attribute annoted on ActionResult constructor and helper method parameters to indicate + /// that the parameter is used to set the "statusCode" for the ActionResult. + /// + /// Analyzers match this parameter by type name. This allows users to annotate custom results \ custom helpers + /// with a user defined attribute without having to expose this type. + /// + /// + /// This attribute is intentionally marked Inherited=false since the analyzer does not walk the inheritance graph. + /// + /// + /// + /// StatusCodeResult([ActionResultStatusCodeParameter] int statusCode) + /// + [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)] + internal sealed class ActionResultStatusCodeAttribute : Attribute + { + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/StatusCodeValueAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/StatusCodeValueAttribute.cs deleted file mode 100644 index 944f958d62..0000000000 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/StatusCodeValueAttribute.cs +++ /dev/null @@ -1,12 +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; - -namespace Microsoft.AspNetCore.Mvc.Infrastructure -{ - [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)] - internal sealed class StatusCodeValueAttribute : Attribute - { - } -} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/NotFoundObjectResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/NotFoundObjectResult.cs index c8856473aa..11bb6d8732 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/NotFoundObjectResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/NotFoundObjectResult.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Mvc /// Creates a new instance. /// /// The value to format in the entity body. - public NotFoundObjectResult(object value) + public NotFoundObjectResult([ActionResultObjectValue] object value) : base(value) { StatusCode = DefaultStatusCode; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs index e81dbccfa1..bb7fc0522d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs @@ -18,6 +18,7 @@ namespace Microsoft.AspNetCore.Mvc ContentTypes = new MediaTypeCollection(); } + [ActionResultObjectValue] public object Value { get; set; } public FormatterCollection Formatters { get; set; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs index c95b69237e..70de9cdafe 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc /// with the given . /// /// The HTTP status code of the response. - public StatusCodeResult([StatusCodeValue] int statusCode) + public StatusCodeResult([ActionResultStatusCode] int statusCode) { StatusCode = statusCode; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/UnauthorizedObjectResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/UnauthorizedObjectResult.cs index ebab2df60f..f4a9a4556f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/UnauthorizedObjectResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/UnauthorizedObjectResult.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// Creates a new instance. /// - public UnauthorizedObjectResult(object value) : base(value) + public UnauthorizedObjectResult([ActionResultObjectValue] object value) : base(value) { StatusCode = DefaultStatusCode; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/UnprocessableEntityObjectResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/UnprocessableEntityObjectResult.cs index 85c21a9594..dcee40ef05 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/UnprocessableEntityObjectResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/UnprocessableEntityObjectResult.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Mvc /// Creates a new instance. /// /// containing the validation errors. - public UnprocessableEntityObjectResult(ModelStateDictionary modelState) + public UnprocessableEntityObjectResult([ActionResultObjectValue] ModelStateDictionary modelState) : this(new SerializableError(modelState)) { } @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Mvc /// Creates a new instance. /// /// Contains errors to be returned to the client. - public UnprocessableEntityObjectResult(object error) + public UnprocessableEntityObjectResult([ActionResultObjectValue] object error) : base(error) { StatusCode = DefaultStatusCode; diff --git a/test/Mvc.Api.Analyzers.Test/ActualApiResponseMetadataFactoryTest.cs b/test/Mvc.Api.Analyzers.Test/ActualApiResponseMetadataFactoryTest.cs new file mode 100644 index 0000000000..bf4a12fd67 --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/ActualApiResponseMetadataFactoryTest.cs @@ -0,0 +1,369 @@ +// 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.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.ActualApiResponseMetadataFactoryTest; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + public class ActualApiResponseMetadataFactoryTest + { + private static readonly string Namespace = typeof(ActualApiResponseMetadataFactoryTest).Namespace; + + [Fact] + public async Task GetDefaultStatusCode_ReturnsValueDefinedUsingStatusCodeConstants() + { + // Arrange + var compilation = await GetCompilation("GetDefaultStatusCodeTest"); + var attribute = compilation.GetTypeByMetadataName(typeof(TestActionResultUsingStatusCodesConstants).FullName).GetAttributes()[0]; + + // Act + var actual = ActualApiResponseMetadataFactory.GetDefaultStatusCode(attribute); + + // Assert + Assert.Equal(412, actual); + } + + [Fact] + public async Task GetDefaultStatusCode_ReturnsValueDefinedUsingHttpStatusCast() + { + // Arrange + var compilation = await GetCompilation("GetDefaultStatusCodeTest"); + var attribute = compilation.GetTypeByMetadataName(typeof(TestActionResultUsingHttpStatusCodeCast).FullName).GetAttributes()[0]; + + // Act + var actual = ActualApiResponseMetadataFactory.GetDefaultStatusCode(attribute); + + // Assert + Assert.Equal(302, actual); + } + + [Fact] + public async Task InspectReturnExpression_ReturnsNull_IfReturnExpressionCannotBeFound() + { + // Arrange & Act + var source = @" + using Microsoft.AspNetCore.Mvc; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + [ApiController] + public class TestController : ControllerBase + { + public IActionResult Get(int id) + { + return new DoesNotExist(id); + } + } +}"; + var project = DiagnosticProject.Create(GetType().Assembly, new[] { source }); + var compilation = await project.GetCompilationAsync(); + var symbolCache = new ApiControllerSymbolCache(compilation); + + var returnType = compilation.GetTypeByMetadataName($"{Namespace}.TestController"); + var syntaxTree = returnType.DeclaringSyntaxReferences[0].SyntaxTree; + + var method = (IMethodSymbol)returnType.GetMembers().First(); + var methodSyntax = syntaxTree.GetRoot().FindNode(method.Locations[0].SourceSpan); + var returnStatement = methodSyntax.DescendantNodes().OfType().First(); + + var actualResponseMetadata = ActualApiResponseMetadataFactory.InspectReturnStatementSyntax( + symbolCache, + compilation.GetSemanticModel(syntaxTree), + returnStatement, + CancellationToken.None); + + // Assert + Assert.Null(actualResponseMetadata); + } + + [Fact] + public async Task InspectReturnExpression_ReturnsStatusCodeFromDefaultStatusCodeAttributeOnActionResult() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Equal(401, actualResponseMetadata.Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResult() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.True(actualResponseMetadata.Value.IsDefaultResponse); + } + + [Fact] + public async Task InspectReturnExpression_ReturnsStatusCodeFromStatusCodePropertyAssignment() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Equal(201, actualResponseMetadata.Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_ReturnsStatusCodeFromConstructorAssignment() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Equal(204, actualResponseMetadata.Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_ReturnsStatusCodeFromHelperMethod() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Equal(302, actualResponseMetadata.Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_UsesExplicitlySpecifiedStatusCode_ForActionResultWithDefaultStatusCode() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Equal(422, actualResponseMetadata.Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_ReadsStatusCodeConstant() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Equal(423, actualResponseMetadata.Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_DoesNotReadLocalFieldWithConstantValue() + { + // This is a gap in the analyzer. We're using this to document the current behavior and not an expecation. + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.Null(actualResponseMetadata); + } + + [Fact] + public async Task InspectReturnExpression_FallsBackToDefaultStatusCode_WhenAppliedStatusCodeCannotBeRead() + { + // This is a gap in the analyzer. We're using this to document the current behavior and not an expecation. + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Equal(400, actualResponseMetadata.Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_SetsReturnType_WhenLiteralTypeIsSpecifiedInConstructor() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata?.ReturnType); + Assert.Equal("TestModel", actualResponseMetadata.Value.ReturnType.Name); + } + + [Fact] + public async Task InspectReturnExpression_SetsReturnType_WhenLocalValueIsSpecifiedInConstructor() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata?.ReturnType); + Assert.Equal("TestModel", actualResponseMetadata.Value.ReturnType.Name); + } + + [Fact] + public async Task InspectReturnExpression_SetsReturnType_WhenValueIsReturned() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata?.ReturnType); + Assert.Equal("TestModel", actualResponseMetadata.Value.ReturnType.Name); + } + + [Fact] + public async Task InspectReturnExpression_ReturnsNullReturnType_IfValueIsNotSpecified() + { + // Arrange & Act + var actualResponseMetadata = await RunInspectReturnStatementSyntax(); + + // Assert + Assert.NotNull(actualResponseMetadata); + Assert.Null(actualResponseMetadata.Value.ReturnType); + } + + [Fact] + public async Task TryGetActualResponseMetadata_ActionWithActionResultOfTReturningOkResult() + { + // Arrange + var typeName = typeof(TryGetActualResponseMetadataController).FullName; + var methodName = nameof(TryGetActualResponseMetadataController.ActionWithActionResultOfTReturningOkResult); + + // Act + var (success, responseMetadatas, _) = await TryGetActualResponseMetadata(typeName, methodName); + + // Assert + Assert.True(success); + Assert.Collection( + responseMetadatas, + metadata => + { + Assert.False(metadata.IsDefaultResponse); + Assert.Equal(200, metadata.StatusCode); + }); + } + + [Fact] + public async Task TryGetActualResponseMetadata_ActionWithActionResultOfTReturningModel() + { + // Arrange + var typeName = typeof(TryGetActualResponseMetadataController).FullName; + var methodName = nameof(TryGetActualResponseMetadataController.ActionWithActionResultOfTReturningModel); + + // Act + var (success, responseMetadatas, _) = await TryGetActualResponseMetadata(typeName, methodName); + + // Assert + Assert.True(success); + Assert.Collection( + responseMetadatas, + metadata => + { + Assert.True(metadata.IsDefaultResponse); + }); + } + + [Fact] + public async Task TryGetActualResponseMetadata_ActionReturningNotFoundAndModel() + { + // Arrange + var typeName = typeof(TryGetActualResponseMetadataController).FullName; + var methodName = nameof(TryGetActualResponseMetadataController.ActionReturningNotFoundAndModel); + + // Act + var (success, responseMetadatas, testSource) = await TryGetActualResponseMetadata(typeName, methodName); + + // Assert + Assert.True(success); + Assert.Collection( + responseMetadatas, + metadata => + { + Assert.False(metadata.IsDefaultResponse); + Assert.Equal(204, metadata.StatusCode); + AnalyzerAssert.DiagnosticLocation(testSource.MarkerLocations["MM1"], metadata.ReturnStatement.GetLocation()); + + }, + metadata => + { + Assert.True(metadata.IsDefaultResponse); + AnalyzerAssert.DiagnosticLocation(testSource.MarkerLocations["MM2"], metadata.ReturnStatement.GetLocation()); + }); + } + + private async Task<(bool result, IList responseMetadatas, TestSource testSource)> TryGetActualResponseMetadata(string typeName, string methodName) + { + var testSource = MvcTestSource.Read(GetType().Name, "TryGetActualResponseMetadataTests"); + var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source }); + + var compilation = await GetCompilation("TryGetActualResponseMetadataTests"); + + var type = compilation.GetTypeByMetadataName(typeName); + var method = (IMethodSymbol)type.GetMembers(methodName).First(); + var symbolCache = new ApiControllerSymbolCache(compilation); + + var syntaxTree = method.DeclaringSyntaxReferences[0].SyntaxTree; + var methodSyntax = (MethodDeclarationSyntax)syntaxTree.GetRoot().FindNode(method.Locations[0].SourceSpan); + var semanticModel = compilation.GetSemanticModel(syntaxTree); + + var result = ActualApiResponseMetadataFactory.TryGetActualResponseMetadata(symbolCache, semanticModel, methodSyntax, CancellationToken.None, out var responseMetadatas); + + return (result, responseMetadatas, testSource); + } + + private async Task RunInspectReturnStatementSyntax([CallerMemberName]string test = null) + { + // Arrange + var compilation = await GetCompilation("InspectReturnExpressionTests"); + var symbolCache = new ApiControllerSymbolCache(compilation); + + var controllerType = compilation.GetTypeByMetadataName(typeof(TestFiles.InspectReturnExpressionTests.TestController).FullName); + var syntaxTree = controllerType.DeclaringSyntaxReferences[0].SyntaxTree; + + var method = (IMethodSymbol)Assert.Single(controllerType.GetMembers(test)); + var methodSyntax = syntaxTree.GetRoot().FindNode(method.Locations[0].SourceSpan); + var returnStatement = methodSyntax.DescendantNodes().OfType().First(); + + return ActualApiResponseMetadataFactory.InspectReturnStatementSyntax( + symbolCache, + compilation.GetSemanticModel(syntaxTree), + returnStatement, + CancellationToken.None); + } + + private async Task RunInspectReturnStatementSyntax(string source, string test) + { + var project = DiagnosticProject.Create(GetType().Assembly, new[] { source }); + var compilation = await project.GetCompilationAsync(); + var symbolCache = new ApiControllerSymbolCache(compilation); + + var returnType = compilation.GetTypeByMetadataName($"{Namespace}.{test}"); + var syntaxTree = returnType.DeclaringSyntaxReferences[0].SyntaxTree; + + var method = (IMethodSymbol)returnType.GetMembers().First(); + var methodSyntax = syntaxTree.GetRoot().FindNode(method.Locations[0].SourceSpan); + var returnStatement = methodSyntax.DescendantNodes().OfType().First(); + + return ActualApiResponseMetadataFactory.InspectReturnStatementSyntax( + symbolCache, + compilation.GetSemanticModel(syntaxTree), + returnStatement, + CancellationToken.None); + } + + private Task GetCompilation(string test) + { + var testSource = MvcTestSource.Read(GetType().Name, test); + var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source }); + + return project.GetCompilationAsync(); + } + } +} diff --git a/test/Mvc.Api.Analyzers.Test/AddResponseTypeAttributeCodeFixProviderIntegrationTest.cs b/test/Mvc.Api.Analyzers.Test/AddResponseTypeAttributeCodeFixProviderIntegrationTest.cs index 755fe9b136..f0cfbff1e5 100644 --- a/test/Mvc.Api.Analyzers.Test/AddResponseTypeAttributeCodeFixProviderIntegrationTest.cs +++ b/test/Mvc.Api.Analyzers.Test/AddResponseTypeAttributeCodeFixProviderIntegrationTest.cs @@ -21,6 +21,9 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers [Fact] public Task CodeFixAddsMissingStatusCodes() => RunTest(); + [Fact] + public Task CodeFixAddsMissingStatusCodesAndTypes() => RunTest(); + [Fact] public Task CodeFixWithConventionAddsMissingStatusCodes() => RunTest(); @@ -36,6 +39,9 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers [Fact] public Task CodeFixAddsNumericLiteralForNonExistingStatusCodeConstants() => RunTest(); + [Fact] + public Task CodeFixAddsResponseTypeWhenDifferentFromErrorType() => RunTest(); + [Fact] public Task CodeFixAddsStatusCodesFromMethodParameters() => RunTest(); diff --git a/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs b/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs index 5e68cdbe54..77c5b0d751 100644 --- a/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs +++ b/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs @@ -42,6 +42,71 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers public Task NoDiagnosticsAreReturned_ForReturnStatementsInLocalFunctions() => RunNoDiagnosticsAreReturned(); + [Fact] + public async Task DiagnosticsAreReturned_ForIncompleteActionResults() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; + +namespace Test +{ + [ApiController] + [Route(""[controller]/[action]"") + public class TestController : ControllerBase + { + public IActionResult Get(int id) + { + if (id == 0) + { + /*MM*/return NotFound(); + } + + return; + } + } +}"; + var testSource = TestSource.Read(source); + var expectedLocation = testSource.DefaultMarkerLocation; + + // Act + var result = await Executor.GetDiagnosticsAsync(testSource.Source); + + // Assert + var diagnostic = Assert.Single(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); + AnalyzerAssert.DiagnosticLocation(expectedLocation, diagnostic.Location); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenActionDoesNotCompile() + { + // Arrange + var source = @" +namespace Test +{ + [ApiController] + [Route(""[controller]/[action]"") + public class TestController : ControllerBase + { + public IActionResult Get(int id) + { + if (id == 0) + { + return NotFound(); + } + + return Ok(); + } + } +}"; + + // Act + var result = await Executor.GetDiagnosticsAsync(source); + + // Assert + Assert.DoesNotContain(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); + } + [Fact] public Task DiagnosticsAreReturned_IfMethodWithProducesResponseTypeAttribute_ReturnsUndocumentedStatusCode() => RunTest(ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode, 404); diff --git a/test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs b/test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs index fcfc65ebef..eee4a862f3 100644 --- a/test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs +++ b/test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ImplicitResponse; - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ImplicitResponse; - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 200); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 200, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -49,7 +49,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(200, AttributeData, Mock.Of()); - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -67,7 +67,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(201, AttributeData, Mock.Of()); - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -85,7 +85,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(201, AttributeData, Mock.Of()); - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 200); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 200, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -99,7 +99,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(302, AttributeData, Mock.Of()); - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 302); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 302, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ForProducesDefaultResponse(AttributeData, Mock.Of()); - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, actualStatusCode); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, actualStatusCode, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -130,7 +130,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ForProducesDefaultResponse(AttributeData, Mock.Of()); - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 204); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 204, null); // Act var matches = declaredMetadata.Matches(actualMetadata); @@ -144,7 +144,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers { // Arrange var declaredMetadata = DeclaredApiResponseMetadata.ForProducesDefaultResponse(AttributeData, Mock.Of()); - var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, null); // Act var matches = declaredMetadata.Matches(actualMetadata); diff --git a/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs b/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs index c417a199fe..57d823e8c6 100644 --- a/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs +++ b/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs @@ -1,15 +1,11 @@ // 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.Generic; using System.Linq; -using System.Runtime.CompilerServices; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Analyzer.Testing; using Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Xunit; namespace Microsoft.AspNetCore.Mvc.Api.Analyzers @@ -218,12 +214,12 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers } [Fact] - public async Task GetResponseMetadata_WIthProducesResponseTypeAndApiConventionMethod() + public async Task GetResponseMetadata_WithProducesResponseTypeAndApiConventionMethod() { // Arrange var compilation = await GetResponseMetadataCompilation(); var controller = compilation.GetTypeByMetadataName($"{Namespace}.{nameof(GetResponseMetadata_ControllerActionWithAttributes)}"); - var method = (IMethodSymbol)controller.GetMembers(nameof(GetResponseMetadata_ControllerActionWithAttributes.GetResponseMetadata_WIthProducesResponseTypeAndApiConventionMethod)).First(); + var method = (IMethodSymbol)controller.GetMembers(nameof(GetResponseMetadata_ControllerActionWithAttributes.GetResponseMetadata_WithProducesResponseTypeAndApiConventionMethod)).First(); var symbolCache = new ApiControllerSymbolCache(compilation); // Act @@ -362,193 +358,75 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers } [Fact] - public async Task GetDefaultStatusCode_ReturnsValueDefinedUsingStatusCodeConstants() + public async Task GetErrorResponseType_ReturnsProblemDetails_IfNoAttributeIsDiscovered() { // Arrange - var compilation = await GetCompilation("GetDefaultStatusCodeTest"); - var attribute = compilation.GetTypeByMetadataName(typeof(TestActionResultUsingStatusCodesConstants).FullName).GetAttributes()[0]; + var compilation = await GetCompilation(nameof(GetErrorResponseType_ReturnsProblemDetails_IfNoAttributeIsDiscovered)); + var expected = compilation.GetTypeByMetadataName(typeof(ProblemDetails).FullName); - // Act - var actual = SymbolApiResponseMetadataProvider.GetDefaultStatusCode(attribute); - - // Assert - Assert.Equal(412, actual); - } - - [Fact] - public async Task GetDefaultStatusCode_ReturnsValueDefinedUsingHttpStatusCast() - { - // Arrange - var compilation = await GetCompilation("GetDefaultStatusCodeTest"); - var attribute = compilation.GetTypeByMetadataName(typeof(TestActionResultUsingHttpStatusCodeCast).FullName).GetAttributes()[0]; - - // Act - var actual = SymbolApiResponseMetadataProvider.GetDefaultStatusCode(attribute); - - // Assert - Assert.Equal(302, actual); - } - - [Fact] - public async Task InspectReturnExpression_ReturnsNull_IfReturnExpressionCannotBeFound() - { - // Arrange & Act - var source = @" - using Microsoft.AspNetCore.Mvc; - -namespace Microsoft.AspNetCore.Mvc.Api.Analyzers -{ - [ApiController] - public class InspectReturnExpression_ReturnsNull_IfReturnExpressionCannotBeFound : ControllerBase - { - public IActionResult Get(int id) - { - return new DoesNotExist(id); - } - } -}"; - var actualResponseMetadata = await RunInspectReturnStatementSyntax(source, nameof(InspectReturnExpression_ReturnsNull_IfReturnExpressionCannotBeFound)); - - // Assert - Assert.Null(actualResponseMetadata); - } - - [Fact] - public async Task InspectReturnExpression_ReturnsStatusCodeFromDefaultStatusCodeAttributeOnActionResult() - { - // Arrange & Act - var actualResponseMetadata = await RunInspectReturnStatementSyntax(); - - // Assert - Assert.NotNull(actualResponseMetadata); - Assert.Equal(401, actualResponseMetadata.Value.StatusCode); - } - - [Fact] - public async Task InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResult() - { - // Arrange & Act - var actualResponseMetadata = await RunInspectReturnStatementSyntax(); - - // Assert - Assert.NotNull(actualResponseMetadata); - Assert.True(actualResponseMetadata.Value.IsDefaultResponse); - } - - [Fact] - public async Task TryGetActualResponseMetadata_ActionWithActionResultOfTReturningOkResult() - { - // Arrange - var typeName = typeof(TryGetActualResponseMetadataController).FullName; - var methodName = nameof(TryGetActualResponseMetadataController.ActionWithActionResultOfTReturningOkResult); - - // Act - var (success, responseMetadatas, _) = await TryGetActualResponseMetadata(typeName, methodName); - - // Assert - Assert.True(success); - Assert.Collection( - responseMetadatas, - metadata => - { - Assert.False(metadata.IsDefaultResponse); - Assert.Equal(200, metadata.StatusCode); - }); - } - - [Fact] - public async Task TryGetActualResponseMetadata_ActionWithActionResultOfTReturningModel() - { - // Arrange - var typeName = typeof(TryGetActualResponseMetadataController).FullName; - var methodName = nameof(TryGetActualResponseMetadataController.ActionWithActionResultOfTReturningModel); - - // Act - var (success, responseMetadatas, _) = await TryGetActualResponseMetadata(typeName, methodName); - - // Assert - Assert.True(success); - Assert.Collection( - responseMetadatas, - metadata => - { - Assert.True(metadata.IsDefaultResponse); - }); - } - - [Fact] - public async Task TryGetActualResponseMetadata_ActionReturningNotFoundAndModel() - { - // Arrange - var typeName = typeof(TryGetActualResponseMetadataController).FullName; - var methodName = nameof(TryGetActualResponseMetadataController.ActionReturningNotFoundAndModel); - - // Act - var (success, responseMetadatas, testSource) = await TryGetActualResponseMetadata(typeName, methodName); - - // Assert - Assert.True(success); - Assert.Collection( - responseMetadatas, - metadata => - { - Assert.False(metadata.IsDefaultResponse); - Assert.Equal(204, metadata.StatusCode); - AnalyzerAssert.DiagnosticLocation(testSource.MarkerLocations["MM1"], metadata.ReturnStatement.GetLocation()); - - }, - metadata => - { - Assert.True(metadata.IsDefaultResponse); - AnalyzerAssert.DiagnosticLocation(testSource.MarkerLocations["MM2"], metadata.ReturnStatement.GetLocation()); - }); - } - - private async Task<(bool result, IList responseMetadatas, TestSource testSource)> TryGetActualResponseMetadata(string typeName, string methodName) - { - var testSource = MvcTestSource.Read(GetType().Name, "TryGetActualResponseMetadataTests"); - var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source }); - - var compilation = await GetCompilation("TryGetActualResponseMetadataTests"); - - var type = compilation.GetTypeByMetadataName(typeName); - var method = (IMethodSymbol)type.GetMembers(methodName).First(); + var type = compilation.GetTypeByMetadataName(typeof(GetErrorResponseType_ReturnsProblemDetails_IfNoAttributeIsDiscoveredController).FullName); + var method = (IMethodSymbol)type.GetMembers("Action").First(); var symbolCache = new ApiControllerSymbolCache(compilation); - var syntaxTree = method.DeclaringSyntaxReferences[0].SyntaxTree; - var methodSyntax = (MethodDeclarationSyntax)syntaxTree.GetRoot().FindNode(method.Locations[0].SourceSpan); - var semanticModel = compilation.GetSemanticModel(syntaxTree); + // Act + var result = SymbolApiResponseMetadataProvider.GetErrorResponseType(symbolCache, method); - var result = SymbolApiResponseMetadataProvider.TryGetActualResponseMetadata(symbolCache, semanticModel, methodSyntax, CancellationToken.None, out var responseMetadatas); - - return (result, responseMetadatas, testSource); + // Assert + Assert.Same(expected, result); } - private async Task RunInspectReturnStatementSyntax([CallerMemberName]string test = null) + [Fact] + public async Task GetErrorResponseType_ReturnsTypeDefinedAtAssembly() { // Arrange - var testSource = MvcTestSource.Read(GetType().Name, test); - return await RunInspectReturnStatementSyntax(testSource.Source, test); - } + var compilation = await GetCompilation(nameof(GetErrorResponseType_ReturnsTypeDefinedAtAssembly)); + var expected = compilation.GetTypeByMetadataName(typeof(GetErrorResponseType_ReturnsTypeDefinedAtAssemblyModel).FullName); - private async Task RunInspectReturnStatementSyntax(string source, string test) - { - var project = DiagnosticProject.Create(GetType().Assembly, new[] { source }); - var compilation = await project.GetCompilationAsync(); + var type = compilation.GetTypeByMetadataName(typeof(GetErrorResponseType_ReturnsTypeDefinedAtAssemblyController).FullName); + var method = (IMethodSymbol)type.GetMembers("Action").First(); var symbolCache = new ApiControllerSymbolCache(compilation); - var returnType = compilation.GetTypeByMetadataName($"{Namespace}.{test}"); - var syntaxTree = returnType.DeclaringSyntaxReferences[0].SyntaxTree; + // Act + var result = SymbolApiResponseMetadataProvider.GetErrorResponseType(symbolCache, method); - var method = (IMethodSymbol)returnType.GetMembers().First(); - var methodSyntax = syntaxTree.GetRoot().FindNode(method.Locations[0].SourceSpan); - var returnStatement = methodSyntax.DescendantNodes().OfType().First(); + // Assert + Assert.Same(expected, result); + } - return SymbolApiResponseMetadataProvider.InspectReturnStatementSyntax( - symbolCache, - compilation.GetSemanticModel(syntaxTree), - returnStatement, - CancellationToken.None); + [Fact] + public async Task GetErrorResponseType_ReturnsTypeDefinedAtController() + { + // Arrange + var compilation = await GetCompilation(nameof(GetErrorResponseType_ReturnsTypeDefinedAtController)); + var expected = compilation.GetTypeByMetadataName(typeof(GetErrorResponseType_ReturnsTypeDefinedAtControllerModel).FullName); + + var type = compilation.GetTypeByMetadataName(typeof(GetErrorResponseType_ReturnsTypeDefinedAtControllerController).FullName); + var method = (IMethodSymbol)type.GetMembers("Action").First(); + var symbolCache = new ApiControllerSymbolCache(compilation); + + // Act + var result = SymbolApiResponseMetadataProvider.GetErrorResponseType(symbolCache, method); + + // Assert + Assert.Same(expected, result); + } + + [Fact] + public async Task GetErrorResponseType_ReturnsTypeDefinedAtAction() + { + // Arrange + var compilation = await GetCompilation(nameof(GetErrorResponseType_ReturnsTypeDefinedAtAction)); + var expected = compilation.GetTypeByMetadataName(typeof(GetErrorResponseType_ReturnsTypeDefinedAtActionModel).FullName); + + var type = compilation.GetTypeByMetadataName(typeof(GetErrorResponseType_ReturnsTypeDefinedAtActionController).FullName); + var method = (IMethodSymbol)type.GetMembers("Action").First(); + var symbolCache = new ApiControllerSymbolCache(compilation); + + // Act + var result = SymbolApiResponseMetadataProvider.GetErrorResponseType(symbolCache, method); + + // Assert + Assert.Same(expected, result); } private Task GetResponseMetadataCompilation() => GetCompilation("GetResponseMetadataTests"); diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetDefaultStatusCodeTest.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/GetDefaultStatusCodeTest.cs similarity index 100% rename from test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetDefaultStatusCodeTest.cs rename to test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/GetDefaultStatusCodeTest.cs diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/InspectReturnExpressionTests.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/InspectReturnExpressionTests.cs new file mode 100644 index 0000000000..c7e12719f4 --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/InspectReturnExpressionTests.cs @@ -0,0 +1,81 @@ +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.InspectReturnExpressionTests +{ + public class TestController : ControllerBase + { + public object InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResult() + { + return new TestModel(); + } + + public IActionResult InspectReturnExpression_ReturnsStatusCodeFromDefaultStatusCodeAttributeOnActionResult() + { + return Unauthorized(); + } + + public IActionResult InspectReturnExpression_ReturnsStatusCodeFromStatusCodePropertyAssignment() + { + return new ObjectResult(new object()) { StatusCode = 201 }; + } + + public IActionResult InspectReturnExpression_ReturnsStatusCodeFromConstructorAssignment() + { + return new StatusCodeResult(204); + } + + public IActionResult InspectReturnExpression_ReturnsStatusCodeFromHelperMethod() + { + return StatusCode(302); + } + + public IActionResult InspectReturnExpression_UsesExplicitlySpecifiedStatusCode_ForActionResultWithDefaultStatusCode() + { + return new BadRequestObjectResult(new object()) + { + StatusCode = StatusCodes.Status422UnprocessableEntity, + }; + } + + public IActionResult InspectReturnExpression_ReadsStatusCodeConstant() + { + return StatusCode(StatusCodes.Status423Locked); + } + + public IActionResult InspectReturnExpression_DoesNotReadLocalFieldWithConstantValue() + { + var statusCode = StatusCodes.Status429TooManyRequests; + return StatusCode(statusCode); + } + + public IActionResult InspectReturnExpression_FallsBackToDefaultStatusCode_WhenAppliedStatusCodeCannotBeRead() + { + var statusCode = StatusCodes.Status422UnprocessableEntity; + return new BadRequestObjectResult(new object()) { StatusCode = statusCode }; + } + + public IActionResult InspectReturnExpression_SetsReturnType_WhenLiteralTypeIsSpecifiedInConstructor() + { + return new BadRequestObjectResult(new TestModel()); + } + + public IActionResult InspectReturnExpression_SetsReturnType_WhenLocalValueIsSpecifiedInConstructor() + { + var local = new TestModel(); + return new BadRequestObjectResult(local); + } + + public IActionResult InspectReturnExpression_ReturnsNullReturnType_IfValueIsNotSpecified() + { + return NotFound(); + } + + public ActionResult InspectReturnExpression_SetsReturnType_WhenValueIsReturned() + { + var local = new TestModel(); + return local; + } + } + + public class TestModel { } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/TryGetActualResponseMetadataTests.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/TryGetActualResponseMetadataTests.cs similarity index 92% rename from test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/TryGetActualResponseMetadataTests.cs rename to test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/TryGetActualResponseMetadataTests.cs index 38e4bc1a81..389b605adb 100644 --- a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/TryGetActualResponseMetadataTests.cs +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/ActualApiResponseMetadataFactoryTest/TryGetActualResponseMetadataTests.cs @@ -1,7 +1,7 @@ using System.Collections.Generic; using System.Threading.Tasks; -namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.ActualApiResponseMetadataFactoryTest { public class TryGetActualResponseMetadataController : ControllerBase { diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsMissingStatusCodesAndTypes.Input.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsMissingStatusCodesAndTypes.Input.cs new file mode 100644 index 0000000000..43f07eca63 --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsMissingStatusCodesAndTypes.Input.cs @@ -0,0 +1,25 @@ +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers._INPUT_ +{ + [ApiController] + [Route("[controller]/[action]")] + public class CodeFixAddsMissingStatusCodesAndTypes : ControllerBase + { + [ProducesResponseType(StatusCodes.Status404NotFound)] + public IActionResult GetItem(int id) + { + if (id == 0) + { + return NotFound(); + } + + if (id == 1) + { + return BadRequest(ModelState); + } + + return Ok(new object()); + } + } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsMissingStatusCodesAndTypes.Output.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsMissingStatusCodesAndTypes.Output.cs new file mode 100644 index 0000000000..a758564bcf --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsMissingStatusCodesAndTypes.Output.cs @@ -0,0 +1,28 @@ +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers._OUTPUT_ +{ + [ApiController] + [Route("[controller]/[action]")] + public class CodeFixAddsMissingStatusCodesAndTypes : ControllerBase + { + [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ModelBinding.ModelStateDictionary), StatusCodes.Status400BadRequest)] + [ProducesDefaultResponseType] + public IActionResult GetItem(int id) + { + if (id == 0) + { + return NotFound(); + } + + if (id == 1) + { + return BadRequest(ModelState); + } + + return Ok(new object()); + } + } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsResponseTypeWhenDifferentFromErrorType.Input.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsResponseTypeWhenDifferentFromErrorType.Input.cs new file mode 100644 index 0000000000..02d2e72efb --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsResponseTypeWhenDifferentFromErrorType.Input.cs @@ -0,0 +1,26 @@ +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers._INPUT_ +{ + [ProducesErrorResponseType(typeof(CodeFixAddsResponseTypeWhenDifferentErrorModel))] + [ApiController] + [Route("[controller]/[action]")] + public class CodeFixAddsResponseTypeWhenDifferentFromErrorType : ControllerBase + { + public IActionResult GetItem(int id) + { + if (id == 0) + { + return NotFound(new CodeFixAddsResponseTypeWhenDifferentErrorModel()); + } + + if (id == 1) + { + var validationProblemDetails = new ValidationProblemDetails(ModelState); + return BadRequest(validationProblemDetails); + } + + return Ok(new object()); + } + } + + public class CodeFixAddsResponseTypeWhenDifferentErrorModel { } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsResponseTypeWhenDifferentFromErrorType.Output.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsResponseTypeWhenDifferentFromErrorType.Output.cs new file mode 100644 index 0000000000..f6be4784e5 --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/AddResponseTypeAttributeCodeFixProviderIntegrationTest/CodeFixAddsResponseTypeWhenDifferentFromErrorType.Output.cs @@ -0,0 +1,32 @@ +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers._OUTPUT_ +{ + [ProducesErrorResponseType(typeof(CodeFixAddsResponseTypeWhenDifferentErrorModel))] + [ApiController] + [Route("[controller]/[action]")] + public class CodeFixAddsResponseTypeWhenDifferentFromErrorType : ControllerBase + { + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ValidationProblemDetails), StatusCodes.Status400BadRequest)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesDefaultResponseType] + public IActionResult GetItem(int id) + { + if (id == 0) + { + return NotFound(new CodeFixAddsResponseTypeWhenDifferentErrorModel()); + } + + if (id == 1) + { + var validationProblemDetails = new ValidationProblemDetails(ModelState); + return BadRequest(validationProblemDetails); + } + + return Ok(new object()); + } + } + + public class CodeFixAddsResponseTypeWhenDifferentErrorModel { } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsProblemDetails_IfNoAttributeIsDiscovered.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsProblemDetails_IfNoAttributeIsDiscovered.cs new file mode 100644 index 0000000000..7e7834edbe --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsProblemDetails_IfNoAttributeIsDiscovered.cs @@ -0,0 +1,7 @@ +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest +{ + public class GetErrorResponseType_ReturnsProblemDetails_IfNoAttributeIsDiscoveredController + { + public void Action() { } + } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtAction.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtAction.cs new file mode 100644 index 0000000000..9e8f37d0c6 --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtAction.cs @@ -0,0 +1,13 @@ +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest +{ + [ProducesErrorResponseType(typeof(ModelStateDictionary))] + public class GetErrorResponseType_ReturnsTypeDefinedAtActionController + { + [ProducesErrorResponseType(typeof(GetErrorResponseType_ReturnsTypeDefinedAtActionModel))] + public void Action() { } + } + + public class GetErrorResponseType_ReturnsTypeDefinedAtActionModel { } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtAssembly.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtAssembly.cs new file mode 100644 index 0000000000..b5199490d3 --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtAssembly.cs @@ -0,0 +1,13 @@ +using Microsoft.AspNetCore.Mvc; + +[assembly: ProducesErrorResponseType(typeof(Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest.GetErrorResponseType_ReturnsTypeDefinedAtAssemblyModel))] + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest +{ + public class GetErrorResponseType_ReturnsTypeDefinedAtAssemblyController + { + public void Action() { } + } + + public class GetErrorResponseType_ReturnsTypeDefinedAtAssemblyModel { } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtController.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtController.cs new file mode 100644 index 0000000000..b731f3f0eb --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetErrorResponseType_ReturnsTypeDefinedAtController.cs @@ -0,0 +1,10 @@ +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest +{ + [ProducesErrorResponseType(typeof(GetErrorResponseType_ReturnsTypeDefinedAtControllerModel))] + public class GetErrorResponseType_ReturnsTypeDefinedAtControllerController + { + public void Action() { } + } + + public class GetErrorResponseType_ReturnsTypeDefinedAtControllerModel { } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetResponseMetadataTests.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetResponseMetadataTests.cs index 9040a07256..389617609b 100644 --- a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetResponseMetadataTests.cs +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/GetResponseMetadataTests.cs @@ -48,7 +48,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers [ProducesResponseType(204)] [ApiConventionMethod(typeof(DefaultApiConventions), nameof(DefaultApiConventions.Find))] - public IActionResult GetResponseMetadata_WIthProducesResponseTypeAndApiConventionMethod() => null; + public IActionResult GetResponseMetadata_WithProducesResponseTypeAndApiConventionMethod() => null; } public class Person { } diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResult.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResult.cs deleted file mode 100644 index 02a1ffb40a..0000000000 --- a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResult.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace Microsoft.AspNetCore.Mvc.Api.Analyzers -{ - public class InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResult : ControllerBase - { - public object Get() - { - return new InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResultModel(); - } - } - - public class InspectReturnExpression_ReturnsDefaultResponseMetadata_IfReturnedTypeIsNotActionResultModel { } -} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/InspectReturnExpression_ReturnsStatusCodeFromDefaultStatusCodeAttributeOnActionResult.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/InspectReturnExpression_ReturnsStatusCodeFromDefaultStatusCodeAttributeOnActionResult.cs deleted file mode 100644 index 94d66474f5..0000000000 --- a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/InspectReturnExpression_ReturnsStatusCodeFromDefaultStatusCodeAttributeOnActionResult.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace Microsoft.AspNetCore.Mvc.Api.Analyzers -{ - public class InspectReturnExpression_ReturnsStatusCodeFromDefaultStatusCodeAttributeOnActionResult : ControllerBase - { - public IActionResult Get() - { - return Unauthorized(); - } - } -}