From 53b5952908107a5339504e056a49daed8c02fb28 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 16 Sep 2022 19:47:15 -0700 Subject: [PATCH 1/3] Fix DetectMisplacedLambdaAttribute analyzer and tests --- .../DetectMisplacedLambdaAttribute.cs | 52 ++++++++++++++----- ...arpWebApplicationBuilderCodeFixVerifier.cs | 1 + 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DetectMisplacedLambdaAttribute.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DetectMisplacedLambdaAttribute.cs index 126f0a9c25d9..79fa7dd994c3 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DetectMisplacedLambdaAttribute.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DetectMisplacedLambdaAttribute.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -22,23 +23,17 @@ private static void DetectMisplacedLambdaAttribute( // Hello(); // return "foo"; // } - IMethodSymbol? methodSymbol = null; - // () => Hello() has a single child which is a BlockOperation so we check to see if - // expression associated with that operation is an invocation expression - if (lambda.ChildOperations.FirstOrDefault().Syntax is InvocationExpressionSyntax innerInvocation) + // All lambdas have a single child which is a BlockOperation. We search ChildOperations for + // the invocation expression. + if (lambda.ChildOperations.Count != 1 || lambda.ChildOperations.FirstOrDefault() is not IBlockOperation blockOperation) { - methodSymbol = lambda.Symbol; - } - - if (lambda.ChildOperations.FirstOrDefault().ChildOperations.FirstOrDefault() is IReturnOperation returnOperation - && returnOperation.ReturnedValue is IInvocationOperation returnedInvocation) - { - methodSymbol = returnedInvocation.TargetMethod; + Debug.Fail("Expected a single top-level BlockOperation for all lambdas."); + return; } // If no method definition was found for the lambda, then abort. - if (methodSymbol is null) + if (GetReturnedInvocation(blockOperation) is not IMethodSymbol methodSymbol) { return; } @@ -73,5 +68,38 @@ static bool IsInValidNamespace(INamespaceSymbol? @namespace) return false; } + + static IMethodSymbol? GetReturnedInvocation(IBlockOperation blockOperation) + { + foreach (var op in blockOperation.ChildOperations.Reverse()) + { + if (op is IReturnOperation returnStatement) + { + if (returnStatement.ReturnedValue is IInvocationOperation invocationReturn) + { + return invocationReturn.TargetMethod; + } + + // Sometimes expression backed lambdas include an IReturnOperation with a null ReturnedValue + // right after the IExpressionStatementOperation whose Operation is the real ReturnedValue, + // so we keep looking backwards rather than returning null immediately. + } + else if (op is IExpressionStatementOperation expression) + { + if (expression.Operation is IInvocationOperation invocationExpression) + { + return invocationExpression.TargetMethod; + } + + return null; + } + else + { + return null; + } + } + + return null; + } } } diff --git a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpWebApplicationBuilderCodeFixVerifier.cs b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpWebApplicationBuilderCodeFixVerifier.cs index aa8841c83be5..8073893db702 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpWebApplicationBuilderCodeFixVerifier.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpWebApplicationBuilderCodeFixVerifier.cs @@ -68,6 +68,7 @@ public WebApplicationBuilderAnalyzerTest() TrimAssemblyExtension(typeof(Microsoft.Extensions.Hosting.HostingHostBuilderExtensions).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.ConfigureHostBuilder).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.ConfigureWebHostBuilder).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.EndpointRoutingApplicationBuilderExtensions).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Hosting.HostingAbstractionsWebHostBuilderExtensions).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.Extensions.Logging.ILoggingBuilder).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.Extensions.Logging.ConsoleLoggerExtensions).Assembly.Location), From ccfd9abb78ea2e1f3ceb651fb23310e5ef0e4166 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 19 Sep 2022 09:49:51 -0700 Subject: [PATCH 2/3] Add AspNetCoreAnalyzers to Build.props --- eng/Build.props | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/Build.props b/eng/Build.props index 6599b9d6f23f..d0dff953ae75 100644 --- a/eng/Build.props +++ b/eng/Build.props @@ -160,6 +160,7 @@