diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index aaa63f9c6abac..b28851d5ab230 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -4397,11 +4397,11 @@ private ImmutableArray VisitArguments( Debug.Assert(!arguments.IsDefault); bool shouldReturnNotNull = false; - (ImmutableArray argumentsNoConversions, ImmutableArray conversions) = RemoveArgumentConversions(arguments, refKindsOpt); + (ImmutableArray argumentsMostlyNoConversions, ImmutableArray conversions) = RemoveArgumentConversions(arguments, refKindsOpt); // Visit the arguments and collect results ImmutableArray results; - (results, argumentsNoConversions, argsToParamsOpt, refKindsOpt) = VisitArgumentsEvaluate(node.Syntax, argumentsNoConversions, refKindsOpt, parametersOpt, argsToParamsOpt, expanded); + (results, argumentsMostlyNoConversions, argsToParamsOpt, refKindsOpt) = VisitArgumentsEvaluate(node.Syntax, argumentsMostlyNoConversions, refKindsOpt, parametersOpt, argsToParamsOpt, expanded); // Re-infer method type parameters if ((object)method != null && method.IsGenericMethod) @@ -4409,7 +4409,7 @@ private ImmutableArray VisitArguments( if (HasImplicitTypeArguments(node)) { var binder = (node as BoundCall)?.BinderOpt ?? (node as BoundCollectionElementInitializer)?.BinderOpt ?? throw ExceptionUtilities.UnexpectedValue(node); - method = InferMethodTypeArguments(binder, method, GetArgumentsForMethodTypeInference(results, argumentsNoConversions), refKindsOpt, argsToParamsOpt, expanded); + method = InferMethodTypeArguments(binder, method, GetArgumentsForMethodTypeInference(results, argumentsMostlyNoConversions), refKindsOpt, argsToParamsOpt, expanded); parametersOpt = method.Parameters; } if (ConstraintsHelper.RequiresChecking(method)) @@ -4434,11 +4434,11 @@ private ImmutableArray VisitArguments( continue; } - var argumentNoConversion = argumentsNoConversions[i]; - var argument = i < arguments.Length ? arguments[i] : argumentsNoConversions[i]; + var argumentMostlyNoConversion = argumentsMostlyNoConversions[i]; + var argument = i < arguments.Length ? arguments[i] : argumentMostlyNoConversion; VisitArgumentConversionAndInboundAssignmentsAndPreConditions( - GetConversionIfApplicable(argument, argumentNoConversion), - argumentNoConversion, + GetConversionIfApplicable(argument, argumentMostlyNoConversion, avoidTypelessExpressions: true), + argumentMostlyNoConversion, conversions.IsDefault || i >= conversions.Length ? Conversion.Identity : conversions[i], GetRefKind(refKindsOpt, i), parameter, @@ -4498,8 +4498,8 @@ private ImmutableArray VisitArguments( // so just assume that the conversions have the same nullability as the underlying result var argument = arguments[i]; var result = results[i]; - var argumentNoConversion = argumentsNoConversions[i]; - TrackAnalyzedNullabilityThroughConversionGroup(TypeWithState.Create(argument.Type, result.RValueType.State), argument as BoundConversion, argumentNoConversion); + var argumentMostlyNoConversion = argumentsMostlyNoConversions[i]; + TrackAnalyzedNullabilityThroughConversionGroup(TypeWithState.Create(argument.Type, result.RValueType.State), argument as BoundConversion, argumentMostlyNoConversion); } } @@ -5111,7 +5111,7 @@ void learnFromPostConditions(BoundExpression argument, FlowAnalysisAnnotations p if (refKind == RefKind.None) { var before = argument; - (argument, conversion) = RemoveConversion(argument, includeExplicitConversions: false); + (argument, conversion) = RemoveConversion(argument, includeExplicitConversions: false, avoidTypelessExpressions: true); if (argument != before) { SnapshotWalkerThroughConversionGroup(before, argument); @@ -5395,7 +5395,7 @@ private void CheckMethodConstraints(SyntaxNode syntax, MethodSymbol method) /// (Currently, the only visit method that passes `includeExplicitConversions: true` /// is VisitConversion. All other callers are handling implicit conversions only.) /// - private static (BoundExpression expression, Conversion conversion) RemoveConversion(BoundExpression expr, bool includeExplicitConversions) + private static (BoundExpression expression, Conversion conversion) RemoveConversion(BoundExpression expr, bool includeExplicitConversions, bool avoidTypelessExpressions = false) { ConversionGroup group = null; while (true) @@ -5404,7 +5404,16 @@ private static (BoundExpression expression, Conversion conversion) RemoveConvers { break; } + var conversion = (BoundConversion)expr; + + if (avoidTypelessExpressions && + conversion.ConversionKind == ConversionKind.ImplicitReference && + conversion.Operand is BoundLiteral { Type: null }) + { + break; + } + if (group != conversion.ConversionGroupOpt && group != null) { // E.g.: (C)(B)a @@ -6044,13 +6053,13 @@ private bool HasTopLevelNullabilityConversion(TypeWithAnnotations source, TypeWi /// Gets the conversion node for passing to , /// if one should be passed. /// - private static BoundConversion GetConversionIfApplicable(BoundExpression conversionOpt, BoundExpression convertedNode) + private static BoundConversion GetConversionIfApplicable(BoundExpression conversionOpt, BoundExpression convertedNode, bool avoidTypelessExpressions = false) { Debug.Assert(conversionOpt is null || convertedNode == conversionOpt // Note that convertedNode itself can be a BoundConversion, so we do this check explicitly // because the below calls to RemoveConversion could potentially strip that conversion. - || convertedNode == RemoveConversion(conversionOpt, includeExplicitConversions: false).expression - || convertedNode == RemoveConversion(conversionOpt, includeExplicitConversions: true).expression); + || convertedNode == RemoveConversion(conversionOpt, includeExplicitConversions: false, avoidTypelessExpressions).expression + || convertedNode == RemoveConversion(conversionOpt, includeExplicitConversions: true, avoidTypelessExpressions).expression); return conversionOpt == convertedNode ? null : (BoundConversion)conversionOpt; } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index ec1702e2ca27e..4cbf97f04a6af 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -4045,6 +4045,147 @@ void M(T t) ); } + [Fact, WorkItem(43536, "https://github.com/dotnet/roslyn/issues/43536")] + public void MethodTypeInference_WithDefaultAndNull() + { + var source = @" +class C +{ + void M(string s) + { + Infer(s, default).ToString(); // 1 + Infer(s, null).ToString(); // 2 + } + + T Infer(T t1, T t2) => throw null!; +}"; + + var comp = CreateNullableCompilation(source); + comp.VerifyDiagnostics( + // (6,9): warning CS8602: Dereference of a possibly null reference. + // Infer(s, default).ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Infer(s, default)").WithLocation(6, 9), + // (7,9): warning CS8602: Dereference of a possibly null reference. + // Infer(s, null).ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Infer(s, null)").WithLocation(7, 9) + ); + } + + [Fact, WorkItem(43536, "https://github.com/dotnet/roslyn/issues/43536")] + public void MethodTypeInference_WithNull_TwoConversions() + { + var source = @" +class C +{ + void M(string s) + { + M2(null); // 1 + } + + void M2(Optional o) => throw null!; +} + +public readonly struct Optional +{ + public Optional(T value) => throw null!; + public static implicit operator Optional(T value) => throw null!; +} +"; + var comp = CreateNullableCompilation(source); + comp.VerifyDiagnostics( + // (6,12): warning CS8625: Cannot convert null literal to non-nullable reference type. + // M2(null); // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 12) + ); + } + + [Fact, WorkItem(43536, "https://github.com/dotnet/roslyn/issues/43536")] + public void MethodTypeInference_WithLambdaReturningDefaultAndNull() + { + var source = @" +class C +{ + void M(string s1, string? s2) + { + Infer(s1, () => default); // 1 + Infer(s1, () => null); // 2 + + Infer(s2, () => default); + Infer(s2, () => null); + } + + T Infer(T t1, System.Func t2) => throw null!; +}"; + + var comp = CreateCompilationWithIndexAndRange(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (6,25): warning CS8603: Possible null reference return. + // Infer(s1, () => default); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReturn, "default").WithLocation(6, 25), + // (7,25): warning CS8603: Possible null reference return. + // Infer(s1, () => null); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReturn, "null").WithLocation(7, 25) + ); + } + + [Fact, WorkItem(43536, "https://github.com/dotnet/roslyn/issues/43536")] + public void MethodTypeInference_WithNullAndNestedNullability() + { + var source = @" +class C +{ + void M(C c1, C c2) + { + Infer(c1, default).ToString(); + Infer(c1, null).ToString(); + + Infer(c2, default).ToString(); // 1 + Infer(c2, null).ToString(); // 2 + } + + T Infer(C? t1, C? t2) => throw null!; +}"; + + var comp = CreateCompilationWithIndexAndRange(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (9,9): warning CS8602: Dereference of a possibly null reference. + // Infer(c2, default).ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Infer(c2, default)").WithLocation(9, 9), + // (10,9): warning CS8602: Dereference of a possibly null reference. + // Infer(c2, null).ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Infer(c2, null)").WithLocation(10, 9) + ); + } + + [Fact, WorkItem(43536, "https://github.com/dotnet/roslyn/issues/43536")] + public void MethodTypeInference_WithNullAndNestedNullability2() + { + var source = @" +class C +{ + void M(string c1, string? c2) + { + Infer(c1, default).ToString(); + Infer(c1, null).ToString(); + + Infer(c2, default).ToString(); // 1 + Infer(c2, null).ToString(); // 2 + } + + T Infer(T t1, C? t2) => throw null!; +}"; + + var comp = CreateCompilationWithIndexAndRange(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (9,9): warning CS8602: Dereference of a possibly null reference. + // Infer(c2, default).ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Infer(c2, default)").WithLocation(9, 9), + // (10,9): warning CS8602: Dereference of a possibly null reference. + // Infer(c2, null).ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Infer(c2, null)").WithLocation(10, 9) + ); + } + [Fact] public void SpeakableInference_MethodTypeInference_WithTuple() { @@ -29801,8 +29942,8 @@ public class C { public void Main() { - Copy(null, out string s); // warn - s/*T:string!*/.ToString(); // ok + Copy(null, out string s); // 1 + s/*T:string?*/.ToString(); // 2 } public static void Copy(T key, out T value) => throw null!; } @@ -29811,9 +29952,12 @@ public void Main() VerifyOutVar(c, "string!"); c.VerifyTypes(); c.VerifyDiagnostics( - // (6,14): warning CS8625: Cannot convert null literal to non-nullable reference type. - // Copy(null, out string s); // warn - Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 14) + // (6,24): warning CS8600: Converting null literal or possible null value to non-nullable type. + // Copy(null, out string s); // 1 + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "string s").WithLocation(6, 24), + // (7,9): warning CS8602: Dereference of a possibly null reference. + // s/*T:string?*/.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(7, 9) ); }