Skip to content

Comments

Add source generator for Arguments.From#18837

Open
gvkries wants to merge 31 commits intomainfrom
gvkries/source-generator
Open

Add source generator for Arguments.From#18837
gvkries wants to merge 31 commits intomainfrom
gvkries/source-generator

Conversation

@gvkries
Copy link
Member

@gvkries gvkries commented Feb 13, 2026

This pull request introduces significant improvements to how arguments are passed to shapes by leveraging source generators for strongly-typed argument classes, optimizes the internal implementation of the Arguments system, and updates several modules to use these new patterns. It also adds new dependencies and project references to support these changes.

Shape Arguments Refactoring and Source Generators Integration:

  • Introduced strongly-typed argument classes (e.g., NavigationArguments, MenuItemArguments, ContentZoneArguments) for shape creation, decorated with [GenerateArguments] to enable source-generated property access and improved performance. Updated shape creation calls throughout the codebase to use these new argument types instead of anonymous objects. [1] [2] [3] [4] [5] [6] [7] [8]

  • Added the OrchardCore.SourceGenerators project and included it as an analyzer reference in the build system, ensuring that argument source generation is available for all projects except itself. [1] [2]

  • Updated documentation navigation to include the new "Source Generators" module.

Arguments System Improvements:

  • Refactored Arguments.cs to optimize named/positional argument handling: replaced lists with arrays and ArraySegments, used FrozenDictionary for faster lookups, improved zero-argument cases, and provided a generic From<T> method that leverages source-generated property access for types marked with [GenerateArguments]. [1] [2] [3] [4] [5]

Dependency and Build System Updates:

  • Added new NuGet dependencies: Microsoft.CodeAnalysis.Analyzers and Microsoft.CodeAnalysis.CSharp to support source generator functionality.

  • Updated build properties to enable interceptors for the new generated code.

Fixes #15671

@github-actions
Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

Refactored the Arguments utility and related types to return shared static empty instances instead of allocating new NamedEnumerable<T> objects for empty argument/name collections. Updated INamedEnumerable<T> to inherit from IReadOnlyCollection<T> and added a Count property for efficient checks. Introduced Arguments<T>.Empty for strongly-typed empty instances. Adjusted constructors and From methods to use arrays directly and return empty instances when appropriate. Improved parameter checks in ShapeFactoryExtensions for robustness and type safety. Suppressed CA1710 warning for INamedEnumerable<T>.
Introduced ArgumentsFromInterceptor source generator to optimize Arguments.From<T>() calls with anonymous types using C# interceptors, eliminating reflection overhead. Added comprehensive documentation covering usage, migration, and troubleshooting. Updated project files to support generated interceptor code. Added tests for interceptor and source generation paths.
@gvkries gvkries marked this pull request as ready for review February 16, 2026 16:49
…perty-based argument access. Updates source generator, Arguments.From, and tests to use the new pattern. Improves performance by avoiding array allocations and simplifies the property access model.
@gvkries
Copy link
Member Author

gvkries commented Feb 20, 2026

I added some small benchmarks, here are the result on my machine:

| Method                                           | Mean         | Error     | StdDev     | Gen0   | Allocated |
|------------------------------------------------- |-------------:|----------:|-----------:|-------:|----------:|
| 'Generated - Create'                             |     5.365 ns | 0.1578 ns |  0.1550 ns | 0.0024 |      40 B |
| 'Generated - Enumerate All'                      |    22.171 ns | 0.2657 ns |  0.2485 ns | 0.0052 |      88 B |
| 'Generated - Access By Name (1 property)'        |    18.105 ns | 0.3292 ns |  0.3079 ns | 0.0038 |      64 B |
| 'Generated - Access By Name (all 3 properties)'  |    47.272 ns | 0.4085 ns |  0.3621 ns | 0.0067 |     112 B |
| 'Reflection - Create'                            |    72.154 ns | 0.5639 ns |  0.5274 ns | 0.0110 |     184 B |
| 'Reflection - Enumerate All'                     |   130.163 ns | 2.6200 ns |  3.0172 ns | 0.0129 |     216 B |
| 'Reflection - Access By Name (1 property)'       |   264.390 ns | 1.4693 ns |  1.3744 ns | 0.0448 |     752 B |
| 'Reflection - Access By Name (all 3 properties)' |   311.164 ns | 6.2543 ns | 12.3454 ns | 0.0448 |     752 B |
| 'Anonymous - Create'                             |    43.130 ns | 0.7978 ns |  0.7072 ns | 0.0134 |     224 B |
| 'Anonymous - Enumerate All'                      |    60.903 ns | 1.0290 ns |  0.9625 ns | 0.0153 |     256 B |
| 'Anonymous - Access By Name (1 property)'        |   242.234 ns | 3.2552 ns |  3.0449 ns | 0.0491 |     824 B |
| 'Anonymous - Access By Name (all 3 properties)'  |   258.396 ns | 3.1335 ns |  2.7777 ns | 0.0491 |     824 B |
| 'Generated (10 props) - Create and Access 2'     |    42.862 ns | 0.4499 ns |  0.3989 ns | 0.0076 |     128 B |
| 'Reflection (10 props) - Create and Access 2'    | 1,370.329 ns | 7.3737 ns |  6.8974 ns | 0.1278 |    2152 B |
| 'Generated (10 props) - Lookup All By Name'      |   348.518 ns | 1.8103 ns |  1.5117 ns | 0.0272 |     456 B |

@gvkries
Copy link
Member Author

gvkries commented Feb 20, 2026

I removed the extra IArgumentProvider, the source generator is now directly implementing INamedEnumerable<T>.

Copy link
Member

@hishamco hishamco left a comment

Choose a reason for hiding this comment

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

Incredible!!

Finally, we have a source generator in OC, after my first attempt to generate the resources manifest

The only thing I need to change is move the docs outside the modules.

@gvkries
Copy link
Member Author

gvkries commented Feb 20, 2026

Sure, any suggestion? Also for the project itself, should it stay there?

@hishamco
Copy link
Member

The project should be there, IMHO, same as what I did a long time ago. We need to change the docs location

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.

Replace Arguments.From by source code generation.

3 participants