Skip to content

Call data portal operation methods explicitly via bundled source generators - #4925

Open
rockfordlhotka wants to merge 11 commits into
mainfrom
feature/explicit-operation-method-calls
Open

rockfordlhotka wants to merge 11 commits into
mainfrom
feature/explicit-operation-method-calls

Conversation

@rockfordlhotka

Copy link
Copy Markdown
Member

Closes #4359

Data portal operation methods ([Fetch], [FetchChild], …) are private and invoked dynamically, so IDEs gray them out and the trimmer can remove them. This PR makes the CSLA source generators part of the Csla package, makes every operation method statically referenced through a generated interface, and adds opt-in strongly typed data portal extension methods.

Generators bundled in the Csla package

  • The DataPortalInterfaces generator and its tests are merged into the AutoImplementProperties generator project, which is now packed into the Csla nupkg (analyzers/dotnet/cs) instead of shipping as its own package.
  • [CslaImplementProperties], [CslaImplementPropertiesInterface<T>] and [CslaIgnoreProperty] move into Csla.dll; the Attributes project and the Microsoft.Bcl.HashCode dependency are removed.
  • buildTransitive/Csla.targets raises CSLABUILD001 when a project still references the retired Csla.Generator.AutoImplementProperties.CSharp package.

Always-generated operations interface (server side)

  • For every partial class declaring operation methods (abstract classes included), the generator emits an internal nested IDataPortalOperations interface with one explicitly implemented member per operation method, named with the operation name (e.g. Fetch__Int32). Concrete classes also get IDataPortalOperationMapping / IDataPortalOperationNamedMapping implementations that call through the interface.
  • Discovery now uses ForAttributeWithMetadataName with equatable models grouped by type, so partial classes spread across files work and the pipeline caches correctly.
  • Fixes found along the way:
    • inject parameter order
    • invalid identifiers for arrays and tuples
    • nullable value types
    • InjectAttribute subclasses and keyed services
    • ValueTask
    • generic criteria
    • sync/async parity with reflection dispatch
    • overloads that share an operation name, with a CSLADP002 info diagnostic
  • Csla's abstract base classes that declare operation methods are now partial.

Pre-resolved invokers (runtime)

  • New IDataPortalOperationInvoker<T> and IChildDataPortalOperationInvoker<T> ([EditorBrowsable(Never)]), implemented by DataPortal<T>. They take a generated operation name and RunLocal flag, so the client skips reflection-based method lookup.
  • ChildDataPortal and DataPortalTarget pass operation names through for CreateChild/FetchChild named dispatch.
  • When the invoker receives a single covariant array criteria value such as string[], it is no longer split into separate criteria values.

[DataPortalExtensions] generator (client side)

  • Opt-in per class via [DataPortalExtensions(Prefix = "...")]. It generates sync and async extension methods on IDataPortal<T> (Create/Fetch/Execute/Delete) and IChildDataPortal<T> (CreateChild/FetchChild).
  • [Inject] parameters are omitted and default values are preserved. The extension method is internal when the business type or a parameter type is internal.
  • Each method falls back to the existing params methods for custom or mock IDataPortal<T> implementations.
  • Warnings are reported for:
    • names that IDataPortal<T> instance methods would hide
    • duplicate signatures
    • private parameter types
    • generic, abstract or non-CSLA types
  • [NoDataPortalExtension] excludes a method.
  • Adapted from Stefan Ossendorf's MIT-licensed Csla.DataPortalExtensions, with attribution headers and a third-party notice in the release notes.

Analyzers

  • CSLA0024 (Warning): type with data portal operation methods should be partial, plus a code fix that adds partial to the type and its containing types.
  • CSLA0025 (Warning): use the generated data portal extension method instead of IDataPortal<T> methods with untyped criteria.

Tests and verification

  • Generator snapshot tests for the operations and extensions generators, plus stage-caching tests for both.
  • csla.netcore.test integration tests:
    • every generated root and child operation, sync and async, asserting that named dispatch was used
    • [RunLocal] bypassing the configured proxy
    • fallback for a custom IDataPortal<T>
    • sync/async exception parity with reflection
    • generated interface member names matching DataPortalOperationNameHelper
  • Analyzer tests for CSLA0024 (and its code fix) and CSLA0025.
  • dotnet build Source\csla.test.sln and the full test run pass.
  • Packaging was checked with a scratch consumer project using the local nupkg:
    • generated code works
    • the retired-package guard fires
    • IDE0051 no longer fires on private operation methods
    • a trimmed publish keeps the private operation method and generated interface, with no trim warnings about them

Not in this PR

  • Samples/ still reference the retired generator package. They pin CSLA 10 packages, so the reference should be removed when they move to CSLA 11.
  • Follow-up items from the plan: AutoSerialization packaging, Execute(T command) extensions, named dispatch for Update-family child operations, the existing sync Delete and IChildDataPortal.CreateChildAsync bugs, and the inject analyzer.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Dn4aKN7kqcG78x1HBWApk3

rockfordlhotka and others added 5 commits September 13, 2026 17:44
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dn4aKN7kqcG78x1HBWApk3
…enerated operations interface

- Merge the DataPortalInterfaces generator and its tests into the
  AutoImplementProperties generator project; move the AutoImplement
  attributes into Csla core and drop the Microsoft.Bcl.HashCode dependency
- Pack the generator into the Csla nupkg (analyzers/dotnet/cs) and add a
  buildTransitive guard against the retired standalone package
- Rewrite discovery with ForAttributeWithMetadataName, grouped by type
  with equatable models; emit an internal nested IDataPortalOperations
  interface for every partial class (abstract classes included)
- Fix inject parameter order, identifier sanitizing, nullable value
  types, inject detection, ValueTask, generic criteria, sync/async parity,
  keyed services, and duplicate operation names
- Make Csla abstract base classes partial

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dn4aKN7kqcG78x1HBWApk3
…xtension usage analyzer

CSLA0024 warns when a class declaring data portal operation methods, or
any of its containing types, is not partial; the code fix adds partial to
the class and all non-partial containing types.

CSLA0025 warns when untyped criteria-based IDataPortal<T>,
IChildDataPortal<T>, or DataPortal<T> methods are called for a business
type marked with [DataPortalExtensions]. Generated code is skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dn4aKN7kqcG78x1HBWApk3
…generator

- Add IDataPortalOperationInvoker<T> and IChildDataPortalOperationInvoker<T>,
  implemented by DataPortal<T>, which take a generated operation name and
  RunLocal flag so the client skips reflection-based method lookup
- Flow operation names through ChildDataPortal and DataPortalTarget for
  CreateChild/FetchChild named dispatch
- Wrap a single covariant array criteria value so it is not split
- Add the DataPortalExtensions generator (adapted from Stefan Ossendorf's
  MIT Csla.DataPortalExtensions) emitting sync and async extensions on
  IDataPortal<T>/IChildDataPortal<T> with fallback for custom portals
- Add generator snapshot, stage caching, integration and name consistency
  tests; update release notes and docs

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dn4aKN7kqcG78x1HBWApk3
The Update form matched only inner (per-framework) builds, so the outer
pack build omitted the retired-package guard from the nupkg.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dn4aKN7kqcG78x1HBWApk3

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.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Bundles CSLA source generators into Csla, adds generated data portal operation dispatch, and introduces strongly typed data portal extensions.

Changes:

  • Merges operation and extension generators into the bundled generator package.
  • Adds generated interfaces, named dispatch, invoker APIs, and trim-friendly operation references.
  • Adds analyzers, packaging guards, and extensive generator/integration tests.
File summaries
File Description
Source/tests/Csla.test/Csla.Tests.csproj Updated as part of this pull request.
Source/tests/csla.netcore.test/csla.netcore.test.csproj Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/TestHelper.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.SingleFetchWithCriteria#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.SingleCreateNoParams#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.RootAndChildAttributes#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.OperationWithInjectParam#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.OperationWithInjectAllowNull#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.NestedClass#TestApp.Outer.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.MultipleOverloads#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.FileScopedNamespace#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.ExecuteCommand#TestApp.MyCommand.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.DeleteWithCriteria#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Helpers/Snapshots/DataPortalInterfaceGeneratorTests.AsyncMethod#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests/Csla.Generator.DataPortalInterfaces.CSharp.Tests.csproj Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.MyTestMethod#BOTest.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ushort[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ushort[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ushort-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ushort_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ulong[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ulong[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ulong-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=ulong_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=uint[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=uint[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=uint-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=uint_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=string[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=string[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=string-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=string_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=short[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=short[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=short-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=short_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=sbyte[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=sbyte[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=sbyte-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=sbyte_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=object[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=object[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=object-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=object_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=long[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=long[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=long-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=long_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=int[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=int[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=int-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=int_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=float[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=float[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=float-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=float_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=double[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=double[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=double-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=double_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=decimal[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=decimal[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=decimal-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=decimal_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=char[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=char[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=char-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=char_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=byte[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=byte[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=byte-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=byte_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=bool[]-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=bool[]_isNullable=True#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=bool-_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Snapshots/AutoImplementPropertiesGeneratorTests.Case09_type=bool_isNullable=False#BusinessBaseTestClass.g.received.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.SingleFetchWithCriteria#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.SingleCreateNoParams#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.RootAndChildAttributes#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.PartialClassAcrossFiles#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.ParamsArrayAndKeywordNames#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.OperationWithInjectParam#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.OperationWithInjectAllowNull#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.NestedClass#TestApp.Outer.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.MultipleOverloads#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.GenericBusinessObject#TestApp.Lookup_1.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.FileScopedNamespace#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.ExistingNamedMapping#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.ExecuteCommand#TestApp.MyCommand.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.DuplicateOperationName#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.DuplicateOperationName.verified.txt Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.DerivedFromBaseWithOperations#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.DerivedFromBaseWithOperations#TestApp.PersonBase_1.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.DeleteWithCriteria#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.AsyncMethod#TestApp.PersonEdit.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalOperations/Snapshots/DataPortalOperationsGeneratorTests.AbstractClass#TestApp.PersonBase_1.DataPortalOperations.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.Prefix#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.ParameterAccessibility#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.ParameterAccessibility.verified.txt Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.NoDataPortalExtension#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.NestedType#TestApp.Outer.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.InvalidTargets.verified.txt Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.InternalType#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.HiddenNames#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.HiddenNames.verified.txt Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.GenericTypeSkipped.verified.txt Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.ExecuteCommand#TestApp.RecalculateCommand.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.DuplicateExtension#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.DuplicateExtension.verified.txt Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.DefaultValues#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.ChildOperations#TestApp.LineItem.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/DataPortalExtensions/Snapshots/DataPortalExtensionsGeneratorTests.ArrayCriteria#TestApp.PersonEdit.DataPortalExtensions.g.verified.cs Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.Tests/Csla.Generator.AutoImplementProperties.CSharp.Tests.csproj Updated as part of this pull request.
Source/tests/Csla.Generator.AutoImplementProperties.CSharp.TestObjects/Csla.Generator.AutoImplementProperties.TestObjects.csproj Updated as part of this pull request.
Source/Csla/Server/DataPortalTarget.cs Updated as part of this pull request.
Source/Csla/ReadOnlyListBase.cs Updated as part of this pull request.
Source/Csla/ReadOnlyBindingListBase.cs Updated as part of this pull request.
Source/Csla/ReadOnlyBase.cs Updated as part of this pull request.
Source/Csla/NoDataPortalExtensionAttribute.cs Updated as part of this pull request.
Source/Csla/NameValueListBase.cs Updated as part of this pull request.
Source/Csla/DynamicListBase.cs Updated as part of this pull request.
Source/Csla/DynamicBindingListBase.cs Updated as part of this pull request.
Source/Csla/DataPortalExtensionsAttribute.cs Updated as part of this pull request.
Source/Csla/CslaImplementPropertiesInterfaceAttribute.cs Updated as part of this pull request.
Source/Csla/CslaImplementPropertiesAttribute.cs Updated as part of this pull request.
Source/Csla/CslaIgnorePropertyAttribute.cs Updated as part of this pull request.
Source/Csla/Csla.csproj Updated as part of this pull request.
Source/Csla/CommandBase.cs Updated as part of this pull request.
Source/Csla/BusinessListBase.cs Updated as part of this pull request.
Source/Csla/BusinessDocumentBase.cs Updated as part of this pull request.
Source/Csla/BusinessBindingListBase.cs Updated as part of this pull request.
Source/Csla/buildTransitive/Csla.targets Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Extractors/GenerationResults.cs Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Extractors/ExtractedTypeDefinition.cs Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Extractors/ExtractedOperationParameter.cs Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Extractors/ExtractedOperationMethod.cs Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Extractors/ExtractedContainerDefinition.cs Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Discovery/DefinitionExtractionContext.cs Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Discovery/ContainerDefinitionsExtractor.cs Updated as part of this pull request.
Source/Csla.Generators/cs/DataPortalInterfaces/Csla.Generator.DataPortalInterfaces.CSharp/Csla.Generator.DataPortalInterfaces.CSharp.csproj Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/readme.md Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/tools/uninstall.ps1 Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/tools/install.ps1 Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/Internals/EquatableArray.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/TrackingNames.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/Models/OperationTypeModel.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/Models/OperationParameterModel.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/Models/OperationMethodModel.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/Models/LocationInfo.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalExtensions/ExtensionTypeModel.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/Csla.Generator.AutoImplementProperties.CSharp.csproj Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/AutoImplement/ExtractedTypeDefinition.cs Updated as part of this pull request.
Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.Attributes.CSharp/Csla.Generator.AutoImplementProperties.Attributes.CSharp.csproj Updated as part of this pull request.
Source/Csla.Benchmarks/Csla.Benchmarks.csproj Updated as part of this pull request.
Source/Csla.Analyzers/Csla.Analyzers/UseGeneratedDataPortalExtensionAnalyzerConstants.cs Updated as part of this pull request.
Source/Csla.Analyzers/Csla.Analyzers/TypeWithOperationsShouldBePartialAnalyzerConstants.cs Updated as part of this pull request.
Source/Csla.Analyzers/Csla.Analyzers/TypeWithOperationsShouldBePartialAddPartialCodeFix.cs Updated as part of this pull request.
Source/Csla.Analyzers/Csla.Analyzers/Properties/Resources.resx Updated as part of this pull request.
Source/Csla.Analyzers/Csla.Analyzers/Properties/Resources.Designer.cs Updated as part of this pull request.
Source/Csla.Analyzers/Csla.Analyzers/Constants.cs Updated as part of this pull request.
Source/Csla.Analyzers/Csla.Analyzers/AnalyzerReleases.Unshipped.md Updated as part of this pull request.
docs/code-generators.md Updated as part of this pull request.
docs/analyzers/index.md Updated as part of this pull request.
docs/analyzers/CSLA0025-UseGeneratedDataPortalExtensionAnalyzer.md Updated as part of this pull request.
docs/analyzers/CSLA0024-TypeWithOperationsShouldBePartialAnalyzer.md Updated as part of this pull request.
Review details

Files not reviewed (1)

  • Source/Csla.Analyzers/Csla.Analyzers/Properties/Resources.Designer.cs: Generated file

Suppressed comments (6)

Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalExtensions/DataPortalExtensionsBuilder.cs:133

  • Prefix is an arbitrary string attribute property, but it is concatenated directly into the generated method name here. A value such as Prefix = "My-" or Prefix = "123" produces invalid C# and breaks the consumer build; validate the prefix as an identifier fragment and report a diagnostic (or otherwise escape/reject it) before emitting these methods.
        foreach (var isAsync in new[] { true, false })
        {
          var name = type.Prefix + baseName + (isAsync ? "Async" : string.Empty);
          if (type.HiddenNames.Contains(name))

Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalExtensions/DataPortalExtensionsBuilder.cs:135

  • This check uses a union of the IDataPortal<T> and IChildDataPortal<T> member names for every generated extension. As a result, a root operation method named CreateChild (or a child operation method named Fetch) is suppressed even though that name is only an instance member of the other receiver interface, so a valid extension is never emitted. Keep the hidden-name sets receiver-specific and select the set for isChild.
          if (type.HiddenNames.Contains(name))
          {
            diagnostics.Add(Diagnostic(DataPortalOperationsDiagnostics.ExtensionNameHiddenId, method.Location, name, type.TypeName, $"{receiverInterface}<T>.{name}"));

Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/DataPortalOperationsBuilder.cs:295

  • The generated interface implementation invokes the operation method directly, bypassing ServiceProviderMethodCaller.CallMethodTryAsync/LateBoundObject.CallMethodTryAsyncDI, which wrap operation and injection failures in CallMethodException. Reflection dispatch and DataPortalExceptionHandler rely on that wrapper, so generated dispatch changes the exception type/inner-exception shape and can bypass exception-inspector behavior. Route generated calls through a shared wrapper or reproduce the same wrapping semantics.
      var call = $"{OperationsLocal}.{memberName}({string.Join(", ", arguments)})";
      writer.WriteLine(method.IsAsync ? $"await {call}.ConfigureAwait(false);" : $"{call};");

Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/DataPortalOperationsDiagnostics.cs:66

  • This diagnostic is configured with DiagnosticSeverity.Error, so a private operation parameter makes the consumer build fail rather than merely reporting a warning. The PR description lists private parameter types among the warning cases; unless a build-breaking diagnostic is intentional, this should be Warning (or the description should explicitly call out the breaking behavior).
    public static readonly DiagnosticDescriptor InaccessibleParameterType = new(
      id: InaccessibleParameterTypeId,
      title: "Operation parameter type is not accessible",
      messageFormat: "The data portal extension method for '{0}' cannot be generated because parameter '{1}' has type '{2}', which is not accessible outside its containing type",
      category: Category,
      defaultSeverity: DiagnosticSeverity.Error,
      isEnabledByDefault: true);

Source/Csla/DataPortalT.cs:610

  • The special case deliberately excludes an exact object[], but a generated extension can legally have a single criterion typed as object or IEnumerable<object> and receive an object[] value. That value is then returned as the criteria object and GetCriteriaArray expands it into multiple criteria values, so the generated one-parameter operation cannot be dispatched (and reflection may select the wrong shape). The pre-resolved path should preserve any single array value as one criterion, including an exact object[].
    Source/Csla/Server/DataPortalTarget.cs:220
  • The child dispatch fallback has the same exception ambiguity as the root path: an operation that throws DataPortalOperationNotSupportedException is treated as an unsupported generated match and is invoked again through reflection. That changes exception/side-effect behavior for child operations; only the generated 'not matched' result should trigger fallback.
  • Files reviewed: 200/204 changed files
  • Comments generated: 10
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 199 to 200
object?[] parameters = criteriaArray!;
await CallMethodTryAsyncDI<T>(isSync, parameters).ConfigureAwait(false);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 77e2158, in the generated code instead of DataPortalTarget. The generated dispatch now wraps the operation call in try/catch and rethrows via DataPortalOperationHelper.CreateCallMethodException, producing the same CallMethodException as reflection dispatch. A DataPortalOperationNotSupportedException thrown by an operation now arrives wrapped, so the fallback catch blocks (root and child, both lines) never see it and the operation is not run again. Only the generated "not matched" throw reaches those blocks. The XML docs on IDataPortalOperationMapping and IDataPortalOperationNamedMapping now state this contract for hand-written implementations. New integration tests: FetchAsync_OperationThrowsNotSupported_IsNotInvokedAgain (operation runs exactly once) and FetchAsync_OperationThrows_WrapsExceptionLikeReflectionDispatch (exception chain matches the reflection path).

Comment thread Source/Csla/buildTransitive/Csla.targets
- Wrap exceptions from generated operation calls in CallMethodException,
  matching reflection dispatch; an operation that throws
  DataPortalOperationNotSupportedException is no longer invoked again
- Bind explicit in criteria to a local in named dispatch
- Resolve Nullable<T> injected services by their declared type and treat
  nullable inject parameters as optional, as reflection does
- Escape keyword extension names, validate the Prefix (CSLADP008), and
  choose receiver/invoker names that cannot collide with parameters
- Check hidden extension names against the receiving portal interface only
- Report inaccessible parameter types as a warning
- Keep a single object[] criteria value as one criterion in the invokers
- CSLA0025 only reports when a matching extension method is generated

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6AuArAiFMmEoBmSNbD8YG
@rockfordlhotka

Copy link
Copy Markdown
Member Author

Responses to the suppressed comments in the Copilot review summary (all addressed in 77e2158):

  • DataPortalExtensionsBuilder.cs:133 — arbitrary Prefix: Fixed. A prefix that isn't a valid C# identifier now reports the new CSLADP008 warning and no extensions are generated for the type. CSLA0025 also skips such types. Covered by the InvalidPrefix test.
  • DataPortalExtensionsBuilder.cs:135 — union of hidden names: Fixed. Hidden names are kept per receiver interface; see the inline reply.
  • DataPortalOperationsBuilder.cs:295 — no CallMethodException wrapping: Fixed. Generated calls are wrapped via the new DataPortalOperationHelper.CreateCallMethodException, so the exception shape (and DataPortalExceptionHandler inspection) matches reflection dispatch. Service resolution stays outside the wrapper, as in reflection. FetchAsync_OperationThrows_WrapsExceptionLikeReflectionDispatch compares the exception chain of both paths.
  • DataPortalOperationsDiagnostics.cs:66 — CSLADP006 severity: Fixed. It is now a Warning, consistent with the other skipped-extension diagnostics and with the PR description.
  • DataPortalT.cs:610 — exact object[] single criterion: Fixed. Any single reference-type array value, including an exact object[], is now wrapped so it stays one criterion. FetchAsync_ObjectArrayForSingleObjectCriteria_IsNotSplit passes an object[] to an operation taking object through named dispatch.
  • DataPortalTarget.cs:220 — child fallback ambiguity: Fixed by the same generated-code wrapping as the root path. Operation exceptions reach DataPortalTarget as CallMethodException, so only the generated "not matched" exception triggers fallback.

dotnet build Source\csla.test.sln and the full CI test run (--filter TestCategory!=SkipOnCIServer --settings Source/test.runsettings) pass.

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.

🟡 Changes recommended

Unresolved critical and moderate correctness issues remain in generated dispatch, extension generation, and analyzers.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

  • Source/Csla.Analyzers/Csla.Analyzers/Properties/Resources.Designer.cs: Generated file

Suppressed comments (6)

Source/Csla.Analyzers/Csla.Analyzers/TypeWithOperationsShouldBePartialAnalyzer.cs:115

  • This analyzer matches only exact attribute symbols, while CSLA operation attributes are extensible and runtime discovery accepts derived attributes. A non-partial type using a custom FetchAttribute subclass therefore gets no CSLA0024 warning/code fix, even though the operations generator also needs a partial declaration to preserve the method. Match the attribute inheritance chain consistently with runtime discovery.
    private static bool HasOperationMethods(INamedTypeSymbol typeSymbol, HashSet<INamedTypeSymbol> operationAttributes)
    {
      foreach (var member in typeSymbol.GetMembers())
      {
        if (member is IMethodSymbol method)
        {
          foreach (var attribute in method.GetAttributes())
          {
            if (attribute.AttributeClass is not null && operationAttributes.Contains(attribute.AttributeClass))
            {

Source/Csla.Analyzers/Csla.Analyzers/UseGeneratedDataPortalExtensionAnalyzer.cs:236

  • The analyzer checks only the criteria count and never verifies argument conversions against the generated overload. For a business type with only [Fetch] Fetch(int id), a valid untyped call such as portal.FetchAsync("x") is reported as CSLA0025 even though the generated Fetch extension requires an int and cannot replace that call. Match the invocation's argument conversions to the selected operation parameters before reporting.
        if (criteriaCount is { } count)
        {
          var required = criteria.Count(p => !p.HasExplicitDefaultValue && !p.IsParams);
          var hasParams = criteria.Count > 0 && criteria[criteria.Count - 1].IsParams;
          if (count < required || (!hasParams && count > criteria.Count))

Source/Csla.Analyzers/Csla.Analyzers/UseGeneratedDataPortalExtensionAnalyzer.cs:242

  • This return true path does not apply the extension generator's collision selection. For example, if the selected method for a shared operation name has [NoDataPortalExtension], SelectMethods skips that winner and also skips its collision losers, so no extension is generated; this analyzer still finds the non-annotated loser and reports CSLA0025. Mirror the generator's collision/signature filtering before claiming that a replacement extension exists.
        return true;

Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/DataPortalOperationsBuilder.cs:279

  • Injected services are resolved while building the argument list, before the try that wraps the operation call. A missing required service, unsupported keyed provider, or other resolution failure therefore escapes as its original exception, whereas reflection-based dispatch resolves DI inside CallMethodTryAsync and the surrounding LateBoundObject path wraps the failure in CallMethodException. Include generated argument/service resolution in the same wrapping path so generated and reflection dispatch preserve the same exception contract.
        if (parameter.IsInjected)
        {
          var variable = $"__c{candidateIndex}i{injectIndex++}";
          writer.WriteLine($"var {variable} = {FormatServiceResolution(parameter)};");
          arguments.Add(ArgumentPrefix(parameter) + variable);

Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/IncrementalDataPortalOperationsGenerator.cs:65

  • ForAttributeWithMetadataName only supplies methods carrying the exact built-in attribute metadata name. The runtime resolver accepts derived operation attributes (GetCustomAttributes<T>(true)), so a valid custom attribute derived from FetchAttribute/another operation attribute is invoked by reflection but receives no generated interface or mapping here; its private method can still be trimmed. Discover derived operation attributes as well, or explicitly align the runtime/analyzer contract to reject them.
        var entries = context.SyntaxProvider.ForAttributeWithMetadataName(
            OperationDiscovery.GetAttributeMetadataName(kind),
            predicate: static (node, _) => node is MethodDeclarationSyntax,
            transform: (ctx, ct) => OperationDiscovery.ExtractMethod(ctx, kind, ct))

Source/Csla.Generators/cs/AutoImplementProperties/Csla.Generator.AutoImplementProperties.CSharp/DataPortalOperations/OperationDiscovery.cs:441

  • The generated keyed-service expression loses the runtime type of small integral keys. For example, [Inject(Key = (short)1)] stores an Int16 key in the attribute, but FormatPrimitive emits 1, which is boxed as Int32 when passed to GetKeyedService; keyed lookup then misses a registration made with (short)1. Preserve the primitive type with explicit casts for short, ushort, byte, and sbyte (and add a test for a non-int key).
      int or short or ushort or byte or sbyte => Convert.ToString(value, CultureInfo.InvariantCulture),
  • Files reviewed: 206/210 changed files
  • Comments generated: 3
  • Review effort level: Lite

MetadataName = OperationDiscovery.GetFullMetadataName(type),
Namespace = @namespace,
FullyQualifiedName = type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat),
ExtensionClassName = string.Concat(containers.Select(c => c.Name + "_")) + type.Name + "DataPortalExtensions",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 4c411d4, though not by renaming. The generated class is static partial, so Outer_InnerDataPortalExtensions from a top-level Outer_Inner and from a nested Outer.Inner merge into one class instead of being duplicate definitions. Their methods don't clash, because the receivers are IDataPortal<Outer_Inner> and IDataPortal<Outer.Inner>. The one real conflict was accessibility: a public part and an internal part fail with CS0262. Now only a public business type puts public on its part; other parts omit the modifier, so the parts always agree. Each method still gets public or internal from its business type. The new ExtensionClassNameCollision test (public Outer_Inner + internal Outer.Inner) compiles with no errors. I kept the naming, since an encoded name would change every nested type's class name.

Comment on lines +111 to +115
if (candidate.InjectCount > winner.InjectCount)
winner = candidate;
}
selected.Add(winner);
foreach (var loser in ordered.Where(m => !ReferenceEquals(m, winner)))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 4c411d4. SelectNamedDispatchMethods now marks a collision as ambiguous when several methods share the highest inject count. Ambiguous groups are left out of both named and type-based dispatch, so the call falls through to reflection and throws the same AmbiguousMatchException. They're reported with the new CSLADP009 warning; CSLADP002 stays for collisions with a clear winner. Covered by the AmbiguousOperationName generator test.

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.

Do CSLADP009 and CSLADP002 still exist?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, both still exist. Since cbe92b0 they're reported by DataPortalOperationsAnalyzer instead of the generator, so they can be suppressed in source. The descriptors are in DataPortalOperationsDiagnostics.

  • CSLADP002 (info): two methods map to the same operation and differ only in [Inject] parameters. The method with more injected parameters is used, which is what reflection does too. That's the behavior your colleague saw; the diagnostic now makes the choice visible.
  • CSLADP009 (warning): same as above, but with the same number of injected parameters. The generated code leaves them out, so the call falls back to reflection, which throws AmbiguousMatchException.

Both are now documented (see the reply on DataPortalOperationsDiagnostics.cs).

Comment on lines +308 to +311
var builder = new StringBuilder("partial ").Append(keyword).Append(' ').Append(EscapeIdentifier(type.Name));
if (type.TypeParameters.Length > 0)
builder.Append('<').Append(string.Join(", ", type.TypeParameters.Select(t => EscapeIdentifier(t.Name)))).Append('>');
return builder.ToString();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No change. C# only requires constraints to match on partial parts that declare them; a part with no where clauses takes the constraints from the parts that have them (CS0265 is only for two parts with different constraints). The AbstractClass and DerivedFromBaseWithOperations tests use PersonBase<T> where T : PersonBase<T>, and their helper asserts the output compilation has no errors or warnings — both pass. Omitting the constraints also avoids copying constraints that reference types needing extra qualification.

- Resolve injected services inside the CallMethodException wrapper so
  resolution failures surface like reflection dispatch
- Leave operation methods tied on injected parameter count to reflection,
  which reports an ambiguous match, and report CSLADP009
- Keep the type of short/ushort/byte/sbyte constants such as service keys
- Only public business types state the extension class accessibility so
  partial parts from types mapping to the same class name merge
- CSLA0025 checks argument conversions and skips operations that lose to
  an overload sharing their operation name

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6AuArAiFMmEoBmSNbD8YG
@rockfordlhotka

Copy link
Copy Markdown
Member Author

Responses to the suppressed comments in the second Copilot review (changes in 4c411d4):

  • TypeWithOperationsShouldBePartialAnalyzer.cs:115 and IncrementalDataPortalOperationsGenerator.cs:65 — derived operation attributes: No change for now. The generator and CSLA0024 both deliberately recognize only the built-in operation attributes, so they are consistent with each other. A method marked with a custom subclass of FetchAttribute still works through reflection dispatch, as before; it just doesn't get generated dispatch. Finding derived attributes would mean semantic analysis of every attributed method in the compilation instead of using ForAttributeWithMetadataName, at a real IDE performance cost, for a pattern that's rarely used. If that scenario needs trim safety, it can be a follow-up issue.
  • UseGeneratedDataPortalExtensionAnalyzer.cs:236 — argument conversions: Fixed. When criteria are passed in expanded params form, each argument must implicitly convert to the matching criteria parameter (including params element types and null for nullable targets). New tests: AnalyzeWhenCriteriaTypesDoNotMatchAnyExtension (portal.FetchAsync("x") against Fetch(int) is not reported) and AnalyzeWhenCriteriaConvertToExtensionParameters.
  • UseGeneratedDataPortalExtensionAnalyzer.cs:242 — collision selection: Fixed. The analyzer skips operation methods that lose to another method with the same criteria types, using the generator's rule (most injected parameters, ties to the first declared). A [NoDataPortalExtension] winner therefore suppresses the diagnostic. Covered by AnalyzeWhenPreferredOverloadHasNoDataPortalExtension.
  • DataPortalOperationsBuilder.cs:279 — service resolution outside the wrapper: Fixed. Injected service resolution (and binding in criteria to locals) now happens inside the try, so resolution failures are wrapped in CallMethodException as in LateBoundObject.CallMethodTryAsyncDI. The async-on-sync check stays outside, since it already throws CallMethodException, just as the reflection path rethrows it without wrapping again. New integration test: FetchAsync_MissingRequiredService_WrapsExceptionLikeReflectionDispatch compares the exception chain with reflection dispatch.
  • OperationDiscovery.cs:441 — small integral keys: Fixed. short, ushort, byte and sbyte constants are emitted with casts (e.g. (short)(-1), (byte)2), so boxed keyed-service keys keep their type. The KeyedInject test now includes short and byte keys.

The full CI test run passes.

@ossendorf-at-hoelscher

Copy link
Copy Markdown

I'll review it at home later. Two things:

  1. Is there an option to disable the generation of sync-extensions? If not could we add a compiler flag to disable them? At work we must only use the async variations to improve throughput.
  2. It's best practice to only add diagnostics from analyzers NOT generators. A generator has to run to emit diagnostics which we don't want as little as possible to run. https://github.com/dotnet/roslyn/blob/main/docs/features/incremental-generators.cookbook.md#issue-diagnostics

…nly extensions

- Move the CSLADP002-CSLADP009 diagnostics out of the data portal generators into
  DataPortalOperationsAnalyzer and DataPortalExtensionsAnalyzer, which share the
  generators' selection rules. Diagnostics now use source locations, so they can
  be suppressed with #pragma.
- Add the CslaGenerateSyncDataPortalExtensions MSBuild property (default true);
  set it to false to generate only async [DataPortalExtensions] methods.
  CSLA0025 no longer reports sync calls when sync extensions are not generated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6AuArAiFMmEoBmSNbD8YG
@rockfordlhotka

Copy link
Copy Markdown
Member Author

Thanks @ossendorf-at-hoelscher, both addressed in cbe92b0.

1. Opting out of sync extensions: Added. Set the CslaGenerateSyncDataPortalExtensions MSBuild property to false and only the async extension methods are generated:

<PropertyGroup>
  <CslaGenerateSyncDataPortalExtensions>false</CslaGenerateSyncDataPortalExtensions>
</PropertyGroup>

It defaults to true. The property is exposed to the compiler through CompilerVisibleProperty in the package's buildTransitive/Csla.targets, so no other setup is needed. When sync generation is off:

  • Diagnostics about sync names that would be hidden or duplicated (CSLADP005/CSLADP007) are no longer reported.
  • CSLA0025 no longer flags calls to the sync IDataPortal<T> methods, since there is no extension to suggest.

New tests: AsyncOnly (generator) and AnalyzeWhenSyncExtensionsAreNotGenerated (CSLA0025). I also tested it end to end with a project that imports Csla.targets.

2. Diagnostics from analyzers instead of generators: Agreed, and done. The generators no longer report any diagnostics. Two new analyzers in the bundled generator assembly report the same IDs:

  • DataPortalOperationsAnalyzer: CSLADP002, CSLADP009
  • DataPortalExtensionsAnalyzer: CSLADP003–CSLADP008

The analyzers call the same code the generators use to decide what to emit (OperationDiscovery, SelectNamedDispatchMethods, and a new DataPortalExtensionsBuilder.Analyze), so they can't drift apart. The diagnostic snapshot tests now run the analyzers and assert that the generators report nothing. The messages and locations match what the generators produced before. Diagnostics now also have real source locations, so #pragma warning disable works on them, which it didn't when the generator reported them.

The full CI test run passes.

@StefanOssendorf

StefanOssendorf commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Two other things:

  1. Is it possible to enable the extension generation for a whole assembly without annotating every business object? Would that make sense? Since my implementation is used on a dedicated type as the extensions host it scanned the assembly and used all types.

I had something like this in mind:

[assembly: DataPortalExtensions(Prefix = "Portal")]
  1. I'm not a friend for the Async-Methodsuffix which is the reason why my implementation doesn't add it by default but it's possible through a compiler flag. My question is: Would it be bad to make that possible due to my taste (compiler flag with Async as default suffix but overridable) or should we stick with the async and I have to deal with it? 😅

@StefanOssendorf StefanOssendorf 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.

I'm not done yet, but a first batch.

Comment thread docs/analyzers/CSLA0025-UseGeneratedDataPortalExtensionAnalyzer.md Outdated
Comment thread docs/analyzers/index.md Outdated
Comment thread docs/code-generators.md Outdated
Starting with CSLA 9, there are code generators available as NuGet packages. These packages are designed to work with the CSLA .NET framework to generate code for your business objects as part of the .NET build process.

* [AutoImplementProperties](https://github.com/MarimerLLC/csla/tree/main/Source/Csla.Generators/cs/AutoImplementProperties) - Uses code generation to radically reduce the amount of code you need to write to declare properties in a business domain class
* [AutoImplementProperties](https://github.com/MarimerLLC/csla/tree/main/Source/Csla.Generators/cs/AutoImplementProperties) - Uses code generation to radically reduce the amount of code you need to write to declare properties in a business domain class. Starting with CSLA 11 this generator is included in the `Csla` package, along with the data portal operations generator (explicit, reflection-free calls to data portal operation methods) and the `[DataPortalExtensions]` generator (strongly typed data portal extension methods).

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.

DataPortalExtensionGenerator should get it's own paragraph

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done. The data portal operations generator and the data portal extensions generator now each have their own entry. The entry for your Csla.DataPortalExtensions package now notes that CSLA 11 and later include the generator in the Csla package. (b4f0938)

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.

I meant a paragraph like [AutoImplementProperties] or [AutoSerialization].

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in b8e5994. DataPortalOperations and DataPortalExtensions now have their own linked entries, like AutoImplementProperties and AutoSerialization. They point to two new pages, data-portal-operations-generator.md and data-portal-extensions-generator.md.

&& applicationContext.ExecutionLocation == ApplicationContext.ExecutionLocations.Server)
return;

throw new CallMethodException(

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.

Can't we use method from line 56 here? Looks like the same setup to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it now throws CreateCallMethodException(target, methodName, new NotSupportedException(...)), so both paths build the exception the same way. (b4f0938)

Comment thread Source/Csla/Server/DataPortalOperationHelper.cs Outdated
Comment thread Source/Csla/Server/DataPortalOperationHelper.cs Outdated
Comment thread Source/Csla/Server/DataPortalTarget.cs
Comment thread Source/Csla/BusinessBindingListBase.cs
…x, code fix, guards

- Allow [assembly: DataPortalExtensions] to generate extensions for every eligible
  business class; a class Prefix overrides the assembly Prefix, and
  [NoDataPortalExtension] can now exclude a class.
- Add the CslaDataPortalExtensionsAsyncSuffix MSBuild property (default Async,
  none for no suffix). With no suffix sync methods are not generated (CSLADP011);
  an invalid suffix is reported (CSLADP010). CSLA0025 mirrors both settings.
- Add a CSLA0025 code fix that replaces the untyped call with the generated
  extension method and adds a using directive when needed.
- The generator no longer creates diagnostic data: DataPortalExtensionsBuilder.Analyze
  takes an optional reporter used only by the analyzer; DiagnosticInfo is removed.
- Reject empty or white-space operation names in ChildDataPortal and the
  DataPortal<T> operation invokers; add null guards and reuse
  CreateCallMethodException in DataPortalOperationHelper.
- Docs: CSLA0009 index entry, separate generator entries in code-generators.md,
  CSLA0025 code fix, release notes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6AuArAiFMmEoBmSNbD8YG
@rockfordlhotka

Copy link
Copy Markdown
Member Author

@StefanOssendorf both are in b4f0938 (and I replied inline to the first review batch).

1. Assembly-level attribute: Yes, it makes sense, and it's added:

[assembly: DataPortalExtensions(Prefix = "Portal")]
  • It generates extensions for every business class in the assembly that can have them: non-abstract, non-generic, accessible outside its containing type, and implementing ICslaObject. Records are skipped too. Classes that don't qualify are skipped silently. The CSLADP003/CSLADP004 "invalid target" warnings only appear for classes that carry the attribute themselves.
  • A Prefix on a class attribute takes precedence over the assembly prefix. A class attribute without Prefix inherits the assembly prefix.
  • [NoDataPortalExtension] can now also go on a class, to opt it out.
  • An invalid assembly prefix is reported once, as CSLADP008 on the assembly attribute.
  • The generator no longer needs ForAttributeWithMetadataName on the class attribute. The attribute info now comes from the operation-type pipeline it already has, plus a small options model built from the compilation and build properties.
  • CSLA0025 follows the assembly attribute, including for business types from referenced assemblies.

2. Async suffix: Added as an MSBuild property, Async by default:

<CslaDataPortalExtensionsAsyncSuffix>none</CslaDataPortalExtensionsAsyncSuffix>
  • MSBuild passes an unset CompilerVisibleProperty to the compiler as an empty value, so an empty value can't mean "no suffix". That's why the value none means no suffix. Any other value is used as the suffix, for example Task → GetByIdTask.
  • With no suffix, the sync and async methods would have the same names. So sync methods aren't generated then, and CSLADP011 suggests setting CslaGenerateSyncDataPortalExtensions to false. With both set, you get your original naming: portal.PortalGetById(42) returning Task<T>.
  • A value that isn't valid in an identifier is reported as CSLADP010, and Async is used instead.
  • CSLA0025 and its new code fix use the configured suffix.

The full CI test run passes. I also checked both settings end to end with a project that imports the package's Csla.targets.

@rockfordlhotka

Copy link
Copy Markdown
Member Author

@StefanOssendorf thank you for the quality feedback. I think all your suggestions have been addressed - any more coming?

@StefanOssendorf

Copy link
Copy Markdown
Contributor

@StefanOssendorf thank you for the quality feedback. I think all your suggestions have been addressed - any more coming?

Yes. I'll continue to review tomorrow. I'll build a local alpha to test in a console/lib project.

@StefanOssendorf

Copy link
Copy Markdown
Contributor

Btw how does the new invocation with a name handle method overloads?
A colleague showed me today that calling a Create Child where two methods only differing in injected dependencies always the one with more injections is chosen. I think with the new one the correct one is used or is it an error/warning?

@rockfordlhotka

Copy link
Copy Markdown
Member Author

Btw how does the new invocation with a name handle method overloads? A colleague showed me today that calling a Create Child where two methods only differing in injected dependencies always the one with more injections is chosen. I think with the new one the correct one is used or is it an error/warning?

This sounds like a different or preexisting issue that should be filed?

@StefanOssendorf

Copy link
Copy Markdown
Contributor

I can't pack a pre-release on my local machine. I can build the solution fine but dotnet pack csla.build.sln -o /home/stefan/GitHub/nuget-prereleases -c Release ends with an error and no Csla.nupkg.
Can you verify that, please?

@StefanOssendorf StefanOssendorf 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.

Some thins to this review:

  1. I can't pack the solution locally. Maybe it's an dotnet issue on linux or there is something wrong with the changes.
  2. I skimmed through the changes. There are a lot of changes and tbh: with the usage of AI I tend to care less about the code structure because the AI is doing that job now.

Comment on lines +111 to +115
if (candidate.InjectCount > winner.InjectCount)
winner = candidate;
}
selected.Add(winner);
foreach (var loser in ordered.Where(m => !ReferenceEquals(m, winner)))

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.

Do CSLADP009 and CSLADP002 still exist?

/// <param name="allowNull">True if the parameter allows a null value.</param>
/// <exception cref="ArgumentNullException"><paramref name="serviceProvider"/> or <paramref name="serviceType"/> is <see langword="null"/>.</exception>
/// <exception cref="InvalidOperationException">The service is required and not registered.</exception>
public static object? GetService(IServiceProvider serviceProvider, Type serviceType, bool allowNull)

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.

Where exactly is this method used? And wouldn't it be better to either create two methods object? GetService(SP, Typ) and object GetRequiredService(SP, Type) or just use use serviceProvider.Get[Required]Service(type)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The generated dispatch code used it to resolve [Inject] parameters. Agreed, the helpers added nothing. b8e5994 removes GetService and GetKeyedService, and the generated code now makes the same calls that reflection makes in ServiceProviderMethodCaller:

  • optional: serviceProvider.GetService(type)
  • required: ServiceProviderServiceExtensions.GetRequiredService(serviceProvider, type)
  • keyed: ServiceProviderKeyedServiceExtensions.GetKeyedService / GetRequiredKeyedService(serviceProvider, type, key)

Csla references M.E.DI.Abstractions 10.0 on every target, so these are always available to consumers.

/// <param name="allowNull">True if the parameter allows a null value.</param>
/// <exception cref="ArgumentNullException"><paramref name="serviceProvider"/> or <paramref name="serviceType"/> is <see langword="null"/>.</exception>
/// <exception cref="InvalidOperationException">The service is required and not registered, or the provider does not support keyed services.</exception>
public static object? GetKeyedService(IServiceProvider serviceProvider, Type serviceType, object? serviceKey, bool allowNull)

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.

Same question as for GetService

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Same change, removed. See the reply above.

Comment thread releasenotes.md Outdated
Comment thread releasenotes.md Outdated
- Fix dotnet pack on Linux: Csla.csproj packed Csla.Analyzers.dll from a
  hard-coded bin\Release path, but the analyzers build to Bin\. Both the
  analyzers and the bundled generator are now packed from GetTargetPath.
- Generated dispatch resolves [Inject] parameters by calling the
  Microsoft.Extensions.DependencyInjection methods directly; remove
  DataPortalOperationHelper.GetService and GetKeyedService.
- Remove the nullable accumulator in GetOperationTypes and add a test for
  business objects without operation methods.
- Document CSLADP002-CSLADP011, add help links to the descriptors, and add
  pages for the data portal operations and extensions generators.
- Release notes: clarify the sync extensions property and drop the partial
  base classes item from breaking changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rockfordlhotka

Copy link
Copy Markdown
Member Author

@StefanOssendorf thanks for the second round. Everything is in b8e5994, and I replied inline to each comment.

dotnet pack on Linux: I reproduced it in a Linux container (mcr.microsoft.com/dotnet/sdk:11.0.100-rc.1) with your exact command:

NuGet.Build.Tasks.Pack.targets(232,5): error : Could not find a part of the path '/work/bin/Release/netstandard2.0'. [/work/Source/Csla/Csla.csproj]

It isn't caused by this PR: main fails the same way on Linux. Csla.csproj packed the analyzers from a hard-coded ..\..\bin\Release\netstandard2.0\Csla.Analyzers.dll, but Csla.Analyzers builds to Bin\ (capital B). Windows paths aren't case-sensitive, so CI never noticed.

The fix: a CslaPackBundledAnalyzers target now gets the paths of both Csla.Analyzers.dll and the bundled generator from the projects with GetTargetPath, so the casing is right in any configuration. A build-order-only ProjectReference makes sure the analyzers are built first. With the fix, the same command on Linux creates all 17 packages, and Csla.nupkg contains analyzers/dotnet/cs/Csla.Analyzers.dll, analyzers/dotnet/cs/Csla.Generator.AutoImplementProperties.CSharp.dll and buildTransitive/Csla.targets. You should be able to build your local alpha now.

On your second point: understood. If anything in the structure gets in your way while you test the alpha, please flag it.

rockfordlhotka added a commit that referenced this pull request Oct 8, 2026
- Remove the DataPortal_XYZ/Child_XYZ name-matching fallback from
  ServiceProviderMethodCaller and the DataPortalOptions.UseLegacyOperationMethods option
- Add operation attributes to legacy-named test operation methods
- Replace the legacy fallback tests with tests that attribute-less
  legacy-named methods are not found
- Draft release notes, upgrade guide, and analyzer doc updates

Base class changes (Phase 2) and analyzer changes (Phase 3) follow after #4925.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

Call data portal operation methods explicitly

4 participants