Skip to content

Commit

Permalink
Fix S6605 and S6617 FP: Should not be applied to expressions used by …
Browse files Browse the repository at this point in the history
…EntityFramework #7286 (#7332)

Co-authored-by: Sebastien Marichal <[email protected]>
  • Loading branch information
1 parent 37ee147 commit d5416d1
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 16 deletions.
12 changes: 12 additions & 0 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/InsteadOfAny.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ public sealed class InsteadOfAny : InsteadOfAnyBase<SyntaxKind, InvocationExpres
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

protected override HashSet<SyntaxKind> ExitParentKinds { get; } = new()
{
SyntaxKind.MethodDeclaration,
SyntaxKind.ConstructorDeclaration,
SyntaxKind.DestructorDeclaration,
SyntaxKind.GetAccessorDeclaration,
SyntaxKind.SetAccessorDeclaration,
SyntaxKind.CompilationUnit,
SyntaxKindEx.LocalFunctionStatement,
SyntaxKindEx.InitAccessorDeclaration
};

protected override bool IsSimpleEqualityCheck(InvocationExpressionSyntax node, SemanticModel model) =>
GetArgumentExpression(node, 0) is SimpleLambdaExpressionSyntax lambda
&& lambda.Parameter.Identifier.ValueText is var lambdaVariableName
Expand Down
3 changes: 2 additions & 1 deletion analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public sealed partial class KnownType
public static readonly KnownType Microsoft_Azure_WebJobs_FunctionNameAttribute = new("Microsoft.Azure.WebJobs.FunctionNameAttribute");
public static readonly KnownType Microsoft_Data_Sqlite_SqliteCommand = new("Microsoft.Data.Sqlite.SqliteCommand");
public static readonly KnownType Microsoft_EntityFrameworkCore_DbContextOptionsBuilder = new("Microsoft.EntityFrameworkCore.DbContextOptionsBuilder");
public static readonly KnownType Microsoft_EntityFrameworkCore_DbSet_TEntity = new("Microsoft.EntityFrameworkCore.DbSet", "TEntity");
public static readonly KnownType Microsoft_EntityFrameworkCore_Migrations_Migration = new("Microsoft.EntityFrameworkCore.Migrations.Migration");
public static readonly KnownType Microsoft_EntityFrameworkCore_MySQLDbContextOptionsExtensions = new("Microsoft.EntityFrameworkCore.MySQLDbContextOptionsExtensions");
public static readonly KnownType Microsoft_EntityFrameworkCore_NpgsqlDbContextOptionsExtensions = new("Microsoft.EntityFrameworkCore.NpgsqlDbContextOptionsExtensions");
Expand Down Expand Up @@ -457,7 +458,7 @@ public sealed partial class KnownType
public static readonly KnownType System_StackOverflowException = new("System.StackOverflowException");
public static readonly KnownType System_STAThreadAttribute = new("System.STAThreadAttribute");
public static readonly KnownType System_String = new("System.String");
public static readonly KnownType System_String_Array = new("System.String") { IsArray = true};
public static readonly KnownType System_String_Array = new("System.String") { IsArray = true };
public static readonly KnownType System_StringComparison = new("System.StringComparison");
public static readonly KnownType System_SystemException = new("System.SystemException");
public static readonly KnownType System_Text_RegularExpressions_Regex = new("System.Text.RegularExpressions.Regex");
Expand Down
27 changes: 26 additions & 1 deletion analyzers/src/SonarAnalyzer.Common/Rules/InsteadOfAnyBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ public abstract class InsteadOfAnyBase<TSyntaxKind, TInvocationExpression> : Son
KnownType.System_Collections_Generic_HashSet_T,
KnownType.System_Collections_Generic_SortedSet_T);

protected static readonly ImmutableArray<KnownType> DbSetTypes = ImmutableArray.Create(
KnownType.System_Data_Entity_DbSet_TEntity,
KnownType.Microsoft_EntityFrameworkCore_DbSet_TEntity);

protected abstract ILanguageFacade<TSyntaxKind> Language { get; }
protected abstract HashSet<TSyntaxKind> ExitParentKinds { get; }

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(existsRule, containsRule);

Expand All @@ -62,7 +67,8 @@ protected override void Initialize(SonarAnalysisContext context) =>
&& Language.Syntax.HasExactlyNArguments(invocation, 1)
&& Language.Syntax.TryGetOperands(invocation, out var left, out var right)
&& IsCorrectCall(right, c.SemanticModel)
&& c.SemanticModel.GetTypeInfo(left).Type is { } type)
&& c.SemanticModel.GetTypeInfo(left).Type is { } type
&& !IsUsedByEntityFramework(invocation, c.SemanticModel))
{
if (ExistsTypes.Any(x => type.DerivesFrom(x)))
{
Expand Down Expand Up @@ -124,4 +130,23 @@ private static bool IsCorrectCall(SyntaxNode right, SemanticModel model) =>

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.DerivesFromAny(DbSetTypes))
{
return true;
}
}
while (!Language.Syntax.IsAnyKind(node, ExitParentKinds));

return false;
}
}
10 changes: 10 additions & 0 deletions analyzers/src/SonarAnalyzer.VisualBasic/Rules/InsteadOfAny.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ public sealed class InsteadOfAny : InsteadOfAnyBase<SyntaxKind, InvocationExpres
{
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;

protected override HashSet<SyntaxKind> ExitParentKinds { get; } = new()
{
SyntaxKind.SubStatement,
SyntaxKind.SubNewStatement,
SyntaxKind.FunctionStatement,
SyntaxKind.GetAccessorStatement,
SyntaxKind.SetAccessorStatement,
SyntaxKind.CompilationUnit,
};

protected override bool IsSimpleEqualityCheck(InvocationExpressionSyntax node, SemanticModel model) =>
GetArgumentExpression(node, 0) is SingleLineLambdaExpressionSyntax lambda
&& lambda.SubOrFunctionHeader.ParameterList.Parameters is { Count: 1 } parameters
Expand Down
36 changes: 29 additions & 7 deletions analyzers/tests/SonarAnalyzer.UnitTest/Rules/InsteadOfAnyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace SonarAnalyzer.UnitTest.Rules;
public class InsteadOfAnyTest
{
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.InsteadOfAny>();
private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.InsteadOfAny>();

[TestMethod]
public void InsteadOfAny_CS() =>
Expand All @@ -40,22 +41,43 @@ public void InsteadOfAny_TopLevelStatements() =>
.WithTopLevelStatements()
.AddReferences(MetadataReferenceFacade.SystemCollections)
.Verify();
#endif

[TestMethod]
public void InsteadOfAny_EntityFramework() =>
builderCS.AddPaths("InsteadOfAny.EntityFramework.cs")
builderCS.AddPaths("InsteadOfAny.EntityFramework.Core.cs")
.WithOptions(ParseOptionsHelper.FromCSharp8)
.AddReferences(GetReferencesEntityFrameworkNetCore("2.2.6").Concat(NuGetMetadataReference.SystemComponentModelTypeConverter()))
.AddReferences(GetReferencesEntityFrameworkNetCore())
.Verify();

internal static IEnumerable<MetadataReference> GetReferencesEntityFrameworkNetCore(string entityFrameworkVersion) =>
Enumerable.Empty<MetadataReference>()
.Concat(NuGetMetadataReference.MicrosoftEntityFrameworkCore(entityFrameworkVersion))
.Concat(NuGetMetadataReference.MicrosoftEntityFrameworkCoreRelational(entityFrameworkVersion));
#if NETFRAMEWORK

[TestMethod]
public void InsteadOfAny_EntityFramework_Framework() =>
builderCS.AddPaths("InsteadOfAny.EntityFramework.Framework.cs")
.AddReferences(GetReferencesEntityFrameworkNetFramework())
.Verify();

#endif

[TestMethod]
public void ExistsInsteadOfAny_VB() =>
new VerifierBuilder<VB.InsteadOfAny>().AddPaths("InsteadOfAny.vb").Verify();
builderVB.AddPaths("InsteadOfAny.vb").Verify();

[TestMethod]
public void InsteadOfAny_EntityFramework_VB() =>
builderVB.AddPaths("InsteadOfAny.EntityFramework.Core.vb")
.AddReferences(GetReferencesEntityFrameworkNetCore())
.Verify();

private static IEnumerable<MetadataReference> GetReferencesEntityFrameworkNetCore() =>
Enumerable.Empty<MetadataReference>()
.Concat(NuGetMetadataReference.MicrosoftEntityFrameworkCore("2.2.6"))
.Concat(NuGetMetadataReference.MicrosoftEntityFrameworkCoreRelational("2.2.6"))
.Concat(NuGetMetadataReference.SystemComponentModelTypeConverter());

private static IEnumerable<MetadataReference> GetReferencesEntityFrameworkNetFramework() =>
Enumerable.Empty<MetadataReference>()
.Concat(NuGetMetadataReference.EntityFramework());

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;

// https://github.com/SonarSource/sonar-dotnet/issues/7286
Expand All @@ -10,19 +11,32 @@ public class MyEntity
public int Id { get; set; }
}

public class MyDbContext : DbContext
public class FirstContext: DbContext
{
public DbSet<MyEntity> MyEntities { get; set; }
}

public void GetEntities(MyDbContext dbContext, List<int> ids)
public class SecondContext: DbContext
{
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id == i)); // Noncompliant - FP, should raise in context of EntityFramework queries. Exist cannot be translated to SQL query
public DbSet<MyEntity> SecondEntities { get; set; }
}


public void GetEntities(FirstContext dbContext, List<int> ids)
{
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id == i)); // Compliant
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Equals(i))); // Compliant
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id > i)); // Compliant

_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id is i)); // Error [CS0150]
// Noncompliant@+1
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id is 2)); // Error [CS8122]
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Equals(i))); // Noncompliant - FP, should raise in context of EntityFramework queries. Exist cannot be translated to SQL query
// This will generate a runtime error as EF does not know how to translate it to SQL Query
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id > i)); // Noncompliant - FP
}

public async Task GetEntitiesAsync(SecondContext secondContext, List<int> ids)
{
_ = await secondContext
.SecondEntities
.Where(e => ids.Any(i => e.Id == i))
.ToListAsync();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Imports System.Collections.Generic
Imports System.Linq
Imports Microsoft.EntityFrameworkCore

Public Class EntityFrameworkTestcases
Public Class MyEntity2
Public Property Id As Integer
End Class

Public Class MyDbContext
Inherits DbContext

Public Property MyEntities As DbSet(Of MyEntity2)

Public Sub GetEntities(ByVal dbContext As MyDbContext, ByVal ids As List(Of Integer))
Dim __ As IQueryable(Of MyEntity2) = dbContext.MyEntities.Where(Function(e) ids.Any(Function(i) e.Id = i)) ' Compliant
__ = dbContext.MyEntities.Where(Function(e) ids.Any(Function(i) e.Equals(i))) ' Compliant
__ = dbContext.MyEntities.Where(Function(e) ids.Any(Function(i) e.Id > i)) ' Compliant
__ = dbContext.MyEntities.Where(Function(e) ids.Any(Function(i) TypeOf e.Id Is i)) ' Error [BC30002]
End Sub
End Class
End Class
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Remoting.Contexts;
using System.Threading.Tasks;
using System.Data.Entity;

// https://github.com/SonarSource/sonar-dotnet/issues/7286
public class EntityFrameworkReproGH7286
{
public class MyEntity
{
public int Id { get; set; }
}

public class FirstContext : DbContext
{
public DbSet<MyEntity> MyEntities { get; set; }
}

public class SecondContext : DbContext
{
public DbSet<MyEntity> SecondEntities { get; set; }
}


public void GetEntities(FirstContext dbContext, List<int> ids)
{
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id == i)); // Compliant
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Equals(i))); // Compliant
_ = dbContext.MyEntities.Where(e => ids.Any(i => e.Id > i)); // Compliant
}

public async Task GetEntitiesAsync(SecondContext secondContext, List<int> ids)
{
_ = await secondContext
.SecondEntities
.Where(e => ids.Any(i => e.Id == i))
.ToListAsync();
}
}

0 comments on commit d5416d1

Please sign in to comment.