Skip to content

Complete source generators for queries#84

Open
enisn wants to merge 9 commits into
developfrom
source-generators
Open

Complete source generators for queries#84
enisn wants to merge 9 commits into
developfrom
source-generators

Conversation

@enisn

@enisn enisn commented Dec 26, 2025

Copy link
Copy Markdown
Owner

Introduces the ApplyFilterGenerator Roslyn source generator to generate efficient ApplyFilter extension methods at compile time, reducing reflection overhead. Adds comprehensive documentation for query generation flow and source generator usage. Provides sample DTOs, models, and test scaffolding to demonstrate and validate generator functionality.

Introduces the ApplyFilterGenerator Roslyn source generator to generate efficient ApplyFilter extension methods at compile time, reducing reflection overhead. Adds comprehensive documentation for query generation flow and source generator usage. Provides sample DTOs, models, and test scaffolding to demonstrate and validate generator functionality.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a comprehensive Roslyn source generator (ApplyFilterGenerator) to generate compile-time filter application code, eliminating reflection overhead and enabling AOT compatibility. It includes extensive documentation, test infrastructure with sample models/filters, and sandbox examples to demonstrate the generator's functionality.

  • Adds ApplyFilterGenerator to generate efficient ApplyFilter extension methods at compile time
  • Introduces comprehensive test infrastructure with sample models, filters, and smoke tests
  • Provides detailed documentation covering generator usage, features, and query generation flow

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
tests/AutoFilterer.Generators.Tests/TestDataHelper.cs Test data factory for Books, Authors, and Publishers with navigation relationships
tests/AutoFilterer.Generators.Tests/GeneratorSmokeTests.cs Smoke tests validating generated code compiles and executes correctly for various filter types
tests/AutoFilterer.Generators.Tests/FilterGeneratorTests.cs Refactored existing tests into separate namespace to avoid naming conflicts
tests/AutoFilterer.Generators.Tests/Environment/Models/TestModels.cs Entity models (Book, Author, Publisher) for testing
tests/AutoFilterer.Generators.Tests/Environment/Dtos/TestFilters.cs Comprehensive filter DTOs testing all generator features
tests/AutoFilterer.Generators.Tests/AutoFilterer.Generators.Tests.csproj Project configuration updated to net10.0 target
src/AutoFilterer.Generators/ApplyFilterGenerator.cs Core incremental source generator implementing filter-to-LINQ translation
sandbox/WebApplication.API/Models/Author.cs Sample entity model for sandbox demo
sandbox/WebApplication.API/Dtos/AuthorFilterWithGenerator.cs Sample filter DTO demonstrating generator usage
docs/en/generators/SourceGenerator-ApplyFilter.md Complete user guide for the ApplyFilter source generator
QUERY_GENERATION_FLOW.md Technical deep-dive into query generation architecture and flow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sb.AppendLine("using System;");
sb.AppendLine("using System.Linq;");
sb.AppendLine("using AutoFilterer.Abstractions;");
sb.AppendLine("using AutoFilterer;");

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated code references 'AutoFilterer.Sorting' namespace at line 244, but the using statements don't include 'using AutoFilterer.Enums;'. Depending on the LEGACY_NAMESPACE flag, the Sorting enum is in either 'AutoFilterer.Enums' or 'AutoFilterer' namespace. Since line 113 includes 'using AutoFilterer;', this should work for non-legacy builds, but for projects with LEGACY_NAMESPACE defined, the generated code will fail to compile. Consider adding 'using AutoFilterer.Enums;' to the generated using statements to ensure compatibility with both build configurations.

Suggested change
sb.AppendLine("using AutoFilterer;");
sb.AppendLine("using AutoFilterer;");
sb.AppendLine("using AutoFilterer.Enums;");

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +43
var books = GetSampleBooks();
foreach (var author in authors)
{
author.Books = books.Where(b => b.AuthorId == author.Id).ToList();
foreach (var book in author.Books)
{
book.Author = author;
}
}

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling GetSampleBooks() creates a new list of Book instances each time, but these are then shared across all Author instances. When setting book.Author = author on line 41, this modifies the shared Book instances. This could lead to unexpected behavior if GetSampleAuthors is called multiple times in a test, as the Book.Author reference will point to the last Author that was assigned. Consider creating independent Book instances for each Author, or documenting that this method should only be called once per test to establish the relationships.

Copilot uses AI. Check for mistakes.
var isNullable = filterProp.NullableAnnotation == Microsoft.CodeAnalysis.NullableAnnotation.Annotated;
if (isNullable)
{
sb.AppendLine($" if (filter.{fpName} != null) source = source.Where(x => x.{epName}.Equals(filter.{fpName}));");

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated code for nullable value type comparisons uses x.{epName}.Equals(filter.{fpName}), but if the entity property is nullable and the filter value is not null, this will cause a compilation error because it's calling Equals on a nullable type without null-checking. The correct code should be either 'x.{epName} == filter.{fpName}' (which handles nullable comparison correctly) or add a null check like 'x.{epName}.HasValue && x.{epName}.Value.Equals(filter.{fpName}.Value)'.

Suggested change
sb.AppendLine($" if (filter.{fpName} != null) source = source.Where(x => x.{epName}.Equals(filter.{fpName}));");
sb.AppendLine($" if (filter.{fpName} != null) source = source.Where(x => x.{epName} == filter.{fpName});");

Copilot uses AI. Check for mistakes.
var sortByProp = filterSymbol.GetMembers().OfType<IPropertySymbol>().FirstOrDefault(p => p.Name == "SortBy");
if (sortByProp != null)
{
sb.AppendLine(" if (filter.SortBy == AutoFilterer.Sorting.Descending)");

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated code references 'AutoFilterer.Sorting.Descending', but based on the Sorting enum definition, the namespace should be 'AutoFilterer.Enums.Sorting' (or just 'AutoFilterer' depending on the LEGACY_NAMESPACE flag). The tests use 'AutoFilterer.Enums.Sorting.Descending' (line 89 in GeneratorSmokeTests.cs), which suggests the generated code should match this pattern. This will cause compilation errors in the generated code.

Suggested change
sb.AppendLine(" if (filter.SortBy == AutoFilterer.Sorting.Descending)");
sb.AppendLine(" if (filter.SortBy == AutoFilterer.Enums.Sorting.Descending)");

Copilot uses AI. Check for mistakes.
Comment on lines +296 to +317
var fpName = filterProp.Name;
var epName = entityProp.Name;

// Get FilterOption (Any or All)
var filterOption = "Any"; // default
var filterOptionArg = collectionFilterAttr.ConstructorArguments.Length > 0 ? collectionFilterAttr.ConstructorArguments[0] : default;
if (filterOptionArg.Value != null)
{
// The value is an integer enum value
var enumValue = filterOptionArg.Value.ToString();
filterOption = enumValue == "0" ? "Any" : "All";
}
else
{
var namedArg = collectionFilterAttr.NamedArguments.FirstOrDefault(kv => kv.Key == "FilterOption").Value;
if (namedArg.Value != null)
{
var enumValue = namedArg.Value.ToString();
filterOption = enumValue == "0" ? "Any" : "All";
}
}

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for extracting filter option (Any/All) from CollectionFilterAttribute is duplicated in three places: EmitCollectionFilter (lines 300-316) and BuildNestedCollectionCondition (lines 531-546). Consider extracting this into a shared helper method like GetCollectionFilterOption that takes an AttributeData and returns the filter option string. This would reduce duplication and make maintenance easier.

Suggested change
var fpName = filterProp.Name;
var epName = entityProp.Name;
// Get FilterOption (Any or All)
var filterOption = "Any"; // default
var filterOptionArg = collectionFilterAttr.ConstructorArguments.Length > 0 ? collectionFilterAttr.ConstructorArguments[0] : default;
if (filterOptionArg.Value != null)
{
// The value is an integer enum value
var enumValue = filterOptionArg.Value.ToString();
filterOption = enumValue == "0" ? "Any" : "All";
}
else
{
var namedArg = collectionFilterAttr.NamedArguments.FirstOrDefault(kv => kv.Key == "FilterOption").Value;
if (namedArg.Value != null)
{
var enumValue = namedArg.Value.ToString();
filterOption = enumValue == "0" ? "Any" : "All";
}
}
string GetCollectionFilterOption(AttributeData collectionFilterAttr)
{
var filterOption = "Any"; // default
var filterOptionArg = collectionFilterAttr.ConstructorArguments.Length > 0
? collectionFilterAttr.ConstructorArguments[0]
: default;
if (filterOptionArg.Value != null)
{
// The value is an integer enum value
var enumValue = filterOptionArg.Value.ToString();
filterOption = enumValue == "0" ? "Any" : "All";
}
else
{
var namedArg = collectionFilterAttr.NamedArguments
.FirstOrDefault(kv => kv.Key == "FilterOption")
.Value;
if (namedArg.Value != null)
{
var enumValue = namedArg.Value.ToString();
filterOption = enumValue == "0" ? "Any" : "All";
}
}
return filterOption;
}
var fpName = filterProp.Name;
var epName = entityProp.Name;
// Get FilterOption (Any or All)
var filterOption = GetCollectionFilterOption(collectionFilterAttr);

Copilot uses AI. Check for mistakes.
Comment on lines +199 to +220
// For each CompareTo target property, emit comparisons
foreach (var attr in compareToAttrs2)
{
var combineArg = attr.NamedArguments.FirstOrDefault(kv => kv.Key == "CombineWith").Value;
var combineWith = combineArg.Value?.ToString() ?? "Or";

var propNamesConst = attr.ConstructorArguments.Length > 0 ? attr.ConstructorArguments[0] : default;
if (propNamesConst.Kind == TypedConstantKind.Array)
{
foreach (var item in propNamesConst.Values)
{
var targetName = item.Value?.ToString();
if (string.IsNullOrWhiteSpace(targetName)) continue;
var entityProp = ResolveMemberByPath(entitySymbol, targetName);
if (entityProp == null) continue;
EmitScalarComparison(sb, entitySymbol, prop, entityProp, combineWith);
}
}
else
{
var targetName = propNamesConst.Value?.ToString();
if (string.IsNullOrWhiteSpace(targetName)) continue;

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for extracting target property names from CompareToAttribute constructor arguments is duplicated across multiple locations (lines 205-216, 433-447, 487-501). This pattern of checking for array vs. single value and extracting target names appears at least three times. Consider extracting this into a shared helper method like ExtractTargetPropertyNames that takes a TypedConstant and returns a List of string. This would improve maintainability and reduce the risk of inconsistent behavior.

Suggested change
// For each CompareTo target property, emit comparisons
foreach (var attr in compareToAttrs2)
{
var combineArg = attr.NamedArguments.FirstOrDefault(kv => kv.Key == "CombineWith").Value;
var combineWith = combineArg.Value?.ToString() ?? "Or";
var propNamesConst = attr.ConstructorArguments.Length > 0 ? attr.ConstructorArguments[0] : default;
if (propNamesConst.Kind == TypedConstantKind.Array)
{
foreach (var item in propNamesConst.Values)
{
var targetName = item.Value?.ToString();
if (string.IsNullOrWhiteSpace(targetName)) continue;
var entityProp = ResolveMemberByPath(entitySymbol, targetName);
if (entityProp == null) continue;
EmitScalarComparison(sb, entitySymbol, prop, entityProp, combineWith);
}
}
else
{
var targetName = propNamesConst.Value?.ToString();
if (string.IsNullOrWhiteSpace(targetName)) continue;
static List<string> ExtractTargetPropertyNames(TypedConstant propNamesConst)
{
var result = new List<string>();
if (propNamesConst.Kind == TypedConstantKind.Array)
{
foreach (var item in propNamesConst.Values)
{
var targetName = item.Value?.ToString();
if (!string.IsNullOrWhiteSpace(targetName))
{
result.Add(targetName);
}
}
}
else
{
var targetName = propNamesConst.Value?.ToString();
if (!string.IsNullOrWhiteSpace(targetName))
{
result.Add(targetName);
}
}
return result;
}
// For each CompareTo target property, emit comparisons
foreach (var attr in compareToAttrs2)
{
var combineArg = attr.NamedArguments.FirstOrDefault(kv => kv.Key == "CombineWith").Value;
var combineWith = combineArg.Value?.ToString() ?? "Or";
var propNamesConst = attr.ConstructorArguments.Length > 0 ? attr.ConstructorArguments[0] : default;
var targetNames = ExtractTargetPropertyNames(propNamesConst);
foreach (var targetName in targetNames)
{

Copilot uses AI. Check for mistakes.
Comment on lines +361 to +366
foreach (var iface in type.AllInterfaces)
{
if (iface.Name == "IFilter" && iface.ContainingNamespace?.ToDisplayString() == "AutoFilterer.Abstractions")
{
return true;
}

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Suggested change
foreach (var iface in type.AllInterfaces)
{
if (iface.Name == "IFilter" && iface.ContainingNamespace?.ToDisplayString() == "AutoFilterer.Abstractions")
{
return true;
}
var implementsIFilter = type.AllInterfaces.Any(iface =>
iface.Name == "IFilter" &&
iface.ContainingNamespace?.ToDisplayString() == "AutoFilterer.Abstractions");
if (implementsIFilter)
{
return true;

Copilot uses AI. Check for mistakes.
Comment on lines +157 to +166
foreach (var item in propNamesConst.Values)
{
var targetName = item.Value?.ToString();
if (string.IsNullOrWhiteSpace(targetName)) continue;
var entityProp = ResolveMemberByPath(entitySymbol, targetName);
if (entityProp != null)
{
EmitCollectionFilter(sb, entitySymbol, prop, entityProp, collectionFilterAttr, "x");
}
}

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Copilot uses AI. Check for mistakes.
Comment on lines +208 to +213
foreach (var item in propNamesConst.Values)
{
var targetName = item.Value?.ToString();
if (string.IsNullOrWhiteSpace(targetName)) continue;
var entityProp = ResolveMemberByPath(entitySymbol, targetName);
if (entityProp == null) continue;

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Suggested change
foreach (var item in propNamesConst.Values)
{
var targetName = item.Value?.ToString();
if (string.IsNullOrWhiteSpace(targetName)) continue;
var entityProp = ResolveMemberByPath(entitySymbol, targetName);
if (entityProp == null) continue;
var entityProps = propNamesConst.Values
.Select(v => v.Value?.ToString())
.Where(targetName => !string.IsNullOrWhiteSpace(targetName))
.Select(targetName => ResolveMemberByPath(entitySymbol, targetName))
.Where(entityProp => entityProp != null);
foreach (var entityProp in entityProps)
{

Copilot uses AI. Check for mistakes.
{
Assert.NotNull(typeof(MappingTest.AllTypesTestTypeFilter));

var filter = new MappingTest.AllTypesTestTypeFilter();

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to filter is useless, since its value is never read.

Suggested change
var filter = new MappingTest.AllTypesTestTypeFilter();
var filter = new MappingTest.AllTypesTestTypeFilter();
Assert.NotNull(filter);

Copilot uses AI. Check for mistakes.
enisn added 5 commits January 16, 2026 10:40
Add AGENTS.md and expand generator capabilities and tests. Changes to ApplyFilterGenerator introduce a Batch-1 safe generation path, parity prefilters, null-guard builders, single-target resolution helpers, and safer handling for Sort/SortBy and pagination properties. FilterGenerator now parses new GenerateAutoFilterAttribute options (BaseClass, UseStringFilter, UseRangeForNumbers/Dates, GenerateForEnumProperties), adds property-skipping heuristics, and emits DTOs accordingly. TypeMapping entries adjusted for bool/Guid/DateTimeOffset and other fixes. StringFilter and StringFilterOptions updated to reuse a local source expression and wrap comparisons with GuardTargetNotNull to avoid null-reference issues. A large set of generator tests and parity baseline tests were added/updated to cover the new behaviors.
Add AGENTS.md with guidance for autonomous coding agents and repository conventions. Refactor ApplyFilterGenerator: replace the old Batch1 safe gate with a BuildFilterState-driven approach that emits a generated Where condition when applicable; add extensive helper routines (BuildPropertyState/BuildTargetState/BuildCollectionState/BuildNestedFilterState, string/operator/range handling, path guards, CombineStates, etc.). Emit ordering and pagination logic and generate nested filter helpers (HasFilters/ApplyGenerated helpers) to support collection and nested filter scenarios. Update GeneratorAotPathTests to include Level1Filter_AllAnyAnyAnyAnyAnyAny.
@enisn

enisn commented Apr 21, 2026

Copy link
Copy Markdown
Owner Author

@copilot Apply your review feedbacks and ensure all the generated code by source generators is compiled with tests when possible

Copilot AI commented Apr 21, 2026

Copy link
Copy Markdown

@copilot Apply your review feedbacks and ensure all the generated code by source generators is compiled with tests when possible

Addressed the review feedback and validated generator output compilation through tests in commits 4a54933 and c336f16. I applied the actionable generator/test fixes, removed duplicated extraction logic, fixed nullable comparison generation, and ran generator/solution test runs successfully in this environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants