Enable nullable reference types by default #7130
Conversation
|
I think I'm on board with this? I wasn't willing to do it myself and I still find the approach kinda ugly, but it does encourage modernization and make the old-and-busted unannotated files stand out. And I don't have a better approach--without a mechanism to enforce "new files get nullable enabled by default/if you do major surgery in a file enable it" we were just kinda sitting still. |
drewnoakes
force-pushed
the
dev/drnoakes/nullable-by-default
branch
from
a141513
to
9f75c90
Compare
This turns C# 9's nullable reference types on by default for all projects in the solution, and opts out unannotated files by adding `#nullable disable` to them. This has two nice properties: 1. New code will be written with nullability in mind as the feature will be on by default. 2. It's easy to find unannotated code by searching for `#nullable disable`. We have taken this approach in the .NET Project System and in CPS, and it's worked very well for us. Code under `Deprecated` is unchanged.
drewnoakes
force-pushed
the
dev/drnoakes/nullable-by-default
branch
from
9f75c90
to
d7724dd
Compare
|
I made this change with a regex, matching on There are a lot of file changes here and reviewing is a pain. The strategy I used to validate this approach was review all files that had more or less than two line changes (the addition of The one-line changes are all removals of I reviewed all the others. There were some incorrect substitutions in code blocks. I've fixed those. The other changes here look good to me. |
I learned this morning that there's a VS refactoring to do this! We discussed this with the team and we're planning to go ahead with this approach--thanks for piloting it out for us! |
| #nullable disable | ||
|
|
There was a problem hiding this comment.
Just move this above the #ifs?
There was a problem hiding this comment.
Fixed. Merged latest main and resolved conflict.
Forgind
added
the
merge-when-branch-open
Per the comment, this was removable once we turned on nullable. Which was done in dotnet#7130.
Per the comment, this was removable once we turned on nullable. Which was done in #7130.
Context
In #7093 (comment) we observed that adding
#nullable enablewould better document the intended nullness of values.It would be good if new code was
#nullable enableby default.Changes Made
This turns C# 9's nullable reference types on by default for all projects in the solution, and opts out unannotated files by adding
#nullable disableto them.This has two nice properties:
#nullable disable.Testing
CI. The nullable reference types feature does not impact code at runtime, other than the addition of metadata.
Notes
We have taken this approach in the .NET Project System and in CPS, and it's worked very well for us.
Code under
Deprecatedis unchanged.This is the kind of change a team should discuss first. I was curious if it'd be an easy change for MSBuild and by the time I had an answer, I also had a PR. I will not be offended if this is not a change you wish to make at this time.