Skip to content

Port nullability annotations to refs XmlDocument and XmlSerializer - #41474

Merged
3 commits merged into
dotnet:masterfrom
jozkee:nullability_9
Aug 28, 2020
Merged

Port nullability annotations to refs XmlDocument and XmlSerializer#41474
3 commits merged into
dotnet:masterfrom
jozkee:nullability_9

Conversation

@jozkee

Copy link
Copy Markdown
Member

Contributes to #2339

For System.Xml.XmlDocument all types were forwarded to ReaderWriter assembly so I only enabled nullability in the csprojs.
For System.Xml.XmlSerializer some types were forwarded to ReaderWriter but some other types were in place.

@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @buyaa-n, @krwq
See info in area-owners.md if you want to be subscribed.

@jeffhandleyjeffhandley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but please get a second review

StringReader strreader = new StringReader(s);
XmlSerializer deserializerWithOutRootAttribute = ObjectStorage.GetXmlSerializer(_dataType);
return (deserializerWithOutRootAttribute.Deserialize(strreader));
return (deserializerWithOutRootAttribute.Deserialize(strreader))!;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also IMO we should file an issue to investigate if this should return nullable here and below

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #41497.

Comment threadsrc/libraries/System.Xml.XmlSerializer/ref/System.Xml.XmlSerializer.cs Outdated
@krwq

krwq commented Aug 28, 2020

Copy link
Copy Markdown
Member

LGTM, please file an issue for the potential System.Data.Common issue (or TODO-NULLABLE)

@jozkeejozkee self-assigned this Aug 28, 2020
@ghost

Copy link
Copy Markdown

Hello @jozkee!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost
ghost merged commit c587f9d into dotnet:masterAug 28, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@jozkee
jozkee deleted the nullability_9 branch March 24, 2021 19:35
This pull request was closed.
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jozkee@krwq@jeffhandley@Dotnet-GitSync-Bot