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

Fix TypeWithAnnotations.ToTypeWithState() for (untyped) null literal #46344

Merged
merged 1 commit into from
Jul 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,7 @@ private bool ExactOrBoundsNullableInference(ExactOrBoundsKind kind, TypeWithAnno
return false;

// True if the type is nullable.
bool isNullableOnly(TypeWithAnnotations type)
static bool isNullableOnly(TypeWithAnnotations type)
=> type.NullableAnnotation.IsAnnotated();
}

Expand Down
9 changes: 4 additions & 5 deletions src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -772,11 +772,6 @@ internal bool IsSameAs(TypeWithAnnotations other)
/// </summary>
internal TypeWithState ToTypeWithState()
{
if (Type is null)
{
return default;
}

// This operation reflects reading from an lvalue, which produces an rvalue.
// Reading from a variable of a type parameter (that could be substituted with a nullable type), but which
// cannot itself be annotated (because it isn't known to be a reference type), may yield a null value
Expand All @@ -785,6 +780,10 @@ internal TypeWithState ToTypeWithState()

static NullableFlowState getFlowState(TypeSymbol type, NullableAnnotation annotation)
{
if (type is null)
{
return annotation.IsAnnotated() ? NullableFlowState.MaybeNull : NullableFlowState.NotNull;
}
if (type.IsPossiblyNullableReferenceTypeTypeParameter())
{
return annotation switch { NullableAnnotation.Annotated => NullableFlowState.MaybeDefault, NullableAnnotation.NotAnnotated => NullableFlowState.MaybeNull, _ => NullableFlowState.NotNull };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51082,12 +51082,12 @@ static void G(string x)
new[] { source }, options: WithNonNullTypesTrue(),
parseOptions: TestOptions.Regular8);
comp.VerifyDiagnostics(
// (9,54): warning CS8603: Possible null reference return.
// (9,9): warning CS8602: Dereference of a possibly null reference.
// F(() => { if (x.Length > 0) return x; return null; }).ToString();
Diagnostic(ErrorCode.WRN_NullReferenceReturn, "null").WithLocation(9, 54),
// (10,45): warning CS8603: Possible null reference return.
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "F(() => { if (x.Length > 0) return x; return null; })").WithLocation(9, 9),
// (10,9): warning CS8602: Dereference of a possibly null reference.
// F(() => { if (x.Length == 0) return null; return x; }).ToString();
Diagnostic(ErrorCode.WRN_NullReferenceReturn, "null").WithLocation(10, 45));
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "F(() => { if (x.Length == 0) return null; return x; })").WithLocation(10, 9));
Copy link
Member Author

Choose a reason for hiding this comment

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

Restores behavior from before #45993.

}

[Fact]
Expand Down Expand Up @@ -133331,6 +133331,160 @@ internal interface IB : IA { }
comp.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem(46342, "https://github.com/dotnet/roslyn/issues/46342")]
public void LambdaNullReturn_01()
{
var source =
@"#nullable enable
using System;
class Program
{
static void F<T>(Func<T> f)
{
}
static void M<T>(bool b, T t) where T : class
{
F(() =>
{
if (b) return null;
return t;
});
}
}";
var comp = CreateCompilation(source);
comp.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem(46342, "https://github.com/dotnet/roslyn/issues/46342")]
public void LambdaNullReturn_02()
{
var source =
@"#nullable enable
using System;
class Program
{
static void F<T>(Func<T> f)
{
}
static void M<T>(bool b, T t) where T : class?
{
F(() =>
{
if (b) return null;
return t;
});
}
}";
var comp = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
comp.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem(46342, "https://github.com/dotnet/roslyn/issues/46342")]
public void LambdaNullReturn_03()
{
var source =
@"#nullable enable
using System;
class Program
{
static void F<T>(Func<T> f)
{
}
static void M<T>(T? t) where T : class
{
F(() =>
{
if (t is null) return null;
return t;
});
}
}";
var comp = CreateCompilation(source);
comp.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem(46342, "https://github.com/dotnet/roslyn/issues/46342")]
public void LambdaNullReturn_04()
{
var source =
@"#nullable enable
using System;
class Program
{
static void F<T>(Func<T> f)
{
}
static void M<T>(T? t) where T : class?
{
F(() =>
{
if (t is null) return null;
return t;
});
}
}";
var comp = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
comp.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem(46342, "https://github.com/dotnet/roslyn/issues/46342")]
public void LambdaNullReturn_05()
{
var source =
@"#nullable enable
using System;
using System.Threading.Tasks;
class Program
{
static void F<T>(Func<Task<T>> f)
{
}
static void M<T>(T? t) where T : class
{
F(async () =>
{
await Task.Yield();
if (t is null) return null;
return t;
});
}
}";
var comp = CreateCompilation(source);
comp.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem(46342, "https://github.com/dotnet/roslyn/issues/46342")]
public void LambdaNullReturn_06()
{
var source =
@"#nullable enable
using System;
using System.Threading.Tasks;
class Program
{
static void F<T>(Func<Task<T>> f)
{
}
static void M<T>(T? t) where T : class?
{
F(async () =>
{
await Task.Yield();
if (t is null) return null;
return t;
});
}
}";
var comp = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
comp.VerifyEmitDiagnostics();
}

Copy link
Member

Choose a reason for hiding this comment

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

Could we add similar tests with default? File an issue if misbehaving.

Copy link
Member Author

@cston cston Jul 27, 2020

Choose a reason for hiding this comment

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

return default; results in "warning CS8603: Possible null reference return" with 16.7 and this PR.
return default(T); compiles without warnings with both.

Added tests in #46405.

[Fact]
[WorkItem(45862, "https://github.com/dotnet/roslyn/issues/45862")]
public void Issue_45862()
Expand Down