Skip to content

Generate explicit IShape wrappers for display drivers#19087

Open
Copilot wants to merge 10 commits intomainfrom
copilot/replace-castle-core-with-code-generation
Open

Generate explicit IShape wrappers for display drivers#19087
Copilot wants to merge 10 commits intomainfrom
copilot/replace-castle-core-with-code-generation

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 29, 2026

Description

CreateAsync<TModel> for non-IShape models currently relies on Castle class proxies plus a ShapeViewModel mixin. This change adds a source-generated wrapper path for supported call sites while preserving the existing Castle proxy behavior as the fallback.

  • Shape creation

    • Keep direct construction for types that already implement IShape.
    • Keep Castle proxy generation in ShapeFactoryExtensions as the runtime fallback when no generated wrapper path is available.
    • Use generated wrapper types for intercepted strongly typed CreateAsync<TModel...> call sites instead of routing runtime activation through a generated-type registry.
    • Extend generated wrapper coverage to display-driver-created shapes by intercepting DisplayDriverBase.Initialize<TModel>(...) call sites in consuming projects.
  • Source generation

    • Add a Roslyn generator that finds relevant CreateAsync<TModel...> call sites for non-IShape classes.
    • Generate wrapper subclasses that implement IShape/IPositioned directly, mimicking ShapeViewModel behavior without allocating an internal ShapeViewModel instance.
    • Generate interceptor methods for concrete CreateAsync<TModel...> and display-driver Initialize<TModel>(...) call sites so wrappers can be instantiated directly without a registry.
    • Handle member name conflicts in generated wrappers by emitting new only when the model type or its base types already define conflicting members.
  • Dependency cleanup

    • Keep the Castle.Core dependency because it remains the fallback path for non-generated scenarios.
  • Coverage

    • Add focused tests for:
      • strongly typed shape creation returning generated subclasses for intercepted call sites
      • generated wrappers exposing IShape members directly with the same behavior as ShapeViewModel
      • Castle fallback behavior when a shape type is not covered by the generated path
      • display-driver Initialize<TModel>(...) creating generated strongly typed wrappers
      • generated wrappers no longer embedding a ShapeViewModel field
var shape = await factory.CreateAsync<TestShapeViewModel>(model =>
{
    model.Title = "Generated";
});

Assert.IsAssignableFrom<TestShapeViewModel>(shape);
Assert.Equal("Generated", shape.Title);
Assert.Equal(shape.Metadata, ((IShape)shape).Metadata);

Testing

  • dotnet test --project test/OrchardCore.Tests/OrchardCore.Tests.csproj --filter-class "OrchardCore.Tests.DisplayManagement.ShapeFactoryTests"

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Replace Castle.Core with source code generation for CreateShape Replace Castle shape proxies with generated shape wrappers Mar 29, 2026
Copilot AI requested a review from gvkries March 29, 2026 16:27
@gvkries
Copy link
Copy Markdown
Member

gvkries commented Mar 30, 2026

@copilot Generating shape proxies with Castle must be kept as a fallback. Additionally, avoid using a registry for generated types, if possible.

Copilot AI and others added 2 commits March 30, 2026 09:05
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 30, 2026

@copilot Generating shape proxies with Castle must be kept as a fallback. Additionally, avoid using a registry for generated types, if possible.

Addressed in dcf71ba. The runtime Castle proxy path is restored as the fallback in CreateStronglyTypedShape, and the generated path now uses interceptor-generated wrappers at concrete CreateAsync<TModel...> call sites instead of a runtime registry.

Copilot AI changed the title Replace Castle shape proxies with generated shape wrappers Generate shape wrappers with Castle fallback Mar 30, 2026
@gvkries
Copy link
Copy Markdown
Member

gvkries commented Mar 30, 2026

@copilot Simplify the source generator by only intercepting calls to public static ValueTask<IShape> CreateAsync<TModel, TState>(this IShapeFactory factory, string shapeType, Func<TModel, TState, ValueTask> initializeAsync, TState state) where TModel : class in the ShapeFactoryExtensions class. This should be enough, because all other calls end up calling this extension method.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 30, 2026

@copilot Simplify the source generator by only intercepting calls to `public static ValueTask CreateAsync<TModel, TState>(this IShapeFactory factory, string shapeType, Func<TModel, TSt...

I investigated this path, but simplifying the generator to only intercept that CreateAsync<TModel, TState>(..., string, Func<TModel, TState, ValueTask>, TState) overload does not currently work with the registry-less approach. The only call sites to that overload in the repo are the open-generic forwarding calls inside ShapeFactoryExtensions, so the generator no longer sees a concrete TModel to emit a wrapper for and the flow falls back to Castle. I reverted that experiment and kept the branch in the last passing state. If you want, I can either keep the broader concrete-call-site interception, or explore a different runtime lookup approach for generated wrappers.

@gvkries
Copy link
Copy Markdown
Member

gvkries commented Mar 30, 2026

@copilot Can you intercept the nested static ValueTask<IShape> ShapeFactory(Func<TModel, TState, ValueTask> init, TState state) method at line 153 instead?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 30, 2026

@copilot Can you intercept the nested static ValueTask<IShape> ShapeFactory(Func<TModel, TState, ValueTask> init, TState state) method at line 153 instead?

I checked that path, but intercepting the nested ShapeFactory(...) local function would have the same limitation: the generator only sees that one open-generic call inside ShapeFactoryExtensions, not the concrete consumer call sites that provide the actual TModel types. Since OrchardCore.SourceGenerators runs as an analyzer in the consuming project, the current concrete call-site interception is what lets it generate wrapper subclasses without a registry.

@gvkries
Copy link
Copy Markdown
Member

gvkries commented Mar 30, 2026

@copilot The issue is that all calls to the shape factory (extensions) via display drivers are not detected and no source gets generated for all the shapes creating by them. We need to find a better approach, e.g. try to refactor the ShapeFactoryExtensions so that it's easier to intercept. Or you may need to intercept calls to the display drivers Initialize or Factory methods as well. Check what the best approach is.

Copilot AI and others added 2 commits March 30, 2026 12:37
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 30, 2026

@copilot The issue is that all calls to the shape factory (extensions) via display drivers are not detected and no source gets generated for all the shapes creating by them. We need to find a ...

Addressed in 51b3407. I extended the generator to intercept display-driver Initialize<TModel>(...) call sites directly, so strongly typed shapes created through display drivers now get generated wrappers as well, while Castle remains the fallback for non-generated paths.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • learn.microsoft.com
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Generate shape wrappers with Castle fallback Generate shape wrappers for display drivers with Castle fallback Mar 30, 2026
@gvkries
Copy link
Copy Markdown
Member

gvkries commented Mar 30, 2026

@copilot The generated shapes should not allocate an ShapeViewModel internally, implement IShape on them explicitly. Mimic the existing implementation of ShapeViewModel for that.

Copilot AI and others added 2 commits March 30, 2026 13:40
Agent-Logs-Url: https://github.com/OrchardCMS/OrchardCore/sessions/679b3934-3f02-46c7-952a-88927a1f8e84

Co-authored-by: gvkries <7062785+gvkries@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OrchardCMS/OrchardCore/sessions/679b3934-3f02-46c7-952a-88927a1f8e84

Co-authored-by: gvkries <7062785+gvkries@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 30, 2026

@copilot The generated shapes should not allocate an ShapeViewModel internally, implement IShape on them explicitly. Mimic the existing implementation of ShapeViewModel for that.

Addressed in 936f670. The generated wrappers now implement the IShape state directly and mirror ShapeViewModel behavior without allocating an internal ShapeViewModel instance.

Copilot AI changed the title Generate shape wrappers for display drivers with Castle fallback Generate explicit IShape wrappers for display drivers with Castle fallback Mar 30, 2026
@gvkries gvkries marked this pull request as ready for review March 30, 2026 15:04
Previously, sorting was triggered whenever the _sorted flag was false, even if the _items list was empty. Now, sorting only occurs if there are items in the list, preventing unnecessary operations on empty collections.
@gvkries gvkries changed the title Generate explicit IShape wrappers for display drivers with Castle fallback Generate explicit IShape wrappers for display drivers Mar 31, 2026
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.

Castle.Core could be replaced by source code generation

2 participants