Skip to content

Commit

Permalink
Enhancement to EFCore analyzer
Browse files Browse the repository at this point in the history
  • Loading branch information
SteveDunn committed Oct 30, 2024
1 parent 7645a45 commit 303644d
Show file tree
Hide file tree
Showing 10 changed files with 423 additions and 57 deletions.
1 change: 1 addition & 0 deletions Vogen.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpCodeStyle/ARGUMENTS_LITERAL/@EntryValue">Named</s:String>
<s:Boolean x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/WRAP_AFTER_INVOCATION_LPAR/@EntryValue">True</s:Boolean>
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/WRAP_ARGUMENTS_STYLE/@EntryValue">CHOP_IF_LONG</s:String>
<s:String x:Key="/Default/CodeStyle/CSharpVarKeywordUsage/ForOtherTypes/@EntryValue">UseVarWhenEvident</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateConstants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=15b5b1f1_002D457c_002D4ca6_002Db278_002D5615aedc07d3/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"&gt;&lt;ElementKinds&gt;&lt;Kind Name="READONLY_FIELD" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;&lt;/Policy&gt;</s:String>
Expand Down
79 changes: 62 additions & 17 deletions src/Vogen/Rules/DoNotCompareWithPrimitivesInEfCoreAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ namespace Vogen.Rules;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class DoNotCompareWithPrimitivesInEfCoreAnalyzer : DiagnosticAnalyzer
{
private static readonly ImmutableHashSet<string> _knownNames = new[] { "Where", "Single", "SkipWhile", "TakeWhile", "Select" }.ToImmutableHashSet();
private static readonly ImmutableHashSet<string> _knownNames =
new[] { "Where", "Single", "SkipWhile", "TakeWhile", "Select" }.ToImmutableHashSet();

// ReSharper disable once ArrangeObjectCreationWhenTypeEvident - current bug in Roslyn analyzer means it
// won't find this and will report:
Expand All @@ -36,46 +37,62 @@ public override void Initialize(AnalysisContext context)
context.EnableConcurrentExecution();

context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression);
context.RegisterSyntaxNodeAction(AnalyzeQieryExpression, SyntaxKind.QueryExpression);
context.RegisterSyntaxNodeAction(AnalyzeQueryExpression, SyntaxKind.QueryExpression);
}

private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
{
var invocationExpr = (InvocationExpressionSyntax) context.Node;
if (invocationExpr.Expression is not MemberAccessExpressionSyntax memberAccessExpr) return;
if (invocationExpr.Expression is not MemberAccessExpressionSyntax memberAccessExpr)
{
return;
}

if (!_knownNames.Contains(memberAccessExpr.Name.Identifier.Text))
{
return;
}

if (!IsAMemberOfDbSet(context, memberAccessExpr.Expression)) return;
ExpressionSyntax expressionSyntax = memberAccessExpr.Expression;

if (!IsAMemberOfDbSet(context, expressionSyntax))
{
return;
}

foreach (ArgumentSyntax eachArgument in invocationExpr.ArgumentList.Arguments.Where(e => e.Expression is LambdaExpressionSyntax))
foreach (ArgumentSyntax eachArgument in
invocationExpr.ArgumentList.Arguments.Where(e => e.Expression is LambdaExpressionSyntax))
{
foreach (BinaryExpressionSyntax eachBinaryExpression in eachArgument.DescendantNodes().OfType<BinaryExpressionSyntax>())
{
ITypeSymbol? left = context.SemanticModel.GetTypeInfo(eachBinaryExpression.Left).Type;
ITypeSymbol? right = context.SemanticModel.GetTypeInfo(eachBinaryExpression.Right).Type;

if (left is null || right is null) continue;
if (left is null || right is null)
{
continue;
}

// Check if left is ValueObject and right is integer
if (IsValueObject(left) && right.SpecialType == SpecialType.System_Int32)
{
context.ReportDiagnostic(DiagnosticsCatalogue.BuildDiagnostic(_rule, left.Name, eachBinaryExpression.GetLocation()));
context.ReportDiagnostic(
DiagnosticsCatalogue.BuildDiagnostic(_rule, left.Name, eachBinaryExpression.GetLocation()));
}
}
}
}

private static void AnalyzeQieryExpression(SyntaxNodeAnalysisContext context)
private static void AnalyzeQueryExpression(SyntaxNodeAnalysisContext context)
{
var queryExpr = (QueryExpressionSyntax) context.Node;
var whereClauses = queryExpr.Body.DescendantNodes().OfType<WhereClauseSyntax>();
var fromClause = queryExpr.FromClause;

if (!IsAMemberOfDbSet(context, fromClause.Expression)) return;
if (!IsAMemberOfDbSet(context, fromClause.Expression))
{
return;
}

foreach (var eachArgument in whereClauses)
{
Expand All @@ -84,12 +101,16 @@ private static void AnalyzeQieryExpression(SyntaxNodeAnalysisContext context)
ITypeSymbol? left = context.SemanticModel.GetTypeInfo(eachBinaryExpression.Left).Type;
ITypeSymbol? right = context.SemanticModel.GetTypeInfo(eachBinaryExpression.Right).Type;

if (left is null || right is null) continue;
if (left is null || right is null)
{
continue;
}

// Check if left is ValueObject and right is integer
if (IsValueObject(left) && right.SpecialType == SpecialType.System_Int32)
{
context.ReportDiagnostic(DiagnosticsCatalogue.BuildDiagnostic(_rule, left.Name, eachBinaryExpression.GetLocation()));
context.ReportDiagnostic(
DiagnosticsCatalogue.BuildDiagnostic(_rule, left.Name, eachBinaryExpression.GetLocation()));
}
}
}
Expand All @@ -99,20 +120,44 @@ private static bool IsAMemberOfDbSet(SyntaxNodeAnalysisContext context, Expressi
{
var symbolInfo = context.SemanticModel.GetSymbolInfo(expressionSyntax);

if (symbolInfo.Symbol is not IPropertySymbol ps) return false;
ITypeSymbol? typeSymbol = null;

if (symbolInfo.Symbol is IPropertySymbol ps)
{
typeSymbol = ps.Type;
}

if (symbolInfo.Symbol is ILocalSymbol ls)
{
typeSymbol = ls.Type;
}

if (typeSymbol is null)
{
return false;
}

var dbSetType = context.SemanticModel.Compilation.GetTypeByMetadataName("Microsoft.EntityFrameworkCore.DbSet`1");

if (dbSetType is null) return false;

return InheritsFrom(ps.Type, dbSetType);
if (dbSetType is null)
{
return false;
}

return InheritsFrom(typeSymbol, dbSetType);
}

private static bool IsValueObject(ITypeSymbol type) =>

private static bool IsValueObject(ITypeSymbol type) =>
type is INamedTypeSymbol symbol && VoFilter.IsTarget(symbol);

private static bool InheritsFrom(ITypeSymbol? type, INamedTypeSymbol baseType)
private static bool InheritsFrom(ITypeSymbol? type, INamedTypeSymbol? baseType)
{
if (baseType is null)
{
return false;
}

while (type != null)
{
if (SymbolEqualityComparer.Default.Equals(type.OriginalDefinition, baseType))
Expand Down
1 change: 1 addition & 0 deletions src/Vogen/Vogen.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" PrivateAssets="all"/>
<PackageReference Include="Microsoft.CodeAnalysis.AnalyzerUtilities" Version="3.3.4" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,53 @@ await Run(
WithDiagnostics("VOG034", DiagnosticSeverity.Error, "Age", 0));
}

[Fact]
public async Task Triggers_when_found_in_IQueryableOfDbSet_in_separate_expression()
{
var source = _source + """
public static class Test
{
public static void FilterItems()
{
using var ctx = new DbContext();
DbSet<EmployeeEntity> step1 = ctx.Entities;
var entities = step1.Where(e => {|#0:e.Age == 50|});
}
}
""";
var sources = await CombineUserAndGeneratedSource(source);

await Run(
sources,
WithDiagnostics("VOG034", DiagnosticSeverity.Error, "Age", 0));
}

[Fact(Skip = "It would be nice if this did work, but I couldn't get it working. Please see the thread at https://github.com/SteveDunn/Vogen/issues/684")]
public async Task Triggers_when_found_in_IQueryableOfDbSet_in_separate_expression_twice_removed()
{
var source = _source + """
public static class Test
{
public static void FilterItems()
{
using var ctx = new DbContext();
DbSet<EmployeeEntity> step1 = ctx.Entities;
var step2 = step1.Take(4);
var entities = step2.Where(e => {|#0:e.Age == 50|});
}
}
""";
var sources = await CombineUserAndGeneratedSource(source);

await Run(
sources,
WithDiagnostics("VOG034", DiagnosticSeverity.Error, "Age", 0));
}

[Fact]
public async Task Triggers_when_found_using_query_syntax()
{
Expand Down
75 changes: 75 additions & 0 deletions tests/Testbench/EfCoreTest/EfCoreScenario.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Query.Internal;

namespace Vogen.EfCoreTest;

public static class EfCoreScenario
{
public static void Run()
{
using var context = new MyContext();
//context.Database.Migrate();

AddAndSaveItems(amount: 10);
AddAndSaveItems(amount: 10);

PrintItems();

FilterItems();

return;

static void AddAndSaveItems(int amount)
{
using var context = new MyContext();

for (int i = 0; i < amount; i++)
{
var entity = new EmployeeEntity
{
Name = Name.From("Fred #" + i),
Age = Age.From(42 + i),
Department = Department.From("Quarry"),
HireDate = HireDate.From(new DateOnly(1066, 12, 13))
};

context.Entities.Add(entity);
}

context.SaveChanges();
}

static void PrintItems()
{
using var ctx = new MyContext();

var entities = ctx.Entities.ToList();
Console.WriteLine(string.Join(Environment.NewLine, entities.Select(e => $"ID: {e.Id.Value}, Name: {e.Name}, Age: {e.Age}")));
}

static void FilterItems()
{
Console.WriteLine();
Console.WriteLine("FILTERING ITEMS...");
using var ctx = new MyContext();

int age = 50;
// var entities = from e in ctx.Entities where e != null && e.Age == age select e;
// var entities2 = ctx.Entities.Where(e => e != null && e.Age == age);

DbSet<EmployeeEntity> step1 = ctx.Entities;
//IQueryable<EmployeeEntity> step2 = step1.Take(4);
var step3 = step1.Where(e => e.Age == age);



// Console.WriteLine(string.Join(Environment.NewLine, entities.Select(e => $"ID: {e.Id.Value}, Name: {e.Name}, Age: {e.Age}")));

}
}

}
Loading

0 comments on commit 303644d

Please sign in to comment.