diff --git a/DiagnosticsIdentifiers.cs b/DiagnosticsIdentifiers.cs index fcd483535..5191e0b12 100644 --- a/DiagnosticsIdentifiers.cs +++ b/DiagnosticsIdentifiers.cs @@ -20,6 +20,10 @@ internal static class DiagnosticIdentifiers /// public const string UnitOfWorkAddRangeNestedCollectionId = "HFW1003"; // Category: Usage + /// + /// FilteringCollection used within an expression tree evaluated by a database query. + /// + public const string FilteringCollectionInExpressionTreeId = "HFW1004"; // Category: Usage } } diff --git a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers.Tests/FilteringCollections/FilteringCollectionInExpressionTreeAnalyzerTests.cs b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers.Tests/FilteringCollections/FilteringCollectionInExpressionTreeAnalyzerTests.cs new file mode 100644 index 000000000..c0836426b --- /dev/null +++ b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers.Tests/FilteringCollections/FilteringCollectionInExpressionTreeAnalyzerTests.cs @@ -0,0 +1,474 @@ +using Havit.Data.EntityFrameworkCore.Patterns.Analyzers.FilteringCollections; +using Havit.Model.Collections.Generic; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; + +namespace Havit.Data.EntityFrameworkCore.Patterns.Analyzers.Tests.FilteringCollections; + +[TestClass] +public class FilteringCollectionInExpressionTreeAnalyzerTests +{ + /// + /// Model, na kterém jsou postaveny všechny testy: Master.Children je FilteringCollection nad namapovanou + /// kolekcí Master.ChildrenIncludingDeleted, Master.Others je FilteringCollection bez namapovaného protějšku. + /// Součástí je i Include extension metoda se stejnou signaturou, jakou má EF Core (aby testy nemusely + /// referencovat EF Core) a metoda přijímající Expression mimo IQueryable (tvar, jaký má např. FluentValidation RuleFor). + /// + private const string ModelDeclarations = @" +using System; +using System.Collections.Generic; +using System.Linq; +using System.Linq.Expressions; +using System.Threading.Tasks; +using Havit.Data.Patterns.DataLoaders; +using Havit.Model.Collections.Generic; + +namespace TestNamespace +{ + public class Child + { + public int Id { get; set; } + public DateTime? Deleted { get; set; } + public Master Master { get; set; } + } + + public class Master + { + public int Id { get; set; } + public List ChildrenIncludingDeleted { get; } = new List(); + public FilteringCollection Children { get; } + public FilteringCollection Others { get; } + + public Master() + { + Children = new FilteringCollection(ChildrenIncludingDeleted, child => child.Deleted == null); + Others = new FilteringCollection(ChildrenIncludingDeleted, child => child.Deleted != null); + } + } + + public class MasterBase + { + public List ItemsIncludingDeleted { get; } = new List(); + } + + public class DerivedMaster : MasterBase + { + public FilteringCollection Items { get; } + + public DerivedMaster() + { + Items = new FilteringCollection(ItemsIncludingDeleted, child => child.Deleted == null); + } + } + + public static class QueryableExtensions + { + public static IQueryable Include(this IQueryable source, Expression> navigationPropertyPath) => source; + } + + public static class Validation + { + public static void RuleFor(Expression> propertyPath) + { + } + } +} +"; + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_WhereWithAnyOnFilteringCollection_ReportsDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Where(master => {|#0:master.Children|}.Any()); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Children", "'ChildrenIncludingDeleted' with an explicit filter")); + } + + /// + /// Nejzrádnější případ: ve finální projekci EF Core nehlásí chybu, jen kolekci vůbec nenačte a vrátí prázdno. + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_CountInProjection_ReportsDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Select(master => new { master.Id, Count = {|#0:master.Children|}.Count }); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Children", "'ChildrenIncludingDeleted' with an explicit filter")); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_NestedSubqueryInProjection_ReportsDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Select(master => new { Ids = {|#0:master.Children|}.Select(child => child.Id).ToList() }); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Children", "'ChildrenIncludingDeleted' with an explicit filter")); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_IncludeOfFilteringCollection_ReportsDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Include(master => {|#0:master.Children|}); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Children", "'ChildrenIncludingDeleted' with an explicit filter")); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_ExpressionVariable_ReportsDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public Expression> GetFilter() + { + return master => {|#0:master.Children|}.Any(); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Children", "'ChildrenIncludingDeleted' with an explicit filter")); + } + + /// + /// Kolekce bez namapovaného protějšku XIncludingDeleted - hláška nemá co navrhnout jménem. + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_WithoutIncludingDeletedCounterpart_ReportsDiagnosticWithFallbackSuggestion() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Where(master => {|#0:master.Others|}.Any()); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Others", "the underlying mapped collection with an explicit filter")); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_MappedCollection_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Where(master => master.ChildrenIncludingDeleted.Any(child => child.Deleted == null)); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_DataLoaderLoad_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IDataLoader dataLoader, Master master) + { + dataLoader.Load(master, item => item.Children); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_DataLoaderLoadAllAsync_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public async Task TestMethod(IDataLoader dataLoader, IEnumerable masters) + { + await dataLoader.LoadAllAsync(masters, item => item.Children); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_DataLoaderThenLoad_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IDataLoader dataLoader, IEnumerable children) + { + dataLoader.LoadAll(children, child => child.Master).ThenLoad(master => master.Children); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + /// + /// Cizí API přijímající Expression (např. FluentValidation RuleFor) není dotaz do databáze - hlásit se nemá. + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_ExpressionOutsideQuery_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod() + { + Validation.RuleFor>(master => master.Children); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_InMemoryLinq_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(List masters) + { + masters.Where(master => master.Children.Any()).ToList(); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_DirectAccess_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public int TestMethod(Master master) + { + return master.Children.Count; + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + /// + /// Query syntax neobsahuje žádný lambda syntax node - dotaz je na lambdy přeložený až v operation tree. + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_QuerySyntax_ReportsDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + var result = from master in masters + where {|#0:master.Children|}.Any() + select master; + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Children", "'ChildrenIncludingDeleted' with an explicit filter")); + } + + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_QuerySyntaxOverInMemoryCollection_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(List masters) + { + var result = from master in masters + where master.Children.Any() + select master; + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + /// + /// Vnořená (quoted) lambda uvnitř outer expression tree - diagnostika se musí hlásit právě jednou. + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_NestedQuotedLambda_ReportsDiagnosticOnce() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Where(master => masters.Any(other => {|#0:master.Children|}.Any())); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Children", "'ChildrenIncludingDeleted' with an explicit filter")); + } + + /// + /// Namapovaný protějšek XIncludingDeleted deklarovaný na bázové entitě - návrh ho musí najít přes dědičnost. + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_IncludingDeletedCounterpartOnBaseType_ReportsDiagnosticWithSuggestion() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IQueryable masters) + { + masters.Where(master => {|#0:master.Items|}.Any()); + } + } +}"; + + await VerifyAnalyzerAsync(source, ExpectedDiagnostic("Items", "'ItemsIncludingDeleted' with an explicit filter")); + } + + /// + /// Lambda předaná data loaderu se přeskakuje celá, bez ohledu na tvar (delší property path by data loader + /// odmítl za běhu vlastní - srozumitelnou - výjimkou, analyzer ji neřeší). + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_DataLoaderWithNonWholeBodyLambda_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IDataLoader dataLoader, Master master) + { + dataLoader.Load(master, item => item.Children.First().Master); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + /// + /// Params overload data loaderu - lambda je v operation tree zabalená do pole (ArrayCreation/ArrayInitializer). + /// + [TestMethod] + public async Task FilteringCollectionInExpressionTreeAnalyzer_DataLoaderParamsOverload_DoesNotReportDiagnostic() + { + const string source = ModelDeclarations + @" +namespace TestNamespace +{ + public class TestClass + { + public void TestMethod(IDataLoader dataLoader, Master master) + { + dataLoader.Load(master, item => item.Children, item => item.ChildrenIncludingDeleted); + } + } +}"; + + await VerifyAnalyzerAsync(source); + } + + private static DiagnosticResult ExpectedDiagnostic(string propertyName, string suggestion) + { + return new DiagnosticResult(Analyzers.Diagnostics.FilteringCollectionInExpressionTree) + .WithLocation(0) + .WithArguments(propertyName, suggestion); + } + + private static async Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest + { + TestState = + { + Sources = { source }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }, + }; + + test.TestState.AdditionalReferences.Add(typeof(FilteringCollection<>).Assembly); + test.TestState.AdditionalReferences.Add(typeof(Data.Patterns.DataLoaders.IDataLoader).Assembly); + + test.ExpectedDiagnostics.AddRange(expected); + + await test.RunAsync(); + } +} diff --git a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/AnalyzerReleases.Unshipped.md b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/AnalyzerReleases.Unshipped.md index f2b7fad65..b71a33bf7 100644 --- a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/AnalyzerReleases.Unshipped.md +++ b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/AnalyzerReleases.Unshipped.md @@ -1,2 +1,8 @@ ; Unshipped analyzer release ; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +HFW1004 | Usage | Warning | Diagnostics diff --git a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Diagnostics.cs b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Diagnostics.cs index 9f87c2f55..b048b3aae 100644 --- a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Diagnostics.cs +++ b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Diagnostics.cs @@ -47,4 +47,23 @@ public static class Diagnostics isEnabledByDefault: true, description: "Detects when IEnumerable is passed to AddForInsert, AddForInsertAsync, AddForUpdate, or AddForDelete methods instead of a single entity." ); + + /// + /// Represents a diagnostic descriptor that identifies and reports cases where a member of type + /// FilteringCollection<T> is used within an expression tree which is translated to a database query. + /// + /// + /// FilteringCollection<T> is an in-memory wrapper over the underlying (mapped) collection, it is not + /// a mapped navigation property. Entity Framework Core is therefore not able to translate it to SQL: the query either + /// fails at runtime, or - when the member is used within the final projection - silently returns no data at all. + /// + public static readonly DiagnosticDescriptor FilteringCollectionInExpressionTree = new DiagnosticDescriptor( + id: DiagnosticIdentifiers.FilteringCollectionInExpressionTreeId, + title: "FilteringCollection used within a database query", + messageFormat: "'{0}' is a FilteringCollection which cannot be translated to SQL - the query fails at runtime or silently returns no data. Use {1} instead.", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "Detects when a FilteringCollection member is used within an expression tree (LINQ to Entities query). FilteringCollection is an in-memory wrapper which Entity Framework Core cannot translate to SQL." + ); } diff --git a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/FilteringCollections/FilteringCollectionConstants.cs b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/FilteringCollections/FilteringCollectionConstants.cs new file mode 100644 index 000000000..8a59ba67c --- /dev/null +++ b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/FilteringCollections/FilteringCollectionConstants.cs @@ -0,0 +1,20 @@ +namespace Havit.Data.EntityFrameworkCore.Patterns.Analyzers.FilteringCollections; + +internal static class FilteringCollectionConstants +{ + internal const string FilteringCollectionMetadataName = "Havit.Model.Collections.Generic.FilteringCollection`1"; + + // Konvence pojmenování namapovaného protějšku (X -> XIncludingDeleted) je zduplikovaná vůči runtime substituci + // v PropertyLoadSequenceResolverIncludingDeletedFilteringCollectionsSubstitution (Havit.Data.EntityFrameworkCore.Patterns). + // Analyzer projekt na Patterns referencovat nemůže. Při změně konvence je potřeba upravit obě místa. + internal const string IncludingDeletedSuffix = "IncludingDeleted"; + + internal const string ExpressionOfTDelegateMetadataName = "System.Linq.Expressions.Expression`1"; + + internal const string QueryableMetadataName = "System.Linq.IQueryable"; + internal const string QueryableOfTMetadataName = "System.Linq.IQueryable`1"; + + internal const string DataLoaderMetadataName = "Havit.Data.Patterns.DataLoaders.IDataLoader"; + internal const string FluentDataLoaderMetadataName = "Havit.Data.Patterns.DataLoaders.IFluentDataLoader`1"; + internal const string FluentDataLoaderExtensionsMetadataName = "Havit.Data.Patterns.DataLoaders.FluentDataLoaderExtensions"; +} diff --git a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/FilteringCollections/FilteringCollectionInExpressionTreeAnalyzer.cs b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/FilteringCollections/FilteringCollectionInExpressionTreeAnalyzer.cs new file mode 100644 index 000000000..7be988af9 --- /dev/null +++ b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/FilteringCollections/FilteringCollectionInExpressionTreeAnalyzer.cs @@ -0,0 +1,293 @@ +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Havit.Data.EntityFrameworkCore.Patterns.Analyzers.FilteringCollections; + +/// +/// Analyzer that detects usages of FilteringCollection<T> members within expression trees (LINQ to Entities queries). +/// +/// +/// FilteringCollection<T> is an in-memory wrapper over the underlying (mapped) collection, therefore Entity Framework Core +/// cannot translate it to SQL. Such a query either fails at runtime, or - when the member is used within the final projection - +/// silently returns no data at all. +/// +/// The analysis is operation-based (/), +/// so it covers both method syntax and query syntax - query clauses are already lowered to expression tree lambdas +/// in the operation tree. +/// +/// +/// Data loaders (IDataLoader, IFluentDataLoader) are excluded: they do support FilteringCollection<T> +/// by substituting the XIncludingDeleted collection. Expression trees consumed outside of a database query +/// (validation rules, mocking setups, ...) have the shape entity => entity.Collection; that shape is reported only when +/// the expression is passed to an IQueryable method (e.g. Include). +/// +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class FilteringCollectionInExpressionTreeAnalyzer : DiagnosticAnalyzer +{ + private static readonly ImmutableArray supportedDiagnostics = ImmutableArray.Create(Diagnostics.FilteringCollectionInExpressionTree); + + /// + public override ImmutableArray SupportedDiagnostics => supportedDiagnostics; + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(compilationStartContext => + { + KnownTypes knownTypes = KnownTypes.TryResolve(compilationStartContext.Compilation); + if (knownTypes == null) + { + // The compilation does not reference FilteringCollection at all - do not register any action. + return; + } + + compilationStartContext.RegisterOperationAction( + operationContext => AnalyzeMemberReference(operationContext, knownTypes), + OperationKind.PropertyReference, + OperationKind.FieldReference); + }); + } + + private static void AnalyzeMemberReference(OperationAnalysisContext context, KnownTypes knownTypes) + { + var memberReference = (IMemberReferenceOperation)context.Operation; + + if (!TryGetFilteringCollectionItemType(memberReference.Type, knownTypes, out ITypeSymbol itemType)) + { + return; + } + + // The member reference is relevant only when it sits inside a lambda converted to an expression tree. + // Each member reference is visited exactly once, so nested (quoted) lambdas need no deduplication; + // the outermost expression tree lambda determines the consumer (data loader, IQueryable method, ...). + IAnonymousFunctionOperation outermostExpressionTreeLambda = null; + for (IOperation current = memberReference.Parent; current != null; current = current.Parent) + { + if ((current is IAnonymousFunctionOperation anonymousFunction) && IsConvertedToExpressionTree(anonymousFunction, knownTypes)) + { + outermostExpressionTreeLambda = anonymousFunction; + } + } + + if (outermostExpressionTreeLambda == null) + { + return; + } + + IInvocationOperation enclosingInvocation = GetEnclosingInvocation(outermostExpressionTreeLambda); + + if ((enclosingInvocation != null) && IsDataLoaderInvocation(enclosingInvocation, knownTypes)) + { + return; + } + + // entity => entity.Collection - the shape used by data loaders, validation rules or mocking setups. + // Within a query (Include, Select, ...) it is still an error, anywhere else it is a legitimate usage. + if (IsWholeLambdaBody(memberReference, outermostExpressionTreeLambda) + && ((enclosingInvocation == null) || !IsQueryableInvocation(enclosingInvocation, knownTypes))) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create( + Diagnostics.FilteringCollectionInExpressionTree, + memberReference.Syntax.GetLocation(), + memberReference.Member.Name, + GetSuggestion(memberReference.Member, itemType))); + } + + /// + /// Returns true when the anonymous function is converted to System.Linq.Expressions.Expression<TDelegate> + /// (an expression tree), not to a delegate. + /// + private static bool IsConvertedToExpressionTree(IAnonymousFunctionOperation anonymousFunction, KnownTypes knownTypes) + { + // An expression tree conversion is an IConversionOperation to Expression wrapping the anonymous function + // (IDelegateCreationOperation is used for conversions to a delegate type only). + return (anonymousFunction.Parent is IConversionOperation conversion) + && (conversion.Type is INamedTypeSymbol convertedType) + && SymbolEqualityComparer.Default.Equals(convertedType.OriginalDefinition, knownTypes.ExpressionOfTDelegate); + } + + /// + /// Returns true when the member reference (modulo implicit conversions) forms the whole body of the lambda, + /// i.e. the lambda has the shape entity => entity.Collection. + /// + private static bool IsWholeLambdaBody(IMemberReferenceOperation memberReference, IAnonymousFunctionOperation lambda) + { + IOperation current = memberReference; + while ((current.Parent is IConversionOperation conversion) && conversion.IsImplicit) + { + current = conversion; + } + + return (current.Parent is IReturnOperation returnOperation) && (returnOperation.Parent == lambda.Body); + } + + /// + /// Returns the invocation the lambda is passed to as an argument (incl. a params array of expressions), or null. + /// + private static IInvocationOperation GetEnclosingInvocation(IAnonymousFunctionOperation lambda) + { + IOperation current = lambda.Parent; + while (current is IDelegateCreationOperation or IConversionOperation or IArrayInitializerOperation or IArrayCreationOperation) + { + current = current.Parent; + } + + return (current is IArgumentOperation argument) + ? argument.Parent as IInvocationOperation + : null; + } + + private static bool IsDataLoaderInvocation(IInvocationOperation invocation, KnownTypes knownTypes) + { + IMethodSymbol methodSymbol = invocation.TargetMethod; + INamedTypeSymbol containingType = (methodSymbol.ReducedFrom ?? methodSymbol).ContainingType; + if (containingType == null) + { + return false; + } + + return IsDataLoaderType(containingType, knownTypes) || containingType.AllInterfaces.Any(interfaceType => IsDataLoaderType(interfaceType, knownTypes)); + } + + private static bool IsDataLoaderType(INamedTypeSymbol type, KnownTypes knownTypes) + { + INamedTypeSymbol typeDefinition = type.OriginalDefinition; + + return SymbolEqualityComparer.Default.Equals(typeDefinition, knownTypes.DataLoader) + || SymbolEqualityComparer.Default.Equals(typeDefinition, knownTypes.FluentDataLoader) + || SymbolEqualityComparer.Default.Equals(typeDefinition, knownTypes.FluentDataLoaderExtensions); + } + + private static bool IsQueryableInvocation(IInvocationOperation invocation, KnownTypes knownTypes) + { + IMethodSymbol methodSymbol = invocation.TargetMethod; + + if (IsQueryable(methodSymbol.ReceiverType, knownTypes) || IsQueryable(methodSymbol.ReturnType, knownTypes)) + { + return true; + } + + // non-reduced form of an extension method: Queryable.Include(source, navigationPropertyPath) + return (methodSymbol.Parameters.Length > 0) && IsQueryable(methodSymbol.Parameters[0].Type, knownTypes); + } + + private static bool IsQueryable(ITypeSymbol type, KnownTypes knownTypes) + { + if (type == null) + { + return false; + } + + return IsQueryableInterface(type, knownTypes) || ((type is INamedTypeSymbol namedType) && namedType.AllInterfaces.Any(interfaceType => IsQueryableInterface(interfaceType, knownTypes))); + } + + private static bool IsQueryableInterface(ITypeSymbol type, KnownTypes knownTypes) + { + ITypeSymbol typeDefinition = type.OriginalDefinition; + + return SymbolEqualityComparer.Default.Equals(typeDefinition, knownTypes.Queryable) + || SymbolEqualityComparer.Default.Equals(typeDefinition, knownTypes.QueryableOfT); + } + + private static ITypeSymbol GetMemberType(ISymbol symbol) + { + return symbol switch + { + IPropertySymbol property => property.Type, + IFieldSymbol field => field.Type, + _ => null + }; + } + + private static bool TryGetFilteringCollectionItemType(ITypeSymbol type, KnownTypes knownTypes, out ITypeSymbol itemType) + { + for (ITypeSymbol currentType = type; currentType != null; currentType = currentType.BaseType) + { + if ((currentType is INamedTypeSymbol namedType) + && SymbolEqualityComparer.Default.Equals(namedType.OriginalDefinition, knownTypes.FilteringCollection)) + { + itemType = namedType.TypeArguments[0]; + return true; + } + } + + itemType = null; + return false; + } + + /// + /// Returns the name of the mapped counterpart (XIncludingDeleted) when the entity declares one - the very same substitution + /// the data loader does. Falls back to a generic wording when there is no such member. + /// + private static string GetSuggestion(ISymbol memberSymbol, ITypeSymbol itemType) + { + string expectedName = memberSymbol.Name + FilteringCollectionConstants.IncludingDeletedSuffix; + + for (INamedTypeSymbol type = memberSymbol.ContainingType; type != null; type = type.BaseType) + { + foreach (ISymbol candidate in type.GetMembers(expectedName)) + { + if ((GetMemberType(candidate) is INamedTypeSymbol candidateType) && IsEnumerableOf(candidateType, itemType)) + { + return $"'{expectedName}' with an explicit filter"; + } + } + } + + return "the underlying mapped collection with an explicit filter"; + } + + private static bool IsEnumerableOf(INamedTypeSymbol type, ITypeSymbol itemType) + { + return type.AllInterfaces.Concat([type]).Any(candidateType => + (candidateType.OriginalDefinition.SpecialType == SpecialType.System_Collections_Generic_IEnumerable_T) + && SymbolEqualityComparer.Default.Equals(candidateType.TypeArguments.FirstOrDefault(), itemType)); + } + + /// + /// Symbols of well-known types, resolved once per compilation. Data loader types may be null (the compilation + /// does not have to reference Havit.Data.Patterns) - never matches null. + /// + private sealed class KnownTypes + { + public INamedTypeSymbol FilteringCollection { get; private set; } + public INamedTypeSymbol ExpressionOfTDelegate { get; private set; } + public INamedTypeSymbol Queryable { get; private set; } + public INamedTypeSymbol QueryableOfT { get; private set; } + public INamedTypeSymbol DataLoader { get; private set; } + public INamedTypeSymbol FluentDataLoader { get; private set; } + public INamedTypeSymbol FluentDataLoaderExtensions { get; private set; } + + public static KnownTypes TryResolve(Compilation compilation) + { + INamedTypeSymbol filteringCollection = compilation.GetTypeByMetadataName(FilteringCollectionConstants.FilteringCollectionMetadataName); + INamedTypeSymbol expressionOfTDelegate = compilation.GetTypeByMetadataName(FilteringCollectionConstants.ExpressionOfTDelegateMetadataName); + + if ((filteringCollection == null) || (expressionOfTDelegate == null)) + { + return null; + } + + return new KnownTypes + { + FilteringCollection = filteringCollection, + ExpressionOfTDelegate = expressionOfTDelegate, + Queryable = compilation.GetTypeByMetadataName(FilteringCollectionConstants.QueryableMetadataName), + QueryableOfT = compilation.GetTypeByMetadataName(FilteringCollectionConstants.QueryableOfTMetadataName), + DataLoader = compilation.GetTypeByMetadataName(FilteringCollectionConstants.DataLoaderMetadataName), + FluentDataLoader = compilation.GetTypeByMetadataName(FilteringCollectionConstants.FluentDataLoaderMetadataName), + FluentDataLoaderExtensions = compilation.GetTypeByMetadataName(FilteringCollectionConstants.FluentDataLoaderExtensionsMetadataName), + }; + } + } +} diff --git a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Havit.Data.EntityFrameworkCore.Patterns.Analyzers.csproj b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Havit.Data.EntityFrameworkCore.Patterns.Analyzers.csproj index e83d2b829..71aa8b13a 100644 --- a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Havit.Data.EntityFrameworkCore.Patterns.Analyzers.csproj +++ b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/Havit.Data.EntityFrameworkCore.Patterns.Analyzers.csproj @@ -17,7 +17,7 @@ - 2.10.1 + 2.11.0 false true @@ -26,14 +26,10 @@ + - - - - - diff --git a/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/README.md b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/README.md new file mode 100644 index 000000000..2ad286a4d --- /dev/null +++ b/Havit.Data.EntityFrameworkCore.Patterns.Analyzers/README.md @@ -0,0 +1,21 @@ +HAVIT .NET Framework Extensions - Entity Framework Core Data Patterns - Analyzers + +## Účel nuget balíčku +* Balíček obsahuje Roslyn analyzery, které v compile time hlásí chybná použití API knihovny + `Havit.Data.EntityFrameworkCore.Patterns` (resp. `Havit.Data.Patterns`). +* Jde o vývojovou závislost (`DevelopmentDependency`), do runtime se nic nepřenáší. + +## Jak balíček použít +* Zaregistrujte nuget balíček `Havit.Data.EntityFrameworkCore.Patterns.Analyzers` do projektů, + ve kterých se pracuje s `IUnitOfWork`, s modelem a s dotazy do databáze + (typicky `Model`, `DataLayer`, `Services`, `Facades`). +* Jednotlivá pravidla lze standardně konfigurovat v `.editorconfig` + (např. `dotnet_diagnostic.HFW1004.severity = error`). + +## Pravidla + +| ID | Pravidlo | +| --- | --- | +| HFW1002 | `IEnumerable` předaný do `IUnitOfWork.AddFor*` metody, která očekává jednu entitu. | +| HFW1003 | Vnořená kolekce (`IEnumerable>`) předaná do `IUnitOfWork.AddRangeFor*` metody. | +| HFW1004 | `FilteringCollection` použitá v expression tree, tedy v dotazu do databáze. Kolekce je in-memory wrapper nad namapovanou kolekcí, EF Core ji nepřeloží do SQL: dotaz buď spadne za běhu, nebo (ve finální projekci) tiše nevrátí žádná data. Řešením je použít namapovanou kolekci `XIncludingDeleted` s explicitním filtrem. Pokrývá method syntax i query syntax. Načítání přes `IDataLoader`/`IFluentDataLoader` hlášeno není - data loader `FilteringCollection` podporuje. | diff --git a/Havit.Data.EntityFrameworkCore.Patterns/DataLoaders/Internal/PropertyLoadSequenceResolverIncludingDeletedFilteringCollectionsSubstitution.cs b/Havit.Data.EntityFrameworkCore.Patterns/DataLoaders/Internal/PropertyLoadSequenceResolverIncludingDeletedFilteringCollectionsSubstitution.cs index 4a71e9e03..e4600fcbb 100644 --- a/Havit.Data.EntityFrameworkCore.Patterns/DataLoaders/Internal/PropertyLoadSequenceResolverIncludingDeletedFilteringCollectionsSubstitution.cs +++ b/Havit.Data.EntityFrameworkCore.Patterns/DataLoaders/Internal/PropertyLoadSequenceResolverIncludingDeletedFilteringCollectionsSubstitution.cs @@ -19,6 +19,8 @@ public override PropertyToLoad[] GetPropertiesToLoad(Express { // pokud jde o kolekci // a existuje vlastnost s pojmenováním "IncludingDeleted" na konci + // (konvence sufixu je zduplikovaná v analyzeru HFW1004 - Havit.Data.EntityFrameworkCore.Patterns.Analyzers, + // FilteringCollectionConstants.IncludingDeletedSuffix; při změně konvence je potřeba upravit obě místa) // která obsahuje prvky stejného typu // pak provedeme substituci if (propertyToLoad.IsCollection)