Commit Graph

3 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
romanett 16c54e52d9 Keep application certificates which do not name a CertificateType (#4348)
# Description

`SecurityConfiguration.ApplicationCertificates`'s setter removed every
identifier whose certificate type is not supported, and
`CertificateIdentifier.CertificateType` defaults to unset. An otherwise
correct identifier passed to the recommended
`AddSecurityConfiguration(ArrayOf<CertificateIdentifier> certIdList,
...)` overload was therefore dropped silently, and validation then
failed with `ApplicationCertificate must be specified.` — pointing at
the wrong cause. The obsolete `AddSecurityConfiguration(string
subjectName, ...)` overload escaped this because it assigns the legacy
singular `ApplicationCertificate` property, whose setter fills in
`RsaSha256ApplicationCertificateType` itself. The result: the
replacement advertised by the `[Obsolete]` message could not be used
without also setting `CertificateType`, which that message does not
mention.

**Changes:**

- `SecurityConfiguration.ApplicationCertificates`
(`src/Opc.Ua.Core/Schema/ApplicationConfiguration.cs`) now treats an
unset `CertificateType` as the default RSA-SHA256 application
certificate type instead of dropping the entry — the same assumption the
legacy single `ApplicationCertificate` element carries, so both paths
agree on what a valid `CertificateIdentifier` is. Fixing the setter also
covers callers that assign `ApplicationCertificates` directly, not just
the builder overload.
- Identifiers dropped for a genuinely unsupported type are no longer
discarded without a trace. They are remembered and reported by
`SecurityConfiguration.Validate`, which has an `ITelemetryContext` —
this replaces the `// TODO: Log when ITelemetry instance is available`
that was exactly this bug:
- a warning naming the rejected certificate types is logged whenever any
were filtered out;
- when the collection ends up empty *because* of the filtering, the
error now reads `No supported application certificate configured: N
certificate identifier(s) were rejected because their CertificateType is
not supported (...)` instead of `ApplicationCertificate must be
specified.`. The original message is kept for the genuinely-empty case.

**Notes for reviewers:**

- Behaviour is unchanged for every configuration that already names a
certificate type; all shipped and test config XMLs set
`CertificateTypeString`.
- `FindApplicationCertificateAsync` already had a fallback treating a
null certificate type as RsaSha256, and `ApplicationInstance` fills in
the type from the created/validated certificate when it is null —
defaulting in the setter is consistent with both and yields the same RSA
certificate.
- The unsupported-type filter itself is untouched; only unset types are
now kept.

## Related Issues

- Fixes #4315

## Checklist

- [x] 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.
- `SecurityConfigurationTests`: an untyped identifier survives as
RsaSha256 and contributes `Basic256Sha256` to
`SupportedSecurityPolicies`; an unsupported type
(`EccCurve25519ApplicationCertificateType`) is removed and reported by
`Validate` with `BadConfigurationError`; a supported entry survives
alongside an unsupported sibling without failing validation.
- `ApplicationConfigurationBuilderTests`: end-to-end reproduction from
the issue, building a client through
`AddSecurityConfiguration(ArrayOf<CertificateIdentifier>, pkiRoot)` +
`CreateAsync()`.
- [ ] 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.
- Ran on net10.0: `Opc.Ua.Configuration.Tests` 225/225 passed,
`Opc.Ua.Core.Tests` 4368 passed / 86 skipped / 0 failed. The full
solution across .NET Framework was not run locally — relying on CI for
that.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-30 20:27:20 +00:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00