From a52896cc4a0e99b34aef62e20decbfd93919e2f3 Mon Sep 17 00:00:00 2001 From: Charles Stoner Date: Thu, 10 May 2018 13:27:51 -0700 Subject: [PATCH] Warning for null default value of non-nullable parameter (#26726) --- .../Portable/FlowAnalysis/NullableWalker.cs | 8 +- .../Symbols/Source/ParameterHelpers.cs | 23 +- .../Source/SourceComplexParameterSymbol.cs | 25 ++ .../AttributeTests_CallerInfoAttributes.cs | 4 +- .../Semantic/Semantics/StaticNullChecking.cs | 334 ++++++++++++++++-- .../CSharpDeclareAsNullableCodeFixTests.cs | 6 +- 6 files changed, 353 insertions(+), 47 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index a80f9a2443e..fa81d9b3e96 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -114,13 +114,7 @@ public static void Analyze(CSharpCompilation compilation, MethodSymbol member, B return; } - var flags = ((CSharpParseOptions)node.SyntaxTree.Options).GetNullableReferenceFlags(); - var walker = new NullableWalker( - compilation, - member, - node, - callbackOpt); - + var walker = new NullableWalker(compilation, member, node, callbackOpt); try { bool badRegion = false; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs b/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs index a56ec6eb0a2..96df6d00dfd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs @@ -492,10 +492,25 @@ private static bool IsValidDefaultValue(BoundExpression expression) // // Also when valuetype S has a parameterless constructor, // new S() is clearly not a constant expression and should produce an error - return (expression.ConstantValue != null) || - (expression.Kind == BoundKind.DefaultExpression) || - (expression.Kind == BoundKind.ObjectCreationExpression && - IsValidDefaultValue((BoundObjectCreationExpression)expression)); + if (expression.ConstantValue != null) + { + return true; + } + while (true) + { + switch (expression.Kind) + { + case BoundKind.DefaultExpression: + return true; + case BoundKind.ObjectCreationExpression: + return IsValidDefaultValue((BoundObjectCreationExpression)expression); + case BoundKind.SuppressNullableWarningExpression: + expression = ((BoundSuppressNullableWarningExpression)expression).Expression; + break; + default: + return false; + } + } } private static bool IsValidDefaultValue(BoundObjectCreationExpression expression) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs index c68fe25af3a..bc305c61345 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs @@ -212,10 +212,35 @@ protected ConstantValue MakeDefaultExpression(DiagnosticBag diagnostics, Binder } } + if (parameterType.IsNullable == false && + parameterType.IsReferenceType && + convertedExpression.ConstantValue?.IsNull == true && + !suppressNullableWarning(convertedExpression)) + { + diagnostics.Add(ErrorCode.WRN_NullAsNonNullable, parameterSyntax.Default.Value.Location); + } + // represent default(struct) by a Null constant: var value = convertedExpression.ConstantValue ?? ConstantValue.Null; VerifyParamDefaultValueMatchesAttributeIfAny(value, defaultSyntax.Value, diagnostics); return value; + + bool suppressNullableWarning(BoundExpression expr) + { + while (true) + { + switch (expr.Kind) + { + case BoundKind.Conversion: + expr = ((BoundConversion)expr).Operand; + break; + case BoundKind.SuppressNullableWarningExpression: + return true; + default: + return false; + } + } + } } public override string MetadataName diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_CallerInfoAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_CallerInfoAttributes.cs index 4528217846d..70ad512a8d0 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_CallerInfoAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_CallerInfoAttributes.cs @@ -2454,7 +2454,7 @@ static void Main(string[] args) "; var compilation = CreateCompilationWithMscorlib45( - new[] { SyntaxFactory.ParseSyntaxTree(source, path: @"C:\filename", encoding: Encoding.UTF8) }, + new[] { SyntaxFactory.ParseSyntaxTree(source, options: TestOptions.Regular7, path: @"C:\filename", encoding: Encoding.UTF8) }, options: TestOptions.ReleaseExe); compilation.VerifyDiagnostics( @@ -2524,7 +2524,7 @@ static void Main(string[] args) } "; - var compilation = CreateCompilationWithMscorlib45(new SyntaxTree[] { SyntaxFactory.ParseSyntaxTree(source, path: @"C:\filename") }).VerifyDiagnostics( + var compilation = CreateCompilationWithMscorlib45(new SyntaxTree[] { SyntaxFactory.ParseSyntaxTree(source, options: TestOptions.Regular7, path: @"C:\filename") }).VerifyDiagnostics( // C:\filename(7,38): error CS4018: CallerFilePathAttribute cannot be applied because there are no standard conversions from type 'string' to type 'int' // static void M1([CallerLineNumber,CallerFilePath,CallerMemberName] int i = 0) { Console.WriteLine(); } Diagnostic(ErrorCode.ERR_NoConversionForCallerFilePathParam, "CallerFilePath").WithArguments("string", "int"), diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs index 4e6302e99b8..de793af9db6 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs @@ -17925,39 +17925,9 @@ static T F(IEnumerable e) /// Default value for non-nullable parameter /// should not result in a warning at the call site. /// + [WorkItem(26626, "https://github.com/dotnet/roslyn/issues/26626")] [Fact] - public void NullDefaultValueFromSource() - { - var source = -@"class C -{ - public static void F(object o = null) - { - } -} -class Program -{ - static void Main() - { - C.F(); - C.F(null); - } -}"; - var comp = CreateCompilation( - source, - parseOptions: TestOptions.Regular8); - comp.VerifyDiagnostics( - // (12,13): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. - // C.F(null); - Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(12, 13)); - } - - /// - /// Default value for non-nullable parameter - /// should not result in a warning at the call site. - /// - [Fact] - public void NullDefaultValueFromMetadata() + public void ParameterDefaultValue_FromMetadata() { var source0 = @"public class C @@ -17969,7 +17939,10 @@ public static void F(object o = null) var comp0 = CreateCompilation( source0, parseOptions: TestOptions.Regular8); - comp0.VerifyDiagnostics(); + comp0.VerifyDiagnostics( + // (3,37): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // public static void F(object o = null) + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(3, 37)); var ref0 = comp0.EmitToImageReference(); var source1 = @@ -17991,6 +17964,301 @@ static void Main() Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 13)); } + [WorkItem(26626, "https://github.com/dotnet/roslyn/issues/26626")] + [Fact] + public void ParameterDefaultValue_01() + { + var source = +@"class C +{ + const string? S0 = null; + const string? S1 = """"; + static string? F() => string.Empty; + static void F0(string s = null) { } + static void F1(string s = default) { } + static void F2(string s = default(string)) { } + static void F3(string x = (string)null, string y = (string?)null) { } + static void F4(string s = S0) { } + static void F5(string s = S1) { } + static void F6(string s = F()) { } + static void M() + { + F0(); + F1(); + F2(); + F3(); + F4(); + F5(); + F6(); + F0(null); + F0(string.Empty); + } +}"; + var comp = CreateCompilation(source, parseOptions: TestOptions.Regular8); + comp.VerifyDiagnostics( + // (6,31): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F0(string s = null) { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 31), + // (7,31): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F1(string s = default) { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(7, 31), + // (8,31): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F2(string s = default(string)) { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default(string)").WithLocation(8, 31), + // (9,31): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F3(string x = (string)null, string y = (string?)null) { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "(string)null").WithLocation(9, 31), + // (9,56): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F3(string x = (string)null, string y = (string?)null) { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "(string?)null").WithLocation(9, 56), + // (10,31): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F4(string s = S0) { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "S0").WithLocation(10, 31), + // (12,31): error CS1736: Default parameter value for 's' must be a compile-time constant + // static void F6(string s = F()) { } + Diagnostic(ErrorCode.ERR_DefaultValueMustBeConstant, "F()").WithArguments("s").WithLocation(12, 31), + // (22,12): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F0(null); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(22, 12)); + } + + [WorkItem(26626, "https://github.com/dotnet/roslyn/issues/26626")] + [Fact] + public void ParameterDefaultValue_02() + { + var source = +@"class C +{ + const string? S0 = null; + static void F0(string s = null!) { } + static void F1(string x = (string)null!, string y = ((string)null)!) { } + static void F2(string x = default!, string y = default(string)!) { } + static void F3(string s = (S0!)!) { } + static void M() + { + F0(); + F1(); + F2(); + F3(); + F1(x: null); + F1(y: null); + F2(null!, null); + } +}"; + var comp = CreateCompilation(source, parseOptions: TestOptions.Regular8); + comp.VerifyDiagnostics( + // (14,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F1(x: null); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(14, 15), + // (15,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F1(y: null); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(15, 15), + // (16,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F2(null!, null); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(16, 19)); + } + + [WorkItem(26626, "https://github.com/dotnet/roslyn/issues/26626")] + [Fact] + public void ParameterDefaultValue_03() + { + var source = +@"interface I { } +class C : I { } +class P +{ + static void F0(T t = default) { } + static void F1(T t = null) where T : class { } + static void F2(T t = default) where T : struct { } + static void F3(T t = default) where T : new() { } + static void F4(T t = null) where T : C { } + static void F5(T t = default) where T : I { } + static void F6(T t = default) where T : U { } + static void G0() + { + F0(); + F0(default); + F0(new object()); + F1(); + F1(default); + F1(new object()); + F2(); + F2(default); + F2(2); + F3(); + F3(default); + F3(new object()); + F4(); + F4(default); + F4(new C()); + F5(); + F5(default); + F5(new C()); + F6(); + F6(default); + F6(new object()); + } + static void G0() + { + F0(); // 0 + F0(default); // 0 + F6(); // 0 + F6(default); // 0 + } + static void G1() where T : class + { + F0(); // 1 + F0(default); // 1 + F1(); // 1 + F1(default); // 1 + F6(); // 1 + F6(default); // 1 + } + static void G2() where T : struct + { + F0(); // 2 + F0(default); // 2 + F2(); // 2 + F2(default); // 2 + F3(); // 2 + F3(default); // 2 + F6(); // 2 + F6(default); // 2 + } + static void G3() where T : new() + { + F0(); // 3 + F0(default); // 3 + F0(new T()); // 3 + F3(); // 3 + F3(default); // 3 + F3(new T()); // 3 + F6(); // 3 + F6(default); // 3 + F6(new T()); // 3 + } + static void G4() where T : C + { + F0(); // 4 + F0(default); // 4 + F1(); // 4 + F1(default); // 4 + F4(); // 4 + F4(default); // 4 + F5(); // 4 + F5(default); // 4 + F6(); // 4 + F6(default); // 4 + } + static void G5() where T : I + { + F0(); // 5 + F0(default); // 5 + F5(); // 5 + F5(default); // 5 + F6(); // 5 + F6(default); // 5 + } + static void G6() where T : U + { + F0(); // 6 + F0(default); // 6 + F6(); // 6 + F6(default); // 6 + } +}"; + var comp = CreateCompilation(source, parseOptions: TestOptions.Regular8); + comp.VerifyDiagnostics( + // (6,29): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F1(T t = null) where T : class { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 29), + // (9,29): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static void F4(T t = null) where T : C { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(9, 29), + // (15,20): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F0(default); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(15, 20), + // (18,20): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F1(default); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(18, 20), + // (24,20): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F3(default); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(24, 20), + // (27,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F4(default); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(27, 15), + // (30,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F5(default); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(30, 15), + // (33,28): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F6(default); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(33, 28), + // (46,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F0(default); // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(46, 15), + // (48,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F1(default); // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(48, 15), + // (50,18): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F6(default); // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(50, 18), + // (78,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F0(default); // 4 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(78, 15), + // (80,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F1(default); // 4 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(80, 15), + // (82,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F4(default); // 4 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(82, 15), + // (84,15): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F5(default); // 4 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(84, 15), + // (86,18): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // F6(default); // 4 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(86, 18)); + + // No warnings with C#7.3. + comp = CreateCompilation(source, parseOptions: TestOptions.Regular7_3); + comp.VerifyDiagnostics(); + } + + [WorkItem(26626, "https://github.com/dotnet/roslyn/issues/26626")] + [Fact] + public void ParameterDefaultValue_04() + { + var source = +@"partial class C +{ + static partial void F(object? x = null, object y = null); + static partial void F(object? x, object y) { } + static partial void G(object x, object? y); + static partial void G(object x = null, object? y = null) { } + static void M() + { + F(); + G(); + } +}"; + var comp = CreateCompilation(source, parseOptions: TestOptions.Regular8); + comp.VerifyDiagnostics( + // (6,34): warning CS1066: The default value specified for parameter 'x' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments + // static partial void G(object x = null, object? y = null) { } + Diagnostic(ErrorCode.WRN_DefaultValueForUnconsumedLocation, "x").WithArguments("x").WithLocation(6, 34), + // (6,38): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static partial void G(object x = null, object? y = null) { } + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 38), + // (6,52): warning CS1066: The default value specified for parameter 'y' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments + // static partial void G(object x = null, object? y = null) { } + Diagnostic(ErrorCode.WRN_DefaultValueForUnconsumedLocation, "y").WithArguments("y").WithLocation(6, 52), + // (3,56): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // static partial void F(object? x = null, object y = null); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(3, 56), + // (10,9): error CS7036: There is no argument given that corresponds to the required formal parameter 'x' of 'C.G(object, object)' + // G(); + Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "G").WithArguments("x", "C.G(object, object?)").WithLocation(10, 9)); + } + [Fact] public void InvalidThrowTerm() { diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/Nullable/CSharpDeclareAsNullableCodeFixTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/Nullable/CSharpDeclareAsNullableCodeFixTests.cs index 38f043af6d0..d8387970fd7 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/Nullable/CSharpDeclareAsNullableCodeFixTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/Nullable/CSharpDeclareAsNullableCodeFixTests.cs @@ -251,10 +251,14 @@ public async Task FixPropertyDeclaration_ArrowBody() [WorkItem(26626, "https://github.com/dotnet/roslyn/issues/26626")] public async Task FixOptionalParameter() { - await TestMissingInRegularAndScriptAsync( + await TestInRegularAndScript1Async( @"class Program { static void M(string x = [|null|]) { } +}", +@"class Program +{ + static void M(string? x = null) { } }", parameters: s_nullableFeature); } } -- GitLab