Skip to content

Commit

Permalink
Fix S6605 FP: Should not fire in expressions (#7671)
Browse files Browse the repository at this point in the history
  • Loading branch information
martin-strecker-sonarsource committed Jul 26, 2023
1 parent 32f382d commit 967a5d9
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,7 @@ node switch

public override bool IsMemberAccessOnKnownType(SyntaxNode memberAccess, string name, KnownType knownType, SemanticModel semanticModel) =>
Cast<MemberAccessExpressionSyntax>(memberAccess).IsMemberAccessOnKnownType(name, knownType, semanticModel);

public override bool IsInExpressionTree(SemanticModel model, SyntaxNode node) =>
node.IsInExpressionTree(model);
}
1 change: 1 addition & 0 deletions analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public abstract class SyntaxFacade<TSyntaxKind>
public abstract bool TryGetOperands(SyntaxNode invocation, out SyntaxNode left, out SyntaxNode right);
public abstract bool HasExactlyNArguments(SyntaxNode invocation, int count);
public abstract bool IsMemberAccessOnKnownType(SyntaxNode memberAccess, string name, KnownType knownType, SemanticModel semanticModel);
public abstract bool IsInExpressionTree(SemanticModel model, SyntaxNode node);

protected static T Cast<T>(SyntaxNode node) where T : SyntaxNode =>
node as T ?? throw new InvalidCastException($"A {node.GetType().Name} node can not be cast to a {typeof(T).Name} node.");
Expand Down
21 changes: 1 addition & 20 deletions analyzers/src/SonarAnalyzer.Common/Rules/InsteadOfAnyBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected InsteadOfAnyBase()
&& Language.Syntax.TryGetOperands(invocation, out var left, out var right)
&& IsCorrectCall(right, c.SemanticModel)
&& c.SemanticModel.GetTypeInfo(left).Type is { } type
&& !IsUsedByEntityFramework(invocation, c.SemanticModel))
&& !Language.Syntax.IsInExpressionTree(c.SemanticModel, invocation))
{
if (ExistsTypes.Any(x => type.DerivesFrom(x)))
{
Expand Down Expand Up @@ -126,23 +126,4 @@ protected bool HasValidInvocationOperands(TInvocationExpression invocation, stri

private void RaiseIssue(SonarSyntaxNodeReportingContext c, SyntaxNode invocation, DiagnosticDescriptor rule, string methodName) =>
c.ReportIssue(Diagnostic.Create(rule, Language.Syntax.NodeIdentifier(invocation)?.GetLocation(), methodName));

// See https://github.com/SonarSource/sonar-dotnet/issues/7286
private bool IsUsedByEntityFramework(SyntaxNode node, SemanticModel model)
{
do
{
node = node.Parent;

if (Language.Syntax.IsKind(node, Language.SyntaxKind.InvocationExpression)
&& Language.Syntax.TryGetOperands(node, out var left, out var _)
&& model.GetTypeInfo(left).Type.DerivesOrImplements(KnownType.System_Linq_IQueryable))
{
return true;
}
}
while (!Language.Syntax.IsAnyKind(node, ExitParentKinds));

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,7 @@ public override SyntaxNode RemoveConditionalAccess(SyntaxNode node)

public override bool IsMemberAccessOnKnownType(SyntaxNode memberAccess, string name, KnownType knownType, SemanticModel semanticModel) =>
Cast<MemberAccessExpressionSyntax>(memberAccess).IsMemberAccessOnKnownType(name, knownType, semanticModel);

public override bool IsInExpressionTree(SemanticModel model, SyntaxNode node) =>
node.IsInExpressionTree(model);
}
32 changes: 32 additions & 0 deletions analyzers/tests/SonarAnalyzer.UnitTest/TestCases/InsteadOfAny.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;

public class TestClass
{
Expand Down Expand Up @@ -262,3 +263,34 @@ public class ClassB

public bool Any(Func<int, bool> predicate) => false;
}

public class ExpressionTree
{
// https://github.com/SonarSource/sonar-dotnet/issues/7508
public void Repro_7508()
{
Expression<Func<List<int>, bool>> containsThree = list => list.Any(el => el == 3); // Compliant (IsInExpressionTree)
}

class Customer
{
public ICollection<Order> Orders { get; } // Orders is an IEnumerable<T> and not IQueryable<T>
}
class Order
{
public int Id { get; }
}

private void NestedQuery(IQueryable<Customer> customers) // typically a DbSet<Customer> https://learn.microsoft.com/en-us/dotnet/api/microsoft.entityframeworkcore.dbset-1
{
// Typical query in EF (https://learn.microsoft.com/en-us/ef/core/modeling/relationships/one-to-many)

// (order => order.Id > 0) is not an Expression<Func<..>> nor is Orders an IQueryable<T>
// But the surrounding "customers.Where" are and so we do not raise.
var qry1 = customers.Where(customer => customer.Orders.Any(order => order.Id > 0)); // Compliant. In expression tree context

var qry2 = from customer in customers
where customer.Orders.Any(order => order.Id > 0) // Compliant. In expression tree context
select customer;
}
}

0 comments on commit 967a5d9

Please sign in to comment.