Commit Graph

2 Commits

Author SHA1 Message Date
romanett 9b034f8291 Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700)
# 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>
2026-10-08 08:48:13 +02:00
Marc Schier be61d598a9 Fix startup NodeManager lifecycle and imported method arguments (#4427)
# Description

Make startup-composed application NodeManagers participate in the live
lifecycle and bind imported NodeSet2 Method argument properties to their
typed `MethodState` children.

This change:

- adopts application NodeManagers created before startup into
`INodeManagerLifecycle.Registrations` after server startup succeeds,
while keeping diagnostics, configuration, and core managers protected;
- retains per-manager startup external-reference ownership and exact
routing positions so reload, removal, rollback, overlapping namespaces,
synchronous adapters, and server restart preserve existing behavior;
counted immutable reference snapshots avoid quadratic startup matching;
- rejects duplicate factory output across active, unpublished, and
retired lifecycle generations without deleting or disposing the live
manager; callers can catch the public
`NodeManagerAlreadyRegisteredException`;
- binds valid namespace-zero `Argument[]` `PropertyType`
`InputArguments` and `OutputArguments` Variables to
`MethodState.InputArguments` and `MethodState.OutputArguments` from
either `HasProperty` direction, without requiring `ParentNodeId`,
including local namespace-URI reference targets;
- preserves authored metadata during NodeSet typed-child promotion
without changing the initialization behavior of unrelated generated
nodes;
- retains generic-import guards for custom and malformed signatures,
duplicate standard-signature validation, and upstream factory-driven
generated-slot replacement and reconciliation callbacks;
- updates lifecycle and Runtime NodeSet documentation and clarifies
`OnServerStartedAsync` timing for both `StartAsync` overloads, including
the unopened default host returned to the caller.

This is a focused follow-up to #4418. That PR preserves registrations
from multiple fluent builders, but does not change lifecycle ownership
or NodeSet child typing. The branch includes upstream `master` through
`da7412c42`, including #4396, #4432, #4437, #4438, and #4440.

The completed review fixes in `686d0561f` are included. The latest base
merge (`a29fc6ca8`) combines them with #4432's shared batch/fluent
importer and generated state factories. One iterative linker supports
available parent nodes, deferred parent side tables, application-owned
handles, and placeholder reconciliation while retaining reference-driven
Method argument binding and URI handling. Upstream cancellation, GDS
assembly renames, certificate leak scoping, and xRegistry changes are
preserved.

## Related Issues

- Fixes #4421
- Fixes #4422

## CI regression fix

`e1a4436f0` fixes the GDS failure reported on Ubuntu and macOS. Copying
permission metadata in general `NodeState` initialization applied
`SecurityAdmin`-only defaults to generated GDS custom certificate groups
and denied their existing GDS-admin Browse requests. Metadata
preservation is now confined to NodeSet typed-child promotion; imported
Method argument permissions remain intact, while unrelated generated
nodes retain their existing initialization behavior.

Local Release validation also exposed a 260-character certificate path
in the startup-failure test added by this PR. Its isolated PKI now uses
a short temporary path. No assertions, authorization checks, or CI gates
were weakened, and no pre-existing tests were changed.

## macOS ARM64 NativeAOT suspension

Per maintainer request, `a15971f32` suspends only the failing
`macos-latest` / `osx-arm64` NativeAOT matrix entry. Re-enabling it once
the compiler contains the runtime fix is tracked in #4443. Intel macOS,
Linux, and Windows NativeAOT coverage, managed macOS tests, and all
other CI gates remain enabled. This is an explicit coverage suspension,
not a claim that the underlying compiler/linker defect is fixed.

The macOS ARM64 NativeAOT job on `a29fc6ca8` fails during native linking
with
`ld: Assertion failed: (_addend == uniqueIndex && "too many large
addends")`.
Both attempts of [run
34191755950](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34191755950)
hit the same assertion before the native test executable can run; the
build/test summary failure is downstream.

This matches the NativeAOT Mach-O relocation issue in
dotnet/runtime#119380, fixed by relocation-anchor emission in
dotnet/runtime#124721 and its .NET 10 backport dotnet/runtime#132171.
The failing `Microsoft.DotNet.ILCompiler` 10.0.11 package's nuspec
points to `dotnet/dotnet` commit
`e2f47b0110ed922f21a1522da67279133ce28f32` (July 23); its object writer
does not contain the anchor fix merged on August 12. Version 10.0.11 is
currently the latest stable package.

The base commit passed on the same runner image and SDK, so this is not
being dismissed as a generally broken runner or a test flake. The
precise PR-specific object-layout trigger has not been isolated. A
compiler containing the relocation fix is needed before restoring the
lane. No test code, assertions, coverage thresholds, or shared tooling
settings were changed.

## Validation

Latest base-merge validation on both `net10.0` and `net48`, Release:

- Import helpers and argument-binding regressions: 51 passed per
framework.
- Batch/fluent import, generated-state overlays, runtime/startup
lifecycle, cross-source references, routing, reference snapshots, and
node-authoring selection: 344 passed per framework.
- Merge-specific cases cover available Method parents with
forward/inverse and URI/index references while retaining an
application-owned `NodeId` handle.
- The unchanged upstream `NodeSetImportIntegrationTests.cs:210` CA1861
warning remains; no diagnostic suppression or unrelated cleanup was
added.

Release CI-fix validation on both `net10.0` and `net48`:

- `CustomCertificateGroupIntegrationTest`: 4 passed, including the
unchanged CI failure.
- `UANodeSetHelpersTests`: 8 passed, including imported security
metadata.
- `StartupRuntimeNodeSetLifecycleTests`: 5 passed.

Earlier Debug merge and review validation on both frameworks:

- Lifecycle, RuntimeNodeSet, fluent, and OperationContext selection
after merging `978e6d1af`: 638 passed per framework.
- Startup/transport-binding and RuntimeNodeSet integration selection
after merging `017856f4a`: 28 passed per framework.
- Public exception contract and duplicate-registration selection in
`fbf08428b`: 6 passed per framework.

Targeted test invocations rebuilt the affected projects successfully.
The full `UA.slnx` suite was not rerun locally.

## 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.
- [x] 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.
- [x] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bb8a834-7695-4ae6-84cb-2471c76d1b12
2026-09-08 11:11:53 +00:00