Source generator: typed NodeSet method signatures and referenced model accessors - #4124
Merged
Merged
Conversation
This was referenced Jul 30, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates the OPC UA model source generator to improve method signature typing and dependency interoperability, including “fluent-accessors-only” generation support and fixes for NodeSet-derived model details (method declarations, state-machine numbers, symbolic names).
Changes:
- Add typed NodeSet method signature support (including method-type grouping by declared signature) and fix method declaration linking for overrides to keep
Callresolution working. - Extend dependency metadata to carry fluent-accessor capability + method identity details, and scan referenced assemblies for fluent accessor providers (enabling “fluent-accessors-only” mode).
- Fix NodeSet-derived state/transition number emission and normalize type
SymbolicNamenamespaces for UA types.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Opc.Ua.SourceGeneration/SourceGenerator.cs | Adds new diagnostics for fluent-accessors-only failures/config. |
| tools/Opc.Ua.SourceGeneration/ReferencedFluentAccessorProviderScanner.cs | Scans referenced assemblies for fluent accessor provider attributes. |
| tools/Opc.Ua.SourceGeneration/OPCFoundation.Opc.Ua.SourceGeneration.props | Exposes MSBuild property for fluent-accessors-only mode to the generator. |
| tools/Opc.Ua.SourceGeneration/ModelSourceGenerator.cs | Wires referenced accessor providers into the incremental generator pipeline. |
| tools/Opc.Ua.SourceGeneration/ModelCompilationOptions.cs | Adds FluentAccessorsOnly option surfaced from MSBuild. |
| tools/Opc.Ua.SourceGeneration/ModelCompilation.cs | Propagates fluent-accessors-only inputs, diagnostics, and dependency producer selection logic. |
| tools/Opc.Ua.SourceGeneration/AnalyzerReleases.Unshipped.md | Registers new diagnostics MODELGEN014/015. |
| tools/Opc.Ua.SourceGeneration.Core/Templating/Tokens.cs | Adds tokens used for optional-field encoding-mask templating. |
| tools/Opc.Ua.SourceGeneration.Core/Schema/NodeSetToModelDesign.cs | Normalizes type symbolic name namespaces; groups method declarations by signature; orders emitted methods deterministically. |
| tools/Opc.Ua.SourceGeneration.Core/Schema/ModelDesignValidator.cs | Fixes method argument population and method declaration linking during merge/override validation; improves dependency-method argument linking. |
| tools/Opc.Ua.SourceGeneration.Core/Schema/ModelDesignExtensions.cs | Resolves method state class names via effective method identity + signature-derived argument presence. |
| tools/Opc.Ua.SourceGeneration.Core/ModelFluentAccessorProviderReference.cs | Introduces reference record for fluent-accessor providers in referenced assemblies. |
| tools/Opc.Ua.SourceGeneration.Core/ModelDependencyReference.cs | Makes dependency payload decode resilient to malformed payloads (cached null on failure). |
| tools/Opc.Ua.SourceGeneration.Core/Generators/StateMachineIdsGenerator.cs | Reads state/transition numbers from NodeSet-style VariableDesign (DecodedValue/DefaultValue). |
| tools/Opc.Ua.SourceGeneration.Core/Generators/ObjectTypeProxyGenerator.cs | Resolves method signatures against merged instances for overrides. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateTemplates.cs | Adds template for registering predefined nodes for typed method states. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs | Updates method argument detection and override handling; adjusts replace-child logic for fixed vs placeholder slots. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/ModelDependencyGenerator.cs | Extends dependency payload with fluent-accessor capability and method identity/declaration metadata. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/MethodDesignArgumentResolver.cs | Centralizes effective method definition/signature resolution for typed argument consumers. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/FluentBuilderTemplates.cs | Emits assembly-level fluent accessor provider attribute in generated fluent builder output. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/FluentBuilderGenerator.cs | Adds model URI token + resolves override method signatures and ObjectType state CLR naming. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/DataTypeTemplates.cs | Adds overridable encoding-mask field-name list and tokenized encode/decode mask blocks. |
| tools/Opc.Ua.SourceGeneration.Core/Generators/DataTypeGenerator.cs | Emits encoding-mask override/virtual modifiers and conditional encode/decode blocks for inherited optional fields. |
| tools/Opc.Ua.SourceGeneration.Core/Generators.cs | Adds fluent-accessors-only generation path, referenced provider handling, namespace-name deconfliction, and new overloads. |
| tools/Opc.Ua.SourceGeneration.Core/GeneratorOptions.cs | Adds FluentAccessorsOnly generator option. |
| tools/Opc.Ua.SourceGeneration.Core/Dependency/ModelDependencyV1.cs | Adds fluent accessor capability flag and method identity trailer serialization. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Schema/NodeSetToModelDesignTests.cs | Tests type symbolic name namespace normalization behavior. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/VendorRobotics.NodeSet2.xml | Adds fixture NodeSet for vendor robotics subtype scenarios. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/TypedMethodArguments.NodeSet2.xml | Adds fixture NodeSet for typed method argument generation. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/MethodTypeOverride.ModelDesign.xml | Adds fixture ModelDesign for method type override behavior. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/DuplicateMethodNames.NodeSet2.xml | Adds fixture NodeSet for duplicate method name + signature grouping. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/DeclarationBackedMethodConsumer.NodeSet2.xml | Adds fixture NodeSet for declaration-backed method scenarios. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/GeneratorsTests.cs | Asserts original public GenerateCode signatures remain available. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/StateMachineIdsGeneratorTests.cs | Adds regression test for NodeSet-style state/transition numbers. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeStateGeneratorTests.cs | Adds coverage for typed method args, duplicate method grouping, method type overrides, and fixed-child-slot logic. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/ModelDependencyGeneratorTests.cs | Tests method identity/declaration serialization and legacy payload compatibility. |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Dependency/ModelDependencyV1Tests.cs | Adds round-trip tests for fluent accessor capability metadata. |
| tests/Opc.Ua.Di.Tests/PumpInstanceNodeIdRegressionTests.cs | Pins new method argument property nodes introduced by typed signatures. |
Comments suppressed due to low confidence (1)
tools/Opc.Ua.SourceGeneration/SourceGenerator.cs:1
helpLinkUrishould be a valid absolute URI. Usingwww.opcfoundation.org(no scheme) can produce broken links in IDE tooling; prefer anhttps://...URL (and apply the same fix toFluentAccessorsOnlyOptionsError).
marcschier
added a commit
that referenced
this pull request
Jul 30, 2026
Cover the fluent-accessors-only path and stop the argument resolver walking the same chain repeatedly. - ValidateFluentAccessorsOnlyTarget had no tests at all. It is now internal (the test project already has InternalsVisibleTo) and FluentAccessorsOnlyTargetTests asserts all nine outcomes: unknown model URI, an assembly that already provides accessors, a provider for a different prefix not blocking the target, a malformed payload, no payload-bearing producer, a producer supplying another prefix, unknown legacy capability, a producer that already emitted accessors, and the one acceptance path. - Restores the nine end-to-end fluent-accessors-only tests in ModelDependencyScannerTests, including the generation-output one (FluentAccessorsOnlyEmitsAccessorsAgainstReferencedStateTypes). Splitting the stack had reverted that whole file to master because a single test in it uses the Robotics NodeSet as its fixture; only that test and its helpers are held back for the Robotics change at the top of the stack. - ResolveMethodDefinition resolved each candidate target twice, once through ResolveMethodInputs and once through ResolveMethodOutputs, each walking the chain again from scratch with its own visited list. The new DelegatesTo helper resolves the target once and reads both argument arrays from it. The resolution is deliberately still not cached between calls: the model is mutated while it is validated, and a generator that carried state between passes would stop being deterministic. Opc.Ua.SourceGeneration.Core.Tests 3748, Opc.Ua.SourceGeneration.Tests 93. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
cristipogacean
approved these changes
Jul 31, 2026
…l accessors Second in the #4096 stack, on top of the core stack change. Generator only; no runtime library changes. - Methods imported from a NodeSet now get a typed signature. Methods that share a declared signature are grouped onto one generated MethodType, named after the owner when the plain name would collide, so two unrelated Execute methods no longer share a class. - An override that keeps the signature it inherits keeps the declaration it overrides. Pointing MethodDeclarationNode at the override itself made MethodDeclarationId self referential, and OPC 10000-4 lets a caller invoke a method with the NodeId of its declaration in the ObjectType or any supertype, so Call stopped resolving and returned Bad_MethodInvalid. An override that genuinely changes its signature still becomes its own declaration. - StateNumbers and TransitionNumbers were emitted empty for every NodeSet sourced model, because such children arrive as VariableDesign rather than PropertyDesign and were filtered out. Any companion model built from a NodeSet was writing StateNumber 0 on the wire, a Part 16 defect well beyond the models in this stack. The ModelDesign path is unchanged. - Referenced models can now contribute fluent accessors, carried by a model dependency payload so a downstream model can extend an upstream one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Review feedback on #4123 asked for the `is { } x` form to be rewritten everywhere in the stack. This is the only remaining occurrence outside the state machine dispatcher. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Cover the fluent-accessors-only path and stop the argument resolver walking the same chain repeatedly. - ValidateFluentAccessorsOnlyTarget had no tests at all. It is now internal (the test project already has InternalsVisibleTo) and FluentAccessorsOnlyTargetTests asserts all nine outcomes: unknown model URI, an assembly that already provides accessors, a provider for a different prefix not blocking the target, a malformed payload, no payload-bearing producer, a producer supplying another prefix, unknown legacy capability, a producer that already emitted accessors, and the one acceptance path. - Restores the nine end-to-end fluent-accessors-only tests in ModelDependencyScannerTests, including the generation-output one (FluentAccessorsOnlyEmitsAccessorsAgainstReferencedStateTypes). Splitting the stack had reverted that whole file to master because a single test in it uses the Robotics NodeSet as its fixture; only that test and its helpers are held back for the Robotics change at the top of the stack. - ResolveMethodDefinition resolved each candidate target twice, once through ResolveMethodInputs and once through ResolveMethodOutputs, each walking the chain again from scratch with its own visited list. The new DelegatesTo helper resolves the target once and reads both argument arrays from it. The resolution is deliberately still not cached between calls: the model is mutated while it is validated, and a generator that carried state between passes would stop being deterministic. Opc.Ua.SourceGeneration.Core.Tests 3748, Opc.Ua.SourceGeneration.Tests 93. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
marcschier
force-pushed
the
marcschier/3827-2-source-generator
branch
from
July 31, 2026 12:01
b603dad to
5dc5a28
Compare
#4120 removed SourceGenerator.Foundations and reshaped SourceGenerator.Guard to take an Action. Resolved by keeping master's guard shape and adding this change's ReferencedAccessorProviders argument to the ModelCompilation construction, and by replacing the generated ModelSourceGeneratorHoist wrapper with the generator itself in the tests this change added, matching how master now drives CSharpGeneratorDriver. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
marcschier
added a commit
that referenced
this pull request
Aug 1, 2026
…provider (#4125) **Stack 3/5** — base #4124. Part of the #4096 split; see #4096 for the overall picture. > Review after #4123 and #4124. ## What this is The OPC 10000-210 Relative Spatial Location and OPC 10000-211 Global Positioning companion models, with their server and client libraries, plus the ISA-95 migration onto the shared geolocation seam. ## Why it is one change A deployment writes **one** provider implementation and both OPC 10000-211 and OPC 10030 are served from it. The specifications disagree on representation, not on content: - GPOS publishes the structured `GlobalLocationDataType` and validates the EPSG code against the configured CRS. - ISA-95 publishes the same sample as the `String` array that OPC 10030 §8.3.2.2 requires. Splitting them would mean two provider interfaces for one physical sensor, which is exactly what this avoids. The seam itself lives in `Opc.Ua.Server` and landed in 1/5. ## Contents - `Opc.Ua.Positioning`, `.Server` and `.Client`. - `Opc.Ua.ISA95.Server` moves onto `IGeoLocationProvider`; the three ISA-95-specific provider files are removed because the shared seam and its in-memory implementation replace them. - **Fixes a conformance gap**: the ISA-95 binder wrote a scalar where the type declares `ValueRank` OneOrMoreDimensions. It now writes `String[]`. - **Fixes a liveness bug**: a provider that cannot push is polled at the Variable's `MinimumSamplingInterval` rather than installed once, so a non-pushing provider no longer serves its start-up sample with `Good` status forever. - `samples/MinimalIsa95Server`. ## Validation `dotnet build UA.slnx` — 0 errors. | Suite | Result | | --- | --- | | Opc.Ua.Positioning.Tests | 69 / 0 | | Opc.Ua.ISA95.Tests | 136 / 0 | | Opc.Ua.Di.Tests | 304 / 0 | --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
marcschier
added a commit
that referenced
this pull request
Aug 1, 2026
…nector (#4126) **Stack 4/5** — base #4125. Part of the #4096 split; see #4096 for the overall picture. > Review after #4123, #4124 and #4125. ## What this is The generic OPC UA to OpenUSD binding, scene materialization, and the connector that drives a live stage from a running server. ## Contents **Binding (Part 1)** — `Opc.Ua.OpenUsd` and `Opc.Ua.OpenUsd.Server`. A server describes how its Variables drive USD prims, and serves the USD layers it refers to as Part 5 files with a content digest. **Scene materialization (Part 2)** — `Opc.Ua.OpenUsdScene` and `Opc.Ua.OpenUsdScene.Server`, including the USDA writer. - **Escaping fix**: string values, composition-arc references, variant set names and selections, `doc` and prim names were all emitted unescaped, so a value carrying a quote or a newline could author its own layer syntax. Every emit site now goes through the escapers. The tests assert the right property — not that the payload is absent (it legitimately survives inside the escaped literal) but that no payload becomes its own line of layer syntax. **Client** — `Opc.Ua.OpenUsd.Client` streams live values into an `IUsdSink`. `CompositeUsdSink` fans out to several sinks; the file sink interpolates between server samples so motion is continuous rather than stepped at the publishing interval. **Connector** — `tools/Opc.Ua.OpenUsd.Connector` composes a stage from a live server. The optional in-process viewport lives in a separate assembly probed at run time, so the connector still runs — and says why — when it is absent. - **Fix**: the asset cache and PKI root moved off the shared temp directory to `LocalApplicationData`. **Sample** — `PumpDeviceIntegrationServer` gains an OpenUSD representation of the pump. ## Notes for the reviewer - The viewport assembly is net10.0-only. It is what exercises the `Directory.Build.targets` change from 1/5: without it, the `net8.0` leg of the per-TFM CI build fails with `CS0433` because the `System.Threading.Lock` polyfill collides with the real type. - `UsdAssetDelivery` hashes through `SHA256.Create` rather than the static `HashData`, which does not exist on the net472/net48 targets this test project also builds for. ## Validation `dotnet build UA.slnx` — 0 errors, 0 warnings. | Suite | Result | | --- | --- | | Opc.Ua.OpenUsd.Tests | 621 / 0 | | Opc.Ua.Di.Tests | 321 / 0 | --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
marcschier
added a commit
that referenced
this pull request
Aug 1, 2026
#4127) **Stack 5/5** — base #4126. Part of the #4096 split; see #4096 for the overall picture. Closes the API work for #3827. > Review after #4123, #4124, #4125 and #4126. This branch is content-identical to the original #4096 branch, so merging the stack lands exactly what #4096 contained. ## The problem this solves OPC 40010 1.02 defines a rich robot **topology** but **no motion verbs at all** — its actuation surface is the SystemOperation and TaskControl state machines plus a Controller `Programs` directory. Meanwhile [URML](https://github.com/URML-MARS/URML) names OPC UA Robotics as its canonical non-ROS substrate and asks for `MoveTo` / `Grasp` / `Release` methods the companion spec does not define, so today it needs a hand-maintained per-deployment NodeId mapping file. This closes that gap in two clearly separated tiers, and never presents the second as standard. ## Contents **Server** — stock `RoboticsNodeManager`, ordered `IRoboticsModelProvider` composition, `IRoboticsConfigurator` code-behind, and validated fluent topology builders for systems, controllers, motion devices, axes, power trains, motors, gears, drives, safety states, software, task controls and task modules, with the correct OPC 40010 semantic references. **Standard operations** — `AddSystemOperation` and `AddTaskControlOperation` drive the Part 16 state machines. An illegal cause is rejected with `BadInvalidState` before the handler runs; a handler returning a bad `ServiceResult` does not move the machine; and a transition already in flight rejects a concurrent one rather than letting both commit. **Programs** — binds the optional `FileDirectoryType` through the reusable `IFileDirectoryBinder` added in 1/5. **Operation conventions** — opt-in and explicitly non-normative `MoveTo` / `MoveJ` / `MoveL` / `Grasp` / `Release` / `PickFrom` / `PlaceAt` / `SwapTool` / `SetOutput` / `CallProgram`, created **only** in an application-owned namespace — the OPC UA, DI, IA and Robotics namespaces are rejected — plus a generic `AddOperation<TRequest, TResponse>` extension point. **Client** — `RoboticsClient` extends the DI client, is subtype aware throughout, and provides typed topology snapshots, the standard operation and task-control clients, `ProgramsAsync`, streaming observation and a DI factory. Verbs resolve by BrowseName, so no NodeId mapping file is needed. **Sample** — `MinimalRobotServer` composes five models (Robotics, IA, DI, OpenUSD, RSL/GPOS) purely through `IRoboticsModelProvider` contributions, which is the real proof that the stock hosting is sufficient. It follows the published KUKA KR 16-2 kinematics and axis limits with an eased pick-and-place cycle. Also restores the vendor-subtype scanner test held back from 2/5, which uses the Robotics NodeSet as its fixture. ## Defects found and fixed along the way - **`ControllerType.CurrentUser`** is Mandatory in OPC 40010 but was neither built nor validated. Now has a builder and a validation error. - **`MotionDeviceType.TaskControlReference`** was absent because it must target a `TaskControlOperationType` instance that did not exist. Now wired. ## Validation `dotnet build UA.slnx` — 0 errors, 0 warnings. | Suite | Result | | --- | --- | | Opc.Ua.Robotics.Tests | 103 / 0 | | Opc.Ua.OpenUsd.Tests | 635 / 0 | | Opc.Ua.Di.Tests | 342 / 0 | | Opc.Ua.SourceGeneration.Tests | 94 / 0 | | Opc.Ua.Aot.Tests (NativeAOT) | 120 / 0 | --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack 2/5 — base #4123. Part of the #4096 split; see #4096 for the overall picture.
What this is
Source generator only; no runtime library changes.
Contents
MethodType, named after its owner when the plain name would collide, so two unrelatedExecutemethods no longer share a class.MethodDeclarationNodeat the override itself madeMethodDeclarationIdself-referential. OPC 10000-4 lets a caller invoke a method with the NodeId of its declaration in the ObjectType or any supertype, andMethodState.MethodDeclarationIdis what the server matches on, soCallstopped resolving and returnedBad_MethodInvalidfor every alarm subtype. An override that genuinely changes its signature still becomes its own declaration.StateNumbers/TransitionNumberswere emitted empty for every NodeSet-sourced model, because such children arrive asVariableDesignrather thanPropertyDesignand were filtered out. Any companion model built from a NodeSet was writingStateNumber = 0on the wire — a Part 16 defect well beyond the models in this stack. The ModelDesign path is unchanged.Notes for the reviewer
tests/Opc.Ua.SourceGeneration.Testsis left at itsmastercontent here. Its new vendor-subtype test uses the Robotics NodeSet as its fixture, so it can only land with the Robotics change at the top of the stack (5/5).PumpInstanceNodeIdRegressionTestsgains sixInputArguments/OutputArgumentsentries for argument-less standard condition methods. Typed method signatures materialise those properties from theConditionTypedeclaration even where the method takes none. That is a real deviation, pinned rather than hidden so it stays visible; it is worth a follow-up.Validation
dotnet build UA.slnx— 0 errors.History and GDS are listed because they are what the method-declaration fix above is load-bearing for: without it they fail with 10 and 370 errors respectively.