Enable Nullable analysis and fix warnings - #365
Conversation
… return types in UnmarshalledDataSource
There was a problem hiding this comment.
Pull request overview
Enables C# nullable reference type analysis across the Ignite UI for Blazor codebase (including the Lite project) and updates library code, stories, and tests to address the resulting warnings—primarily by adding nullable annotations, null guards, and safer event JSON decoding.
Changes:
- Enabled
<Nullable>enable</Nullable>forIgniteUI.Blazor.Liteand applied nullable annotations across the library. - Updated event args / JSON (de)serialization paths to tolerate
nulldictionaries and nullable payload members. - Updated unit tests and stories to compile under nullable analysis (mostly via
!null-forgiving in assertions/handlers).
Reviewed changes
Copilot reviewed 158 out of 181 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/IgniteUI.Blazor.Tests/TreeTests.cs | Update test asserts for nullable event args/results |
| tests/IgniteUI.Blazor.Tests/TileManagerTests.cs | Update test asserts for nullable event args/results |
| tests/IgniteUI.Blazor.Tests/TabsTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/SwitchTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/StepperTests.cs | Update test asserts for nullable event args/results |
| tests/IgniteUI.Blazor.Tests/SplitterTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/SelectTests.cs | Update test asserts for nullable event args/results |
| tests/IgniteUI.Blazor.Tests/RangeSliderTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/RadioTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/DropdownTests.cs | Update test asserts for nullable results |
| tests/IgniteUI.Blazor.Tests/DateRangePickerTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/ComboTests.cs | Update test asserts for nullable event args/results |
| tests/IgniteUI.Blazor.Tests/CheckboxTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/ChatTests.cs | Update test asserts for nullable event args |
| tests/IgniteUI.Blazor.Tests/CalendarTests.cs | Update test asserts/casts for nullable event payload |
| tests/IgniteUI.Blazor.Lite.TestBed/Components/Common/TestUtil.cs | Nullability adjustments for date range comparison |
| stories/Components/Stories/Chat.stories.razor | Update story event handlers for nullable message payloads |
| src/IgniteUI.Blazor.Lite.csproj | Enable nullable analysis for Lite project |
| src/componentsBase/WebViewCallback.cs | Nullability adjustments in JS-invokable callbacks/batch handling |
| src/componentsBase/WebInputs/TreeItem.cs | CascadingParameter made nullable |
| src/componentsBase/WebInputs/Tree.cs | Nullable backing field for content items |
| src/componentsBase/WebInputs/TileManager.cs | Nullable backing field for content items |
| src/componentsBase/WebInputs/Tile.cs | CascadingParameter made nullable |
| src/componentsBase/WebInputs/SelectItem.cs | CascadingParameter made nullable |
| src/componentsBase/WebInputs/Select.cs | Nullable backing field for content items |
| src/componentsBase/WebInputs/Rating.cs | Injected logger nullability + parameter coercion nullability |
| src/componentsBase/WebInputs/Input.cs | Injected logger nullability + parameter coercion nullability |
| src/componentsBase/WebInputs/DropdownItem.cs | CascadingParameter made nullable |
| src/componentsBase/WebInputs/Dropdown.cs | Nullable args arrays + nullable backing field for content items |
| src/componentsBase/WebInputs/DateTimeInput.cs | Nullable args arrays passed into interop invocations |
| src/componentsBase/WebInputs/Chat.cs | Nullable args arrays passed into interop invocations |
| src/componentsBase/WebInputs/Accordion.cs | Nullable backing fields + nullable CascadingParameter |
| src/componentsBase/Utils.cs | Update signature for nullable enum member name/out name |
| src/componentsBase/RuntimeHelper.cs | Make runtime/delegates nullable and adjust unmarshalled call plumbing |
| src/componentsBase/RendererMessage.cs | Make payload dictionary values nullable |
| src/componentsBase/RefSink.cs | Ref sink interface updated for nullable ref values/items |
| src/componentsBase/MarshalByValueFactory.cs | Allow nullable typeName / nullable CreateInstance result |
| src/componentsBase/JsonSerializable.cs | Nullability updates for serialization delegate/context/interface |
| src/componentsBase/IgbTemplateContent.razor | Template/context parameters updated for nullable analysis |
| src/componentsBase/IgbComponentRendererContainer.cs | Make component type/root component nullable + nullable event |
| src/componentsBase/EventCallbackExtensions.cs | Reflection field handles made nullable + guarded access |
| src/componentsBase/DataAdapters.cs | Make adapter backing fields/properties nullable for analysis |
| src/componentsBase/CollectionAdapter.cs | Make internal fields nullable + add null-forgiving where needed |
| src/components/Blazor/VoidEventArgs.cs | Nullable event JSON args dictionary types |
| src/components/Blazor/TreeSelectionEventArgsDetail.cs | Nullable selection array + nullable event args dictionaries |
| src/components/Blazor/TreeSelectionEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/TreeItemComponentEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/TreeItem.cs | Nullable parameters/returns + nullable args arrays for interop calls |
| src/components/Blazor/ToggleButton.cs | Nullable parameters + nullable args arrays for interop calls |
| src/components/Blazor/TileComponentEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/TileChangeStateEventArgsDetail.cs | Nullable tile + nullable event args dictionaries |
| src/components/Blazor/TileChangeStateEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ThemeProvider.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Tabs.cs | Nullable backing fields/return + nullable args arrays for interop calls |
| src/components/Blazor/TabComponentEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/Tab.cs | Nullable cascading parent + nullable label + nullable args arrays |
| src/components/Blazor/Step.cs | Nullable args arrays for interop calls |
| src/components/Blazor/SplitterResizeEventArgsDetail.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/SplitterResizeEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/Splitter.cs | Nullable args arrays + nullable script refs + nullable callback arg types |
| src/components/Blazor/Snackbar.cs | Nullable parameters + nullable script refs + nullable callback arg types |
| src/components/Blazor/SliderLabel.cs | Nullable args arrays for interop calls |
| src/components/Blazor/SliderBase.cs | Nullable parameters/options + nullable args arrays + null-forgiving cast |
| src/components/Blazor/SelectItemComponentEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/SelectHeader.cs | Nullable args arrays for interop calls |
| src/components/Blazor/SelectGroup.cs | Nullable items array + nullable args arrays for interop calls |
| src/components/Blazor/Ripple.cs | Nullable args arrays for interop calls |
| src/components/Blazor/RatingSymbol.cs | Nullable args arrays for interop calls |
| src/components/Blazor/RangeSliderValueEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/RangeSliderValue.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/RangeSlider.cs | Nullable aria labels + nullable script refs + nullable callback arg types |
| src/components/Blazor/RadioGroup.cs | Nullable value/returns + nullable args arrays + nullable script refs |
| src/components/Blazor/RadioChangeEventArgsDetail.cs | Nullable value + nullable event args dictionaries |
| src/components/Blazor/RadioChangeEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ProgressBase.cs | Nullable label format + nullable args arrays |
| src/components/Blazor/NumberEventArgs.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/NavDrawerItem.cs | Nullable args arrays for interop calls |
| src/components/Blazor/NavDrawerHeaderItem.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Navbar.cs | Nullable args arrays for interop calls |
| src/components/Blazor/MaskInput.cs | Nullable parameters/returns + nullable args arrays + nullable script refs |
| src/components/Blazor/ListItem.cs | Nullable args arrays for interop calls |
| src/components/Blazor/ListHeader.cs | Nullable args arrays for interop calls |
| src/components/Blazor/List.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Input.cs | Nullable parameters/returns + nullable args arrays + nullable script refs |
| src/components/Blazor/IconMeta.cs | Nullable collection + nullable event args dictionaries + fallback name |
| src/components/Blazor/IconButton.cs | Nullable parameters + doc/comment + nullable args arrays for interop calls |
| src/components/Blazor/Icon.cs | Nullable parameters + nullable args arrays for interop calls |
| src/components/Blazor/HighlightNavigation.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/Highlight.cs | Nullable SearchText + nullable args arrays for interop calls |
| src/components/Blazor/FormatSpecifier.cs | Nullable return values + nullable event args dictionaries |
| src/components/Blazor/FilteringOptions.cs | Nullable filter key |
| src/components/Blazor/ExpansionPanelComponentEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/DropdownItemComponentEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/DropdownHeader.cs | Nullable args arrays for interop calls |
| src/components/Blazor/DropdownGroup.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Divider.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Dialog.cs | Nullable parameters/refs + nullable args arrays for interop calls |
| src/components/Blazor/DateRangeValueEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/DateRangeValueDetail.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/DateRangeDescriptor.cs | Nullable date range value |
| src/components/Blazor/CustomDateRange.cs | Nullable label/date range |
| src/components/Blazor/ComponentValueChangedEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ComponentDateValueChangedEventArgs.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/ComponentDataValueChangedEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ComponentBoolValueChangedEventArgs.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/ComboChangeEventArgsDetail.cs | Nullable arrays/refs/scripts + nullable event args dictionaries |
| src/components/Blazor/ComboChangeEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/CircularGradient.cs | Nullable parameters + nullable args arrays |
| src/components/Blazor/Chip.cs | Nullable args arrays + nullable script refs |
| src/components/Blazor/CheckboxChangeEventArgsDetail.cs | Nullable value + nullable event args dictionaries |
| src/components/Blazor/CheckboxChangeEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ChatRenderContext.cs | Nullable instance |
| src/components/Blazor/ChatOptions.cs | Nullable strings/arrays/renderers |
| src/components/Blazor/ChatMessageRenderContext.cs | Nullable message |
| src/components/Blazor/ChatMessageReactionEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ChatMessageReaction.cs | Nullable message/reaction + nullable event args dictionaries |
| src/components/Blazor/ChatMessageEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ChatMessageAttachmentEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ChatMessageAttachment.cs | Nullable fields + nullable event args dictionaries + fallback name |
| src/components/Blazor/ChatMessage.cs | Nullable fields/arrays + nullable event args dictionaries |
| src/components/Blazor/ChatInputRenderContext.cs | Nullable value |
| src/components/Blazor/ChatDraftMessage.cs | Nullable fields/attachments + nullable event args dictionaries |
| src/components/Blazor/ChatAttachmentRenderContext.cs | Nullable attachment |
| src/components/Blazor/CarouselSlide.cs | Nullable args arrays for interop calls |
| src/components/Blazor/CarouselIndicator.cs | Nullable args arrays for interop calls |
| src/components/Blazor/CardMedia.cs | Nullable args arrays for interop calls |
| src/components/Blazor/CardHeader.cs | Nullable args arrays for interop calls |
| src/components/Blazor/CardContent.cs | Nullable args arrays for interop calls |
| src/components/Blazor/CardActions.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Card.cs | Nullable args arrays for interop calls |
| src/components/Blazor/CalendarResourceStrings.cs | Nullable resource string parameters |
| src/components/Blazor/CalendarFormatOptions.cs | Nullable weekday/month + nullable event args dictionaries |
| src/components/Blazor/CalendarBase.cs | Nullable locale/resource strings + nullable args arrays |
| src/components/Blazor/ButtonGroup.cs | Nullable selected items + nullable args arrays + nullable script refs |
| src/components/Blazor/BaseOptionLike.cs | Nullable value + nullable args arrays |
| src/components/Blazor/BaseComboBox.cs | Nullable args arrays for interop calls |
| src/components/Blazor/BaseAlertLike.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Banner.cs | Nullable args arrays + nullable script refs |
| src/components/Blazor/Badge.cs | Nullable args arrays for interop calls |
| src/components/Blazor/Avatar.cs | Nullable parameters + nullable args arrays |
| src/components/Blazor/ActiveStepChangingEventArgsDetail.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/ActiveStepChangingEventArgs.cs | Nullable detail + nullable event args dictionaries |
| src/components/Blazor/ActiveStepChangedEventArgsDetail.cs | Nullable event args dictionaries + null checks |
| src/components/Blazor/ActiveStepChangedEventArgs.cs | Nullable detail + nullable event args dictionaries |
Suppressed comments (1)
src/componentsBase/DataAdapters.cs:46
RemoteJson.Uriis annotated as nullable, but it is always initialized via the constructor. Keeping it nullable makesToRef()potentially produce malformed refs ("json:::") and pushes unnecessary nullability into callers.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: MayaKirova <10397980+MayaKirova@users.noreply.github.com>
…thread Fix Copilot review findings: null-safety, dash encoding, delegate mismatch
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…in AdjustCapacity.
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
damyanpetev
left a comment
There was a problem hiding this comment.
The public API changes will definitely need a discussion - I think some of the infrastructure handling is propagating up as nullable to events and properties, which seems off to me.
Also we should be careful with marking public props as nullable and that will likely mean per-component per-case reviews to make sure we're not changing anything without good reason and at the end - we should evaluate if we're still left with breaking changes to document.
| // TODO: IgbStep has no CascadingParameter registration and no FindByNameStepper impl | ||
| // so the refs currently resolve to null elements | ||
| }) | ||
| .Event(c => c.ActiveStepChanging, | ||
| argsJson: """{"detail": {"retType": "object", "type": "", "value": {"oldIndex": 0, "newIndex": 1}}}""", | ||
| assert: args => | ||
| { | ||
| Assert.Equal(0, args.Detail.OldIndex); | ||
| Assert.Equal(0, args!.Detail!.OldIndex); |
There was a problem hiding this comment.
Okay, args is not null in this context since it's based on ActiveStepChanging being EventCallback<IgbActiveStepChangingEventArgs> which is not nullable, so at least on ! on all events was added for no reason.
.Detail is nullable, and like many other tests if it could be, perhaps we should check it explicitly (gives of a different, better test fail message) w/ Assert.NotNull and that will also lower warnings after it.
Using this as an example, but true for most/lots;
| private IgbActiveStepChangedEventArgsDetail? _detail; | ||
|
|
||
| /// <summary> | ||
| /// The payload of the event, carrying the index of the step that became active. | ||
| /// </summary> | ||
| [Parameter] | ||
| public IgbActiveStepChangedEventArgsDetail Detail | ||
| public IgbActiveStepChangedEventArgsDetail? Detail |
There was a problem hiding this comment.
Is Detail really optional/null at runtime, or is this just to resolve warnings?
I know the old code came out with backing fields all willy-nilly initialized with null, but that doesn't have to be preserved;
Changing a public property from an T to T? creates a new nullable contract for consuming apps and can introduce warnings (or build failures when warnings are treated as errors).
That's true throughout really - if we don't intend to have a null value there, we shouldn't have it as an option. There's a note to be made about null being an potential circuit failure path, which is likely not how that should be exposed to the customers either.
If the event contract guarantees a detail payload, could we keep the public property non-nullable and address initialization internally instead, for example with required if possible and/or null!, constructor init as alt? We can also ensure that there is an instance in Detail regardless;
| private DateTime? _value = DateTime.MinValue; | ||
|
|
||
| /// <summary> | ||
| /// The current value of the calendar. | ||
| /// Used when <see cref="IgbCalendarBase.Selection"/> is set to <see cref="CalendarSelection.Single"/>. | ||
| /// </summary> | ||
| [Parameter] | ||
| public DateTime Value | ||
| public DateTime? Value |
There was a problem hiding this comment.
This is more than a nullable-annotation change on value types:
DateTime -> DateTime? changes to a completely different type, same for EventCallback<DateTime?> - that's no longer a warning for consumers (like string?), these straight up are breaking changes and will likely affect @bind- or event handler signature mismatches.
Unlike string → string?, these changes can be both source- and binary-breaking for NuGet consumers and may also affect existing @bind-Value usage. Could we confirm that making the calendar value genuinely optional is an intended public API change rather than only a way to resolve nullable warnings?
Mind you, the old pattern used DateTime.MinValue to signify empty, not sure how established that is. If we do still change it though, it should be handled/documented as a breaking change
Same is true for other value types affected (double?, bool?, etc)
There was a problem hiding this comment.
ReturnToDate(null) seems to returns DateTime.MinValue still, so need to be very careful here, cuz it seems to me the machinery shouldn't be able to produce nulls for dates as it stands anyway?
There was a problem hiding this comment.
Note: I do prefer DateTime? myself as well, but that should be a much more deliberate, separate global change. I think we have some other such worked shelved for a similar reason.
… setting from IgniteUI.Blazor.Lite.csproj
fix: two behavioral changes/regressions from nullable annotation pass
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
#365 Pull requests are gated by dependency-review (fails on High and above). The release scans what it ships and records the report as a release asset, but stays advisory so a finding never holds up a publish. PR #365 enables nullable analysis outright and makes the staged migration plan moot, so the doc and its references are removed and the csproj nullable block is left exactly as master has it to keep that PR merging cleanly.
Description
Enable Nullable analysis and fix all warnings that emerged from it,
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
All
How Has This Been Tested?
Checklist:
README.MDCHANGELOG.MDupdates for newly added functionalityCloses #347