Repository navigation
Call data portal operation methods explicitly via bundled source generators - #4925
rockfordlhotka wants to merge 11 commits into
Conversation
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
There was a problem hiding this comment.
🟡 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
Prefixis an arbitrarystringattribute property, but it is concatenated directly into the generated method name here. A value such asPrefix = "My-"orPrefix = "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>andIChildDataPortal<T>member names for every generated extension. As a result, a root operation method namedCreateChild(or a child operation method namedFetch) 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 forisChild.
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 inCallMethodException. Reflection dispatch andDataPortalExceptionHandlerrely 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 asobjectorIEnumerable<object>and receive anobject[]value. That value is then returned as the criteria object andGetCriteriaArrayexpands 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 exactobject[].
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.
| object?[] parameters = criteriaArray!; | ||
| await CallMethodTryAsyncDI<T>(isSync, parameters).ConfigureAwait(false); |
There was a problem hiding this comment.
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).
- 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
|
Responses to the suppressed comments in the Copilot review summary (all addressed in 77e2158):
|
There was a problem hiding this comment.
🟡 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
FetchAttributesubclass 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 asportal.FetchAsync("x")is reported as CSLA0025 even though the generatedFetchextension requires anintand 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 truepath does not apply the extension generator's collision selection. For example, if the selected method for a shared operation name has[NoDataPortalExtension],SelectMethodsskips 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
trythat 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 insideCallMethodTryAsyncand the surroundingLateBoundObjectpath wraps the failure inCallMethodException. 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
ForAttributeWithMetadataNameonly 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 fromFetchAttribute/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 anInt16key in the attribute, butFormatPrimitiveemits1, which is boxed asInt32when passed toGetKeyedService; keyed lookup then misses a registration made with(short)1. Preserve the primitive type with explicit casts forshort,ushort,byte, andsbyte(and add a test for a non-intkey).
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", |
There was a problem hiding this comment.
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.
| if (candidate.InjectCount > winner.InjectCount) | ||
| winner = candidate; | ||
| } | ||
| selected.Add(winner); | ||
| foreach (var loser in ordered.Where(m => !ReferenceEquals(m, winner))) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Do CSLADP009 and CSLADP002 still exist?
There was a problem hiding this comment.
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).
| 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(); |
There was a problem hiding this comment.
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
|
Responses to the suppressed comments in the second Copilot review (changes in 4c411d4):
The full CI test run passes. |
|
I'll review it at home later. Two things:
|
…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
|
Thanks @ossendorf-at-hoelscher, both addressed in cbe92b0. 1. Opting out of sync extensions: Added. Set the <PropertyGroup>
<CslaGenerateSyncDataPortalExtensions>false</CslaGenerateSyncDataPortalExtensions>
</PropertyGroup>It defaults to
New tests: 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:
The analyzers call the same code the generators use to decide what to emit ( The full CI test run passes. |
|
Two other things:
I had something like this in mind: [assembly: DataPortalExtensions(Prefix = "Portal")]
|
StefanOssendorf
left a comment
There was a problem hiding this comment.
I'm not done yet, but a first batch.
| 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). |
There was a problem hiding this comment.
DataPortalExtensionGenerator should get it's own paragraph
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
I meant a paragraph like [AutoImplementProperties] or [AutoSerialization].
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
Can't we use method from line 56 here? Looks like the same setup to me.
There was a problem hiding this comment.
Yes, it now throws CreateCallMethodException(target, methodName, new NotSupportedException(...)), so both paths build the exception the same way. (b4f0938)
…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
|
@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")]
2. Async suffix: Added as an MSBuild property, <CslaDataPortalExtensionsAsyncSuffix>none</CslaDataPortalExtensionsAsyncSuffix>
The full CI test run passes. I also checked both settings end to end with a project that imports the package's |
|
@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. |
|
Btw how does the new invocation with a name handle method overloads? |
This sounds like a different or preexisting issue that should be filed? |
|
I can't pack a pre-release on my local machine. I can build the solution fine but |
StefanOssendorf
left a comment
There was a problem hiding this comment.
Some thins to this review:
- I can't pack the solution locally. Maybe it's an dotnet issue on linux or there is something wrong with the changes.
- 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.
| if (candidate.InjectCount > winner.InjectCount) | ||
| winner = candidate; | ||
| } | ||
| selected.Add(winner); | ||
| foreach (var loser in ordered.Where(m => !ReferenceEquals(m, winner))) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
Same question as for GetService
There was a problem hiding this comment.
Same change, removed. See the reply above.
- 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>
|
@StefanOssendorf thanks for the second round. Everything is in b8e5994, and I replied inline to each comment.
It isn't caused by this PR: The fix: a On your second point: understood. If anything in the structure gets in your way while you test the alpha, please flag it. |
- 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>
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 theCslapackage, 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
analyzers/dotnet/cs) instead of shipping as its own package.[CslaImplementProperties],[CslaImplementPropertiesInterface<T>]and[CslaIgnoreProperty]move intoCsla.dll; the Attributes project and theMicrosoft.Bcl.HashCodedependency are removed.buildTransitive/Csla.targetsraisesCSLABUILD001when a project still references the retiredCsla.Generator.AutoImplementProperties.CSharppackage.Always-generated operations interface (server side)
partialclass declaring operation methods (abstract classes included), the generator emits an internal nestedIDataPortalOperationsinterface with one explicitly implemented member per operation method, named with the operation name (e.g.Fetch__Int32). Concrete classes also getIDataPortalOperationMapping/IDataPortalOperationNamedMappingimplementations that call through the interface.ForAttributeWithMetadataNamewith equatable models grouped by type, so partial classes spread across files work and the pipeline caches correctly.InjectAttributesubclasses and keyed servicesValueTaskpartial.Pre-resolved invokers (runtime)
IDataPortalOperationInvoker<T>andIChildDataPortalOperationInvoker<T>([EditorBrowsable(Never)]), implemented byDataPortal<T>. They take a generated operation name and RunLocal flag, so the client skips reflection-based method lookup.ChildDataPortalandDataPortalTargetpass operation names through for CreateChild/FetchChild named dispatch.string[], it is no longer split into separate criteria values.[DataPortalExtensions]generator (client side)[DataPortalExtensions(Prefix = "...")]. It generates sync and async extension methods onIDataPortal<T>(Create/Fetch/Execute/Delete) andIChildDataPortal<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.paramsmethods for custom or mockIDataPortal<T>implementations.IDataPortal<T>instance methods would hide[NoDataPortalExtension]excludes a method.Analyzers
partialto the type and its containing types.IDataPortal<T>methods with untyped criteria.Tests and verification
csla.netcore.testintegration tests:[RunLocal]bypassing the configured proxyIDataPortal<T>DataPortalOperationNameHelperdotnet build Source\csla.test.slnand the full test run pass.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.DeleteandIChildDataPortal.CreateChildAsyncbugs, and the inject analyzer.🤖 Generated with Claude Code
https://claude.ai/code/session_01Dn4aKN7kqcG78x1HBWApk3