Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable nullable warnings as errors on projects which enabled Nullable Reference Types #7172

Merged
merged 3 commits into from
Dec 15, 2021

Conversation

grokys
Copy link
Member

@grokys grokys commented Dec 15, 2021

What does the pull request do?

  • Enable nullable reference checking via .props file: the Nullable annotations on netstandard2.0 are incomplete and incorrect in places. Ignore nullable warnings on netstandard2.0 and promote them from warnings to errors on later target frameworks.
  • Fix nullable reference errors in the projects that have nullable reference types enabled

The Nullable annotations on netstandard2.0 are incomplete and incorrect in places. Ignore nullable warnings on netstandard2.0 and make them errors on later target frameworks.
The projects that have nullable reference types enabled are now generating errors instead of warnings, so fix those errors.
@grokys grokys requested a review from maxkatz6 December 15, 2021 22:39
Comment on lines +8 to +9
<WarningsAsErrors Condition="'$(TargetFramework)' != 'netstandard2.0'">$(WarningsAsErrors);nullable</WarningsAsErrors>
<NoWarn Condition="'$(TargetFramework)' == 'netstandard2.0'">$(NoWarn);nullable</NoWarn>
Copy link
Contributor

Choose a reason for hiding this comment

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

I am curious why do we need NoWarn here? I think WarningsAsErrors wouldn't simply work for projects with netstandard2.0 tfm. Also what $(WarningsAsErrors); does here? Do we need it here?

Copy link
Member

Choose a reason for hiding this comment

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

WarningsAsErrors makes all nullable warnings as errors. Enabled for net6.
NoWarn makes all nullable warning as nothing. Enabled for netstandard.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also what $(WarningsAsErrors); does here

This causes the setting listed here to be appended to the existing WarningsAsErrors value, so we don't overwrite any values already set for this property.

@maxkatz6 maxkatz6 merged commit 237724a into master Dec 15, 2021
@maxkatz6 maxkatz6 deleted the fixes/enable-existing-nullable-errors branch December 15, 2021 23:01
danwalmsley pushed a commit that referenced this pull request Dec 20, 2021
…le-errors

Enable nullable warnings as errors on projects which enabled Nullable Reference Types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants