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

Inconsistent nullable diagnostics for constructors #48996

Closed
TessenR opened this issue Oct 28, 2020 · 1 comment · Fixed by #49841
Closed

Inconsistent nullable diagnostics for constructors #48996

TessenR opened this issue Oct 28, 2020 · 1 comment · Fixed by #49841

Comments

@TessenR
Copy link

TessenR commented Oct 28, 2020

Version Used:

Branch master (27 Oct 2020)
Latest commit 576eb30 by Sam Harwell:
Merge pull request #48934 from sharwell/no-api-telemetry

Disable ApiUsageIncrementalAnalyzerProvider for production scenarios

Steps to Reproduce:

#nullable enable
using System.Diagnostics.CodeAnalysis;

class C<T>
{
  [NotNull] public T Prop { get; set; }
  
  public C(T t) { Prop = t; } // no warnings
  public C(T t, int x) { Prop = t; Prop.ToString(); } // CS8602
  
  [MemberNotNull("Prop")] public void Init(T t) 
  {
    Prop = t;
  } // CS8774
}

Expected Behavior:
CS8618 is reported for the first constructor

Actual Behavior:
No warnings for the first constructor

Notes
Note that if you add a dereference after the assignment e.g. Prop.ToString(); the compiler correctly emits CS8602 for that. So the compiler recognizes that the property might still be nullable contrary to its annotation yet the assignment suppresses the initialization warning in the constructor. I believe this behavior contradicts the following proposal: https://github.com/dotnet/csharplang/blob/master/proposals/nullable-constructor-analysis.md /cc @RikkiGibson

There's a similar problem in the following code (where annotations makes more sense). The compiler understands that the value of Prop might be nullable after the assignment so it emits CS8602 if I dereference the property in both the constructor and Init method but only warns about leaving the instance in invalid state in Init but not in the constructor

#nullable enable
using System.Diagnostics.CodeAnalysis;

class C<T>
{
  [NotNull, DisallowNull] public T Prop { get; set; }
  
  public C(T t) {
    Prop = t; // CS8607
    if ("".Length > 0) { Prop.ToString(); } // CS8602
  } // no warnings
  
  [MemberNotNull("Prop")] public void Init(T t) 
  {
    Prop = t; // CS8607
    if ("".Length > 0) { Prop.ToString(); } // CS8602
  } // CS8774
}
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 28, 2020
@RikkiGibson RikkiGibson self-assigned this Oct 28, 2020
@RikkiGibson
Copy link
Contributor

Thanks, my best guess is we are not folding the flow analysis annotations when determining the required state for the field when exiting the constructor.

@jaredpar jaredpar added Bug New Language Feature - Nullable Reference Types Nullable Reference Types and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 2, 2020
@jaredpar jaredpar added this to the 16.9 milestone Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants