Skip to content

Conversation

@Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Oct 20, 2025

Current generator implementation doesn't follow Roslyn best practices, and can likely be very slow in big solutions while typing in IDE.

The PR isn't yet complete. All diagnostics produced by the generator needs to move to separate DiagnosticAnalyzer implementations. Before putting some time on it, I'd like to get an initial review and whether or not you are comfortable with the changes.

@CyrusNajmabadi @chsienki If you have time to take a high-level look on the changes here please.

Microsoft Reviewers: Open in CodeFlow

Few notes on the changes:

  • ContextReceiver was deleted. It was already dead code on main.
  • SymbolHolder and SymbolLoader were deleted. It was called in HandleAnnotatedTypes but was unnecessarily and not needed. ForAttributeWithMetadataName already gives us what we want. This was probably a left over from a transition from ISourceGenerator to IIncrementalGenerator.
  • Diagnostics in incremental generators are a nightmare and should never be done IMO. Moving to separate analyzers is far much better.
  • Symbols/compilations in generator pipeline models should always be avoided.
  • Generator pipeline models must implement equality properly.

@Youssef1313 Youssef1313 requested a review from a team as a code owner October 20, 2025 06:27
Copilot AI review requested due to automatic review settings October 20, 2025 06:27
Copy link
Contributor

Copilot AI left a comment

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 refactors the contextual options generator to improve incrementality by following Roslyn best practices for source generators. The main goal is to enhance IDE performance by reducing unnecessary recomputation during typing in large solutions.

Key Changes:

  • Simplified the generator pipeline by processing types individually rather than collecting them first
  • Removed diagnostic validation logic from the generator (to be moved to separate analyzers)
  • Modified the model to avoid storing INamedTypeSymbol references, improving caching behavior

Reviewed Changes

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

File Description
ContextualOptionsGenerator.cs Streamlined the generator pipeline to process types incrementally and removed compilation-dependent logic
Parser.cs Simplified to return single types instead of collections; commented out diagnostic checks pending analyzer migration
OptionsContextType.cs Changed to store primitive string values instead of symbol/syntax references for better incrementality

@Youssef1313 Youssef1313 force-pushed the dev/ygerges/contextual-gen branch from f3e01bc to 0a5db80 Compare October 20, 2025 06:29
@Youssef1313 Youssef1313 force-pushed the dev/ygerges/contextual-gen branch from e5b30ff to 9d671dc Compare October 20, 2025 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant