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 annotations for the whole Toolkit #3958

Open
4 of 39 tasks
Sergio0694 opened this issue Apr 22, 2021 · 2 comments
Open
4 of 39 tasks

Enable nullable annotations for the whole Toolkit #3958

Sergio0694 opened this issue Apr 22, 2021 · 2 comments

Comments

@Sergio0694
Copy link
Member

Overview

C# introduced nullable reference types in C# 8.0, but due to some limitations in frameworks such as UWP we haven't enabled them in the entire Toolkit just yet. That said, as I've documented here, it's still possible to use them, and we should gradually extend support for them in the whole Toolkit, especially in preparation for the WinUI 3 support that's already work in progress, since that targets .NET 5. Right now I have already enabled annotations in the .NET packages, and in some other specific files of the UWP projects, but they're not complete there. I propose the following roadmap:

  • Ensure nullability annotations are enabled for all SDK-style .NET projects (effectively already done)
  • Gradually enable nullability annotations (and C# >= 8.0) in UWP projects
  • Once in the WinUI 3 / .NET 5 branch, do a refactor pass to:
    • Enable the project-level nullability switch (remove #nullable directives too)
    • Remove the unnecessary manual attributes for backporting them on UWP
    • Complete the nullability annotations in UWP packages, if any are missing
    • Improve the nullability annotations with the new ones from the BCL
  • (optional) Enable nullability annotations in unit test projects

NOTE: this will take some time to actually complete and I wouldn't expect this to be done by the next minor release, so this issue is meant to just be used to track progress as we gradually add more and more annotations to the Toolkit 😄

Tracking nullability

Here's a list of all projects and their status with respect to nullability annotations:

.NET

  • Microsoft.Toolkit
  • Microsoft.Toolkit.Diagnostics
  • Microsoft.Toolkit.HighPerformance
  • Microsoft.Toolkit.Mvvm

UWP

  • Microsoft.Toolkit.Uwp
  • Microsoft.Toolkit.Uwp.Connectivity
  • Microsoft.Toolkit.Uwp.DeveloperTools
  • Microsoft.Toolkit.Uwp.Input.GazeInteraction
  • Microsoft.Toolkit.Uwp.Notifications

UWP.UI

  • Microsoft.Toolkit.Uwp.UI (partial, work from [Visual|Logical]Tree extensions revamp #3685 is done)
  • Microsoft.Toolkit.Uwp.UI.Animations (partial, work from Feature/animation apis revamp #3639 is done)
  • Microsoft.Toolkit.Uwp.UI.Behaviors (partial, work from Feature/animation apis revamp #3639 is done)
  • Microsoft.Toolkit.Uwp.UI.Controls
  • Microsoft.Toolkit.Uwp.UI.Controls.Core
  • Microsoft.Toolkit.Uwp.UI.Controls.Core.DesignTools
  • Microsoft.Toolkit.Uwp.UI.Controls.DataGrid
  • Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools
  • Microsoft.Toolkit.Uwp.UI.Controls.Input
  • Microsoft.Toolkit.Uwp.UI.Controls.Input.DesignTools
  • Microsoft.Toolkit.Uwp.UI.Controls.Layout
  • Microsoft.Toolkit.Uwp.UI.Controls.Layout.DesignTools
  • Microsoft.Toolkit.Uwp.UI.Controls.Markdown
  • Microsoft.Toolkit.Uwp.UI.Controls.Markdown.DesignTools
  • Microsoft.Toolkit.Uwp.UI.Controls.Media
  • Microsoft.Toolkit.Uwp.UI.Controls.Primitives
  • Microsoft.Toolkit.Uwp.UI.Controls.Primitives.DesignTools
  • Microsoft.Toolkit.Uwp.UI.Media (partial, work from Feature/animation apis revamp #3639 and Win2D pipeline brushes #3112 is done)

Unit

  • UnitTests.NetCore
  • UnitTests.UWP
  • GazeInputTest
  • UnitTests.XamlIslands
  • UnitTests.XamlIslands.UWPApp
  • UITests.App
  • UITests.Tests.MSTest
  • UITests.Tests.TAEF
  • UnitTests.Notifications.NetCore
  • UnitTests.Notifications.UWP
  • UnitTests.HighPerformance.NetCore
  • UnitTests.HighPerformance.UWP
@Rosuavio
Copy link
Contributor

I think this is a really good initiative! I think we might discover and solve a few bugs from these changes. I think this will bring to light any accidental behavior that was not accounted for and help use lean on the compiler to catch errors for us more often!

@Sergio0694
Copy link
Member Author

Yeah that's absolutely one good advantage that this would bring, it'd both make it easier for consumers to more reliably integrate our APIs into their codebases, and it'd also certainly help us catch little bugs that probably have gotten past us so far 😄
Really glad you like the proposal!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants