Enable nullable in Stack, Libraries (9), and Applications projects#3732
Draft
marcschier wants to merge 114 commits intomasterfrom
Draft
Enable nullable in Stack, Libraries (9), and Applications projects#3732marcschier wants to merge 114 commits intomasterfrom
marcschier wants to merge 114 commits intomasterfrom
Conversation
- Add #nullable enable to AssemblyInfo, HttpsServiceHost, HttpsTransportListener - Make events nullable, fields/properties null! initialized - ValidateClientCertificate parameters nullable to match delegate signature - Local variables and bestPolicy declared nullable Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable to 50 source files - State machine files: use !. for required mandatory state properties - ConditionState/AlarmConditionState/etc: nullable annotations on backing fields - FiniteStateMachineState: nullable parameters for state/transition variables - Types files (ContentFilter/MonitoringFilter/etc): nullable result classes - Validate methods returning ServiceResult? when null=success - ToString IFormattable signature with nullable params - Timer fields and TimerCallback parameters made nullable Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable to ~107 of 187 source files in Opc.Ua.Types - Apply nullable annotations to interface signatures (IDecoder, IEncoder, IType) - Fix override Equals/CompareTo/IFormattable.ToString signatures - Make IDecoder.ReadString/Array methods return nullable to match implementations - IEnumeratedType.TryGetSymbol/IEncoder.WriteSwitchField use nullable out string - Fix Polyfills attributes for nullable awareness - Fix EnumValueTests for nullable IEnumeratedType signature - Fix MonitoredItem.cs for nullable EventFieldList.Message Note: ~80 complex files (BinaryEncoder, NodeState, etc.) remain to be migrated in a future commit. Project builds clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update Read/Write methods to accept string? fieldName since arrays and internal callers pass null. Update JsonEncoder/JsonDecoder implementations to match. Returns also nullable for ReadString/ ReadDataValue/ReadDiagnosticInfo to match JsonDecoder behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable to BuiltIn/Argument, RelativePath, Schema, etc. - Make IEncodable.IsEqual accept nullable IEncodeable - Make IEncoder Read/Write methods accept string? fieldName - Make IEncoder.WriteString/DataValue/DiagnosticInfo/Encodeable values nullable - Make IDecoder.Read* return types nullable to match implementations - Make class operator==/!= accept nullable for proper consumer use - Fix Argument operator== to use direct null check instead of EqualityComparer - Fix downstream NodeCache.cs to use 'is null' for ReferenceDescription comparison - Fix Subscription.cs to use null-forgiving on monitoredItem.RelativePath Progress: 117/187 Opc.Ua.Types files now nullable enabled. 70 complex files (Encoders, NodeState, Variant, etc.) remain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add nullable to BuiltIn types: BrowseDescription, ViewDescription, EnumDefinition, EnumField, EnumValueType, RelativePath, RelativePathFormatter, RolePermissionType, StructureDefinition, StructureField, ReferenceDescription, RelativePathElement - Add nullable to State files: NodeStateFactory - Add nullable to Utils files: ServiceMessageContext, ServiceResultException, etc. - Add nullable to Encoders: EncodableObject, Enumeration - Make DataValue operator==/!= accept nullable - Replace EqualityComparer<T>.Default.Equals with manual null check in operators - Fix downstream callers in Stack/Opc.Ua and Encoders/JsonDecoder.cs Remaining: 54 complex files (BinaryEncoder, BinaryDecoder, NodeState, Variant, etc.) require deeper changes and have ~2700 errors to address. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable to ~49 files in Opc.Ua.Core that build clean - Apply bulk fixes: ToString IFormattable signatures, Equals object override - Make encoder Read/Write methods accept string? fieldName - Make class operator==/!= accept nullable for class types - Replace EqualityComparer<T>.Default.Equals in operators with manual null check - Fix ClientBase.MessageContext to use null-forgiving (! operator) - Fix TraceLoggerProvider to handle nullable Exception parameter - Fix Session.cs and HttpsTransportListener.cs downstream callers Remaining: 81 complex files in Opc.Ua.Core (TcpTransportListener, ApplicationConfiguration, DirectoryCertificateStore, etc.) still require deeper migration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ClientBase.Endpoint, EndpointConfiguration, NullableTransportChannel: Use ! to match IClientBase interface (non-nullable returns). The properties throw ServiceResultException if the channel is in a bad state, otherwise consumers can rely on non-null returns. Also adds #nullable enable to IClientBase.cs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable directive - Update method signatures to match IEncoder (string? fieldName) - Make m_namespaceMappings and m_serverMappings nullable fields - Use null-forgiving for generic WriteEncodeable internal calls - Make CloseAndReturnBuffer return byte[]? - Throw InvalidOperationException in CloseAndReturnText if not MemoryStream - WriteSwitchField fieldName is out string? per interface Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Stack/Opc.Ua.Types/State/ISystemContext.cs - Stack/Opc.Ua.Types/State/NodeBrowser.cs - Stack/Opc.Ua.Types/State/BaseDataVariableState.cs - Stack/Opc.Ua.Types/State/BaseObjectState.cs - Stack/Opc.Ua.Types/State/DataTypeState.cs - Stack/Opc.Ua.Types/Nodes/IOperationContext.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Stack/Opc.Ua.Types/State/BaseTypeState.cs - Stack/Opc.Ua.Types/State/ReferenceTypeState.cs - Stack/Opc.Ua.Types/State/ViewState.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable directive - Update method signatures to match IDecoder (string? fieldName) - Make m_namespaceMappings and m_serverMappings nullable fields - Make ReadString return string? to match interface - Make ReadDataValue and ReadDiagnosticInfo return nullable types - Update ReadStringArray, ReadDataValueArray, ReadDiagnosticInfoArray to return ArrayOf<T?> - Use null-forgiving for generic ReadEncodeable internal calls - Suppress CS8620 for Variant.From with nullable element ArrayOf - Make CallerMemberName functionName parameters nullable Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable directive - Update method signatures to match IEncoder (string? fieldName) - Make m_root, m_destination, m_namespaceMappings, m_serverMappings nullable - BeginField helper now accepts string? fieldName - WriteString, WriteDataValue, WriteDiagnosticInfo accept nullable values - Throw InvalidOperationException in CloseAndReturnText if no destination - WriteSwitchField fieldName is out string? per interface Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Utils/CoreUtils.cs
- Utils/NamespaceTable.cs
- Utils/ServiceResult.cs
- Utils/Buffers/ArrayPoolBufferWriter{T}.cs
- Utils/FileSystem/VirtualFileSystem.cs
- BuiltIn/AmbientMessageContext.cs
- BuiltIn/EnumHelper.cs
- BuiltIn/ITranslatableObject.cs
Plus minor downstream fixes for nullability of ServiceResult properties
and EnumHelper return types in EnumValue.cs, EncodeableFactory.cs, and
ServiceResultException.cs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable directive - Update method signatures to match IDecoder (string? fieldName) - Make m_namespaceMappings, m_serverMappings nullable - Make ReadString return string? to match interface - Make ReadDataValue and ReadDiagnosticInfo return nullable types - Update ReadStringArray, ReadDataValueArray, ReadDiagnosticInfoArray return types - Make Peek return XmlQualifiedName? - Suppress CS8620 for Variant.From with nullable element ArrayOf - Make CallerMemberName functionName parameters nullable - Update CreateBadDecodingError and SafeXmlConvert to accept nullable parameters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Annotates DataValue, ExtensionObject, Matrix, DataContractSurrogates, and EqualityComparers with nullable reference types. Updates the corresponding Equals overloads, IFormattable.ToString signatures, IEquatable<T> contracts, IEqualityComparer<T> contracts, and operator == / != to use nullable parameters where the runtime can be null. Also fixes a downstream consumer in EncodableObject.cs that calls TryGetEncodeable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #nullable enable directive - Update method signatures to match IDecoder (string? fieldName) - Make m_namespaceMappings, m_serverMappings nullable - Make ReadString return string? to match interface - Make ReadDataValue and ReadDiagnosticInfo return nullable types - Update ReadStringArray, ReadDataValueArray, ReadDiagnosticInfoArray return types - Make Peek return XmlQualifiedName? - Make ElementContext.Element nullable - Suppress CS8620 for Variant.From with nullable element ArrayOf/MatrixOf - Make CallerMemberName functionName parameters nullable - Update CreateBadDecodingError and SafeXmlConvert to accept nullable parameters - Use null-forgiving for TypeInfo.GetXmlName results Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- UANodeSet.cs: add nullable annotations to auto-generated XML serialization classes - UANodeSetHelpers.cs: enable nullable, update method signatures for nullable inputs - BinarySchemaValidator.cs: enable nullable, accept nullable string params - OPCBinarySchema.cs: add nullable annotations to auto-generated XML schema classes - SchemaValidator.cs: enable nullable, accept nullable params for Load and Exception helpers - TypeDictionaryValidator.cs: enable nullable, handle nullable schema properties - UA Type Dictionary.cs: add nullable annotations to auto-generated XML classes - XmlSchemaValidator.cs: enable nullable, accept nullable typeName/file params - XmlSchemaValidator2.cs: enable nullable, accept nullable typeName/file params Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…StateCollection.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use null-forgiving for xml after string.IsNullOrEmpty checks (older targets) - Use default! for EqualityComparer<T>.Default.Equals in BinaryEncoder - Use null-forgiving for TypeInfo.GetXmlName results - Add null coalescing for fieldName in error messages - Suppress CS8604 for Variant.From with nullable DataValue Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After enabling #nullable on 52 files in Stack/Opc.Ua.Types, downstream projects had build errors due to stricter nullability annotations flowing through interfaces. This commit fixes those errors so the full solution builds cleanly. Stack/Opc.Ua.Types fixes: - JsonDecoder.cs: nullable string handling in namespace/server URI loops, suppression of generic variance for Variant.From overloads taking ArrayOf/MatrixOf with nullable elements - JsonEncoder.cs: nullable handling for TryGetEncodeable/TryGetAsJson/ SymbolicId - BuiltInType.Obsolete.cs, QualifiedName.cs, RelativePathFormatter.cs, ReferenceDescription.cs, BaseInstanceState.cs: standard nullable annotation/suppression patterns Stack/Opc.Ua, Stack/Opc.Ua.Core fixes: - ContentFilter.cs, MonitoringFilter.cs, NotificationMessage.cs, AlarmConditionState.cs, DialogConditionState.cs, AuditEventState.cs, FiniteStateMachineState.cs, HistoryReadValueId.cs, ReadValueId.cs, WriteValue.cs: nullable annotations for INode?, out parameters from TryGetEncodeable, NumericRange.Parse non-null arguments - ClientBase.cs, ServiceResultExtensions.cs, SecuredApplicationEncoding.cs: matching nullable annotations Libraries/Opc.Ua.Client fixes: - ComplexTypeSystem.cs, DataTypeDefinitionExtension.cs, NodeCache.cs, NodeCacheContext.cs, Session.cs, SessionClientBatched.cs, SessionClientExtensions.cs, SessionConfiguration.cs, Browser.cs, MonitoredItemStatus.cs, Subscription.cs: same patterns Applications/McpServer fixes: - OpcUaJsonHelper.cs, AttributeServiceTools.cs, ConvenienceTools.cs, NodeSetExportTools.cs: same patterns Tests fixes: - EnumHelperTests.cs, ServiceResultTests.cs, ClientBaseTests.cs: match new nullable signatures Source generator fix: - ObjectTypeProxyTemplates.cs: emit pragma to suppress CS8600 in generated TryGetStructure call sites where the analyzer ignores MaybeNullWhen(false) annotations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
24 inline ! operators removed via capture-once locals in 4 files. All
metadata/IL-equivalent: same null-throwing semantics, just collapses
N occurrences of the same X!.Y! chain into one local.
Files modified:
- Server/StandardServer.cs (-8): captured Configuration!/SecurityConfiguration
in OnUpdateConfigurationAsync and status!.Variable! in shutdown lambda
- Configuration/ConfigurationNodeManager.cs (-8): captured
m_serverConfigurationNode! once at top of CreateServerConfiguration
(used 9 times in method)
- Gds.Server.Common/ApplicationsNodeManager.cs (-7): captured
CertificateGroups!.DefaultXxxGroup!.CertificateTypes! once for each
of the 3 groups (used in if + else branches)
- Server/Subscription/MonitoredItem/QueueHandler/EventQueueHandler.cs (-1):
is { } pattern replacing if (x != null) { x!. }
Phase 2' originally targeted ~200 ! removed via capture-once. Actual
yield was 24 because:
- C# flow analysis already collapses subsequent reads after first !
in a scope, so most "repetitive" chains in source produce only 1
bang per distinct property per method
- Most candidates clustered at 2 bangs (below the >=3-! collapse rule)
- Several sites rejected because capture-once would change the
null-throwing point (different conditional branches reach different
lines in the chain)
The remaining ~3,200 inline ! operators in the codebase are largely
correct compile-time assertions of post-init invariants on
source-generated NodeState properties. Changing the source generator
to emit non-nullable would require model-level mandatory/optional
distinction and breaking-change semantics, beyond a metadata-only
cleanup pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The local SDK preview can't build Server (union-pattern issues), masking
30 nullable errors in Quickstarts.Servers consumer code. CI's GA SDK
caught them. Fixes:
Quickstarts.Servers (4 files):
- ReferenceServer.cs:194: guard StatusCode.SymbolicId is { } before passing
to ResourceManager.Add (avoids null deref of internal status code list)
- DataChangeMonitoredItem.cs: ! suppressions on m_lastValue, m_lastError,
DataChangeFilter, m_monitoredItemQueueFactory at use sites where the
fields are guaranteed non-null at the point of use
- AlarmNodeManager.cs:927: NodeHandle? initialHandle (GetManagerHandle is
nullable)
- TestData/TestDataNodeManager.cs: FindTypeState<T>() return now nullable;
consumers use ! since the variable is expected to exist
Library API nullable improvements (correct annotations matching runtime):
- OperationContext.ChannelContext: SecureChannelContext? (was non-null with
= null! initializer hiding the genuine nullability)
- IStoredSubscription.UserIdentityToken: UserIdentityToken? (genuinely null
for anonymous sessions)
- RestoreSubscriptionResult.Subscriptions: IEnumerable<IStoredSubscription>?
(null on failure)
- ViewDescription.IsDefault: parameter ViewDescription? (body already
null-checks)
- IMonitoredItem.QueueValue / MonitoredItem.QueueValue: ServiceResult? error
- MonitoredItem.Filter: MonitoringFilter? (genuinely nullable post-init)
- MonitoredItem ctor: originalFilter, filterToUse, range made nullable
- NodeState.cs: Notifier.Node accesses null-guarded; AddReference target
parameter NodeState? (with null guard)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ration
Bug 1 (Quickstarts.Servers/TestData/*ValueObjectState.cs ×6): Removed redundant
duplicate `GenerateValue(system, UInt32Value!)` line in OnGenerateValues. Investigation
confirmed all properties are already called and the second copy was simple dead code.
Bug 2 (Quickstarts.Servers/DurableSubscription/BatchPersistor.cs): Removed inverted
`!` from regex predicate in DeleteBatch. The method now deletes files matching
the target batch instead of files belonging to other batches.
Bug 3 (Quickstarts.Servers/Alarms/AlarmHolders/AlarmConditionTypeHolder.cs): Replaced
the FIRST `UpdateSuppression()` call with `UpdateShelving()` in SetValue. The
two branch bodies have distinct shelving / suppression message strings; the first
condition was the typo, the second was correct.
Bug 4 (Quickstarts.Servers/Alarms/AlarmNodeManager.cs): Removed dead
`if (sourceControllers == null)` checks at four sites in OnStart/OnStartBranch/
OnEnd/OnWriteAlarmTrigger. GetUnitAlarms returns a non-null shared dictionary
(it ignores the node parameter and just returns m_triggerMap), so the
BadNodeIdUnknown branch was never reachable. Unwrapped the `!= null` guards
and simplified to direct returns.
Bug 6 (Quickstarts.Servers/DurableSubscription/Durable{DataChange,Event}MonitoredItemQueue.cs):
Restore now treats null parameter as `new List<...>()` instead of assigning
null! to a non-null field. BatchPersistor.RestoreSynchronously legitimately
passes `restored?.Values` / `restored?.Events` which can be null when the
decoder yields no batch.
Bug 7 (Quickstarts.Servers/TestData/TestDataNodeManager.cs): Replaced redundant
`reader?.Dispose()` with `reader.Dispose()` at two sites in HistoryReadRaw.
A guarded `if (reader == null) return ...` earlier in the method narrows reader
to non-null for the remainder of the block.
Bug 8 (ConsoleReferenceClient/ClientSamples.cs): Added null guard on
`NodeCache.FindAsync` result in FetchAllNodesNodeCacheAsync; logs a warning
and skips when the starting node is unknown instead of NRE on indexing.
Bugs 9-10 (ConsoleReferenceClient/ClientSamples.cs): Replaced `as`/`!` cast
patterns with `is not ... type pattern { ... }` early returns in
OnMonitoredItemNotification and OnMonitoredItemEventNotification. A type
mismatch now logs a clear warning instead of NRE.
Bug 11 (ConsoleReferenceClient/UAClient.cs): Moved `Console.WriteLine` for
session creation success inside the `if (session != null && session.Connected)`
block in ConnectAsync. The log no longer accesses the late-init Session sentinel
when session creation succeeded but the session is not connected.
All bugs were flagged by sub-agents during the nullable-migration but suppressed
with `!` / `?.` per the migration's zero-behavior-change rule. This commit
applies the actual fixes and removes the four `// TODO` comments left as
follow-up markers in ConsoleReferenceClient.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
13 sites caught by CI's GA SDK after the prior commit (ff1e77d) widened upstream library API parameters to nullable. Local SDK preview's `unions` feature issue masks these locally, so they were not seen during the prior commit's local validation. All sites resolved with `!` operators (justified by upstream invariants: Activate/Publish paths always carry a non-null ChannelContext; m_filterToUse is non-null after Modify/SetMonitoringMode in MonitoredItem; storage properties match the existing late-init `null!` pattern). No public API surface widened. Files modified: - Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs - Libraries/Opc.Ua.Server/Session/Session.cs - Libraries/Opc.Ua.Server/Session/SessionManager.cs - Libraries/Opc.Ua.Server/Session/SessionSecurityPolicyHelper.cs - Libraries/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs - Libraries/Opc.Ua.Server/Subscription/Subscription.cs - Libraries/Opc.Ua.Server/Subscription/SubscriptionManager.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # Applications/Quickstarts.Servers/ReferenceServer/ReferenceServer.cs
This project contains only generated source (OPC UA model code from Opc.Ua.SourceGeneration + OpcUaGdsModel). The generator already produces nullable-clean code (used by Opc.Ua.Types and other migrated projects), so enabling `<Nullable>enable</Nullable>` requires no source changes. Build is clean across all target frameworks (net472/net48/netstandard2.1/ net8.0/net9.0/net10.0). Consumers (Opc.Ua.Gds.Client.Common, Opc.Ua.Gds.Server.Common) compile without new diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrated 46 files from nullable-disabled to `<Nullable>enable</Nullable>`.
Resolved 806 CS86xx warnings down to zero across all 6 target frameworks
(net472/net48/netstandard2.1/net8.0/net9.0/net10.0).
Public API nullability decisions:
- Return types widened to T? where null is a valid result:
IUaPubSubDataStore.ReadPublishedDataItem (key-not-found),
IUaPubSubConnection.CreateNetworkMessages,
UaPubSubConfigurator.{FindObjectById, FindParentForObject,
FindPublishedDataSetByName}, DataCollector.{CollectData,
GetPublishedDataSet}, UaNetworkMessage.DataSetMetaData, etc.
- Properties marked T? on DataSet/Field, JsonNetworkMessage,
UadpNetworkMessage security/optional fields, UdpDiscovery
network-address state, MQTT TLS/credential options.
- Events declared as `EventHandler<T>?` per BCL convention.
- Encoder/Decoder `string fieldName` params widened to `string?`
to align with IEncoder/IDecoder interface contracts.
Latent bugs flagged with TODO comments (per migration zero-behavior-change
rule, not fixed inline):
- IntervalRunner.{Start, ProcessAsync}: m_cancellationToken nulled in
Dispose but dereferenced without ObjectDisposedException check.
- UaPubSubConfigurator.{RemoveWriterGroup/DataSetWriter/ReaderGroup/
DataSetReader}: FindIdForObject(possiblyNull) called before null check.
- DataCollector.{Add,Remove}PublishedDataSet: indexes by .Name without
ArgumentNullException guard.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The merge of origin/master brought in two new code paths that don't satisfy <Nullable>enable</Nullable> on Opc.Ua.Server: - AuditEvents.cs:1641 (CS8604): Added ! to request?.RequestHeader?.AuditEntryId passed to SetChildValue. The Variant implicit operator handles null fine at runtime (preserves prior behavior); the annotation is the only issue. - StandardServer.cs:333 (CS8602): Added ! to context.ChannelContext in CreateSession. CreateSession is post-channel-bind and the channel context is always non-null at this point (same invariant fixed at Session.cs/SessionManager.cs/SessionSecurityPolicyHelper.cs in commit 7e2f415). These were brought in by 2806e35 (Server fix Publishing moreNotifications). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
changed the title
After enabling nullable on Opc.Ua.Server, two override-vs-base contracts became incompatible and surfaced in the Opc.Ua.Gds.Server.Common build: - CS8764 ApplicationsNodeManager.ValidateNode return type: override declares `NodeState?` but base `CustomNodeManager.ValidateNode` declared `NodeState`. Both base and override actually return null on cache miss (base line ~1857: `return handle.Node` where Node may be null; override line ~1791: `return null` when handle is null). Fixed by widening the base return type to `NodeState?` and converting the ~17 same-file `NodeState source = ValidateNode(...)` callers to `NodeState? source` (each caller already had an `if (source == null)` guard immediately after, so flow analysis narrows back to non-null with no behavior change). - CS8765 GlobalDiscoverySampleServer.ValidateRequestAsync parameter: override declared `RequestHeader requestHeader` but base `StandardServer.ValidateRequestAsync` declared `[NotNull] RequestHeader?`. Fixed by matching the base signature in the override (added `using System.Diagnostics.CodeAnalysis`). No runtime behavior change; only annotations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After enabling nullable end-to-end, several downstream errors surfaced: - MonitoredItem ctor (Opc.Ua.Server): widened originalFilter, filterToUse, range parameters to nullable. The ctor body already handled null for these (Filter/m_filterToUse fields are nullable; range null-checked at line 141; originalFilter is-pattern at 117). MemoryBufferMonitoredItem and other derived-class ctor calls now compile cleanly. - DataChangeMonitoredItem.OnReportValueChange: added ! to value/error arguments passed to MonitoredItem.ValueChanged (static method has non-null params; the surrounding code already checks if value != null on the immediately-following line so the call path is non-null in practice). Same for m_queue?.QueueValue(value!, error!). - TestDataNodeManager/AlarmNodeManager: NodeState source = ValidateNode(...) -> NodeState? source (CustomNodeManager.ValidateNode now returns NodeState? after the prior commit; downstream callers in Quickstarts.Servers needed the same change as the Opc.Ua.Server callers). - TestDataNodeManager:207: conditionsFolder!.AddNotifier(...) — the FindPredefinedNode<NodeState> returns nullable; current behavior NREs on null, ! preserves that runtime behavior while satisfying the compiler. - MemoryBufferMonitoredItem.Modify: ! on ModifyAttributes return — the base method now returns ServiceResult? (nullable) but Modify declares ServiceResult; preserves prior behavior (was returning a non-null result from a nullable signature already). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eMonitoredItem After widening m_filterToUse to nullable in commit cc8943d, two diagnostic-helper call sites (ServerUtils.ReportCreateMonitoredItem in both ctors) flagged CS8604 because the helper expects non-null filter. Same pattern used at lines 729 and 817 in commit 7e2f415 - add ! preserves the prior runtime behavior (the diagnostic helper handles null at runtime; the static signature is overly strict). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test project links Applications/ConsoleReferenceClient/ClientSamples.cs as a Compile-Item (line 30 of csproj). After enabling nullable on ConsoleReferenceClient (commit a4d9487), the linked file gained ? annotations that fail with CS8632 in the test project's compile context. Setting <Nullable>annotations</Nullable> enables annotation PARSING without enforcing nullable analysis on the rest of the test code — minimal change that resolves the CS8632 errors without requiring a full Tests/ migration (which is out of scope for this PR). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CI test failures showed the server failing to start with InvalidOperationException in ServerSystemContext.get_OperationContext, called via Copy from DiagnosticsNodeManager.CreateAddressSpaceAsync. Commit a4d9487 changed the getter from null-silent return (with a meaningless !) to throw. This was a runtime behavior change: callers were already handling null returns (CustomNodeManager.cs:3252 has 'OperationContext != null' guard). Throwing breaks startup because DiagnosticsNodeManager runs before any operation has been issued and Copy propagates null via line 172. Restored original null-returning behavior; now declares return type as OperationContext? to match reality. All current call sites already handle null. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e revert Reverting OperationContext getter to nullable (commit 1d9df71) cascaded 8 new CS8604 errors at sites that pass it to non-null param contracts. All are in 'normal operation' code paths (after session/operation has begun) where OperationContext is invariably non-null in practice; preserved with !. Fixed sites: - DiagnosticsNodeManager.cs:316 (ResendData),:495 (ConditionRefresh),:513 (ConditionRefresh2) - MonitoredNode.cs:262 (capture from serverSystemContextToUse), :355 (cachedEntry.Context) - SamplingGroupMonitoredItemManager.cs:95 (CreateMonitoredItem),:195 (ModifyMonitoredItem) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove unresolved 'ISystemContext.OperationContext' cref in XML doc remark that prevented Opc.Ua.Server from building (the type lives in Opc.Ua, not Opc.Ua.Server, and the simple cref form failed to resolve). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… nullable Add ! after .OperationContext (now nullable). The Quickstarts SampleNodeManager diagnostic-info path here does not pre-check OperationContext for null (unlike CustomNodeManager.cs:3252 which does); preserved prior runtime behavior with !. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ComponentCache lookup had inverted TryGetValue checks that caused
NRE on first cache hit and silently returned null after AddNodeToComponentCache:
if (!m_componentCache.TryGetValue(handle.RootId, out entry)) // BUG: when NOT found
{
return entry!.Entry!.FindChildBySymbolicName(...); // entry is null here
}
The ! inversions look like collateral damage from the bulk Server migration
script (commit 245e92b) that flipped many == to != in nullable-related
guards; later cleanup (1988bca, 1581531) caught most but missed these.
Matches the AddNodeToComponentCache pattern (which uses positive TryGetValue).
Fixes test failures: TestComponentCacheAsync, AddNodeToComponentCacheFirstAddCreatesEntry,
AddNodeToComponentCacheSecondAddIncrementsRefCountAndReturnsCachedNode,
AddNodeToComponentCacheWithComponentPath* and others. Also explains the
downstream Server startup InvalidOperationException seen in earlier CI runs
because AsyncCustomNodeManager.DeleteNodeAsync indirectly walked the cache.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…NodeInComponentCache (matches commit deb49de fix)
…FindMethodInTypeHierarchy 1. MasterNodeManager.ValidateRolePermissions: lines 3955/3960 had inverted IsEmpty checks. Comments inside the branches said 'there were no role permissions defined for this node' but the conditions were checking '!IsEmpty'. Removed the ! so the logic matches the comments. Fixes: ValidateRolePermissions_DefaultPermissions_ReturnsGood (and 4 sibling tests). 2. CustomNodeManager.cs:3148, AsyncCustomNodeManager.cs:3302: the per-spec ObjectType-method resolution path declared a NEW local 'MethodState? method2 = FindMethodInTypeHierarchy(...)' instead of assigning to the outer 'method' variable that the next 'if (method == null)' check uses. The result of the type-hierarchy lookup was discarded, so any method call on an Object instance whose method lives on its ObjectType returned BadMethodInvalid. Fixes: CallAsync_InvokesMethodFromObjectTypeAsync, CallAsync_InvokesMethodFromSuperTypeOfObjectTypeAsync. Both bugs were collateral damage from the bulk Server migration script (245e92b) and not caught by the prior cleanup commits (1988bca / 1581531). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sites only flagged in the Release build / net472 (not Debug/net10.0): - MasterNodeManager.cs:1716: context.Session is nullable; ! at use site. - ConsoleReferenceSubscriber/Program.cs:249,255,282,292,298: dataSet.Fields, dataSet, and DataSetMetaData are nullable per Opc.Ua.PubSub migration. Added ! at deref sites (sample-app code path is exercised only with real metadata messages). - ConsoleReferencePublisher/PublishedValuesWrites.cs:352,356: IUaPubSubDataStore.ReadPublishedDataItem now returns DataValue? (key-not-found path); changed local to DataValue? and added explicit null check before dereferencing WrappedValue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The line was 'context.Session!.SaveContinuationPoint(currentCp);' but 'context' itself is nullable in this method (other use sites at lines 1665, 1686 use 'context!'). The ! after Session doesn't help when context.* is the first deref. Added ! after context too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Documents the ~3,810 ! operators remaining after the nullable migration, clustered into 15 root-cause groups with removal strategies, effort estimates, and a 6-PR follow-up roadmap. Also flags 2 latent (expr as T)! bug candidates in Opc.Ua.PubSub/Encoding/PubSubJsonDecoder.cs:928,1293. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace 15 `= null!;` initializers on auto-properties in PubSub EventArgs and DTO classes with the C# 11 `required` modifier. The repo's Stack/Opc.Ua.Types already provides PolySharp-generated RequiredMemberAttribute polyfills for older TFMs (net472/48/standard2.1) so this works across all 6 target frameworks without adding new compat code. Modified EventArgs (1 prop each): ConnectionEventArgs, DataSetReaderEventArgs, DataSetWriterEventArgs, ExtensionFieldEventArgs, PubSubStateChangedEventArgs, PublishedDataSetEventArgs, ReaderGroupEventArgs, WriterGroupEventArgs Multi-property classes converted: ConfigurationUpdatingEventArgs (Parent, NewValue) RawDataReceivedEventArgs (Message, Source, PubSubConnectionConfiguration) DataSetWriterConfigurationResponse (DataSetWriterIds, StatusCodes; DataSetWriterConfig left as null! since it is conditionally set) Caller fix: UaPubSubConnection.GetDataSetWriterDiscoveryResponses refactored from post-construction property assignment to object-initializer syntax to satisfy the required-property contract; preserved the conditional DataSetWriterConfig=null on not-found path (no runtime change). Test updates: 5 tests updated to provide initializers for now-required properties (Message=[], Source=string.Empty, etc.). Skipped: EventArgs whose properties have `internal set;` (populated by framework code, not callers) — keeping `= null!;` is correct there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Server Added 10 [MemberNotNull] attributes across 5 files documenting which late-init fields each lifecycle method populates. The compiler now verifies these methods set the fields, eliminating the need for ! operators at downstream use sites. ServerInternalData.cs (9 attributes, 21 member refs): SetNodeManager, SetMainNodeManagerFactory, CreateServerObjectAsync, SetSessionManager, SetMonitoredItemQueueFactory, SetSubscriptionStore, SetAggregateManager, SetModellingRulesManager Session.UpdateUserIdentity (1 attribute, 2 member refs): EffectiveIdentity, IdentityToken Use-site eliminations (5 ! removed): CustomNodeManager.cs:4972 Server!.NodeManager! -> Server.NodeManager AsyncCustomNodeManager.cs:5162 same pattern StandardServer.cs:3700 ServerInternal.ServerObject! -> .ServerObject The = null!; initializers were preserved because constructors do not call the lifecycle methods (compiler still requires the suppressor); the attributes formally document the contract that flow analysis can use inside any caller that has invoked the lifecycle method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…g fixes
A) Two latent (expr as T)! cast bug fixes in PubSubJsonDecoder.cs:
- Line 928: (serverUriToken as string)! -> (string)serverUriToken (clean
InvalidCastException instead of swallowed-then-NRE in ToServerIndex).
- Line 1293: (array.Value as Array)! -> 'is not Array arrayValue' pattern
match with descriptive ServiceResultException(BadDecodingError).
B) ArraySegmentExtensions.GetArray() helper + bulk migration:
New internal extension Stack/Opc.Ua.Core/Utils/ArraySegmentExtensions.cs
with Debug.Assert + AggressiveInlining. Replaced 61 sites of
ArraySegment.Array! with .GetArray() across 9 files in Opc.Ua.Core
(BufferManager, ArraySegmentStream, CryptoUtils, EncryptedSecret,
RsaUtils, UaSCBinaryChannel + Asymmetric/Rsa/Symmetric).
C) ToArray()!.Cast<T> -> .ToList().OfType<T> in JsonDecoder.cs at lines
3937 and 3941 (NamespaceTable/StringTable construction). ToList yields
non-null List<string?>, OfType filters nulls and yields IEnumerable<string>
matching the consumer signature.
Total: ~68 ! operators eliminated, 2 latent bugs fixed.
Build: Opc.Ua.Core, Opc.Ua.PubSub, Opc.Ua.Types all succeed (0 warnings).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Part A (Dispose-time field cleanup): Only Nonce.cs.m_ecdh qualified for safe conversion (already declared ECDiffieHellman?; just changed Dispose-line null! to null). All other candidate sites (BinaryEncoder/Decoder, XmlDecoder, PubSubJson*, BaseComplexType, MonitoredItem, PooledBuffer, ArraySegmentStream, TcpTransportListener) were rejected by the conservative criteria because their fields are widely accessed without null checks; making them nullable would require new null guards (a runtime behavior change). Part B (Type.GetElementType()! pattern replacement): 15 sites converted from .GetElementType()! to either 'is Type x' pattern matching (compile-time non-null narrowing, no exception path) or '?? throw InvalidOperationException' for assignment cases. Files (sites): Stack/Opc.Ua.Types/BuiltIn/EnumValue.cs (2) Stack/Opc.Ua.Types/BuiltIn/TypeInfo.cs (3) Stack/Opc.Ua.Types/BuiltIn/VariantHelper.cs (1) Stack/Opc.Ua.Types/BuiltIn/Matrix.cs (3) Libraries/Opc.Ua.Client.ComplexTypes/Types/ComplexTypePropertyInfo.cs (3) Libraries/Opc.Ua.PubSub/Encoding/PubSubJsonEncoder.cs (3) Total: 16 ! operators eliminated. Builds clean (Types/Core/PubSub all 0 warnings; pre-existing baseline errors in MonitoredNode/SessionExtensions unrelated to this change). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make `string fieldName` parameter nullable across all encoder/decoder APIs. In binary encoding fieldName is unused (positional encoding) and callers were universally passing `null!`; making the parameter nullable eliminates the ! at every call site. Interface changes (10 methods each): IEncoder.cs: WriteEncodeable*, WriteEnumerated*, WriteEncodeableArray*, WriteEncodeableArrayAsExtensionObjects, WriteEncodeableMatrix* IDecoder.cs: ReadEncodeable*, ReadEnumerated*, ReadEncodeableArray*, ReadEncodeableMatrix, ReadEncodeableArrayAsExtensionObjects, ReadVariantValue Implementations updated: BinaryEncoder/Decoder, XmlEncoder/Decoder, XmlParser (private helper), JsonEncoder/Decoder — 72 method signatures total. AuditEvent.Initialize source parameter: BaseEventState.Initialize and AuditEventState.Initialize: NodeState source -> NodeState? source. Eliminates ! at 25 call sites in AuditEvents.cs (×17+3), Subscription.cs (×2), MonitoredItem.cs, CustomNodeManager.cs (×3). Call-site replacements (88 total): - 63 IEncoder/IDecoder null! -> null sites - 25 e.Initialize(ctx, null!, ...) -> e.Initialize(ctx, null, ...) Source-breaking note: The interface ? annotations on fieldName are source-breaking for downstream consumers with NRT enabled — they will see new warnings when overriding/implementing the interfaces. Binary-compatible. Builds: Types/Core/PubSub all 0 warnings. Server/Client have only the pre-existing `unions` baseline errors (verified via git stash). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # Libraries/Opc.Ua.Client/Browser.cs # Libraries/Opc.Ua.Client/ComplexTypes/NodeCacheResolver.cs # Libraries/Opc.Ua.Client/NodeCache/LruNodeCache.cs # Libraries/Opc.Ua.Client/NodeCache/NodeCache.cs # Libraries/Opc.Ua.Client/NodeCache/NodeCacheObsolete.cs # Libraries/Opc.Ua.Client/Session/DefaultSessionFactory.cs # Libraries/Opc.Ua.Client/Session/Session.cs # Libraries/Opc.Ua.Client/Session/SessionObsolete.cs # Libraries/Opc.Ua.Client/Subscription/MonitoredItem.cs # Libraries/Opc.Ua.Client/Subscription/Subscription.cs # Libraries/Opc.Ua.Server/Fluent/NodeManagerBuilder.cs # Stack/Opc.Ua.Core/Security/Constants/AdditionalParameterNames.cs # Stack/Opc.Ua.Core/Security/Constants/SecurityPolicies.cs # Stack/Opc.Ua.Core/Security/Constants/SecurityPolicyInfo.cs # Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs # Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs # Stack/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.Asymmetric.cs # Stack/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.Rsa.cs # Stack/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.Symmetric.cs # Stack/Opc.Ua.Core/Stack/Tcp/UaSCBinaryClientChannel.cs # Stack/Opc.Ua.Core/Stack/Transport/NullChannel.cs # Stack/Opc.Ua.Core/Stack/Types/X509IdentityTokenHandler.cs # Stack/Opc.Ua.Types/BuiltIn/DataContractSurrogates.cs # Stack/Opc.Ua.Types/BuiltIn/ExtensionObject.cs # Stack/Opc.Ua.Types/BuiltIn/Variant.cs
After merging master (PR #3730), 3 test sites still passed null as the first arg to the 2-arg ConfiguredEndpoint(ConfiguredEndpointCollection, EndpointDescription) ctor whose collection param is non-nullable. The 3-arg overload with nullable collection was already used in production code (DefaultServerRedundancyHandler, ManagedSessionBuilder); apply the same pattern in tests by adding 'configuration: null' to disambiguate to the 3-arg overload. Files: Tests/Opc.Ua.Client.Tests/ClientBuilder/ManagedSessionBuilderTests.cs:54 Tests/Opc.Ua.Client.Tests/ClientBuilder/OpcUaClientServiceCollectionExtensionsTests.cs:44 Tests/Opc.Ua.Client.Tests/Session/ServerRedundancyHandlerTests.cs:375 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Enable C# nullable reference types across all production code projects in the OPC UA stack — Stack/, Libraries/, and Applications/ — applying systematic annotations and minimizing
!(null-forgiving) operators per themigrate-nullable-referencesskill conventions.Scope
Nullable-enabled projects (21 total):
Stack/(4):Opc.Ua.Bindings.Https,Opc.Ua,Opc.Ua.Types,Opc.Ua.CoreLibraries/(9):Opc.Ua.Security.Certificates,Opc.Ua.Configuration,Opc.Ua.Client,Opc.Ua.Client.ComplexTypes,Opc.Ua.Server,Opc.Ua.PubSub,Opc.Ua.Gds.Common,Opc.Ua.Gds.Client.Common,Opc.Ua.Gds.Server.CommonApplications/(8):ConsoleReferenceServer,ConsoleReferenceClient,ConsoleReferencePublisher,ConsoleReferenceSubscriber,MinimalBoilerServer,McpServer,Quickstarts.Serversand the Mono variantTests/projects are intentionally out of scope for this PR.Conventions applied
<Nullable>enable</Nullable>set at project level (no per-file#nullable enabledirectives)[NotNullWhen],[MemberNotNull],[NotNullIfNotNull],[MaybeNullWhen]attributes used where simple?cannot express the contract= null!for late-init fields (paired with[MemberNotNull]on initializer methods)!operators reduced across multiple cleanup passes!justified by upstream invariants or commentsLatent bugs surfaced and fixed
During the audit, ~25 latent null-deref risks and dead-code paths were flagged with TODOs and then fixed in dedicated commits. Examples:
EndpointDescription.SecurityPolicyUri = null!→ useSecurityPolicies.NoneliteralContentFilter.Result/ElementResult(null!)→ useServiceResult.Good(expr as Type)!patterns (13 sites) replaced withis-pattern early-return + log warningBatchPersistor.DeleteBatchhad inverted regex predicate (deleted wrong batches)AlarmConditionTypeHolder.SetValuehad unreachableelse if (UpdateSuppression()); first branch should have beenUpdateShelving()(matching the message body)AlarmNodeManagerhad 4 deadsourceControllers == nullchecks (the dictionary getter is non-null)Durable*MonitoredItemQueue.Restore(null)left fields in inconsistent persisted-but-empty stateConsoleReferenceClientNRE risks onNodeCache.FindAsyncandascasts in subscription notifications[NotNullWhen(true)]so callers no longer need!Master integration
Branch was merged twice with origin/master:
UnionAttributepolyfill,TryGetEncodeable→TryGetValuerename) — propagated through all consumers; nullable annotations preserved across the rename.!fixes inAuditEvents.csandStandardServer.csbecause the new code paths didn't satisfy the now-<Nullable>enable</Nullable>dOpc.Ua.Server.Types of changes
Checklist
Further comments
Source-breaking changes
Adding
?to a public method's return or parameter is metadata-only at the IL level (no behavior change), but is source-breaking for consumers who have NRT enabled — they will see new warnings/errors. Consumers who have NRT disabled will be unaffected. The annotations match the actual runtime behavior already shipping in master.Local validation note
The local SDK preview (
dotnet 10.0.300-preview.0.26177.108) fails on master's new union-pattern code paths (is { IsNull: false }on[Union]-marked types likeNodeId) due to a preview compiler issue. CI uses GA SDK 10.0.7 where these compile cleanly. Errors inMonitoredNode.csand a few related files reported only locally are pre-existing baseline noise unrelated to this PR.