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 reference types for System.Windows.Forms.Primitives #3451

Merged
merged 16 commits into from
Jun 18, 2020

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Jun 16, 2020

Based on work started by @sharwell in #2676, renders it obsolete.

Microsoft Reviewers: Open in CodeFlow

@RussKie RussKie requested a review from JeremyKuhne June 16, 2020 04:05
@RussKie RussKie requested a review from a team as a code owner June 16, 2020 04:05
@ghost ghost assigned RussKie Jun 16, 2020
@RussKie
Copy link
Member Author

RussKie commented Jun 16, 2020

@vladimir-krestov @SergeySmirnov-Akvelon @Ryuugamine please note of NRT annotations for the accessibility-related API.

@RussKie RussKie changed the title Nullable primitives Enable nullable reference types for System.Windows.Forms.Primitives Jun 16, 2020
@@ -832,12 +832,14 @@ HRESULT UiaCore.IAccessibleEx.ConvertReturnedElement(UiaCore.IRawElementProvider

IAccessible? UiaCore.ILegacyIAccessibleProvider.GetIAccessible() => AsIAccessible(this);

object?[] UiaCore.ILegacyIAccessibleProvider.GetSelection()
object[]? UiaCore.ILegacyIAccessibleProvider.GetSelection()
Copy link
Member

Choose a reason for hiding this comment

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

This one worries me- it was clearly object?[] historically. That may have been wrong, but I would want the functional change taken out of this so we discuss and track for regression purposes if we take this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

That was my guess work when I annotated AccessibleObject in #3273 based on the use. Now having annotated the imported interface object[]? looked more appropriate.
Now I'm re-reading docs, and it looks like it may be returning an empty array, if there is no selection - https://docs.microsoft.com/en-us/dotnet/api/system.windows.automation.provider.iselectionprovider.getselection?view=netcore-3.1#remarks

Copy link
Member Author

Choose a reason for hiding this comment

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

On further review, the current annotation is consistent with the existing implementation:

internal virtual UiaCore.IRawElementProviderSimple[]? GetSelection() => null;

object[] /* IRawElementProviderSimple[] */ GetSelection();

So I'm leaving it as is.

@vladimir-krestov please review the API definitions for correctness (i.e. is null appropriate or has to be empty array?) and update, if necessary.

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #3451 into master will increase coverage by 31.91179%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                  @@
##              master       #3451          +/-   ##
====================================================
+ Coverage   66.58567%   98.49746%   +31.91179%     
====================================================
  Files           1338         443         -895     
  Lines         501683      248046      -253637     
  Branches       40864        4088       -36776     
====================================================
- Hits          334049      244319       -89730     
+ Misses        162085        3015      -159070     
+ Partials        5549         712        -4837     
Flag Coverage Δ
#Debug 98.49746% <100.00000%> (+31.91179%) ⬆️
#production ?
#test 98.49746% <100.00000%> (+0.00011%) ⬆️

@RussKie
Copy link
Member Author

RussKie commented Jun 18, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@RussKie RussKie merged commit 00f87c5 into dotnet:master Jun 18, 2020
@ghost ghost added this to the 5.0 Preview7 milestone Jun 18, 2020
kpreisser added a commit to kpreisser/winforms that referenced this pull request Jul 19, 2020
JeremyKuhne pushed a commit that referenced this pull request Jul 22, 2020
@RussKie RussKie deleted the nullable-primitives branch July 23, 2020 04:53
@ghost ghost locked as resolved and limited conversation to collaborators Jan 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants