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

Update BoundCall method based on receiver nullability #31000

Merged
merged 4 commits into from
Nov 9, 2018
Merged

Update BoundCall method based on receiver nullability #31000

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5fd9ae6
Update BoundCall method based on receiver nullability
cston Nov 6, 2018
ad73072
Additional tests
cston Nov 7, 2018
340a850
Misc.
cston Nov 7, 2018
9da8b69
Fix tests
cston Nov 8, 2018
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
36 changes: 28 additions & 8 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1951,19 +1951,26 @@ public override BoundNode VisitCall(BoundCall node)
// Note: we analyze even omitted calls
var method = node.Method;
var receiverOpt = node.ReceiverOpt;
TypeSymbolWithAnnotations receiverType = default;

if (receiverOpt != null && method.MethodKind != MethodKind.Constructor)
{
VisitRvalue(receiverOpt);
receiverType = VisitRvalueWithResult(receiverOpt);
// https://github.com/dotnet/roslyn/issues/30598: Mark receiver as not null
// after arguments have been visited, and only if the receiver has not changed.
CheckPossibleNullReceiver(receiverOpt);
// Update method based on inferred receiver type: see https://github.com/dotnet/roslyn/issues/29605.
}

// https://github.com/dotnet/roslyn/issues/29605 Can we handle some error cases?
// (Compare with CSharpOperationFactory.CreateBoundCallOperation.)
if (!node.HasErrors)
{
if (!receiverType.IsNull)
{
// Update method based on inferred receiver type.
method = (MethodSymbol)AsMemberOfResultType(receiverType, method);
}

ImmutableArray<RefKind> refKindsOpt = node.ArgumentRefKindsOpt;
(ImmutableArray<BoundExpression> arguments, ImmutableArray<Conversion> conversions) = RemoveArgumentConversions(node.Arguments, refKindsOpt);
ImmutableArray<int> argsToParamsOpt = node.ArgsToParamsOpt;
Expand Down Expand Up @@ -2797,9 +2804,9 @@ private TypeSymbolWithAnnotations GetAdjustedResult(TypeSymbolWithAnnotations ty
return type;
}

private Symbol AsMemberOfResultType(Symbol symbol)
private static Symbol AsMemberOfResultType(TypeSymbolWithAnnotations resultType, Symbol symbol)
{
var containingType = _resultType.TypeSymbol as NamedTypeSymbol;
var containingType = resultType.TypeSymbol as NamedTypeSymbol;
if ((object)containingType == null || containingType.IsErrorType())
{
return symbol;
Expand All @@ -2813,6 +2820,9 @@ private static Symbol AsMemberOfType(NamedTypeSymbol containingType, Symbol symb
{
return null;
}
#if DEBUG
bool wasInterface = containingType.IsInterface;
#endif
if (symbol.Kind == SymbolKind.Method)
{
if (((MethodSymbol)symbol).MethodKind == MethodKind.LocalFunction)
Expand All @@ -2831,16 +2841,23 @@ private static Symbol AsMemberOfType(NamedTypeSymbol containingType, Symbol symb
{
return AsMemberOfTupleType((TupleTypeSymbol)containingType, symbol);
}
return symbolDef.SymbolAsMember(containingType);
var result = symbolDef.SymbolAsMember(containingType);
if (result is MethodSymbol resultMethod && resultMethod.IsGenericMethod)
{
return resultMethod.Construct(((MethodSymbol)symbol).TypeArguments);
}
return result;
}
containingType = containingType.BaseTypeNoUseSiteDiagnostics;
if ((object)containingType == null)
{
break;
}
}
// https://github.com/dotnet/roslyn/issues/29967 Handle other cases such as interfaces.
Debug.Assert(symbolDefContainer.IsInterface);
#if DEBUG
// https://github.com/dotnet/roslyn/issues/29967 Handle interfaces.
Debug.Assert(wasInterface);
#endif
return symbol;
}

Expand Down Expand Up @@ -3725,7 +3742,7 @@ private void VisitMemberAccess(BoundExpression receiverOpt, Symbol member, bool

if (!member.IsStatic)
{
member = AsMemberOfResultType(member);
member = AsMemberOfResultType(_resultType, member);
// https://github.com/dotnet/roslyn/issues/30598: For l-values, mark receiver as not null
// after RHS has been visited, and only if the receiver has not changed.
CheckPossibleNullReceiver(receiverOpt);
Expand Down Expand Up @@ -4234,13 +4251,16 @@ public override BoundNode VisitEventAssignmentOperator(BoundEventAssignmentOpera
VisitRvalue(node.ReceiverOpt);
Debug.Assert(!IsConditionalState);
var receiverOpt = node.ReceiverOpt;
var @event = node.Event;
if (!node.Event.IsStatic)
{
@event = (EventSymbol)AsMemberOfResultType(_resultType, @event);
// https://github.com/dotnet/roslyn/issues/30598: Mark receiver as not null
// after arguments have been visited, and only if the receiver has not changed.
CheckPossibleNullReceiver(receiverOpt);
}
VisitRvalue(node.Argument);
// https://github.com/dotnet/roslyn/issues/31018: Check for delegate mismatch.
SetResult(node); // https://github.com/dotnet/roslyn/issues/29969 Review whether this is the correct result
return null;
}
Expand Down
266 changes: 266 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27888,6 +27888,66 @@ void Test1(Test? x1)
);
}

[WorkItem(31018, "https://github.com/dotnet/roslyn/issues/31018")]
[Fact]
public void EventAssignment()
{
var source =
@"#pragma warning disable 0067
using System;
class A<T> { }
class B
{
event Action<A<object?>> E;
static void M1()
{
var b1 = new B();
b1.E += F1; // 1
b1.E += F2; // 2
b1.E += F3;
b1.E += F4;
}
static void M2(Action<A<object>> f1, Action<A<object>?> f2, Action<A<object?>> f3, Action<A<object?>?> f4)
{
var b2 = new B();
b2.E += f1; // 3
b2.E += f2; // 4
b2.E += f3;
b2.E += f4;
}
static void M3()
{
var b3 = new B();
b3.E += (A<object> a) => { }; // 5
b3.E += (A<object>? a) => { }; // 6
b3.E += (A<object?> a) => { };
b3.E += (A<object?>? a) => { }; // 7
Copy link
Member

Choose a reason for hiding this comment

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

This seems unfortunate, but given that this is already how variance works for lambdas/method groups, I don't see a way to change this without making overload resolution rules extremely convoluted and inconsistent.

}
static void F1(A<object> a) { }
static void F2(A<object>? a) { }
static void F3(A<object?> a) { }
static void F4(A<object?>? a) { }
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
// https://github.com/dotnet/roslyn/issues/31018: Report warnings for // 3 and // 4.
comp.VerifyDiagnostics(
// (10,17): warning CS8622: Nullability of reference types in type of parameter 'a' of 'void B.F1(A<object> a)' doesn't match the target delegate 'Action<A<object?>>'.
// b1.E += F1; // 1
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOfTargetDelegate, "F1").WithArguments("a", "void B.F1(A<object> a)", "System.Action<A<object?>>").WithLocation(10, 17),
// (11,17): warning CS8622: Nullability of reference types in type of parameter 'a' of 'void B.F2(A<object>? a)' doesn't match the target delegate 'Action<A<object?>>'.
// b1.E += F2; // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOfTargetDelegate, "F2").WithArguments("a", "void B.F2(A<object>? a)", "System.Action<A<object?>>").WithLocation(11, 17),
// (26,17): warning CS8622: Nullability of reference types in type of parameter 'a' of 'lambda expression' doesn't match the target delegate 'Action<A<object?>>'.
// b3.E += (A<object> a) => { }; // 5
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOfTargetDelegate, "(A<object> a) => { }").WithArguments("a", "lambda expression", "System.Action<A<object?>>").WithLocation(26, 17),
// (27,17): warning CS8622: Nullability of reference types in type of parameter 'a' of 'lambda expression' doesn't match the target delegate 'Action<A<object?>>'.
// b3.E += (A<object>? a) => { }; // 6
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOfTargetDelegate, "(A<object>? a) => { }").WithArguments("a", "lambda expression", "System.Action<A<object?>>").WithLocation(27, 17),
// (29,17): warning CS8622: Nullability of reference types in type of parameter 'a' of 'lambda expression' doesn't match the target delegate 'Action<A<object?>>'.
// b3.E += (A<object?>? a) => { }; // 7
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOfTargetDelegate, "(A<object?>? a) => { }").WithArguments("a", "lambda expression", "System.Action<A<object?>>").WithLocation(29, 17));
}

[Fact]
public void AsOperator_01()
{
Expand Down Expand Up @@ -51406,5 +51466,211 @@ static void Main()
var comp = CreateCompilation(source2, new[] { ref1.WithEmbedInteropTypes(true), CSharpRef }, options: WithNonNullTypesTrue(TestOptions.ReleaseExe));
CompileAndVerify(comp, expectedOutput: "4");
}

[Fact]
public void UpdateFieldFromReceiverType()
{
var source =
@"class A<T> { }
class B<T>
{
internal T F = default!;
}
class Program
{
internal static B<T> Create<T>(T t) => throw null;
static void M1(object x, object? y)
{
var b1 = Create(x);
b1.F = x;
b1.F = y; // 1
}
static void M2(A<object?> x, A<object> y, A<object?> z)
{
var b2 = Create(x);
b2.F = y; // 2
b2.F = z;
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (13,16): warning CS8601: Possible null reference assignment.
// b1.F = y; // 1
Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "y").WithLocation(13, 16),
// (18,16): warning CS8619: Nullability of reference types in value of type 'A<object>' doesn't match target type 'A<object?>'.
// b2.F = y; // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "y").WithArguments("A<object>", "A<object?>").WithLocation(18, 16));
}

[Fact]
public void UpdatePropertyFromReceiverType()
{
var source =
@"class A<T> { }
class B<T>
{
internal T P { get; set; } = default!;
}
class Program
{
internal static B<T> Create<T>(T t) => throw null;
static void M1(object x, object? y)
{
var b1 = Create(x);
b1.P = x;
b1.P = y; // 1
}
static void M2(A<object?> x, A<object> y, A<object?> z)
{
var b2 = Create(x);
b2.P = y; // 2
b2.P = z;
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (13,16): warning CS8601: Possible null reference assignment.
// b1.P = y; // 1
Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "y").WithLocation(13, 16),
// (18,16): warning CS8619: Nullability of reference types in value of type 'A<object>' doesn't match target type 'A<object?>'.
// b2.P = y; // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "y").WithArguments("A<object>", "A<object?>").WithLocation(18, 16));
}

[WorkItem(31018, "https://github.com/dotnet/roslyn/issues/31018")]
[Fact]
public void UpdateEventFromReceiverType()
{
var source =
@"#pragma warning disable 0067
delegate void D<T>(T t);
class C<T>
{
internal event D<T> E;
}
class Program
{
internal static C<T> Create<T>(T t) => throw null;
static void M1(object x, D<object> y, D<object?> z)
{
var c1 = Create(x);
c1.E += y;
c1.E += z; // 1
}
static void M2(object? x, D<object> y, D<object?> z)
{
var c2 = Create(x);
c2.E += y; // 2
c2.E += z;
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
// https://github.com/dotnet/roslyn/issues/31018: Report warnings.
comp.VerifyDiagnostics();
}

[WorkItem(29605, "https://github.com/dotnet/roslyn/issues/29605")]
[Fact]
public void UpdateMethodFromReceiverType_01()
{
var source =
@"class C<T>
{
internal void F(T t) { }
}
class Program
{
internal static C<T> Create<T>(T t) => throw null;
static void M(object x, object? y, string? z)
{
var c = Create(x);
c.F(x);
c.F(y); // 1
c.F(z); // 2
c.F(null); // 3
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (12,13): warning CS8604: Possible null reference argument for parameter 't' in 'void C<object>.F(object t)'.
// c.F(y); // 1
Diagnostic(ErrorCode.WRN_NullReferenceArgument, "y").WithArguments("t", "void C<object>.F(object t)").WithLocation(12, 13),
// (13,13): warning CS8604: Possible null reference argument for parameter 't' in 'void C<object>.F(object t)'.
// c.F(z); // 2
Diagnostic(ErrorCode.WRN_NullReferenceArgument, "z").WithArguments("t", "void C<object>.F(object t)").WithLocation(13, 13),
// (14,13): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter.
// c.F(null); // 3
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(14, 13));
}

[WorkItem(29605, "https://github.com/dotnet/roslyn/issues/29605")]
[Fact]
public void UpdateMethodFromReceiverType_02()
{
var source =
@"class A<T> { }
class B<T>
{
internal void F<U>(U u) where U : A<T> { }
}
class Program
{
internal static B<T> Create<T>(T t) => throw null;
static void M1(object x, A<object> y, A<object?> z)
{
var b1 = Create(x);
b1.F(y);
b1.F(z); // 1
}
static void M2(object? x, A<object> y, A<object?> z)
{
var b2 = Create(x);
b2.F(y); // 2
b2.F(z);
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (13,9): warning CS8631: The type 'A<object?>' cannot be used as type parameter 'U' in the generic type or method 'B<object>.F<U>(U)'. Nullability of type argument 'A<object?>' doesn't match constraint type 'A<object>'.
// b1.F(z); // 1
Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint, "b1.F").WithArguments("B<object>.F<U>(U)", "A<object>", "U", "A<object?>").WithLocation(13, 9),
// (18,9): warning CS8631: The type 'A<object>' cannot be used as type parameter 'U' in the generic type or method 'B<object?>.F<U>(U)'. Nullability of type argument 'A<object>' doesn't match constraint type 'A<object?>'.
// b2.F(y); // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint, "b2.F").WithArguments("B<object?>.F<U>(U)", "A<object?>", "U", "A<object>").WithLocation(18, 9));
}

[WorkItem(29605, "https://github.com/dotnet/roslyn/issues/29605")]
[Fact]
public void UpdateMethodFromReceiverType_03()
{
var source =
@"class C<T>
{
internal static T F() => throw null;
}
class Program
{
internal static C<T> Create<T>(T t) => throw null;
static void M(object x)
{
var c1 = Create(x);
c1.F().ToString();
x = null;
var c2 = Create(x);
c2.F().ToString();
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (11,9): error CS0176: Member 'C<object>.F()' cannot be accessed with an instance reference; qualify it with a type name instead
// c1.F().ToString();
Diagnostic(ErrorCode.ERR_ObjectProhibited, "c1.F").WithArguments("C<object>.F()").WithLocation(11, 9),
// (12,13): warning CS8600: Converting null literal or possible null value to non-nullable type.
// x = null;
Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(12, 13),
// (14,9): error CS0176: Member 'C<object>.F()' cannot be accessed with an instance reference; qualify it with a type name instead
// c2.F().ToString();
Diagnostic(ErrorCode.ERR_ObjectProhibited, "c2.F").WithArguments("C<object>.F()").WithLocation(14, 9));
}
}
}