Skip to content

Use IIncrementalGenerator instead of deprecated ISourceGenerator#83

Merged
enisn merged 3 commits into
enisn:developfrom
curtisy1:incremental-generator
Dec 25, 2025
Merged

Use IIncrementalGenerator instead of deprecated ISourceGenerator#83
enisn merged 3 commits into
enisn:developfrom
curtisy1:incremental-generator

Conversation

@curtisy1

Copy link
Copy Markdown
Contributor

This PR does a few things:

  1. It moves from ISourceGenerator to IIncrementalGenerator, slightly modifying the pipeline
    a. It first gets all classes that are annoted with GenerateAutoFilterAttribute
    b. It then gets the ClassDecorationSyntax of those classes and passes it to the Execute method
  2. Execute now exclusively uses StringBuilder for the generation of classes
  3. Fixed the casing of @namespace in GenerateAutoFilterAttribute
  4. Removes unused code

I validated this works by running the generator tests and looking at the generated classes using dotnet build /p:EmitCompilerGeneratedFiles=true /p:CompilerGeneratedFilesOutputPath=Generated

Fixes #45 and #73

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 modernizes the source generator by migrating from the deprecated ISourceGenerator API to the newer IIncrementalGenerator API, which provides better performance through incremental compilation support.

Key changes:

  • Replaces ISourceGenerator with IIncrementalGenerator using a two-stage pipeline (syntax filtering → semantic analysis)
  • Refactors code generation to exclusively use StringBuilder for better performance
  • Fixes the casing bug in GenerateAutoFilterAttribute constructor parameter

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
FilterGenerator.cs Migrated to IIncrementalGenerator with new Initialize/Execute pipeline, added IsIndexer filtering, improved string type detection, and consolidated code generation using StringBuilder
GenerateAutoFilterAttribute.cs Fixed parameter casing from @Namespace to @namespace
SyntaxReceiver.cs Removed unused legacy syntax receiver class
AttributeSyntaxReceiver.cs Removed unused generic attribute syntax receiver class
Comments suppressed due to low confidence (1)

src/AutoFilterer.Generators/FilterGenerator.cs:133

  • The GetNamespaceRecursively method may fail for the global namespace. When symbol.ContainingNamespace is null, it returns symbol.Name, but for the global namespace, the name might be empty or null, which could lead to an incorrect namespace being generated.
    private static string GetNamespaceRecursively(INamespaceSymbol symbol)
    {
        if (symbol.ContainingNamespace == null)
        {
            return symbol.Name;
        }

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

}
}

return null;

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

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

Returning null from GetSemanticTargetForGeneration could potentially cause issues with nullability warnings in projects with nullable reference types enabled. Consider using a more explicit pattern like returning a nullable type with the proper annotation or using an Option type pattern.

Copilot uses AI. Check for mistakes.

@curtisy1 curtisy1 Dec 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm this is used in a private method and shouldn't happen anyway because the filter is running first, so it's more of a catch-all situation. The calling method does explicitly filter for null afterwards, so I doubt this is an actual issue.

Nullable reference types are disabled so far, so returning a nullable type doesn't really add value here either.

That said, I'm open to suggestions and nullable reference types are something I do want to address in a future PR.

The only useful thing I could do would be wrapping the return type in an Option<T> but that just makes the calling function less readable in my opinion.

Comment thread src/AutoFilterer.Generators/FilterGenerator.cs Outdated

var properties = symbol.GetMembers()
.OfType<IPropertySymbol>()
.Where(x => !x.IsStatic && !x.IsIndexer && x.Kind == SymbolKind.Property);

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

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

The property filtering logic now excludes generic types. The old code excluded properties from generic types with !x.ContainingType.IsGenericType, but the new code removes this check. This means that properties from generic classes will now be included in filter generation, which could be a breaking behavioral change.

Suggested change
.Where(x => !x.IsStatic && !x.IsIndexer && x.Kind == SymbolKind.Property);
.Where(x => !x.IsStatic && !x.IsIndexer && x.Kind == SymbolKind.Property && !x.ContainingType.IsGenericType);

Copilot uses AI. Check for mistakes.

@curtisy1 curtisy1 Dec 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was intentional because excluding the generic classes would break things like Entity<T> where T is int or Guid and used as a generic primary key I think.

I'm open to add it back though

@enisn enisn left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Seems good, thank you for your contribution 🎉

@enisn enisn merged commit b502530 into enisn:develop Dec 25, 2025
3 checks passed
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.

Consider using IIncrementalGenerator interface

3 participants