diff --git a/Mvc.NoFun.sln b/Mvc.NoFun.sln index f775fd9b78..601eda5fad 100644 --- a/Mvc.NoFun.sln +++ b/Mvc.NoFun.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 -VisualStudioVersion = 15.0.26730.10 +VisualStudioVersion = 15.0.27130.2020 MinimumVisualStudioVersion = 15.0.26730.03 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "samples", "samples", "{DAAE4C74-D06F-4874-A166-33305D2643CE}" EndProject @@ -104,6 +104,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "benchmarks", "benchmarks", "{44546170-35BF-448F-88F5-4331AE67AEAE}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Mvc.Analyzers", "src\Microsoft.AspNetCore.Mvc.Analyzers\Microsoft.AspNetCore.Mvc.Analyzers.csproj", "{29454949-4AE0-4B3B-9157-BFC28B7ECD97}" +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Mvc.Analyzers.Test", "test\Microsoft.AspNetCore.Mvc.Analyzers.Test\Microsoft.AspNetCore.Mvc.Analyzers.Test.csproj", "{2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -490,6 +494,30 @@ Global {28D4DA20-6E13-47F9-80AE-D6AA7699CC35}.Release|Mixed Platforms.Build.0 = Release|Any CPU {28D4DA20-6E13-47F9-80AE-D6AA7699CC35}.Release|x86.ActiveCfg = Release|Any CPU {28D4DA20-6E13-47F9-80AE-D6AA7699CC35}.Release|x86.Build.0 = Release|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Debug|Any CPU.Build.0 = Debug|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Debug|x86.ActiveCfg = Debug|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Debug|x86.Build.0 = Debug|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Release|Any CPU.ActiveCfg = Release|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Release|Any CPU.Build.0 = Release|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Release|x86.ActiveCfg = Release|Any CPU + {29454949-4AE0-4B3B-9157-BFC28B7ECD97}.Release|x86.Build.0 = Release|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Debug|Any CPU.Build.0 = Debug|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Debug|x86.ActiveCfg = Debug|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Debug|x86.Build.0 = Debug|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Release|Any CPU.ActiveCfg = Release|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Release|Any CPU.Build.0 = Release|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Release|x86.ActiveCfg = Release|Any CPU + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -528,6 +556,8 @@ Global {CF322BE1-E1FE-4CFD-8FCA-16A14B905D53} = {32285FA4-6B46-4D6B-A840-2B13E4C8B58E} {0AB46520-F441-4E01-B444-08F4D23F8B1B} = {3BA657BF-28B1-42DA-B5B0-1C4601FCF7B1} {28D4DA20-6E13-47F9-80AE-D6AA7699CC35} = {44546170-35BF-448F-88F5-4331AE67AEAE} + {29454949-4AE0-4B3B-9157-BFC28B7ECD97} = {32285FA4-6B46-4D6B-A840-2B13E4C8B58E} + {2E6CDE10-8F96-4B75-B0D9-808F6A01B8BD} = {3BA657BF-28B1-42DA-B5B0-1C4601FCF7B1} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {D003597F-372F-4068-A2F0-353BE3C3B39A} diff --git a/Mvc.sln b/Mvc.sln index 107d31ef52..332bcf9341 100644 --- a/Mvc.sln +++ b/Mvc.sln @@ -1,4 +1,4 @@ -Microsoft Visual Studio Solution File, Format Version 12.00 +Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 VisualStudioVersion = 15.0.26831.3000 MinimumVisualStudioVersion = 15.0.26730.03 @@ -161,6 +161,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "RazorBuildWebSite.Precompil EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "RazorBuildWebSite.Views", "test\WebSites\RazorBuildWebSite.Views\RazorBuildWebSite.Views.csproj", "{8916DDCA-EC2A-4193-B9F3-78CAA1A96D5A}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Mvc.Analyzers", "src\Microsoft.AspNetCore.Mvc.Analyzers\Microsoft.AspNetCore.Mvc.Analyzers.csproj", "{87A3E227-C45E-4141-A59F-402908E651FD}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Mvc.Analyzers.Test", "test\Microsoft.AspNetCore.Mvc.Analyzers.Test\Microsoft.AspNetCore.Mvc.Analyzers.Test.csproj", "{E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -833,6 +837,30 @@ Global {8916DDCA-EC2A-4193-B9F3-78CAA1A96D5A}.Release|Mixed Platforms.Build.0 = Release|Any CPU {8916DDCA-EC2A-4193-B9F3-78CAA1A96D5A}.Release|x86.ActiveCfg = Release|Any CPU {8916DDCA-EC2A-4193-B9F3-78CAA1A96D5A}.Release|x86.Build.0 = Release|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Debug|Any CPU.Build.0 = Debug|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Debug|x86.ActiveCfg = Debug|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Debug|x86.Build.0 = Debug|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Release|Any CPU.ActiveCfg = Release|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Release|Any CPU.Build.0 = Release|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Release|x86.ActiveCfg = Release|Any CPU + {87A3E227-C45E-4141-A59F-402908E651FD}.Release|x86.Build.0 = Release|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Debug|Any CPU.Build.0 = Debug|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Debug|x86.ActiveCfg = Debug|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Debug|x86.Build.0 = Debug|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Release|Any CPU.ActiveCfg = Release|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Release|Any CPU.Build.0 = Release|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Release|x86.ActiveCfg = Release|Any CPU + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -898,6 +926,8 @@ Global {BF8A3392-C3D2-4813-855A-E906564600E1} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} {856D7E25-E033-477D-9ABD-0B50CF428C80} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} {8916DDCA-EC2A-4193-B9F3-78CAA1A96D5A} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} + {87A3E227-C45E-4141-A59F-402908E651FD} = {32285FA4-6B46-4D6B-A840-2B13E4C8B58E} + {E3E09D2F-1FCF-4396-9B09-5A62CA8CC831} = {3BA657BF-28B1-42DA-B5B0-1C4601FCF7B1} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {63D344F6-F86D-40E6-85B9-0AABBE338C4A} diff --git a/build/dependencies.props b/build/dependencies.props index faab41e4e4..29b519962c 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -42,6 +42,7 @@ 2.1.0-preview1-28124 5.2.4-preview1 2.6.1 + 2.6.1 2.1.0-preview1-28124 2.1.0-preview1-28124 2.1.0-preview1-28124 diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ActionsMustNotBeAsyncVoidAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ActionsMustNotBeAsyncVoidAnalyzer.cs new file mode 100644 index 0000000000..6b08beb7e7 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ActionsMustNotBeAsyncVoidAnalyzer.cs @@ -0,0 +1,56 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ActionsMustNotBeAsyncVoidAnalyzer : ControllerAnalyzerBase + { + public static readonly string ReturnTypeKey = "ReturnType"; + + public ActionsMustNotBeAsyncVoidAnalyzer() + : base(DiagnosticDescriptors.MVC1003_ActionsMustNotBeAsyncVoid) + { + } + + protected override void InitializeWorker(ControllerAnalyzerContext analyzerContext) + { + analyzerContext.Context.RegisterSyntaxNodeAction(context => + { + var methodSyntax = (MethodDeclarationSyntax)context.Node; + var method = context.SemanticModel.GetDeclaredSymbol(methodSyntax, context.CancellationToken); + + if (!analyzerContext.IsControllerAction(method)) + { + return; + } + + if (!method.IsAsync || !method.ReturnsVoid) + { + return; + } + + var returnType = analyzerContext.SystemThreadingTask.ToMinimalDisplayString( + context.SemanticModel, + methodSyntax.ReturnType.SpanStart); + + var properties = ImmutableDictionary.Create(StringComparer.Ordinal) + .Add(ReturnTypeKey, returnType); + + var location = methodSyntax.ReturnType.GetLocation(); + context.ReportDiagnostic(Diagnostic.Create( + SupportedDiagnostic, + location, + properties: properties)); + + }, SyntaxKind.MethodDeclaration); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ActionsMustNotBeAsyncVoidFixProvider.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ActionsMustNotBeAsyncVoidFixProvider.cs new file mode 100644 index 0000000000..d953859891 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ActionsMustNotBeAsyncVoidFixProvider.cs @@ -0,0 +1,58 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Editing; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + [Shared] + public class ActionsMustNotBeAsyncVoidFixProvider : CodeFixProvider + { + public sealed override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(DiagnosticDescriptors.MVC1003_ActionsMustNotBeAsyncVoid.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + if (context.Diagnostics.Length == 0) + { + return; + } + + if (!context.Diagnostics[0].Properties.TryGetValue(ActionsMustNotBeAsyncVoidAnalyzer.ReturnTypeKey, out var returnTypeName)) + { + return; + } + + var rootNode = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + const string title = "Fix async void usage."; + context.RegisterCodeFix( + CodeAction.Create( + title, + createChangedDocument: CreateChangedDocumentAsync, + equivalenceKey: title), + context.Diagnostics); + + async Task CreateChangedDocumentAsync(CancellationToken cancellationToken) + { + var returnTypeSyntax = rootNode.FindNode(context.Span); + + var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false); + editor.ReplaceNode(returnTypeSyntax, SyntaxFactory.IdentifierName(returnTypeName)); + + return editor.GetChangedDocument(); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsAreAttributeRoutedAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsAreAttributeRoutedAnalyzer.cs new file mode 100644 index 0000000000..f6817314f6 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsAreAttributeRoutedAnalyzer.cs @@ -0,0 +1,52 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ApiActionsAreAttributeRoutedAnalyzer : ApiControllerAnalyzerBase + { + internal const string MethodNameKey = "MethodName"; + + public ApiActionsAreAttributeRoutedAnalyzer() + : base(DiagnosticDescriptors.MVC1000_ApiActionsMustBeAttributeRouted) + { + } + + protected override void InitializeWorker(ApiControllerAnalyzerContext analyzerContext) + { + analyzerContext.Context.RegisterSymbolAction(context => + { + var method = (IMethodSymbol)context.Symbol; + + if (!analyzerContext.IsApiAction(method)) + { + return; + } + + foreach (var attribute in method.GetAttributes()) + { + if (attribute.AttributeClass.IsAssignableFrom(analyzerContext.RouteAttribute)) + { + return; + } + } + + var properties = ImmutableDictionary.Create(StringComparer.Ordinal) + .Add(MethodNameKey, method.Name); + + var location = method.Locations.Length > 0 ? method.Locations[0] : Location.None; + context.ReportDiagnostic(Diagnostic.Create( + SupportedDiagnostic, + location, + properties: properties)); + + }, SymbolKind.Method); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsAreAttributeRoutedFixProvider.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsAreAttributeRoutedFixProvider.cs new file mode 100644 index 0000000000..e7d697f0a2 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsAreAttributeRoutedFixProvider.cs @@ -0,0 +1,187 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using System.Composition; +using System.Diagnostics; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + [Shared] + public class ApiActionsAreAttributeRoutedFixProvider : CodeFixProvider + { + private static readonly RouteAttributeInfo[] RouteAttributes = new[] + { + new RouteAttributeInfo("HttpGet", TypeNames.HttpGetAttribute, new[] { "Get", "Find" }), + new RouteAttributeInfo("HttpPost", TypeNames.HttpPostAttribute, new[] { "Post", "Create", "Update" }), + new RouteAttributeInfo("HttpDelete", TypeNames.HttpDeleteAttribute, new[] { "Delete", "Remove" }), + new RouteAttributeInfo("HttpPut", TypeNames.HttpPutAttribute, new[] { "Put", "Create", "Update" }), + }; + + public sealed override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(DiagnosticDescriptors.MVC1000_ApiActionsMustBeAttributeRouted.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + if (context.Diagnostics.Length == 0) + { + return; + } + + var rootNode = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + Debug.Assert(context.Diagnostics.Length == 1); + var diagnostic = context.Diagnostics[0]; + var methodName = diagnostic.Properties[ApiActionsAreAttributeRoutedAnalyzer.MethodNameKey]; + + var matchedByKeyword = false; + foreach (var routeInfo in RouteAttributes) + { + foreach (var keyword in routeInfo.KeyWords) + { + // Determine if the method starts with a conventional key and only show relevant routes. + // For e.g. FindPetByCategory would result in HttpGet attribute. + if (methodName.StartsWith(keyword, StringComparison.Ordinal)) + { + matchedByKeyword = true; + + var title = $"Add {routeInfo.Name} attribute"; + context.RegisterCodeFix( + CodeAction.Create( + title, + createChangedDocument: cancellationToken => CreateChangedDocumentAsync(routeInfo.Type, cancellationToken), + equivalenceKey: title), + context.Diagnostics); + } + } + } + + if (!matchedByKeyword) + { + foreach (var routeInfo in RouteAttributes) + { + var title = $"Add {routeInfo.Name} attribute"; + context.RegisterCodeFix( + CodeAction.Create( + title, + createChangedDocument: cancellationToken => CreateChangedDocumentAsync(routeInfo.Type, cancellationToken), + equivalenceKey: title), + context.Diagnostics); + } + } + + async Task CreateChangedDocumentAsync(string attributeName, CancellationToken cancellationToken) + { + var methodNode = (MethodDeclarationSyntax)rootNode.FindNode(context.Span); + + var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false); + var compilation = editor.SemanticModel.Compilation; + var attributeMetadata = compilation.GetTypeByMetadataName(attributeName); + var fromRouteAttribute = compilation.GetTypeByMetadataName(TypeNames.FromRouteAttribute); + + attributeName = attributeMetadata.ToMinimalDisplayString(editor.SemanticModel, methodNode.SpanStart); + + // Remove the Attribute suffix from type names e.g. "HttpGetAttribute" -> "HttpGet" + if (attributeName.EndsWith("Attribute", StringComparison.Ordinal)) + { + attributeName = attributeName.Substring(0, attributeName.Length - "Attribute".Length); + } + + var method = editor.SemanticModel.GetDeclaredSymbol(methodNode); + + var attribute = SyntaxFactory.Attribute( + SyntaxFactory.ParseName(attributeName)); + + var route = GetRoute(fromRouteAttribute, method); + if (!string.IsNullOrEmpty(route)) + { + attribute = attribute.AddArgumentListArguments( + SyntaxFactory.AttributeArgument( + SyntaxFactory.LiteralExpression( + SyntaxKind.StringLiteralExpression, + SyntaxFactory.Literal(route)))); + } + + editor.AddAttribute(methodNode, attribute); + return editor.GetChangedDocument(); + } + } + + private static string GetRoute(ITypeSymbol fromRouteAttribute, IMethodSymbol method) + { + StringBuilder routeNameBuilder = null; + + foreach (var parameter in method.Parameters) + { + if (IsIdParameter(parameter.Name) || parameter.HasAttribute(fromRouteAttribute)) + { + if (routeNameBuilder == null) + { + routeNameBuilder = new StringBuilder(parameter.Name.Length + 2); + } + else + { + routeNameBuilder.Append("/"); + } + + routeNameBuilder + .Append("{") + .Append(parameter.Name) + .Append("}"); + } + } + + return routeNameBuilder?.ToString(); + } + + private static bool IsIdParameter(string name) + { + // Check if the parameter is named "id" (e.g. int id) or ends in Id (e.g. personId) + if (name == null || name.Length < 2) + { + return false; + } + + if (string.Equals("id", name, StringComparison.Ordinal)) + { + return true; + } + + if (name.Length > 3 && name.EndsWith("Id", StringComparison.Ordinal) && char.IsLower(name[name.Length - 3])) + { + return true; + } + + return false; + } + + private struct RouteAttributeInfo + { + public RouteAttributeInfo(string name, string type, string[] keywords) + { + Name = name; + Type = type; + KeyWords = keywords; + } + + public string Name { get; } + + public string Type { get; } + + public string[] KeyWords { get; } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs new file mode 100644 index 0000000000..eb563d7eb5 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer.cs @@ -0,0 +1,94 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer : ApiControllerAnalyzerBase + { + public ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer() + : base(DiagnosticDescriptors.MVC1001_ApiActionsHaveBadModelStateFilter) + { + } + + protected override void InitializeWorker(ApiControllerAnalyzerContext analyzerContext) + { + analyzerContext.Context.RegisterSyntaxNodeAction(context => + { + var methodSyntax = (MethodDeclarationSyntax)context.Node; + if (methodSyntax.Body == null) + { + // Ignore expression bodied methods. + return; + } + + var method = context.SemanticModel.GetDeclaredSymbol(methodSyntax, context.CancellationToken); + if (!analyzerContext.IsApiAction(method)) + { + return; + } + + if (method.ReturnsVoid || method.ReturnType == analyzerContext.SystemThreadingTaskOfT) + { + // Void or Task returning methods. We don't have to check anything here since we're specifically + // looking for return BadRequest(..); + return; + } + + // Only look for top level statements that look like "if (!ModelState.IsValid)" + foreach (var memberAccessSyntax in methodSyntax.Body.DescendantNodes().OfType()) + { + var ancestorIfStatement = memberAccessSyntax.FirstAncestorOrSelf(); + if (ancestorIfStatement == null) + { + // Node's not in an if statement. + continue; + } + + var symbolInfo = context.SemanticModel.GetSymbolInfo(memberAccessSyntax, context.CancellationToken); + + if (!(symbolInfo.Symbol is IPropertySymbol property) || + (property.ContainingType != analyzerContext.ModelStateDictionary) || + !string.Equals(property.Name, "IsValid", StringComparison.Ordinal) || + !IsFalseExpression(memberAccessSyntax)) + { + continue; + } + + var containingBlock = (SyntaxNode)ancestorIfStatement; + if (containingBlock.Parent.Kind() == SyntaxKind.ElseClause) + { + containingBlock = containingBlock.Parent; + } + context.ReportDiagnostic(Diagnostic.Create(SupportedDiagnostic, containingBlock.GetLocation())); + return; + } + }, SyntaxKind.MethodDeclaration); + } + + private static bool IsFalseExpression(MemberAccessExpressionSyntax memberAccessSyntax) + { + switch (memberAccessSyntax.Parent.Kind()) + { + case SyntaxKind.LogicalNotExpression: + // !ModelState.IsValid + return true; + case SyntaxKind.EqualsExpression: + var binaryExpression = (BinaryExpressionSyntax)memberAccessSyntax.Parent; + // ModelState.IsValid == false + // false == ModelState.IsValid + return binaryExpression.Left.Kind() == SyntaxKind.FalseLiteralExpression || + binaryExpression.Right.Kind() == SyntaxKind.FalseLiteralExpression; + } + + return false; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider.cs new file mode 100644 index 0000000000..21245fa0b0 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider.cs @@ -0,0 +1,46 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Editing; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + [Shared] + public class ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider : CodeFixProvider + { + public sealed override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(DiagnosticDescriptors.MVC1001_ApiActionsHaveBadModelStateFilter.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + const string title = "Remove ModelState.IsValid check"; + var rootNode = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + context.RegisterCodeFix( + CodeAction.Create( + title, + createChangedDocument: CreateChangedDocumentAsync, + equivalenceKey: title), + context.Diagnostics); + + async Task CreateChangedDocumentAsync(CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false); + var node = rootNode.FindNode(context.Span); + editor.RemoveNode(node); + + return editor.GetChangedDocument(); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsShouldUseActionResultOfTAnalyzer.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsShouldUseActionResultOfTAnalyzer.cs new file mode 100644 index 0000000000..b6ba8f6be6 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsShouldUseActionResultOfTAnalyzer.cs @@ -0,0 +1,102 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ApiActionsShouldUseActionResultOfTAnalyzer : ApiControllerAnalyzerBase + { + public static readonly string ReturnTypeKey = "ReturnType"; + + public ApiActionsShouldUseActionResultOfTAnalyzer() + : base(DiagnosticDescriptors.MVC1002_ApiActionsShouldReturnActionResultOf) + { + } + + protected override void InitializeWorker(ApiControllerAnalyzerContext analyzerContext) + { + analyzerContext.Context.RegisterSyntaxNodeAction(context => + { + var methodSyntax = (MethodDeclarationSyntax)context.Node; + if (methodSyntax.Body == null) + { + // Ignore expression bodied methods. + } + + var method = context.SemanticModel.GetDeclaredSymbol(methodSyntax, context.CancellationToken); + if (!analyzerContext.IsApiAction(method)) + { + return; + } + + if (method.ReturnsVoid || method.ReturnType.Kind != SymbolKind.NamedType) + { + return; + } + + var declaredReturnType = method.ReturnType; + var namedReturnType = (INamedTypeSymbol)method.ReturnType; + var isTaskOActionResult = false; + if (namedReturnType.ConstructedFrom?.IsAssignableFrom(analyzerContext.SystemThreadingTaskOfT) ?? false) + { + // Unwrap Task. + isTaskOActionResult = true; + declaredReturnType = namedReturnType.TypeArguments[0]; + } + + if (!declaredReturnType.IsAssignableFrom(analyzerContext.IActionResult)) + { + // Method signature does not look like IActionResult MyAction or SomeAwaitable. + // Nothing to do here. + return; + } + + // Method returns an IActionResult. Determine if the method block returns an ObjectResult + foreach (var returnStatement in methodSyntax.DescendantNodes().OfType()) + { + var returnType = context.SemanticModel.GetTypeInfo(returnStatement.Expression, context.CancellationToken); + if (returnType.Type == null || returnType.Type.Kind == SymbolKind.ErrorType) + { + continue; + } + + ImmutableDictionary properties = null; + if (returnType.Type.IsAssignableFrom(analyzerContext.ObjectResult)) + { + // Check if the method signature looks like "return Ok(userModelInstance)". If so, we can infer the type of userModelInstance + if (returnStatement.Expression is InvocationExpressionSyntax invocation && + invocation.ArgumentList.Arguments.Count == 1) + { + var typeInfo = context.SemanticModel.GetTypeInfo(invocation.ArgumentList.Arguments[0].Expression); + var desiredReturnType = analyzerContext.ActionResultOfT.Construct(typeInfo.Type); + if (isTaskOActionResult) + { + desiredReturnType = analyzerContext.SystemThreadingTaskOfT.Construct(desiredReturnType); + } + + var desiredReturnTypeString = desiredReturnType.ToMinimalDisplayString( + context.SemanticModel, + methodSyntax.ReturnType.SpanStart); + + properties = ImmutableDictionary.Create(StringComparer.Ordinal) + .Add(ReturnTypeKey, desiredReturnTypeString); + } + + context.ReportDiagnostic(Diagnostic.Create( + SupportedDiagnostic, + methodSyntax.ReturnType.GetLocation(), + properties: properties)); + } + } + }, SyntaxKind.MethodDeclaration); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsShouldUseActionResultOfTCodeFixProvider.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsShouldUseActionResultOfTCodeFixProvider.cs new file mode 100644 index 0000000000..d75bbed522 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiActionsShouldUseActionResultOfTCodeFixProvider.cs @@ -0,0 +1,55 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Editing; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + [Shared] + public class ApiActionsShouldUseActionResultOfTCodeFixProvider : CodeFixProvider + { + public sealed override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(DiagnosticDescriptors.MVC1002_ApiActionsShouldReturnActionResultOf.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var rootNode = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + foreach (var diagnostic in context.Diagnostics) + { + if (diagnostic.Properties.TryGetValue("ReturnType", out var returnTypeName)) + { + + var title = $"Make return type {returnTypeName}"; + context.RegisterCodeFix( + CodeAction.Create( + title, + createChangedDocument: cancellationToken => CreateChangedDocumentAsync(returnTypeName, cancellationToken), + equivalenceKey: title), + context.Diagnostics); + } + } + + async Task CreateChangedDocumentAsync(string returnTypeName, CancellationToken cancellationToken) + { + var returnType = rootNode.FindNode(context.Span); + + var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false); + editor.ReplaceNode(returnType, SyntaxFactory.IdentifierName(returnTypeName)); + + return editor.GetChangedDocument(); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerAnalyzerBase.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerAnalyzerBase.cs new file mode 100644 index 0000000000..b70d1409de --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerAnalyzerBase.cs @@ -0,0 +1,39 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public abstract class ApiControllerAnalyzerBase : DiagnosticAnalyzer + { + public ApiControllerAnalyzerBase(DiagnosticDescriptor diagnostic) + { + SupportedDiagnostics = ImmutableArray.Create(diagnostic); + } + + protected DiagnosticDescriptor SupportedDiagnostic => SupportedDiagnostics[0]; + + public override ImmutableArray SupportedDiagnostics { get; } + + public sealed override void Initialize(AnalysisContext context) + { + context.RegisterCompilationStartAction(compilationContext => + { + var analyzerContext = new ApiControllerAnalyzerContext(compilationContext); + + // Only do work if ApiControllerAttribute is defined. + if (analyzerContext.ApiControllerAttribute == null) + { + return; + } + + InitializeWorker(analyzerContext); + }); + } + + protected abstract void InitializeWorker(ApiControllerAnalyzerContext analyzerContext); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerAnalyzerContext.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerAnalyzerContext.cs new file mode 100644 index 0000000000..b1e4c81b00 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ApiControllerAnalyzerContext.cs @@ -0,0 +1,63 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ApiControllerAnalyzerContext + { +#pragma warning disable RS1012 // Start action has no registered actions. + public ApiControllerAnalyzerContext(CompilationStartAnalysisContext context) +#pragma warning restore RS1012 // Start action has no registered actions. + { + Context = context; + ApiControllerAttribute = context.Compilation.GetTypeByMetadataName(TypeNames.ApiControllerAttribute); + } + + public CompilationStartAnalysisContext Context { get; } + + public INamedTypeSymbol ApiControllerAttribute { get; } + + private INamedTypeSymbol _routeAttribute; + public INamedTypeSymbol RouteAttribute => GetType(TypeNames.IRouteTemplateProvider, ref _routeAttribute); + + private INamedTypeSymbol _actionResultOfT; + public INamedTypeSymbol ActionResultOfT => GetType(TypeNames.ActionResultOfT, ref _actionResultOfT); + + private INamedTypeSymbol _systemThreadingTask; + public INamedTypeSymbol SystemThreadingTask => GetType(TypeNames.Task, ref _systemThreadingTask); + + private INamedTypeSymbol _systemThreadingTaskOfT; + public INamedTypeSymbol SystemThreadingTaskOfT => GetType(TypeNames.TaskOfT, ref _systemThreadingTaskOfT); + + private INamedTypeSymbol _objectResult; + public INamedTypeSymbol ObjectResult => GetType(TypeNames.ObjectResult, ref _objectResult); + + private INamedTypeSymbol _iActionResult; + public INamedTypeSymbol IActionResult => GetType(TypeNames.IActionResult, ref _iActionResult); + + public INamedTypeSymbol _modelState; + public INamedTypeSymbol ModelStateDictionary => GetType(TypeNames.ModelStateDictionary, ref _modelState); + + public INamedTypeSymbol _nonActionAttribute; + public INamedTypeSymbol NonActionAttribute => GetType(TypeNames.NonActionAttribute, ref _nonActionAttribute); + + + private INamedTypeSymbol GetType(string name, ref INamedTypeSymbol cache) => + cache = cache ?? Context.Compilation.GetTypeByMetadataName(name); + + public bool IsApiAction(IMethodSymbol method) + { + return + method.ContainingType.HasAttribute(ApiControllerAttribute, inherit: true) && + method.DeclaredAccessibility == Accessibility.Public && + !method.IsGenericMethod && + !method.IsAbstract && + !method.IsStatic && + !method.HasAttribute(NonActionAttribute); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/CodeAnalysisExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/CodeAnalysisExtensions.cs new file mode 100644 index 0000000000..a8ae5b8a0f --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/CodeAnalysisExtensions.cs @@ -0,0 +1,78 @@ +// 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.Diagnostics; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + internal static class CodeAnalysisExtensions + { + public static bool HasAttribute(this ITypeSymbol typeSymbol, ITypeSymbol attribute, bool inherit) + { + while (typeSymbol != null) + { + if (typeSymbol.HasAttribute(attribute)) + { + return true; + } + + typeSymbol = typeSymbol.BaseType; + } + + return false; + } + + public static bool HasAttribute(this ISymbol symbol, ITypeSymbol attribute) + { + Debug.Assert(symbol != null); + Debug.Assert(attribute != null); + + foreach (var declaredAttribute in symbol.GetAttributes()) + { + if (declaredAttribute.AttributeClass == attribute) + { + return true; + } + } + + return false; + } + + public static bool IsAssignableFrom(this ITypeSymbol source, INamedTypeSymbol target) + { + Debug.Assert(source != null); + Debug.Assert(target != null); + + if (source == target) + { + return true; + } + + if (target.TypeKind == TypeKind.Interface) + { + foreach (var @interface in source.AllInterfaces) + { + if (@interface == target) + { + return true; + } + } + + return false; + } + + do + { + if (source == target) + { + return true; + } + + source = source.BaseType; + } while (source != null); + + return false; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ControllerAnalyzerBase.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ControllerAnalyzerBase.cs new file mode 100644 index 0000000000..71c497fa29 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ControllerAnalyzerBase.cs @@ -0,0 +1,39 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public abstract class ControllerAnalyzerBase : DiagnosticAnalyzer + { + public ControllerAnalyzerBase(DiagnosticDescriptor diagnostic) + { + SupportedDiagnostics = ImmutableArray.Create(diagnostic); + } + + protected DiagnosticDescriptor SupportedDiagnostic => SupportedDiagnostics[0]; + + public override ImmutableArray SupportedDiagnostics { get; } + + public sealed override void Initialize(AnalysisContext context) + { + context.RegisterCompilationStartAction(compilationContext => + { + var analyzerContext = new ControllerAnalyzerContext(compilationContext); + + // Only do work if ControllerAttribute is defined. + if (analyzerContext.ControllerAttribute == null) + { + return; + } + + InitializeWorker(analyzerContext); + }); + } + + protected abstract void InitializeWorker(ControllerAnalyzerContext analyzerContext); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/ControllerAnalyzerContext.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/ControllerAnalyzerContext.cs new file mode 100644 index 0000000000..503459ba02 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/ControllerAnalyzerContext.cs @@ -0,0 +1,46 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ControllerAnalyzerContext + { +#pragma warning disable RS1012 // Start action has no registered actions. + public ControllerAnalyzerContext(CompilationStartAnalysisContext context) +#pragma warning restore RS1012 // Start action has no registered actions. + { + Context = context; + ControllerAttribute = Context.Compilation.GetTypeByMetadataName(TypeNames.ControllerAttribute); + } + + public CompilationStartAnalysisContext Context { get; } + + public INamedTypeSymbol ControllerAttribute { get; } + + private INamedTypeSymbol _systemThreadingTask; + public INamedTypeSymbol SystemThreadingTask => GetType(TypeNames.Task, ref _systemThreadingTask); + + private INamedTypeSymbol _systemThreadingTaskOfT; + public INamedTypeSymbol SystemThreadingTaskOfT => GetType(TypeNames.TaskOfT, ref _systemThreadingTaskOfT); + + public INamedTypeSymbol _nonActionAttribute; + public INamedTypeSymbol NonActionAttribute => GetType(TypeNames.NonActionAttribute, ref _nonActionAttribute); + + private INamedTypeSymbol GetType(string name, ref INamedTypeSymbol cache) => + cache = cache ?? Context.Compilation.GetTypeByMetadataName(name); + + public bool IsControllerAction(IMethodSymbol method) + { + return + method.ContainingType.HasAttribute(ControllerAttribute, inherit: true) && + method.DeclaredAccessibility == Accessibility.Public && + !method.IsGenericMethod && + !method.IsAbstract && + !method.IsStatic && + !method.HasAttribute(NonActionAttribute); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/DiagnosticDescriptors.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/DiagnosticDescriptors.cs new file mode 100644 index 0000000000..3ad7235cec --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/DiagnosticDescriptors.cs @@ -0,0 +1,46 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public static class DiagnosticDescriptors + { + public static readonly DiagnosticDescriptor MVC1000_ApiActionsMustBeAttributeRouted = + new DiagnosticDescriptor( + "MVC1000", + "Actions on types annotated with ApiControllerAttribute must be attribute routed.", + "Actions on types annotated with ApiControllerAttribute must be attribute routed.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor MVC1001_ApiActionsHaveBadModelStateFilter = + new DiagnosticDescriptor( + "MVC1001", + "Actions on types annotated with ApiControllerAttribute do not require explicit ModelState validity check.", + "Actions on types annotated with ApiControllerAttribute do not require explicit ModelState validity check.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor MVC1002_ApiActionsShouldReturnActionResultOf = + new DiagnosticDescriptor( + "MVC1002", + "Actions on types annotated with ApiControllerAttribute should return ActionResult.", + "Actions on types annotated with ApiControllerAttribute should return ActionResult.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor MVC1003_ActionsMustNotBeAsyncVoid = + new DiagnosticDescriptor( + "MVC2001", + "Controller actions must not have async void signature.", + "Controller actions must not have async void signature.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/Microsoft.AspNetCore.Mvc.Analyzers.csproj b/src/Microsoft.AspNetCore.Mvc.Analyzers/Microsoft.AspNetCore.Mvc.Analyzers.csproj new file mode 100644 index 0000000000..1935fe7bac --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/Microsoft.AspNetCore.Mvc.Analyzers.csproj @@ -0,0 +1,24 @@ + + + CSharp Analyzers for ASP.NET Core MVC. + aspnetcore;aspnetcoremvc + + false + $(ExperimentalVersionPrefix) + $(ExperimentalVersionSuffix) + + netstandard2.0 + false + false + false + + + + + + + + + + + diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/TypeNames.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/TypeNames.cs new file mode 100644 index 0000000000..810c63a7bd --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/TypeNames.cs @@ -0,0 +1,38 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + internal static class TypeNames + { + public const string ControllerAttribute = "Microsoft.AspNetCore.Mvc.ControllerAttribute"; + + public const string ApiControllerAttribute = "Microsoft.AspNetCore.Mvc.ApiControllerAttribute"; + + public const string NonActionAttribute = "Microsoft.AspNetCore.Mvc.NonActionAttribute"; + + public const string IRouteTemplateProvider = "Microsoft.AspNetCore.Mvc.Routing.IRouteTemplateProvider"; + + public const string ActionResultOfT = "Microsoft.AspNetCore.Mvc.ActionResult`1"; + + public const string Task = "System.Threading.Tasks.Task"; + + public const string TaskOfT = "System.Threading.Tasks.Task`1"; + + public const string ObjectResult = "Microsoft.AspNetCore.Mvc.ObjectResult"; + + public const string IActionResult = "Microsoft.AspNetCore.Mvc.IActionResult"; + + public const string ModelStateDictionary = "Microsoft.AspNetCore.Mvc.ModelBinding.ModelStateDictionary"; + + public const string HttpGetAttribute = "Microsoft.AspNetCore.Mvc.HttpGetAttribute"; + + public const string HttpPostAttribute = "Microsoft.AspNetCore.Mvc.HttpPostAttribute"; + + public const string HttpPutAttribute = "Microsoft.AspNetCore.Mvc.HttpPutAttribute"; + + public const string HttpDeleteAttribute = "Microsoft.AspNetCore.Mvc.HttpDeleteAttribute"; + + public const string FromRouteAttribute = "Microsoft.AspNetCore.Mvc.FromRouteAttribute"; + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ActionsMustNotBeAsyncVoidFacts.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ActionsMustNotBeAsyncVoidFacts.cs new file mode 100644 index 0000000000..d5f6bcdf80 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ActionsMustNotBeAsyncVoidFacts.cs @@ -0,0 +1,174 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ActionsMustNotBeAsyncVoidFacts : AnalyzerTestBase + { + protected override DiagnosticAnalyzer DiagnosticAnalyzer { get; } + = new ActionsMustNotBeAsyncVoidAnalyzer(); + + protected override CodeFixProvider CodeFixProvider { get; } + = new ActionsMustNotBeAsyncVoidFixProvider(); + + [Fact] + public async Task NoDiagnosticsAreReturned_FoEmptyScenarios() + { + // Arrange + var test = @""; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenMethodIsNotAControllerAction() + { + // Arrange + var test = +@" +using System.Threading.Tasks; + +public class UserViewModel +{ + public async void Index() => await Task.Delay(10); +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task DiagnosticsAreReturned_WhenMethodIsAControllerAction() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC2001", + Message = "Controller actions must not have async void signature.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 7, 18) } + }; + var test = +@" +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +public class HomeController : Controller +{ + public async void Index() + { + await Response.Body.FlushAsync(); + } +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +public class HomeController : Controller +{ + public async Task Index() + { + await Response.Body.FlushAsync(); + } +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task DiagnosticsAreReturned_WhenActionMethodIsExpressionBodied() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC2001", + Message = "Controller actions must not have async void signature.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 7, 18) } + }; + var test = +@" +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +public class HomeController : Controller +{ + public async void Index() => await Response.Body.FlushAsync(); +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +public class HomeController : Controller +{ + public async Task Index() => await Response.Body.FlushAsync(); +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task CodeFix_ProducesFullyQualifiedNamespaces() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC2001", + Message = "Controller actions must not have async void signature.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 6, 18) } + }; + var test = +@" +using Microsoft.AspNetCore.Mvc; + +public class HomeController : Controller +{ + public async void Index() => await Response.Body.FlushAsync(); +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; + +public class HomeController : Controller +{ + public async System.Threading.Tasks.Task Index() => await Response.Body.FlushAsync(); +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApActionsDoNotRequireExplicitModelValidationCheckFacts.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApActionsDoNotRequireExplicitModelValidationCheckFacts.cs new file mode 100644 index 0000000000..a5f65ee9d0 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApActionsDoNotRequireExplicitModelValidationCheckFacts.cs @@ -0,0 +1,361 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ApiActionsDoNotRequireExplicitModelValidationCheckFacts : AnalyzerTestBase + { + protected override DiagnosticAnalyzer DiagnosticAnalyzer { get; } + = new ApiActionsDoNotRequireExplicitModelValidationCheckAnalyzer(); + + protected override CodeFixProvider CodeFixProvider { get; } + = new ApiActionsDoNotRequireExplicitModelValidationCheckCodeFixProvider(); + + [Fact] + public async Task NoDiagnosticsAreReturned_FoEmptyScenarios() + { + // Arrange + var test = @""; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenTypeIsNotApiController() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +public class HomeController : Controller +{ + public IActionResult Index() => View(); +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenActionDoesNotHaveModelStateCheck() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : Controller +{ + public IActionResult GetPetId() + { + return Ok(new object()); + } +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenAActionsUseExpressionBodies() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : Controller +{ + public IActionResult GetPetId() => ModelState.IsVald ? OK() : BadResult(); +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_ForNonActions() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + private int GetPetIdPrivate() => 0; + protected int GetPetIdProtected() => 0; + public static IActionResult FindPetByStatus(int status) => null; + [NonAction] + public object Reset(int state) => null; +}"; + + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public Task DiagnosticsAndCodeFixes_WhenActionHasModelStateIsValidCheck() + { + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + if (!ModelState.IsValid) + { + return BadRequest(); + } + + return Ok(); + } +}"; + + // Act & Assert + return VerifyAsync(test); + } + + + [Fact] + public Task DiagnosticsAndCodeFixes_WhenActionHasModelStateIsValidCheck_UsingComparisonToFalse() + { + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + if (ModelState.IsValid == false) + { + return BadRequest(); + } + + return Ok(); + } +}"; + + // Act & Assert + return VerifyAsync(test); + } + + [Fact] + public Task DiagnosticsAndCodeFixes_WhenActionHasModelStateIsValidCheck_WithoutBraces() + { + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + if (!ModelState.IsValid) + return BadRequest(); + + return Ok(); + } +}"; + return VerifyAsync(test); + } + + private async Task VerifyAsync(string test) + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC1001", + Message = "Actions on types annotated with ApiControllerAttribute do not require explicit ModelState validity check.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 9, 9) } + }; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + return Ok(); + } +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task DiagnosticsAndCodeFixes_WhenModelStateIsInElseIf() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC1001", + Message = "Actions on types annotated with ApiControllerAttribute do not require explicit ModelState validity check.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 13, 9) } + }; + + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + if (User == null) + { + return Unauthorized(); + } + else if (!ModelState.IsValid) + { + return BadRequest(ModelState); + } + + return Ok(); + } +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + if (User == null) + { + return Unauthorized(); + } + + return Ok(); + } +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task DiagnosticsAndCodeFixes_WhenModelStateIsInNestedBlock() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC1001", + Message = "Actions on types annotated with ApiControllerAttribute do not require explicit ModelState validity check.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 15, 13) } + }; + + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + if (User == null) + { + return Unauthorized(); + } + else + { + if (!ModelState.IsValid) + { + return BadRequest(ModelState); + } + + Debug.Assert(ModelState.Count == 0); + } + + return Ok(); + } +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : ControllerBase +{ + public IActionResult GetPetId() + { + if (User == null) + { + return Unauthorized(); + } + else + { + Debug.Assert(ModelState.Count == 0); + } + + return Ok(); + } +}"; + + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApiActionsAreAttributeRoutedFacts.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApiActionsAreAttributeRoutedFacts.cs new file mode 100644 index 0000000000..71a24bd24f --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApiActionsAreAttributeRoutedFacts.cs @@ -0,0 +1,272 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ApiActionsAreAttributeRoutedFacts : AnalyzerTestBase + { + protected override DiagnosticAnalyzer DiagnosticAnalyzer { get; } + = new ApiActionsAreAttributeRoutedAnalyzer(); + + protected override CodeFixProvider CodeFixProvider { get; } + = new ApiActionsAreAttributeRoutedFixProvider(); + + [Fact] + public async Task NoDiagnosticsAreReturned_FoEmptyScenarios() + { + // Arrange + var test = @""; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenTypeIsNotApiController() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +public class HomeController : Controller +{ + public IActionResult Index() => View(); +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenApiControllerActionHasAttribute() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : Controller +{ + [HttpGet] + public int GetPetId() => 0; +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_ForNonActions() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController : Controller +{ + private int GetPetIdPrivate() => 0; + protected int GetPetIdProtected() => 0; + public static IActionResult FindPetByStatus(int status) => null; + [NonAction] + public object Reset(int state) => null; +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task DiagnosticsAndCodeFixes_WhenApiControllerActionDoesNotHaveAttribute() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC1000", + Message = "Actions on types annotated with ApiControllerAttribute must be attribute routed.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 8, 16) } + }; + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +[Route] +public class PetController : Controller +{ + public int GetPetId() => 0; +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +[Route] +public class PetController : Controller +{ + [HttpGet] + public int GetPetId() => 0; +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task CodeFixes_ApplyFullyQualifiedNames() + { + // Arrange + var test = +@" +[Microsoft.AspNetCore.Mvc.ApiController] +[Microsoft.AspNetCore.Mvc.Route] +public class PetController +{ + public object GetPet() => null; +}"; + var expectedFix = +@" +[Microsoft.AspNetCore.Mvc.ApiController] +[Microsoft.AspNetCore.Mvc.Route] +public class PetController +{ + [Microsoft.AspNetCore.Mvc.HttpGet] + public object GetPet() => null; +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Theory] + [InlineData("id")] + [InlineData("petId")] + public async Task CodeFixes_WithIdParameter(string idParameter) + { + // Arrange + var test = +$@" +using Microsoft.AspNetCore.Mvc; +[ApiController] +[Route] +public class PetController +{{ + public IActionResult Post(string notid, int {idParameter}) => null; +}}"; + var expectedFix = +$@" +using Microsoft.AspNetCore.Mvc; +[ApiController] +[Route] +public class PetController +{{ + [HttpPost(""{{{idParameter}}}"")] + public IActionResult Post(string notid, int {idParameter}) => null; +}}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task CodeFixes_WithRouteParameter() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; +[ApiController] +[Route] +public class PetController +{ + public IActionResult DeletePetByStatus([FromRoute] Status status, [FromRoute] Category category) => null; +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; +[ApiController] +[Route] +public class PetController +{ + [HttpDelete(""{status}/{category}"")] + public IActionResult DeletePetByStatus([FromRoute] Status status, [FromRoute] Category category) => null; +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task CodeFixes_WhenAttributeCannotBeInferred() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; +[ApiController] +[Route] +public class PetController +{ + public IActionResult ModifyPet() => null; +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; +[ApiController] +[Route] +public class PetController +{ + [HttpPut] + public IActionResult ModifyPet() => null; +}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + // There isn't a good way to test all fixes simultaneously. We'll pick the last one to verify when we + // expect to have 4 fixes. + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics, codeFixIndex: 3); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApiActionsShouldUseActionResultOfTFacts.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApiActionsShouldUseActionResultOfTFacts.cs new file mode 100644 index 0000000000..4eafbc8711 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/ApiActionsShouldUseActionResultOfTFacts.cs @@ -0,0 +1,257 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class ApiActionsShouldUseActionResultOfTFacts : AnalyzerTestBase + { + protected override DiagnosticAnalyzer DiagnosticAnalyzer { get; } + = new ApiActionsShouldUseActionResultOfTAnalyzer(); + + protected override CodeFixProvider CodeFixProvider { get; } + = new ApiActionsShouldUseActionResultOfTCodeFixProvider(); + + [Fact] + public async Task NoDiagnosticsAreReturned_FoEmptyScenarios() + { + // Arrange + var test = @""; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenTypeIsNotApiController() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +public class HomeController: ControllerBase +{ + public IActionResult Index() => View(); +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_ForNonActions() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController: ControllerBaseBase +{ + private int GetPetIdPrivate() => 0; + protected int GetPetIdProtected() => 0; + public static IActionResult FindPetByStatus(int status) => null; + [NonAction] + public object Reset(int state) => null; +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenActionAreExpressionBodiedMembers() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +[ApiController] +public class PetController: ControllerBase +{ + public IActionResult GetPetId() => ModelState.IsValid ? OK(new object()) : BadResult(); +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Theory] + [InlineData("Pet")] + [InlineData("List")] + [InlineData("System.Threading.Task")] + public async Task NoDiagnosticsAreReturned_WhenTypeReturnsNonObjectResult(string returnType) + { + // Arrange + var test = +$@" +using Microsoft.AspNetCore.Mvc; + +public class Pet {{ }} + +[ApiController] +public class PetController: ControllerBase +{{ + public {returnType} GetPetId() => null; +}}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_WhenTypeReturnsActionResultOfT() + { + // Arrange + var test = +@" +using Microsoft.AspNetCore.Mvc; + +public class Pet { } + +[ApiController] +public class PetController: ControllerBase +{ + public ActionResult GetPetId() => null; +}"; + var project = CreateProject(test); + + // Act + var result = await GetDiagnosticAsync(project); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task DiagnosticsAreReturned_WhenActionsReturnIActionResult() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC1002", + Message = "Actions on types annotated with ApiControllerAttribute should return ActionResult.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 9, 12) } + }; + var test = +@" +using Microsoft.AspNetCore.Mvc; + +public class Pet {} + +[ApiController] +public class PetController: ControllerBase +{ + public IActionResult GetPet() + { + return Ok(new Pet()); + } +}"; + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; + +public class Pet {} + +[ApiController] +public class PetController: ControllerBase +{ + public ActionResult GetPet() + { + return Ok(new Pet()); + } +}"; + var project = CreateProject(test); + + // Act + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task DiagnosticsAreReturned_WhenActionReturnsAsyncIActionResult() + { + // Arrange + var expectedDiagnostic = new DiagnosticResult + { + Id = "MVC1002", + Message = "Actions on types annotated with ApiControllerAttribute should return ActionResult.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test.cs", 8, 18) } + }; + + var test = +@" +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +[ApiController] +public class PetController: ControllerBase +{ + public async Task GetPet() + { + await Task.Delay(0); + return Ok(new Pet()); + } +} +public class Pet {}"; + + var expectedFix = +@" +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +[ApiController] +public class PetController: ControllerBase +{ + public async Task> GetPet() + { + await Task.Delay(0); + return Ok(new Pet()); + } +} +public class Pet {}"; + var project = CreateProject(test); + + // Act & Assert + var actualDiagnostics = await GetDiagnosticAsync(project); + Assert.DiagnosticsEqual(new[] { expectedDiagnostic }, actualDiagnostics); + + var actualFix = await ApplyCodeFixAsync(project, actualDiagnostics); + Assert.Equal(expectedFix, actualFix, ignoreLineEndingDifferences: true); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/AnalyzerTestBase.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/AnalyzerTestBase.cs new file mode 100644 index 0000000000..7a19460d4e --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/AnalyzerTestBase.cs @@ -0,0 +1,132 @@ +// 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.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; +using Microsoft.Extensions.DependencyModel; + +namespace Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure +{ + public abstract class AnalyzerTestBase : IDisposable + { + private static readonly object WorkspaceLock = new object(); + + public Workspace Workspace { get; private set; } + + protected abstract DiagnosticAnalyzer DiagnosticAnalyzer { get; } + + protected virtual CodeFixProvider CodeFixProvider { get; } + + protected Project CreateProject(string source) + { + var projectId = ProjectId.CreateNewId(debugName: "TestProject"); + var newFileName = "Test.cs"; + var documentId = DocumentId.CreateNewId(projectId, debugName: newFileName); + var metadataReferences = DependencyContext.Load(GetType().Assembly) + .CompileLibraries + .SelectMany(c => c.ResolveReferencePaths()) + .Select(path => MetadataReference.CreateFromFile(path)) + .Cast() + .ToList(); + + lock (WorkspaceLock) + { + if (Workspace == null) + { + Workspace = new AdhocWorkspace(); + } + } + + var solution = Workspace + .CurrentSolution + .AddProject(projectId, "TestProject", "TestProject", LanguageNames.CSharp) + .AddMetadataReferences(projectId, metadataReferences) + .AddDocument(documentId, newFileName, SourceText.From(source)); + + return solution.GetProject(projectId); + } + + protected async Task GetDiagnosticAsync(Project project) + { + var compilation = await project.GetCompilationAsync(); + var compilationWithAnalyzers = compilation.WithAnalyzers(ImmutableArray.Create(DiagnosticAnalyzer)); + var diagnostics = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + return diagnostics.OrderBy(d => d.Location.SourceSpan.Start).ToArray(); + } + + protected Task ApplyCodeFixAsync( + Project project, + Diagnostic[] analyzerDiagnostic, + int codeFixIndex = 0) + { + var diagnostic = analyzerDiagnostic.Single(); + return ApplyCodeFixAsync(project, diagnostic, codeFixIndex); + } + + protected async Task ApplyCodeFixAsync( + Project project, + Diagnostic analyzerDiagnostic, + int codeFixIndex = 0) + { + if (CodeFixProvider == null) + { + throw new InvalidOperationException($"{nameof(CodeFixProvider)} has not been assigned."); + } + + var document = project.Documents.Single(); + var actions = new List(); + var context = new CodeFixContext(document, analyzerDiagnostic, (a, d) => actions.Add(a), CancellationToken.None); + await CodeFixProvider.RegisterCodeFixesAsync(context); + + if (actions.Count == 0) + { + throw new InvalidOperationException("CodeFix produced no actions to apply."); + } + + var updatedSolution = await ApplyFixAsync(actions[codeFixIndex]); + // Todo: figure out why this doesn't work. + // var updatedProject = updatedSolution.GetProject(project.Id); + // await EnsureCompilable(updatedProject); + + var updatedDocument = updatedSolution.GetDocument(document.Id); + var sourceText = await updatedDocument.GetTextAsync(); + return sourceText.ToString(); + } + + private static async Task EnsureCompilable(Project project) + { + var compilation = await project + .WithCompilationOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) + .GetCompilationAsync(); + var diagnostics = compilation.GetDiagnostics(); + if (diagnostics.Length != 0) + { + var message = string.Join( + Environment.NewLine, + diagnostics.Select(d => CSharpDiagnosticFormatter.Instance.Format(d))); + throw new InvalidOperationException($"Compilation failed:{Environment.NewLine}{message}"); + } + } + + private static async Task ApplyFixAsync(CodeAction codeAction) + { + var operations = await codeAction.GetOperationsAsync(CancellationToken.None); + return Assert.Single(operations.OfType()).ChangedSolution; + } + + public void Dispose() + { + Workspace?.Dispose(); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/Assert.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/Assert.cs new file mode 100644 index 0000000000..06461614bf --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/Assert.cs @@ -0,0 +1,168 @@ +// 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.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Simplification; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + internal class Assert : Xunit.Assert + { + public static void DiagnosticsEqual(IEnumerable expected, IEnumerable actual) + { + var expectedCount = expected.Count(); + var actualCount = actual.Count(); + + if (expectedCount != actualCount) + { + throw new DiagnosticsAssertException( + expected, + actual, + $"Mismatch between number of diagnostics returned, expected \"{expectedCount}\" actual \"{actualCount}."); + } + + foreach (var (expectedItem, actualItem) in expected.Zip(actual, (a, b) => (a, b))) + { + if (expectedItem.Line == -1 && expectedItem.Column == -1) + { + if (actualItem.Location != Location.None) + { + throw new DiagnosticAssertException( + expectedItem, + actualItem, + $"Expected: A project diagnostic with no location. Actual {actualItem.Location}."); + } + } + else + { + VerifyDiagnosticLocation(expectedItem, actualItem); + } + + if (actualItem.Id != expectedItem.Id) + { + throw new DiagnosticAssertException( + expectedItem, + actualItem, + $"Expected: Expected id: {expectedItem.Id}. Actual id: {actualItem.Id}."); + } + + if (actualItem.Severity != expectedItem.Severity) + { + throw new DiagnosticAssertException( + expectedItem, + actualItem, + $"Expected: Expected severity: {expectedItem.Severity}. Actual severity: {actualItem.Severity}."); + } + + if (actualItem.GetMessage() != expectedItem.Message) + { + throw new DiagnosticAssertException( + expectedItem, + actualItem, + $"Expected: Expected message: {expectedItem.Message}. Actual message: {actualItem.GetMessage()}."); + } + } + } + + private static void VerifyDiagnosticLocation(DiagnosticResult expected, Diagnostic actual) + { + var actualSpan = actual.Location.GetLineSpan(); + var actualLinePosition = actualSpan.StartLinePosition; + + // Only check line position if there is an actual line in the real diagnostic + if (actualLinePosition.Line > 0) + { + if (actualLinePosition.Line + 1 != expected.Line) + { + throw new DiagnosticAssertException( + expected, + actual, + $"Expected diagnostic to be on line \"{expected.Line}\" was actually on line \"{actualLinePosition.Line + 1}\""); + } + } + + // Only check column position if there is an actual column position in the real diagnostic + if (actualLinePosition.Character > 0) + { + if (actualLinePosition.Character + 1 != expected.Column) + { + throw new DiagnosticAssertException( + expected, + actual, + $"Expected diagnostic to start at column \"{expected.Column}\" was actually on line \"{actualLinePosition.Character + 1}\""); + } + } + } + + private static string FormatDiagnostics(IEnumerable diagnostics) + { + return string.Join(Environment.NewLine, diagnostics.Select(FormatDiagnostic)); + } + + private static string FormatDiagnostic(Diagnostic diagnostic) + { + var builder = new StringBuilder(); + builder.AppendLine(diagnostic.ToString()); + + var location = diagnostic.Location; + if (location == Location.None) + { + builder.Append($"Location unknown: ({diagnostic.Id})"); + } + else + { + True(location.IsInSource, + $"Test base does not currently handle diagnostics in metadata locations. Diagnostic in metadata: {diagnostic}"); + + var linePosition = location.GetLineSpan().StartLinePosition; + builder.Append($"({(linePosition.Line + 1)}, {(linePosition.Character + 1)}, {diagnostic.Id})"); + } + + return builder.ToString(); + } + + private static async Task GetStringFromDocumentAsync(Document document) + { + var simplifiedDoc = await Simplifier.ReduceAsync(document, Simplifier.Annotation); + var root = await simplifiedDoc.GetSyntaxRootAsync(); + root = Formatter.Format(root, Formatter.Annotation, simplifiedDoc.Project.Solution.Workspace); + return root.GetText().ToString(); + } + + private class DiagnosticsAssertException : Xunit.Sdk.EqualException + { + public DiagnosticsAssertException( + IEnumerable expected, + IEnumerable actual, + string message) + : base(expected, actual) + { + Message = message + Environment.NewLine + FormatDiagnostics(actual); + } + + public override string Message { get; } + } + + private class DiagnosticAssertException : Xunit.Sdk.EqualException + { + public DiagnosticAssertException( + DiagnosticResult expected, + Diagnostic actual, + string message) + : base(expected, actual) + { + Message = message + Environment.NewLine + FormatDiagnostic(actual); + } + + public override string Message { get; } + } + + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/DiagnosticResult.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/DiagnosticResult.cs new file mode 100644 index 0000000000..4a99be21fb --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Infrastructure/DiagnosticResult.cs @@ -0,0 +1,70 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure +{ + /// + /// Location where the diagnostic appears, as determined by path, line number, and column number. + /// + public struct DiagnosticResultLocation + { + public DiagnosticResultLocation(string path, int line, int column) + { + if (line < -1) + { + throw new ArgumentOutOfRangeException(nameof(line), "line must be >= -1"); + } + + if (column < -1) + { + throw new ArgumentOutOfRangeException(nameof(column), "column must be >= -1"); + } + + Path = path; + Line = line; + Column = column; + } + + public string Path { get; } + public int Line { get; } + public int Column { get; } + } + + /// + /// Struct that stores information about a Diagnostic appearing in a source + /// + public struct DiagnosticResult + { + private DiagnosticResultLocation[] _locations; + + public DiagnosticResultLocation[] Locations + { + get + { + if (_locations == null) + { + _locations = new DiagnosticResultLocation[] { }; + } + + return _locations; + } + + set => _locations = value; + } + + public DiagnosticSeverity Severity { get; set; } + + public string Id { get; set; } + + public string Message { get; set; } + + public string Path => Locations.Length > 0 ? Locations[0].Path : ""; + + public int Line => Locations.Length > 0 ? Locations[0].Line : -1; + + public int Column => Locations.Length > 0 ? Locations[0].Column : -1; + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Microsoft.AspNetCore.Mvc.Analyzers.Test.csproj b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Microsoft.AspNetCore.Mvc.Analyzers.Test.csproj new file mode 100644 index 0000000000..a7d41638d4 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/Microsoft.AspNetCore.Mvc.Analyzers.Test.csproj @@ -0,0 +1,19 @@ + + + + $(StandardTestTfms) + true + + + + + + + + + + + + + + diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/xunit.runner.json b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/xunit.runner.json new file mode 100644 index 0000000000..1c72a421ad --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/xunit.runner.json @@ -0,0 +1,3 @@ +{ + "shadowCopy": false +} diff --git a/version.props b/version.props index 5c4a7c32d1..efdfa4da19 100644 --- a/version.props +++ b/version.props @@ -2,9 +2,15 @@ 2.1.0 preview1 + + 0.1.0 + alpha1 + $(VersionPrefix) $(VersionPrefix)-$(VersionSuffix)-final t000 + $(VersionSuffix)-$(BuildNumber) + $(ExperimentalVersionSuffix)-$(BuildNumber)