diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SpecifyStringComparison.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SpecifyStringComparison.cs index d2d5e381de..11fdc9fda9 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SpecifyStringComparison.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SpecifyStringComparison.cs @@ -49,9 +49,6 @@ public sealed class SpecifyStringComparisonAnalyzer : DiagnosticAnalyzer isPortedFxCopRule: false, isDataflowRule: false); - private static readonly ImmutableArray s_LambdaOrLocalFunctionKinds = - ImmutableArray.Create(OperationKind.AnonymousFunction, OperationKind.LocalFunction); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule_CA1307, Rule_CA1310); public override void Initialize(AnalysisContext analysisContext) @@ -86,17 +83,11 @@ public override void Initialize(AnalysisContext analysisContext) return; } - if (linqExpressionType != null) + // Check if we are in a Expression> context, in which case it is possible + // that the underlying call doesn't have the comparison option so we want to bail-out. + if (invocationExpression.IsWithinExpressionTree(linqExpressionType)) { - var enclosingLambdaOrLocalFunc = invocationExpression.GetAncestor(s_LambdaOrLocalFunctionKinds); - - // Check if we are in a Expression> context, in which case it is possible - // that the underlying call doesn't have the comparison option so we want to bail-out. - if (enclosingLambdaOrLocalFunc?.Parent?.Type?.OriginalDefinition is { } lambdaType - && linqExpressionType.Equals(lambdaType)) - { - return; - } + return; } // Report correctness issue CA1310 for known string comparison methods that default to culture specific string comparison: diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLength.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLength.cs index ecd98005d6..8fc36b7f0f 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLength.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLength.cs @@ -46,26 +46,43 @@ public sealed override void Initialize(AnalysisContext context) context.EnableConcurrentExecution(); context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - context.RegisterOperationAction( - operationAnalysisContext => AnalyzeInvocationExpression((IInvocationOperation)operationAnalysisContext.Operation, operationAnalysisContext.ReportDiagnostic), - OperationKind.Invocation); - - context.RegisterOperationAction( - operationAnalysisContext => AnalyzeBinaryExpression((IBinaryOperation)operationAnalysisContext.Operation, operationAnalysisContext.ReportDiagnostic), - OperationKind.BinaryOperator); + context.RegisterCompilationStartAction(context => + { + var linqExpressionType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemLinqExpressionsExpression1); + + context.RegisterOperationAction( + operationAnalysisContext => AnalyzeInvocationExpression( + (IInvocationOperation)operationAnalysisContext.Operation, linqExpressionType, + operationAnalysisContext.ReportDiagnostic), + OperationKind.Invocation); + + context.RegisterOperationAction( + operationAnalysisContext => AnalyzeBinaryExpression( + (IBinaryOperation)operationAnalysisContext.Operation, linqExpressionType, + operationAnalysisContext.ReportDiagnostic), + OperationKind.BinaryOperator); + }); } /// /// Check to see if we have an invocation to string.Equals that has an empty string as an argument. /// - private static void AnalyzeInvocationExpression(IInvocationOperation invocationOperation, Action reportDiagnostic) + private static void AnalyzeInvocationExpression(IInvocationOperation invocationOperation, + INamedTypeSymbol? linqExpressionTreeType, Action reportDiagnostic) { if (!invocationOperation.Arguments.IsEmpty) { IMethodSymbol methodSymbol = invocationOperation.TargetMethod; - if (methodSymbol != null && - IsStringEqualsMethod(methodSymbol) && - HasAnEmptyStringArgument(invocationOperation)) + if (methodSymbol == null + || !IsStringEqualsMethod(methodSymbol) + || !HasAnEmptyStringArgument(invocationOperation)) + { + return; + } + + // Check if we are in a Expression> context, in which case it is possible + // that the underlying call doesn't have the helper so we want to bail-out. + if (!invocationOperation.IsWithinExpressionTree(linqExpressionTreeType)) { reportDiagnostic(invocationOperation.Syntax.CreateDiagnostic(s_rule)); } @@ -76,7 +93,8 @@ private static void AnalyzeInvocationExpression(IInvocationOperation invocationO /// Check to see if we have a equals or not equals expression where an empty string is being /// compared. /// - private static void AnalyzeBinaryExpression(IBinaryOperation binaryOperation, Action reportDiagnostic) + private static void AnalyzeBinaryExpression(IBinaryOperation binaryOperation, + INamedTypeSymbol? linqExpressionTreeType, Action reportDiagnostic) { if (binaryOperation.OperatorKind is not BinaryOperatorKind.Equals and not BinaryOperatorKind.NotEquals) @@ -90,7 +108,15 @@ private static void AnalyzeBinaryExpression(IBinaryOperation binaryOperation, Ac return; } - if (IsEmptyString(binaryOperation.LeftOperand) || IsEmptyString(binaryOperation.RightOperand)) + if (!IsEmptyString(binaryOperation.LeftOperand) + && !IsEmptyString(binaryOperation.RightOperand)) + { + return; + } + + // Check if we are in a Expression> context, in which case it is possible + // that the underlying call doesn't have the helper so we want to bail-out. + if (!binaryOperation.IsWithinExpressionTree(linqExpressionTreeType)) { reportDiagnostic(binaryOperation.Syntax.CreateDiagnostic(s_rule)); } diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLengthTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLengthTests.cs index ac97817ea9..87e8f588d0 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLengthTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLengthTests.cs @@ -2,6 +2,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; +using Test.Utilities; using Xunit; using VerifyCS = Test.Utilities.CSharpCodeFixVerifier< Microsoft.NetCore.Analyzers.Runtime.TestForEmptyStringsUsingStringLengthAnalyzer, @@ -108,5 +109,24 @@ void Method() } #endregion + + [Fact, WorkItem(1508, "https://github.com/dotnet/roslyn-analyzers/issues/1508")] + public async Task CA1820_ExpressionTree_NoDiagnostic() + { + await VerifyCS.VerifyAnalyzerAsync(@" +using System.Linq; + +class C +{ + void M(IQueryable strings) + { + var q1 = from s in strings + where s == """" + select s; + + var q2 = strings.Where(s => s.Equals("""")); + } +}"); + } } } diff --git a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs index a257a4f466..dc1a7d90c9 100644 --- a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs @@ -508,12 +508,18 @@ void ProcessLocalOrParameter(ISymbol symbol) private static readonly ImmutableArray s_LambdaAndLocalFunctionKinds = ImmutableArray.Create(OperationKind.AnonymousFunction, OperationKind.LocalFunction); + public static bool IsWithinLambdaOrLocalFunction(this IOperation operation, [NotNullWhen(true)] out IOperation? containingLambdaOrLocalFunctionOperation) { containingLambdaOrLocalFunctionOperation = operation.GetAncestor(s_LambdaAndLocalFunctionKinds); return containingLambdaOrLocalFunctionOperation != null; } + public static bool IsWithinExpressionTree(this IOperation operation, [NotNullWhen(true)] INamedTypeSymbol? linqExpressionTreeType) + => linqExpressionTreeType != null + && operation.GetAncestor(s_LambdaAndLocalFunctionKinds)?.Parent?.Type?.OriginalDefinition is { } lambdaType + && linqExpressionTreeType.Equals(lambdaType); + public static ITypeSymbol? GetPatternType(this IPatternOperation pattern) { return pattern switch