Fix source generator dropping BaseAnalogType EURange/EngineeringUnits on instances (#3964) (#3966)

# Description

The model source generator (`ModelSourceGeneratorGenerateNodeManager`)
split an instance's child nodes into an **always-emitted** list and an
`if (!forInstance)` **type-template** list. Explicitly-declared
descendants of a *concrete predefined instance* whose modelling rule
only resolved to *Optional* — for example the `EURange` /
`EngineeringUnits` properties of a `BaseAnalogType` / `AnalogUnitType`
variable nested under an instance — were placed in the gated list.
Because the per-instance factory is always invoked with `forInstance:
true`, those property children were **never materialized** on the actual
instance. The pre-existing "top-level instance" exception in
`NodeStateGenerator.LoadTemplate_ReplaceChild` only covered an
instance's *direct* children, not deeper descendants (e.g. `EURange`
under `AI01` under `MyEquipment03`).

Both symptoms reported in the issue stem from this single bug:

1. **Instances were missing `Range` / `EU`** — the property nodes were
never created.
2. **`WithEURange` / `WithEngineeringUnits` (in `Configure`) appeared to
do nothing** — with the property nodes missing, the fluent helpers
created *unregistered* replacement nodes whose values were never served
to clients.

## Fix

`NodeStateGenerator.LoadTemplate_ReplaceChild` now also emits
**explicitly-declared descendants of any concrete predefined instance**
(`IsUnderSingletonInstance`, not a type-definition/type-instance
context, explicitly defined) into the always-emitted list. The new
condition is gated on `!UseTypeDefinitionModellingRules`, so SDK-managed
singletons (`Server`, `ServerConfiguration`, …) — generated with that
option **on** — keep the deliberate `#3768` optional-child suppression.
The core stack (generated with the option on) is therefore unaffected;
the change only corrects user models generated with the option off.

## Verification

- **End-to-end**: built a server from the exact NodeSet2 attached to the
issue and browsed/read it with a real OPC UA client.
`MyEquipment03.AI01` / `AO01` now expose `EURange` (`-20..200`) and
`EngineeringUnits`. With the reporter's `Configure` present the values
become `-20..200` / `Kelvin` (`AO01` reads `10`); with no `Configure`
the nodeset-declared values (`°C`, `-20..200`) materialize — confirming
both reported problems are resolved.
- **Regression test** added
(`ConcreteInstanceAnalogChildren_AreMaterializedUnconditionally`) with a
self-contained `AnalogInstance.NodeSet2.xml` fixture. It passes with the
fix and fails without it.
- **No regressions**: full `Opc.Ua.SourceGeneration.Core.Tests` suite
(3702 passed / 8 analyzer-gated skips, including the `#3768`
singleton-dispatch and stack-generation tests), `Opc.Ua.WotCon.Tests`
(342 passed), and clean builds of the DI / GDS / WotCon / Pumps / Boiler
/ Quickstarts model-consuming projects.

## Related Issues

- Fixes #3964
This commit is contained in:
Marc Schier
2026-07-07 16:03:42 +02:00
committed by GitHub
parent 44560fd3aa
commit 533ed1c358
3 changed files with 295 additions and 2 deletions
@@ -28,9 +28,12 @@
* ======================================================================*/
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.Extensions.Logging;
@@ -353,6 +356,136 @@ namespace Opc.Ua.SourceGeneration.Generator.Tests
});
}
/// <summary>
/// Regression test for issue #3964. A concrete predefined instance
/// (<c>AnalogDevice</c>) declares a <c>BaseAnalogType</c> variable
/// (<c>Measurement</c>) whose <c>EURange</c> / <c>EngineeringUnits</c>
/// property children are explicitly present on the instance but only
/// resolve to <c>Optional</c> (inherited from <c>BaseAnalogType</c>,
/// not restated as Mandatory on the instance). Before the fix the
/// generated instance-level factory emitted the creation of those
/// children inside the <c>if (!forInstance)</c> type-template block,
/// so — because the factory is always invoked with
/// <c>forInstance: true</c> — the properties were dropped from the
/// actual instance. They must instead be created unconditionally,
/// exactly like the type-level factory does.
/// </summary>
[Test]
public void ConcreteInstanceAnalogChildren_AreMaterializedUnconditionally()
{
// Arrange
ITelemetryContext telemetry = NUnitTelemetryContext.Create(logLevel: LogLevel.Error);
// Act
Dictionary<string, string> files = GenerateFromNodeSet(
"AnalogInstance.NodeSet2.xml", telemetry);
// Assert
string ex = files.Single(
kv => kv.Key.EndsWith(".NodeStates.ex.g.cs", StringComparison.Ordinal)).Value;
// The concrete-instance factory for the analog variable
// (Create<Instance>_<Var>) is distinct from the type-level
// factory (Create<Type>_<Var>). Only the instance one regressed.
string instanceFactory = ExtractMethodBody(ex, "CreateAnalogDevice_Measurement");
// The variable must be recognised as an analog state so that the
// typed EngineeringUnits / EURange children (and the fluent
// WithEURange / WithEngineeringUnits helpers) are available.
Assert.That(instanceFactory,
Does.Contain("global::Opc.Ua.BaseAnalogState"),
"Measurement should be generated as a BaseAnalogState.");
Assert.Multiple(() =>
{
Assert.That(instanceFactory, Does.Contain("CreateOrReplaceEURange"),
"Instance factory must create the EURange child.");
Assert.That(instanceFactory, Does.Contain("CreateOrReplaceEngineeringUnits"),
"Instance factory must create the EngineeringUnits child.");
});
int idxGate = instanceFactory.IndexOf(
"if (!forInstance)", StringComparison.Ordinal);
int idxEuRange = instanceFactory.IndexOf(
"CreateOrReplaceEURange", StringComparison.Ordinal);
int idxEngineeringUnits = instanceFactory.IndexOf(
"CreateOrReplaceEngineeringUnits", StringComparison.Ordinal);
if (idxGate >= 0)
{
Assert.Multiple(() =>
{
Assert.That(idxEuRange, Is.LessThan(idxGate),
"EURange must be created before the `if (!forInstance)` gate " +
"so it materialises on the actual instance (issue #3964).");
Assert.That(idxEngineeringUnits, Is.LessThan(idxGate),
"EngineeringUnits must be created before the `if (!forInstance)` " +
"gate so it materialises on the actual instance (issue #3964).");
});
}
}
private static Dictionary<string, string> GenerateFromNodeSet(
string nodeSetResource,
ITelemetryContext telemetry)
{
using var fileSystem = new VirtualFileSystem();
string path = Path.Combine(
Directory.GetCurrentDirectory(), "Resources", nodeSetResource);
var nodesets = new global::Opc.Ua.SourceGeneration.NodesetFileCollection(
ImmutableArray.Create(
(path, new global::Opc.Ua.SourceGeneration.NodesetFileOptions())),
fileSystem,
telemetry);
// The test compilation has no Opc.Ua.Server reference, so
// suppress fluent-builder emission (mirrors model-only projects).
nodesets.GenerateCode(
fileSystem,
string.Empty,
telemetry,
new GeneratorOptions { OmitFluentApi = true });
return fileSystem.CreatedFiles
.Where(c => Path.GetExtension(c) == ".cs")
.ToDictionary(c => c, c => Encoding.UTF8.GetString(fileSystem.Get(c)));
}
/// <summary>
/// Extracts the source of the factory method whose signature begins
/// with <c>internal static … {methodName}(</c>. Returns the body from
/// the method definition up to (but not including) the next
/// static-method definition, so callers can assert on ordering within
/// a single method without matching call sites elsewhere in the file.
/// </summary>
private static string ExtractMethodBody(string code, string methodName)
{
System.Text.RegularExpressions.Match match = Regex.Match(
code,
@"internal static [^\r\n]*\b" + Regex.Escape(methodName) + @"\(");
Assert.That(match.Success, Is.True,
$"Method definition '{methodName}' not found in generated code.");
int start = match.Index;
int end = code.Length;
foreach (string marker in new[]
{
"\n internal static",
"\n public static",
"\n private static"
})
{
int idx = code.IndexOf(
marker, start + match.Length, StringComparison.Ordinal);
if (idx >= 0 && idx < end)
{
end = idx;
}
}
return code.Substring(start, end - start);
}
private Mock<IFileSystem> m_mockFileSystem;
private Mock<IModelDesign> m_mockModelDesign;
private Mock<ITelemetryContext> m_mockTelemetry;
@@ -0,0 +1,134 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
* Regression fixture for issue #3964.
*
* A concrete predefined instance ("AnalogDevice") of an ObjectType whose
* mandatory child variable ("Measurement") is a BaseAnalogType. The
* instance explicitly declares the Measurement variable together with its
* EURange / EngineeringUnits property children. Those properties are only
* Optional on BaseAnalogType (they are not restated as Mandatory on the
* concrete instance), so before the fix the generated instance factory
* emitted their creation inside the `if (!forInstance)` type-template block
* and dropped them from the actual instance.
-->
<UANodeSet xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:uax="http://opcfoundation.org/UA/2008/02/Types.xsd"
xmlns="http://opcfoundation.org/UA/2011/03/UANodeSet.xsd">
<NamespaceUris>
<Uri>http://test.org/UA/AnalogInstance/</Uri>
</NamespaceUris>
<Models>
<Model ModelUri="http://test.org/UA/AnalogInstance/" PublicationDate="2024-01-01T00:00:00Z" Version="1.0.0">
<RequiredModel ModelUri="http://opcfoundation.org/UA/" PublicationDate="2024-01-01T00:00:00Z" Version="1.05.03"/>
</Model>
</Models>
<Aliases>
<Alias Alias="Double">i=11</Alias>
<Alias Alias="HasComponent">i=47</Alias>
<Alias Alias="HasProperty">i=46</Alias>
<Alias Alias="Organizes">i=35</Alias>
<Alias Alias="HasSubtype">i=45</Alias>
<Alias Alias="HasTypeDefinition">i=40</Alias>
<Alias Alias="HasModellingRule">i=37</Alias>
</Aliases>
<UAObjectType NodeId="ns=1;i=1000" BrowseName="1:AnalogDeviceType">
<DisplayName>AnalogDeviceType</DisplayName>
<References>
<Reference ReferenceType="HasSubtype" IsForward="false">i=58</Reference>
<Reference ReferenceType="HasComponent">ns=1;i=1001</Reference>
</References>
</UAObjectType>
<UAVariable DataType="Double" NodeId="ns=1;i=1001" BrowseName="1:Measurement" ParentNodeId="ns=1;i=1000">
<DisplayName>Measurement</DisplayName>
<References>
<Reference ReferenceType="HasTypeDefinition">i=15318</Reference>
<Reference ReferenceType="HasModellingRule">i=78</Reference>
<Reference ReferenceType="HasProperty">ns=1;i=1002</Reference>
<Reference ReferenceType="HasProperty">ns=1;i=1003</Reference>
</References>
</UAVariable>
<UAVariable DataType="i=884" NodeId="ns=1;i=1002" BrowseName="EURange" ParentNodeId="ns=1;i=1001">
<DisplayName>EURange</DisplayName>
<References>
<Reference ReferenceType="HasTypeDefinition">i=68</Reference>
<Reference ReferenceType="HasModellingRule">i=78</Reference>
</References>
</UAVariable>
<UAVariable DataType="i=887" NodeId="ns=1;i=1003" BrowseName="EngineeringUnits" ParentNodeId="ns=1;i=1001">
<DisplayName>EngineeringUnits</DisplayName>
<References>
<Reference ReferenceType="HasTypeDefinition">i=68</Reference>
<Reference ReferenceType="HasModellingRule">i=78</Reference>
</References>
</UAVariable>
<UAObject NodeId="ns=1;i=2000" BrowseName="1:AnalogDevice" ParentNodeId="i=85">
<DisplayName>AnalogDevice</DisplayName>
<References>
<Reference ReferenceType="Organizes" IsForward="false">i=85</Reference>
<Reference ReferenceType="HasTypeDefinition">ns=1;i=1000</Reference>
</References>
</UAObject>
<UAVariable DataType="Double" NodeId="ns=1;i=2001" BrowseName="1:Measurement" ParentNodeId="ns=1;i=2000">
<DisplayName>Measurement</DisplayName>
<References>
<Reference ReferenceType="HasComponent" IsForward="false">ns=1;i=2000</Reference>
<Reference ReferenceType="HasTypeDefinition">i=15318</Reference>
<Reference ReferenceType="HasProperty">ns=1;i=2002</Reference>
<Reference ReferenceType="HasProperty">ns=1;i=2003</Reference>
</References>
</UAVariable>
<UAVariable DataType="i=884" NodeId="ns=1;i=2002" BrowseName="EURange" ParentNodeId="ns=1;i=2001">
<DisplayName>EURange</DisplayName>
<References>
<Reference ReferenceType="HasProperty" IsForward="false">ns=1;i=2001</Reference>
<Reference ReferenceType="HasTypeDefinition">i=68</Reference>
</References>
<Value>
<uax:ExtensionObject>
<uax:TypeId>
<uax:Identifier>i=885</uax:Identifier>
</uax:TypeId>
<uax:Body>
<uax:Range>
<uax:Low>-5</uax:Low>
<uax:High>95</uax:High>
</uax:Range>
</uax:Body>
</uax:ExtensionObject>
</Value>
</UAVariable>
<UAVariable DataType="i=887" NodeId="ns=1;i=2003" BrowseName="EngineeringUnits" ParentNodeId="ns=1;i=2001">
<DisplayName>EngineeringUnits</DisplayName>
<References>
<Reference ReferenceType="HasProperty" IsForward="false">ns=1;i=2001</Reference>
<Reference ReferenceType="HasTypeDefinition">i=68</Reference>
</References>
<Value>
<uax:ExtensionObject>
<uax:TypeId>
<uax:Identifier>i=888</uax:Identifier>
</uax:TypeId>
<uax:Body>
<uax:EUInformation>
<uax:NamespaceUri>http://www.opcfoundation.org/UA/units/un/cefact</uax:NamespaceUri>
<uax:UnitId>4408652</uax:UnitId>
<uax:DisplayName>
<uax:Text>°C</uax:Text>
</uax:DisplayName>
<uax:Description>
<uax:Text>degree Celsius</uax:Text>
</uax:Description>
</uax:EUInformation>
</uax:Body>
</uax:ExtensionObject>
</Value>
</UAVariable>
</UANodeSet>
@@ -1579,6 +1579,32 @@ namespace Opc.Ua.SourceGeneration
(node.Parent.Design is not InstanceDesign ||
!m_context.Options.UseTypeDefinitionModellingRules);
// Explicitly-declared descendants of a concrete predefined instance
// (a user-modelled <opc:Object>/<opc:Variable> declared under
// Objects, e.g. a device instance) must be materialised on the
// actual instance regardless of their (possibly type-inherited)
// modelling rule: they are real nodes present in the model, not
// modelling-rule-driven optional children the SDK adds on demand.
// The topLevelInstanceException above only covers the instance's
// DIRECT children (node.Parent.Parent == null); deeper descendants
// — e.g. the EURange / EngineeringUnits properties of a
// BaseAnalogType / AnalogUnitType variable nested under the
// instance (issue #3964) — inherit IsUnderSingletonInstance and
// would otherwise land in the `if (!forInstance)` type-template
// block and be dropped from the instance. SDK-managed singletons
// (Server, ServerConfiguration, …) are generated with
// UseTypeDefinitionModellingRules on and keep the #3768
// optional-child suppression, so they are excluded here.
bool concreteInstanceDescendant =
node.IsUnderSingletonInstance &&
!node.RootIsTypeDefinition &&
node.InstanceOf == null &&
!node.IsNotExplicitlyDefined &&
!m_context.Options.UseTypeDefinitionModellingRules;
bool alwaysEmitInstanceChild =
topLevelInstanceException || concreteInstanceDescendant;
if (context.Token == Tokens.ListOfOptionalChildNodeStates)
{
if (effectiveRule == ModellingRule.Mandatory)
@@ -1588,7 +1614,7 @@ namespace Opc.Ua.SourceGeneration
// Real instance children of a top-level (non-typed) parent belong in
// the always-emitted list so they materialize for the actual instance,
// not only when the node is treated as a type template.
if (topLevelInstanceException)
if (alwaysEmitInstanceChild)
{
return null;
}
@@ -1598,7 +1624,7 @@ namespace Opc.Ua.SourceGeneration
else if (effectiveRule != ModellingRule.Mandatory)
{
// Exception: real instance children of a top-level (non-typed) parent.
if (!topLevelInstanceException)
if (!alwaysEmitInstanceChild)
{
return null;
}