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.

Sign up for GitHub

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

Jump to bottom

Avoid warning assigning to readonly property marked [AllowNull] #39939

Merged
merged 3 commits into from
Nov 22, 2019
Merged

Avoid warning assigning to readonly property marked [AllowNull] #39939

merged 3 commits into from
Nov 22, 2019

Conversation

cston
Copy link
Member

@cston cston commented Nov 21, 2019

Fixes #39926

@cston cston requested a review from a team as a code owner November 21, 2019 19:28
@cston cston added this to the 16.5 milestone Nov 21, 2019
@cston cston requested a review from jcouv November 21, 2019 22:43
RikkiGibson
RikkiGibson reviewed Nov 21, 2019
View reviewed changes
src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Outdated
static FlowAnalysisAnnotations getFieldAnnotations(FieldSymbol field)
{
var property = field.AssociatedSymbol as PropertySymbol;
return property is null ?
Copy link
Contributor

@RikkiGibson RikkiGibson Nov 21, 2019 •
edited by cston
Loading

Choose a reason for hiding this comment

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

consider using field.AssociatedSymbol is PropertySymbol property #Resolved

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Show resolved Hide resolved
src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs Show resolved Hide resolved
src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs Show resolved Hide resolved
@chsienki
Copy link
Contributor

chsienki commented Nov 22, 2019 •
edited by cston
Loading 

    [Fact]

Link to the issue? #Resolved

Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs:29415 in 6d0390d. [](commit_id = 6d0390d, deletion_comment = False)

chsienki
chsienki approved these changes Nov 22, 2019
View reviewed changes
Copy link
Contributor

@chsienki chsienki left a comment

Choose a reason for hiding this comment

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

:shipit:

jcouv
jcouv approved these changes Nov 22, 2019
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 2)
Consider marking the updated test with the work item link

@cston cston merged commit 23f1693 into dotnet:master Nov 22, 2019
@cston cston deleted the 39926 branch November 22, 2019 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RikkiGibson RikkiGibson RikkiGibson left review comments

@chsienki chsienki chsienki approved these changes

@jcouv jcouv jcouv approved these changes

Assignees
No one assigned
Labels
Area-Compilers
Projects
None yet
Milestone
16.5
Development

Successfully merging this pull request may close these issues.

Warning reported for assignment to property marked [AllowNull]
4 participants
@cston @chsienki @RikkiGibson @jcouv