Skip to content

Commit 7bd2c98

Browse files
mavasanimavasani
authored
Merge pull request #4621 from Evangelink/CA1820-expression-tree
Update CA1820 to not report on expression trees
2 parents c49d111 + 4842463 commit 7bd2c98

File tree

4 files changed

+69
-26
lines changed

4 files changed

+69
-26
lines changed

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SpecifyStringComparison.cs

+4-13
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ public sealed class SpecifyStringComparisonAnalyzer : DiagnosticAnalyzer
4949
isPortedFxCopRule: false,
5050
isDataflowRule: false);
5151

52-
private static readonly ImmutableArray<OperationKind> s_LambdaOrLocalFunctionKinds =
53-
ImmutableArray.Create(OperationKind.AnonymousFunction, OperationKind.LocalFunction);
54-
5552
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule_CA1307, Rule_CA1310);
5653

5754
public override void Initialize(AnalysisContext analysisContext)
@@ -86,17 +83,11 @@ public override void Initialize(AnalysisContext analysisContext)
8683
return;
8784
}
8885

89-
if (linqExpressionType != null)
86+
// Check if we are in a Expression<Func<T...>> context, in which case it is possible
87+
// that the underlying call doesn't have the comparison option so we want to bail-out.
88+
if (invocationExpression.IsWithinExpressionTree(linqExpressionType))
9089
{
91-
var enclosingLambdaOrLocalFunc = invocationExpression.GetAncestor(s_LambdaOrLocalFunctionKinds);
92-
93-
// Check if we are in a Expression<Func<T...>> context, in which case it is possible
94-
// that the underlying call doesn't have the comparison option so we want to bail-out.
95-
if (enclosingLambdaOrLocalFunc?.Parent?.Type?.OriginalDefinition is { } lambdaType
96-
&& linqExpressionType.Equals(lambdaType))
97-
{
98-
return;
99-
}
90+
return;
10091
}
10192

10293
// Report correctness issue CA1310 for known string comparison methods that default to culture specific string comparison:

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLength.cs

+39-13
Original file line numberDiff line numberDiff line change
@@ -46,26 +46,43 @@ public sealed override void Initialize(AnalysisContext context)
4646
context.EnableConcurrentExecution();
4747
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
4848

49-
context.RegisterOperationAction(
50-
operationAnalysisContext => AnalyzeInvocationExpression((IInvocationOperation)operationAnalysisContext.Operation, operationAnalysisContext.ReportDiagnostic),
51-
OperationKind.Invocation);
52-
53-
context.RegisterOperationAction(
54-
operationAnalysisContext => AnalyzeBinaryExpression((IBinaryOperation)operationAnalysisContext.Operation, operationAnalysisContext.ReportDiagnostic),
55-
OperationKind.BinaryOperator);
49+
context.RegisterCompilationStartAction(context =>
50+
{
51+
var linqExpressionType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemLinqExpressionsExpression1);
52+
53+
context.RegisterOperationAction(
54+
operationAnalysisContext => AnalyzeInvocationExpression(
55+
(IInvocationOperation)operationAnalysisContext.Operation, linqExpressionType,
56+
operationAnalysisContext.ReportDiagnostic),
57+
OperationKind.Invocation);
58+
59+
context.RegisterOperationAction(
60+
operationAnalysisContext => AnalyzeBinaryExpression(
61+
(IBinaryOperation)operationAnalysisContext.Operation, linqExpressionType,
62+
operationAnalysisContext.ReportDiagnostic),
63+
OperationKind.BinaryOperator);
64+
});
5665
}
5766

5867
/// <summary>
5968
/// Check to see if we have an invocation to string.Equals that has an empty string as an argument.
6069
/// </summary>
61-
private static void AnalyzeInvocationExpression(IInvocationOperation invocationOperation, Action<Diagnostic> reportDiagnostic)
70+
private static void AnalyzeInvocationExpression(IInvocationOperation invocationOperation,
71+
INamedTypeSymbol? linqExpressionTreeType, Action<Diagnostic> reportDiagnostic)
6272
{
6373
if (!invocationOperation.Arguments.IsEmpty)
6474
{
6575
IMethodSymbol methodSymbol = invocationOperation.TargetMethod;
66-
if (methodSymbol != null &&
67-
IsStringEqualsMethod(methodSymbol) &&
68-
HasAnEmptyStringArgument(invocationOperation))
76+
if (methodSymbol == null
77+
|| !IsStringEqualsMethod(methodSymbol)
78+
|| !HasAnEmptyStringArgument(invocationOperation))
79+
{
80+
return;
81+
}
82+
83+
// Check if we are in a Expression<Func<T...>> context, in which case it is possible
84+
// that the underlying call doesn't have the helper so we want to bail-out.
85+
if (!invocationOperation.IsWithinExpressionTree(linqExpressionTreeType))
6986
{
7087
reportDiagnostic(invocationOperation.Syntax.CreateDiagnostic(s_rule));
7188
}
@@ -76,7 +93,8 @@ private static void AnalyzeInvocationExpression(IInvocationOperation invocationO
7693
/// Check to see if we have a equals or not equals expression where an empty string is being
7794
/// compared.
7895
/// </summary>
79-
private static void AnalyzeBinaryExpression(IBinaryOperation binaryOperation, Action<Diagnostic> reportDiagnostic)
96+
private static void AnalyzeBinaryExpression(IBinaryOperation binaryOperation,
97+
INamedTypeSymbol? linqExpressionTreeType, Action<Diagnostic> reportDiagnostic)
8098
{
8199
if (binaryOperation.OperatorKind is not BinaryOperatorKind.Equals and
82100
not BinaryOperatorKind.NotEquals)
@@ -90,7 +108,15 @@ private static void AnalyzeBinaryExpression(IBinaryOperation binaryOperation, Ac
90108
return;
91109
}
92110

93-
if (IsEmptyString(binaryOperation.LeftOperand) || IsEmptyString(binaryOperation.RightOperand))
111+
if (!IsEmptyString(binaryOperation.LeftOperand)
112+
&& !IsEmptyString(binaryOperation.RightOperand))
113+
{
114+
return;
115+
}
116+
117+
// Check if we are in a Expression<Func<T...>> context, in which case it is possible
118+
// that the underlying call doesn't have the helper so we want to bail-out.
119+
if (!binaryOperation.IsWithinExpressionTree(linqExpressionTreeType))
94120
{
95121
reportDiagnostic(binaryOperation.Syntax.CreateDiagnostic(s_rule));
96122
}

‎src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLengthTests.cs

+20
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
using System.Threading.Tasks;
44
using Microsoft.CodeAnalysis.Testing;
5+
using Test.Utilities;
56
using Xunit;
67
using VerifyCS = Test.Utilities.CSharpCodeFixVerifier<
78
Microsoft.NetCore.Analyzers.Runtime.TestForEmptyStringsUsingStringLengthAnalyzer,
@@ -108,5 +109,24 @@ void Method()
108109
}
109110

110111
#endregion
112+
113+
[Fact, WorkItem(1508, "https://github.com/dotnet/roslyn-analyzers/issues/1508")]
114+
public async Task CA1820_ExpressionTree_NoDiagnostic()
115+
{
116+
await VerifyCS.VerifyAnalyzerAsync(@"
117+
using System.Linq;
118+
119+
class C
120+
{
121+
void M(IQueryable<string> strings)
122+
{
123+
var q1 = from s in strings
124+
where s == """"
125+
select s;
126+
127+
var q2 = strings.Where(s => s.Equals(""""));
128+
}
129+
}");
130+
}
111131
}
112132
}

‎src/Utilities/Compiler/Extensions/IOperationExtensions.cs

+6
Original file line numberDiff line numberDiff line change
@@ -508,12 +508,18 @@ void ProcessLocalOrParameter(ISymbol symbol)
508508

509509
private static readonly ImmutableArray<OperationKind> s_LambdaAndLocalFunctionKinds =
510510
ImmutableArray.Create(OperationKind.AnonymousFunction, OperationKind.LocalFunction);
511+
511512
public static bool IsWithinLambdaOrLocalFunction(this IOperation operation, [NotNullWhen(true)] out IOperation? containingLambdaOrLocalFunctionOperation)
512513
{
513514
containingLambdaOrLocalFunctionOperation = operation.GetAncestor(s_LambdaAndLocalFunctionKinds);
514515
return containingLambdaOrLocalFunctionOperation != null;
515516
}
516517

518+
public static bool IsWithinExpressionTree(this IOperation operation, [NotNullWhen(true)] INamedTypeSymbol? linqExpressionTreeType)
519+
=> linqExpressionTreeType != null
520+
&& operation.GetAncestor(s_LambdaAndLocalFunctionKinds)?.Parent?.Type?.OriginalDefinition is { } lambdaType
521+
&& linqExpressionTreeType.Equals(lambdaType);
522+
517523
public static ITypeSymbol? GetPatternType(this IPatternOperation pattern)
518524
{
519525
return pattern switch

0 commit comments

Comments
 (0)