Skip to content

Fix false-positive MODELGEN010 for mixed NodeSet2 + ModelDesign [NodeManager] binding - #3968

Merged
marcschier merged 7 commits into
masterfrom
copilot/source-generated-nodemanager-combination
Jul 8, 2026
Merged

Fix false-positive MODELGEN010 for mixed NodeSet2 + ModelDesign [NodeManager] binding#3968
marcschier merged 7 commits into
masterfrom
copilot/source-generated-nodemanager-combination

Conversation

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Description

Combining a NodeSet2 (object types) with a ModelDesign (object instances) in one
source-generated project is a supported layout, but the "types in NodeSet2 + instances in
ModelDesign" scenario from #3937 failed to build. This PR fixes it and locks the behavior in with
tests.

Two independent defects were involved:

  1. Cross-model TypeDefinition resolution. An instance whose TypeDefinition referenced a
    NodeSet2-defined type failed with MODELGEN003 because the NodeSet2 and ModelDesign inputs were
    processed in separate passes without seeing each other as resolution dependencies (aligned with
    the direction in Fix #3937: resolve ModelDesign references to NodeSet2-defined types #3940). Covered here with a reversed-input-order regression test.
  2. False-positive MODELGEN010. The generator resolves [NodeManager] bindings in two
    independent passes (NodeSet2, then ModelDesign). Each pass validated the full binding list
    against only its own models and reported MODELGEN010 "did not match any model design" for
    anything it did not personally match. A [NodeManager] bound to the NodeSet2 types model was
    therefore false-flagged as unmatched by the ModelDesign pass (and vice-versa) which, under
    TreatWarningsAsErrors, blocked the build. This matched the reported symptoms exactly: it worked
    with only the types file, broke when the instances ModelDesign was added, and built again when the
    [NodeManager] was removed.

Changes

Fix — Tools/Opc.Ua.SourceGeneration.Core/Generators.cs, Tools/Opc.Ua.SourceGeneration/ModelCompilation.cs

  • Both GenerateCode overloads now accept a caller-owned "used" set and a global model count.
    Matches are recorded in the shared set and the unmatched / ambiguity diagnostics are reported
    once, after both passes, via the new Generators.ReportUnmatchedNodeManagerBindings.
  • ModelCompilation owns the shared used-set + global model count across both passes and reports
    unmatched bindings once, instead of per pass.
  • Corrects the single-model fallback and ambiguity detection, which previously used the per-pass
    model count; a selector-less [NodeManager] in a multi-model project now yields a single, clear
    "specify NamespaceUri to disambiguate" diagnostic.

Tests

  • Tests/Opc.Ua.SourceGeneration.Tests/ModelGeneratorTests.cs — reversed AdditionalFiles order for
    mixed cross-model generation, plus e2e regressions: a [NodeManager] bound to the NodeSet2 types
    URI, one bound to the ModelDesign instances URI, and a selector-less binding across two models
    reporting a single ambiguity.
  • Tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeManagerBindingReportingTests.cs (new) —
    unit tests for the shared-set / aggregate-reporting contract.

Docs

  • Docs/ModelDependencies.mdMODELGEN010 was mislabeled ("model skipped" is actually
    MODELGEN013); corrected the entry and added MODELGEN013.
  • Docs/SourceGeneratedNodeManagers.md — how to bind a [NodeManager] in a mixed project, and a
    clarification that ModelSourceGeneratorPrefix/Name rename the generated *State/type classes,
    not the [NodeManager] class (whose name/namespace come from the annotated partial).

Validation

  • Tools/Opc.Ua.SourceGeneration.Core and Tools/Opc.Ua.SourceGeneration build clean (0 warnings /
    0 errors).
  • Opc.Ua.SourceGeneration.Core.Tests: 3708 passed / 0 failed (net10); new tests pass on net48.
  • Opc.Ua.SourceGeneration.Tests: 60 passed / 0 failed (net10); new tests pass on net48.
  • MinimalBoilerServer (a real [NodeManager] + source-generation consumer) builds clean.

Related Issues

Checklist

  • I have signed the CLA and read the CONTRIBUTING doc.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added all necessary documentation.
  • I have verified that my changes do not introduce (new) build or analyzer warnings.
  • I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed.
  • I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
  • I have addressed all PR feedback received.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI changed the title [WIP] Clarify usage of model design and Nodeset2 XML Add regression test for mixed NodeSet2/ModelDesign generation with reversed input order Jul 7, 2026
Copilot AI requested a review from marcschier July 7, 2026 13:40
…Manager] (#3937)

The model source generator resolves [NodeManager] attributes in two
independent passes (NodeSet2 then ModelDesign). Each pass validated the
full binding list against only its own models and reported MODELGEN010
for any binding it did not personally match, so a [NodeManager] bound to
a NodeSet2 types model was false-flagged as unmatched by the ModelDesign
pass (and vice-versa), blocking the build under TreatWarningsAsErrors.

Both GenerateCode overloads now accept a caller-owned used-set and a
global model count; matches are recorded in the shared set and the
unmatched/ambiguity diagnostics are reported once, after both passes, via
the new Generators.ReportUnmatchedNodeManagerBindings. This also corrects
single-model fallback and ambiguity detection, which used the per-pass
model count.

Adds e2e and core regression tests, and fixes the MODELGEN010 entry in
Docs/ModelDependencies.md plus a mixed-project [NodeManager] / Prefix-Name
clarification in Docs/SourceGeneratedNodeManagers.md.
@marcschier
marcschier marked this pull request as ready for review July 7, 2026 16:19
Copilot AI review requested due to automatic review settings July 7, 2026 16:19
@marcschier marcschier changed the title Add regression test for mixed NodeSet2/ModelDesign generation with reversed input order Fix false-positive MODELGEN010 for mixed NodeSet2 + ModelDesign [NodeManager] binding Jul 7, 2026
Comment thread plan.md Outdated

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.

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds regression coverage and updates generator binding logic so mixed NodeSet2 (types) + ModelDesign (instances) projects behave consistently regardless of input file order, and so [NodeManager] binding diagnostics are reported correctly across both generation passes.

Changes:

  • Introduces a shared [NodeManager] binding “used” set across NodeSet2 + ModelDesign passes and defers unmatched-binding reporting until both passes complete.
  • Adds regression tests for reversed AdditionalFiles order and for cross-pass [NodeManager] diagnostic behavior (including selector-less ambiguity).
  • Updates documentation to describe mixed-model [NodeManager] binding behavior and adjusts diagnostic documentation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Tools/Opc.Ua.SourceGeneration/ModelCompilation.cs Shares binding state across NodeSet2/ModelDesign passes and reports unmatched bindings once.
Tools/Opc.Ua.SourceGeneration.Core/Generators.cs Adds shared binding tracking + centralized unmatched/ambiguity reporting helper.
Tests/Opc.Ua.SourceGeneration.Tests/ModelGeneratorTests.cs Adds regression tests for reversed input ordering + cross-pass binding diagnostics.
Tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeManagerBindingReportingTests.cs New unit tests for aggregated unmatched/ambiguity reporting behavior.
Docs/SourceGeneratedNodeManagers.md Documents mixed-project [NodeManager] binding behavior.
Docs/ModelDependencies.md Updates documented meaning/severity of MODELGEN010 and adds MODELGEN013.
Comments suppressed due to low confidence (1)

Tools/Opc.Ua.SourceGeneration.Core/Generators.cs:1

  • The new optional parameter sharedUsedBindings is typed as HashSet<NodeManagerAttributeBinding>, which forces callers to use that concrete collection type. Consider changing it to an interface type like ISet<NodeManagerAttributeBinding> (or at least ICollection<NodeManagerAttributeBinding>) to keep the public API flexible.
/* ========================================================================

Comment thread Tools/Opc.Ua.SourceGeneration/ModelCompilation.cs
Comment thread Tools/Opc.Ua.SourceGeneration/ModelCompilation.cs
Comment thread Tools/Opc.Ua.SourceGeneration.Core/Generators.cs
- ModelCompilation: precompute a HashSet of NodeSet2 paths (Ordinal) and use
  Contains instead of Dictionary.ContainsValue, avoiding the O(n^2) scan over
  m_input; behavior is unchanged.
- Generators: reword the unmatched-[NodeManager] diagnostic to "did not match
  any model" since binding resolution now spans NodeSet2 *and* ModelDesign
  models, not only ModelDesign.
- Tests: resolve the Resources folder from TestContext.CurrentContext.TestDirectory
  instead of Directory.GetCurrentDirectory() for CI/parallel-run stability, and
  update the diagnostic-message assertions.
@marcschier marcschier added the ready Ready to merge once CI Passes label Jul 7, 2026
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.67089% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.28%. Comparing base (76aa837) to head (7e3529f).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
Tools/Opc.Ua.SourceGeneration.Core/Generators.cs 91.37% 0 Missing and 5 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3968      +/-   ##
==========================================
+ Coverage   68.92%   76.28%   +7.36%     
==========================================
  Files        1192     1229      +37     
  Lines      169560   173563    +4003     
  Branches    29329    30055     +726     
==========================================
+ Hits       116861   132411   +15550     
+ Misses      41749    29981   -11768     
- Partials    10950    11171     +221     
Files with missing lines Coverage Δ
Tools/Opc.Ua.SourceGeneration/ModelCompilation.cs 69.18% <100.00%> (+10.70%) ⬆️
Tools/Opc.Ua.SourceGeneration.Core/Generators.cs 90.96% <91.37%> (+20.37%) ⬆️

... and 253 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcschier
marcschier merged commit b15d6e7 into master Jul 8, 2026
255 of 258 checks passed
@marcschier
marcschier deleted the copilot/source-generated-nodemanager-combination branch July 8, 2026 14:54
marcschier added a commit to marcschier/UA-.NETStandard that referenced this pull request Jul 9, 2026
Brings in 5 upstream commits: OPCFoundation#3957 (expand DI/Fluent API injectability +
one-shot ergonomics across client/server/PubSub/discovery/bindings),
OPCFoundation#3968 (fix false-positive MODELGEN010), OPCFoundation#3966 (fix source generator
dropping BaseAnalogType EURange/EngineeringUnits), OPCFoundation#3949 (Apache Kafka
PubSub transport, Part 14 Annex B.2), and OPCFoundation#3959 (MCP server stdio logs
to stderr).

Resolved one conflict in Directory.Packages.props (three regions), all
from our UaLens-specific package entries interleaving with upstream's
new Kafka dependency and a Microsoft.Extensions.* version bump:

 * Kept CommunityToolkit.Mvvm 8.4.2 and added upstream's new
   Confluent.Kafka 2.15.0.
 * Accepted upstream's Microsoft.Extensions.* bump (10.0.8 -> 10.0.9,
   10.6.0 -> 10.7.0) while re-inserting our
   Microsoft.Extensions.Diagnostics.ResourceMonitoring 10.5.0 (a UaLens
   dependency absent from master) in alphabetical position.
 * Kept ScottPlot.Avalonia 5.1.58 and took upstream's SharpFuzz
   2.2.0 -> 2.3.0 bump.

UA.slnx auto-merged and retained our McpServer project path.

UaLens build clean (0 warnings / 0 errors).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Ready to merge once CI Passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Source-Generated NodeManagers combining model design and Nodeset2 XML

6 participants