Annotated remaining Converter classes for nullability #30573
+499
−45
Conversation
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
|
Hey there @@NirmalKumarYuvaraj! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
dotnet-policy-service
bot
added
community ✨
There was a problem hiding this comment.
Pull Request Overview
This PR annotates the remaining converter classes with nullable reference types, improves error handling for invalid inputs, and updates public API signatures across all frameworks.
- Updated converter method signatures in ThicknessTypeConverter, KeyboardTypeConverter, FlexEnumsConverters, and EasingTypeConverter to use nullable annotations.
- Enhanced parsing logic with null checks and explicit exceptions for unsupported or malformed values.
- Updated PublicAPI.Unshipped.txt entries to expose new nullable overloads for converter methods.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Core/src/Converters/ThicknessTypeConverter.cs | Added nullable annotations to signatures and improved parsing error paths. |
| src/Core/src/Converters/PrimitiveTypeConversions.cs | Removed unnecessary #nullable enable directive. |
| src/Core/src/Converters/KeyboardTypeConverter.cs | Refined null guards, updated signature, and improved ConvertTo error logic. |
| src/Core/src/Converters/FlexEnumsConverters.cs | Applied nullable annotations and refined enum parsing in all converters. |
| src/Core/src/Converters/EasingTypeConverter.cs | Introduced null handling in ConvertFrom and updated nullable signatures. |
| src/Core/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Exposed new nullable converter overloads in the public API surface. |
Comments suppressed due to low confidence (3)
src/Core/src/Converters/KeyboardTypeConverter.cs:48
- [nitpick] The long sequence of
ifchecks for eachKeyboardvalue is repetitive; consider using aswitchexpression or a lookup dictionary to improve readability and reduce maintenance overhead.
public override object ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType)
src/Core/src/Converters/EasingTypeConverter.cs:19
- [nitpick] New behavior returns
nullfor anullor whitespace input; please add unit tests to cover these null-returning scenarios and ensure they behave as expected.
if (value is null)
| /// <inheritdoc/> | ||
| public class ThicknessTypeConverter : TypeConverter | ||
| { | ||
| public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType) | ||
| public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) |
There was a problem hiding this comment.
Updating the signature of a public override is a breaking change; please confirm that introducing a nullable context parameter in a patch/minor release is intended.
Copilot uses AI. Check for mistakes.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
This pull request updates multiple type converters across the codebase to improve nullability handling, enhance error handling, and ensure compliance with modern C# standards. The changes primarily involve adding nullable annotations, refining method implementations, and improving readability and maintainability.
Nullability Enhancements:
src/Core/src/Converters/EasingTypeConverter.cs: Updated methods (CanConvertFrom,CanConvertTo,ConvertFrom,ConvertTo, etc.) to use nullable annotations for parameters and return types, ensuring better handling of null values. [1] [2] [3]src/Core/src/Converters/FlexEnumsConverters.cs: Applied nullable annotations across all type converters (FlexJustifyTypeConverter,FlexDirectionTypeConverter, etc.) to improve type safety and nullability handling. [1] [2] [3] [4] [5] [6] [7]Error Handling Improvements:
src/Core/src/Converters/KeyboardTypeConverter.cs: Enhanced error handling by adding null checks and throwingInvalidOperationExceptionorNotSupportedExceptionwhere appropriate, ensuring better handling of unexpected input values.src/Core/src/Converters/ThicknessTypeConverter.cs: Added validation checks and improved error handling for parsing thickness values from strings, ensuring more robust conversions.Codebase Cleanup:
src/Core/src/Converters/PrimitiveTypeConversions.cs: Removed unnecessary#nullable enabledirective, simplifying the file.src/Core/src/Converters/ThicknessTypeConverter.cs: Refined method signatures and added nullable annotations to align with modern C# practices.Issues Fixed
Fixes #28860