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

Extend C# nullable analysis to check for nullable field/property assignments in methods called by constructors #42531

Closed
Ramobo opened this issue Mar 18, 2020 · 5 comments
Labels
Area-Compilers Feature - Nullable Reference Types Nullable Reference Types Resolution-Duplicate The described behavior is tracked in another issue
Milestone

Comments

@Ramobo
Copy link

Ramobo commented Mar 18, 2020

Version Used: C# 8.0, C# Tools 3.6.0-1.20127.6+572b1f0e0da95fd92af710ee58c95e2257cb08e3

Steps to Reproduce:

  1. Enable nullable warnings;
  2. Declare a non-nullable field or property;
  3. Declare a constructor that calls a method;
  4. In the method from step 3, initialize the field/property declared in step 2.

Example:

#nullable enable

public class MyClass
{
    public Exception myField;

    public MyClass() => Initialize(); // Warning CS8618: Non-nullable field 'myField' is uninitialized. Consider declaring the field as nullable.

    private void Initialize() => myField = new Exception();
}

Expected Behavior: The compiler detects that myField is initialized in Initialize(), which is called by the constructor, and does not issue a warning.

Actual Behavior: The compiler issues Warning CS8618: Non-nullable field 'myField' is uninitialized. Consider declaring the field as nullable. on the constructor's line.

@Ramobo Ramobo changed the title Extend C# nullable analysis to consider methods called by constructors Extend C# nullable analysis to check for nullable field/property assignments in methods called by constructors Mar 19, 2020
@jcouv
Copy link
Member

jcouv commented Mar 19, 2020

This was just recently fixed in 16.6p2 with an extension to the language for C# 9.
Mark the Initialize method with [System.Diagnostics.CodeAnalysis.MemberNotNull(nameof(myField))] and set LangVersion to "preview", then the warning on the constructor will disappear.

@jcouv jcouv added Area-Compilers Resolution-Duplicate The described behavior is tracked in another issue Feature - Nullable Reference Types Nullable Reference Types labels Mar 19, 2020
@jcouv jcouv closed this as completed Mar 19, 2020
@jcouv jcouv added this to the 16.6.P2 milestone Mar 19, 2020
@Ramobo
Copy link
Author

Ramobo commented Mar 20, 2020

An attribute for this just feels unclean and unnecessary. I don't know how the analysis works, but this seems like something too simple to require an attribute for, considering that it can analyze if/else branches.

@Ramobo
Copy link
Author

Ramobo commented Jun 29, 2020

@jcouv Seriously, can't this be automatically analyzed — without an attribute?

@jaredpar
Copy link
Member

@Ramobo

This type of analysis would require the compiler to do cross method analysis which is not done today. Doing so would be an enormous undertaking in the compiler. Further it would mean that the results of compilation differ when compiling against methods defined in source and those defined in assemblies (assemblies can be ref assemblies which means method bodies would be absent).

@RikkiGibson
Copy link
Contributor

Related to #32358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature - Nullable Reference Types Nullable Reference Types Resolution-Duplicate The described behavior is tracked in another issue
Projects
None yet
Development

No branches or pull requests

4 participants