# Description
Nullable reference types are now enabled once, in `common.props`, and
every project in the repository builds warning-clean with them.
**Configuration**
- `<Nullable>enable</Nullable>` lives in `common.props`. All per-project
`<Nullable>` settings are removed: 178 project files (enable,
annotations, and the `disable` in the three Aot test projects).
- 419 per-file `#nullable enable`/`restore` directives and the 4
`#nullable disable` directives are removed.
- Kept on purpose:
- The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type
Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`,
`PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the
project setting in generated code, so the directive is required there.
- `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against
the 1.5.378 packages and does not import the repository props.
- `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now
describe the central setting.
**Migration** (the 43 unset and 13 annotations-only projects, roughly
10k diagnostics)
- **Production code: annotated at the declarations.** Covers
`tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the
LDS sample.
- Members, parameters and returns are nullable where null actually
flows.
- Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`,
`IsMethodTypeDesign`).
- `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take
`params object?[]`. `Template.AddReplacement` takes `string?` (rendering
already skips null replacements).
- `IRegisteredServerStore.RegisterAsync` returns
`Task<RegistrationEntry?>`: it returns null when a registration is
removed.
- Dereferences backed by an invariant (for example, the validated model)
use `!`, so runtime behaviour is unchanged.
- **Behavioural changes in production code** (small, deliberate):
- `NodeSetToModelDesign` throws `InvalidOperationException` when the
file is not a NodeSet. Before, it threw a NullReferenceException on the
next line.
- `ModelDesignValidator` starts its lookup tables empty instead of null.
A redundant `if (m_nodes == null) return;` guard is gone.
- **Test code: minimal annotations.**
- `?` on values that can be null, `!` where the test asserts non-null.
- `= null!` for fixture members assigned in SetUp.
- `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the
`IEncodeable` constraint.
- A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`)
folded into the adjacent `IsNull`/`Count` checks.
**Notes for reviewers**
- The diff is large (~1,500 files) but almost entirely mechanical: `?`,
`!` and directive removal. The non-annotation changes are listed above.
- Known follow-up, not done here:
`IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay
non-nullable even though the validator returns null before it loads a
dictionary. Making them nullable would ripple through every generator
call site.
## Related Issues
- Fixes#4439
## Checklist
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] 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.
The "no new warnings" box covers net10.0 and net48: full non-incremental
builds of `UA.slnx`, `tools/SourceGeneration.slnx`,
`tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0
warnings and 0 errors under the repository's warnings-as-errors
settings. net9.0 and net8.0 also had no nullable diagnostics, but they
were not rebuilt after the final fixes. Tests have not been run locally
yet.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
# Description
Fixes the defects a read-only audit reported against `Opc.Ua.Types` at
a05a36055: 98 of its 99 findings for `Variant`/`TypeInfo`, the
identifier types, the four codecs, `NodeState`/`Nodes`,
`Schema`/`Utils`/`Polyfills` and the WoT binding, plus **21 further
defects found while reviewing those fixes** — several of which were
regressions the first pass introduced. Every fix carries a regression
test that fails without it.
`Opc.Ua.Types` is the lowest-level assembly in the stack, so most of
these are silent wrong answers in code every other project sits on
rather than crashes.
## The higher-impact ones
**`Variant` equality, hashing and ordering.** `ValueEquals` and
`CompareTo` — the ContentFilter comparison surface, whose only consumer
is `FilterEvaluator` — now convert between numeric built-in types as
Part 4 §7.7.3's implicit-conversion and data-precedence rules require.
`Variant.Equals` deliberately does **not**: it stays strict on
`BuiltInType`, so it remains consistent with `GetHashCode` and safe as a
`Dictionary`/`HashSet` key. `GetHashCode` read
`Float`/`Double`/`Enumeration`/`DateTime` through the wrong union field
and hashed byte arrays by reference, so equal values hashed apart.
Comparing a null variant against a typed one gave a different answer
depending on which side it was written on; that is now decided once and
symmetrically, and a numeric or boolean zero stays *unequal* to a null
variant — letting it compare equal collapsed both into one bucket of
every `Dictionary`/`HashSet` keyed on `Variant`, made `Equals`
intransitive, and made a `HashSet`'s count depend on insertion order.
`CompareTo` reports an unrelated pair as not comparable instead of
ordering it by raw union bits, and the comparison operators now reject
that sentinel — reading `int.MinValue` as an ordinary negative result
made both `a < b` and `b < a` true, which `OrderBy` detects and throws
on.
**`NodeId.CompareTo` was not transitive.** The null case reported "less
than" in both directions for every identifier shape except a
namespace-zero numeric id, so a `SortedSet<NodeId>` could not find a
`NodeId.Null` it had just inserted.
**Encoding limits.** `MaxStringLength` counts the **bytes** of the
encoded string (Part 3 §5.6.4, Part 5 §6.3.2). The XML codecs counted
UTF-16 code units, so one message context gave two answers and a string
a binary peer rejects survived an XML round trip. The measurement now
lives in a single `EncodingLimits` helper, replacing three separate
copies across the codecs. The JSON decoder additionally enforces the
array, byte-string and string limits it never had — including on the
base64 and shorthand spellings that bypassed the checks entirely, so a
peer could opt out of a limit just by choosing the other legal encoding.
An encoding-limit breach is also no longer downgraded into a successful
decode that keeps the raw over-limit body.
**`Node.Write` threw out of a status-code API.** The attribute type gate
compared built-in types only, which does not separate a scalar from an
array, so an array value passed and the unconditional cast behind it
threw `InvalidCastException` out of a method contracted to return a
`ServiceResult`. It now compares the value rank too, and the structured
permission attributes became writable at all (their data type has no
built-in type, so the gate had rejected every value).
**`CoreUtils.Match`.** Rejects an unmatched trailing character without
breaking the single-character wildcard — an earlier version of this fix
made every pattern ending in `?` return false, which no existing test
covered.
Spec conformance was checked against Part 3, 4, 5 and 6 for each
behavioural change: implicit conversion and data precedence (4 §7.7.3),
`NumericRange` BNF (4 §7.27/A.3), `returnDiagnostics` bits (4 §7.32),
`MinimumSamplingInterval` as a `Duration` (3 §5.6.2), `ServerIndex` as
UInt32 (6 §5.2.2.10), DateTime UTC (6 §5.3.1.6), Union `SwitchField` (6
§5.2.8, §5.3.7, H.10), and the verbose JSON Enumeration form (6
§5.4.4.2).
## Reviewer notes
- **One deliberate behaviour tightening.** XML with more than
`MaxStringLength` non-ASCII *bytes* is now rejected where it previously
passed on *character* count. Nothing in the repo's fixtures trips it,
but it is a real narrowing for a deployment with a low `MaxStringLength`
and non-ASCII display names.
- **One finding deliberately left open.** `NodeState.Initialize` still
does not carry over the optional storage that `CopyTo` copies. It
instantiates a node from a prototype rather than cloning it, and
propagating the prototype's `RolePermissions` makes the GDS
certificate-group nodes unreadable for users that could read them before
(`CustomCertificateGroupNodeExistsInAddressSpaceAsync` fails with
`BadUserAccessDenied`). Whether an instance inherits its prototype's
permissions is a security decision for the address space, so the
omission is now documented in place rather than quietly closed. Happy to
split it into its own issue.
- **Nine existing tests were updated** because they pinned the buggy
behaviour. Each is worth a look:
`EqualsWithDifferentNumericTypesReturnsFalse` asserted the opposite of
Part 4 §7.7.3 and was renamed with a replacement negative case;
`XmlElementGetHashCodeShouldReturnCorrectHashCode` pinned the textual
hash that broke the `Equals` contract; the `CompareTo`-versus-null cases
in `NodeIdTests`/`ExpandedNodeIdTests` asserted the non-antisymmetric
answers; three `DateTimeUtcTests` parse cases depended on the local time
zone.
- 21 of the fixes (5,558 insertions across 74 files) came out of an
adversarial review of the first pass, with each candidate independently
verified against the built assembly rather than by inspection — the
`Variant` null-equality, `NodeId.CompareTo`, `Node.Write`,
`CoreUtils.Match` and JSON-limit items above are all in that group.
## Related Issues
No tracking issue exists — this came from an internal read-only audit
rather than a reported defect. Happy to open one, or to split the WoT,
encoder and `Variant` groups into separate issues if the maintainers
prefer that shape for a change this size.
- Fixes #
## Checklist
_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] 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.
- [x] 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.
### Notes on the checklist
- **Tests:** every fix has a regression test; 9 new test files, ~130 new
test cases. Several were verified discriminating by temporarily
reverting the production line and confirming the test fails.
- **Warnings:** full `UA.slnx` build is clean, and `dotnet format
whitespace --verify-no-changes` reports the same 101 pre-existing
findings in the same file set as an unmodified worktree at the same
commit — none in changed files.
- **Test runs:** all suites were run on **net10.0 only**, so that box is
left unchecked pending a .NET Framework run: Types 10374, Core 4490,
Core.Encoders 2929, Core.Schema 157, Configuration 227, Server 5540,
Client 2217, Gds 1117, PubSub 1369, WotCon 1496, ComplexTypes 2773,
InformationModel 1080, Di 425 — all passing, 0 failures. Two failures in
`Core.Diagnostics.Tests` and `OpenUsd.Tests` are pre-existing on master
and caused by a German locale decimal separator, confirmed against a
clean baseline worktree at the same commit.
- **Documentation:** no separate docs; the new and changed members carry
XML doc comments explaining the spec clause behind each behavioural
decision.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>