Skip to content

Conversation

@halgab
Copy link
Contributor

@halgab halgab commented Aug 3, 2023

Proposed changes

  • Enable nullability in SelectionPanelBase
  • Enable nullability in DockEditor
  • Enable nullability in ContentAlignmentEditor
Microsoft Reviewers: Open in CodeFlow

@halgab halgab requested a review from a team as a code owner August 3, 2023 12:23
@ghost ghost assigned halgab Aug 3, 2023
@ghost ghost added the area-NRT label Aug 3, 2023
protected RadioButton CheckedControl
{
get => _checkedControl;
get => _checkedControl!;
Copy link
Member

Choose a reason for hiding this comment

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

I do not have a real scenario here but forgiving null only propagate error somewhere else in this case and setter doesn't ensure it isn't null. Can you check if you can remove backing field here? Is it possible to enforce null check in this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The setter prevents us from removing the backing field. It ensures the value passed to it isn't null because it calls FocusCheckedControl which crashes with a NRE if CheckedControl is null. So my first version of the annotation of this property was RadioButton? with [DisallowNull]. But I realised that almost all the callsites then need a null forgiving operator, so I thought it was simpler to just do it the way it is here

@dreddy-work dreddy-work added the waiting-author-feedback The team requires more information from the author label Aug 3, 2023
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Aug 3, 2023
dreddy-work
dreddy-work previously approved these changes Aug 3, 2023
@dreddy-work
Copy link
Member

Please resolve conflict.

@dreddy-work dreddy-work enabled auto-merge (squash) August 3, 2023 23:11
@dreddy-work dreddy-work added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Aug 3, 2023
auto-merge was automatically disabled August 4, 2023 08:27

Head branch was pushed to by a user without write access

@dreddy-work dreddy-work enabled auto-merge (squash) August 4, 2023 16:53
@dreddy-work dreddy-work merged commit 2e56be0 into dotnet:main Aug 4, 2023
@ghost ghost added this to the 8.0 RC1 milestone Aug 4, 2023
@ghost ghost removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Aug 4, 2023
@halgab halgab deleted the DockEditor branch August 4, 2023 16:59
dreddy-work pushed a commit that referenced this pull request Aug 4, 2023
* [main] Update dependencies from dotnet/runtime (#9648)

* Update dependencies from https://github.com/dotnet/runtime build 20230802.13

Microsoft.Internal.Runtime.WindowsDesktop.Transport , Microsoft.NET.Sdk.IL , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.ILAsm , Microsoft.NETCore.ILDAsm , Microsoft.NETCore.Platforms , Microsoft.Win32.Registry.AccessControl , Microsoft.Win32.SystemEvents , runtime.win-x64.Microsoft.NETCore.ILAsm , runtime.win-x86.Microsoft.NETCore.ILAsm , System.CodeDom , System.ComponentModel.Composition , System.ComponentModel.Composition.Registration , System.Configuration.ConfigurationManager , System.Data.Odbc , System.Data.OleDb , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices , System.DirectoryServices.AccountManagement , System.DirectoryServices.Protocols , System.IO.Packaging , System.IO.Ports , System.Management , System.Reflection.Context , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.ProtectedData , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceModel.Syndication , System.ServiceProcess.ServiceController , System.Speech , System.Text.Encoding.CodePages , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Windows.Extensions , VS.Redist.Common.NetCore.SharedFramework.x64.8.0
 From Version 8.0.0-rc.1.23401.22 -> To Version 8.0.0-rc.1.23402.13

* Handle `ref` to `in` change (#9654)

A number of Libarary APIs have had their parameters changed from `ref` to `in` (or `readonly ref`).

This change modifies code that we control and disables the "passing ref to in" warning for the interop assembly until we can get microsoft/CsWin32#1014 resolved.

This change requires a more current Roslyn build and as such I've updated it. Note that VS Intellisense does not support this new feature yet. You can filter out intellisense errors by selecting "Build Only" in the error pane in the meantime (this will not remove red squiggles unfortunately).

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jeremy Kuhne <[email protected]>

* [main] Update dependencies from dotnet/arcade (#9661)

[main] Update dependencies from dotnet/arcade

* [main] Update dependencies from dotnet/runtime (#9662)

[main] Update dependencies from dotnet/runtime

* Enable nullability in DockEditor, ContentAlignmentEditor and SelectionPanelBase (#9649)

* annotate SelectionPanelBase

* annotate DockEditor

* annotate ContentAlignmentEditor

---------

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jeremy Kuhne <[email protected]>
Co-authored-by: halgab <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants