From 6517f58bb8add3b0706737f99c506b90ccb00ef5 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Wed, 27 Sep 2023 14:52:30 -0700 Subject: [PATCH 01/11] Collection expressions: analyze nullability of element assignments --- .../Portable/Binder/Binder.ValueChecks.cs | 4 +- .../Portable/Binder/Binder_Conversions.cs | 8 +- .../Semantics/Conversions/Conversions.cs | 8 +- .../Semantics/Conversions/ConversionsBase.cs | 20 +- .../Portable/FlowAnalysis/NullableWalker.cs | 40 +- .../Semantics/CollectionExpressionTests.cs | 768 +++++++++++++++++- 6 files changed, 815 insertions(+), 33 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index 6a6d982357c41..fd85db5d9ddef 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -4025,8 +4025,8 @@ private bool HasLocalScope(BoundCollectionExpression expr) switch (collectionTypeKind) { case CollectionExpressionTypeKind.ReadOnlySpan: - Debug.Assert(elementType is { }); - return !LocalRewriter.ShouldUseRuntimeHelpersCreateSpan(expr, elementType); + Debug.Assert(elementType.Type is { }); + return !LocalRewriter.ShouldUseRuntimeHelpersCreateSpan(expr, elementType.Type); case CollectionExpressionTypeKind.Span: return true; case CollectionExpressionTypeKind.CollectionBuilder: diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs index 2811a17147fb3..7999fd82116d8 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs @@ -768,20 +768,20 @@ private void GenerateImplicitConversionErrorForCollectionExpression( TypeSymbol targetType, BindingDiagnosticBag diagnostics) { - var collectionTypeKind = ConversionsBase.GetCollectionExpressionTypeKind(Compilation, targetType, out var elementType); + var collectionTypeKind = ConversionsBase.GetCollectionExpressionTypeKind(Compilation, targetType, out TypeWithAnnotations elementTypeWithAnnotations); if (collectionTypeKind == CollectionExpressionTypeKind.CollectionBuilder) { - Debug.Assert(elementType is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases. - if (!TryGetCollectionIterationType((ExpressionSyntax)node.Syntax, targetType, out TypeWithAnnotations elementTypeWithAnnotations)) + Debug.Assert(elementTypeWithAnnotations.Type is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases. + if (!TryGetCollectionIterationType((ExpressionSyntax)node.Syntax, targetType, out elementTypeWithAnnotations)) { Error(diagnostics, ErrorCode.ERR_CollectionBuilderNoElementType, node.Syntax, targetType); return; } Debug.Assert(elementTypeWithAnnotations.HasType); - elementType = elementTypeWithAnnotations.Type; } bool reportedErrors = false; + var elementType = elementTypeWithAnnotations.Type; if (collectionTypeKind == CollectionExpressionTypeKind.CollectionInitializer) { diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs index 342c0fceed35c..3115f23534b3d 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs @@ -154,7 +154,7 @@ protected override Conversion GetCollectionExpressionConversion( ref CompoundUseSiteInfo useSiteInfo) { var syntax = node.Syntax; - var collectionTypeKind = GetCollectionExpressionTypeKind(Compilation, targetType, out var elementType); + var collectionTypeKind = GetCollectionExpressionTypeKind(Compilation, targetType, out TypeWithAnnotations elementTypeWithAnnotations); switch (collectionTypeKind) { @@ -163,9 +163,8 @@ protected override Conversion GetCollectionExpressionConversion( case CollectionExpressionTypeKind.CollectionBuilder: { - _binder.TryGetCollectionIterationType((Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax)syntax, targetType, out TypeWithAnnotations elementTypeWithAnnotations); - elementType = elementTypeWithAnnotations.Type; - if (elementType is null) + _binder.TryGetCollectionIterationType((Syntax.ExpressionSyntax)syntax, targetType, out elementTypeWithAnnotations); + if (elementTypeWithAnnotations.Type is null) { return Conversion.NoConversion; } @@ -173,6 +172,7 @@ protected override Conversion GetCollectionExpressionConversion( break; } + var elementType = elementTypeWithAnnotations.Type; Debug.Assert(collectionTypeKind == CollectionExpressionTypeKind.CollectionInitializer || elementType is { }); if (collectionTypeKind == CollectionExpressionTypeKind.CollectionInitializer) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs index 3e4b790891dff..34debf50bc3c9 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs @@ -1624,7 +1624,7 @@ private static bool HasAnonymousFunctionConversion(BoundExpression source, TypeS return IsAnonymousFunctionCompatibleWithType((UnboundLambda)source, destination, compilation) == LambdaConversionResult.Success; } - internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSharpCompilation compilation, TypeSymbol destination, out TypeSymbol? elementType) + internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSharpCompilation compilation, TypeSymbol destination, out TypeWithAnnotations elementType) { Debug.Assert(compilation is { }); @@ -1632,7 +1632,7 @@ internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSh { if (arrayType.IsSZArray) { - elementType = arrayType.ElementType; + elementType = arrayType.ElementTypeWithAnnotations; return CollectionExpressionTypeKind.Array; } } @@ -1650,7 +1650,7 @@ internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSh } else if (implementsIEnumerable(compilation, destination)) { - elementType = null; + elementType = default; return CollectionExpressionTypeKind.CollectionInitializer; } else if (isListInterface(compilation, destination, out elementType)) @@ -1658,18 +1658,18 @@ internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSh return CollectionExpressionTypeKind.ListInterface; } - elementType = null; + elementType = default; return CollectionExpressionTypeKind.None; - static bool isSpanType(CSharpCompilation compilation, TypeSymbol targetType, WellKnownType spanType, [NotNullWhen(true)] out TypeSymbol? elementType) + static bool isSpanType(CSharpCompilation compilation, TypeSymbol targetType, WellKnownType spanType, [NotNullWhen(true)] out TypeWithAnnotations elementType) { if (targetType is NamedTypeSymbol { Arity: 1 } namedType && areEqual(namedType.OriginalDefinition, compilation.GetWellKnownType(spanType))) { - elementType = namedType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0].Type; + elementType = namedType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0]; return true; } - elementType = null; + elementType = default; return false; } @@ -1699,7 +1699,7 @@ static bool implementsIEnumerable(CSharpCompilation compilation, TypeSymbol targ return allInterfaces.Any(static (a, b) => areEqual(a, b), ienumerableType); } - static bool isListInterface(CSharpCompilation compilation, TypeSymbol targetType, [NotNullWhen(true)] out TypeSymbol? elementType) + static bool isListInterface(CSharpCompilation compilation, TypeSymbol targetType, [NotNullWhen(true)] out TypeWithAnnotations elementType) { if (targetType is NamedTypeSymbol { @@ -1712,10 +1712,10 @@ SpecialType.System_Collections_Generic_ICollection_T or TypeArgumentsWithAnnotationsNoUseSiteDiagnostics: [var typeArg] }) { - elementType = typeArg.Type; + elementType = typeArg; return true; } - elementType = null; + elementType = default; return false; } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 2bddf08c8e9f8..7038f883f7162 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -246,7 +246,9 @@ private PooledDictionary s_skippedExpressions = ImmutableArray.Create(BoundKind.ArrayInitialization, BoundKind.ObjectInitializerExpression, BoundKind.CollectionInitializerExpression, - BoundKind.DynamicCollectionElementInitializer); + BoundKind.DynamicCollectionElementInitializer, + BoundKind.CollectionExpressionSpreadElement); // https://github.com/dotnet/roslyn/issues/68786: We should handle spreads + #endif /// @@ -3476,29 +3478,45 @@ protected override void VisitStatement(BoundStatement statement) // https://github.com/dotnet/roslyn/issues/68786: Use inferInitialObjectState() to set the initial // state of the instance: see the call to InheritNullableStateOfTrackableStruct() in particular. int containerSlot = GetOrCreatePlaceholderSlot(node); - bool delayCompletionForType = false; // https://github.com/dotnet/roslyn/issues/68786: Should be true if the collection expression is target typed. + NullableFlowState resultState = NullableFlowState.NotNull; + + var collectionKind = ConversionsBase.GetCollectionExpressionTypeKind(this.compilation, node.Type, out var targetElementType); + if (collectionKind is CollectionExpressionTypeKind.CollectionBuilder) + { + var createMethod = node.CollectionBuilderMethod; + if (createMethod is not null) + { + var readOnlySpan = (NamedTypeSymbol)createMethod.Parameters[0].Type; + targetElementType = readOnlySpan.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0]; + var annotations = createMethod.GetFlowAnalysisAnnotations(); + resultState = ApplyUnconditionalAnnotations(createMethod.ReturnTypeWithAnnotations, annotations).ToTypeWithState().State; + } + } - // https://github.com/dotnet/roslyn/issues/68786: Set nullability of elements from the inferred target type nullability. foreach (var element in node.Elements) { switch (element) { case BoundCollectionElementInitializer initializer: var collectionType = initializer.AddMethod.ContainingType; - var completion = VisitCollectionElementInitializer(initializer, collectionType, delayCompletionForType); - if (completion is { }) - { - // https://github.com/dotnet/roslyn/issues/68786: Complete the analysis later. - completion(containerSlot, collectionType); - } + + var completion = VisitCollectionElementInitializer(initializer, collectionType, + delayCompletionForType: false /* All collection expressions are target-typed */); + + Debug.Assert(completion is null); + break; + case BoundCollectionExpressionSpreadElement spread: + // https://github.com/dotnet/roslyn/issues/68786: We should check the spread + Visit(spread); break; default: - VisitRvalue(element); + _ = VisitOptionalImplicitConversion(element, targetElementType, useLegacyWarnings: false, trackMembers: false, AssignmentKind.Assignment); + break; } } - SetResultType(node, TypeWithState.Create(node.Type, NullableFlowState.NotNull)); + SetResultType(node, TypeWithState.Create(node.Type, resultState)); return null; } diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs index 369cdd1511f81..6b05b485a5c29 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs @@ -7343,11 +7343,16 @@ static void Main() } """; var comp = CreateCompilation(source); - // https://github.com/dotnet/roslyn/issues/68786: // 2 should be reported as a warning (compare with array initializer: new object[] { null }). comp.VerifyEmitDiagnostics( // (7,9): warning CS8602: Dereference of a possibly null reference. // x[0].ToString(); // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x[0]").WithLocation(7, 9)); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x[0]").WithLocation(7, 9), + // (8,23): warning CS8625: Cannot convert null literal to non-nullable reference type. + // object[] y = [null]; // 2 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(8, 23), + // (10,17): warning CS8625: Cannot convert null literal to non-nullable reference type. + // y = [2, null]; // 3 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 17)); } [Fact] @@ -17903,5 +17908,764 @@ public XAttribute(A a) { } Diagnostic(ErrorCode.ERR_BadAttributeParamType, "X").WithArguments("a", "A").WithLocation(12, 2) ); } + + [Fact] + public void ElementNullability_ArrayCollection() + { + string src = """ + #nullable enable + string[] x1 = [null]; + string?[] x2 = [null]; + + #nullable disable + string[] + #nullable enable + x3 = [null]; + """; + + CreateCompilation(src).VerifyEmitDiagnostics( + // (2,16): warning CS8625: Cannot convert null literal to non-nullable reference type. + // string[] x1 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(2, 16) + ); + } + + [Fact] + public void ElementNullability_ArrayCollection_NullableValueTypes() + { + string src = """ + #nullable enable + int[] x1 = [null]; + int?[] x2 = [null]; + + #nullable disable + int[] + #nullable enable + x3 = [null]; + """; + + CreateCompilation(src).VerifyEmitDiagnostics( + // (2,13): error CS0037: Cannot convert null to 'int' because it is a non-nullable value type + // int[] x1 = [null]; + Diagnostic(ErrorCode.ERR_ValueCantBeNull, "null").WithArguments("int").WithLocation(2, 13), + // (2,13): warning CS8619: Nullability of reference types in value of type '' doesn't match target type 'int'. + // int[] x1 = [null]; + Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "null").WithArguments("", "int").WithLocation(2, 13), + // (8,11): error CS0037: Cannot convert null to 'int' because it is a non-nullable value type + // x3 = [null]; + Diagnostic(ErrorCode.ERR_ValueCantBeNull, "null").WithArguments("int").WithLocation(8, 11), + // (8,11): warning CS8619: Nullability of reference types in value of type '' doesn't match target type 'int'. + // x3 = [null]; + Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "null").WithArguments("", "int").WithLocation(8, 11) + ); + } + + [Theory] + [InlineData("System.Span")] + [InlineData("System.ReadOnlySpan")] + public void ElementNullability_SpanCollection(string spanType) + { + string src = $$""" + #nullable enable + {{spanType}} x1 = [null]; + {{spanType}} x2 = [null]; + + #nullable disable + {{spanType}} + #nullable enable + x3 = [null]; + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net70).VerifyEmitDiagnostics( + // (3,35): warning CS8625: Cannot convert null literal to non-nullable reference type. + // System.ReadOnlySpan x2 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null") + ); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_Generic() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection x1 = [null]; + MyCollection x2 = [null]; + + #nullable disable + MyCollection + #nullable enable + x3 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (8,28): warning CS8625: Cannot convert null literal to non-nullable reference type. + // MyCollection x2 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(8, 28) + ); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_Nullable() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection? x1 = [null]; + MyCollection? x2 = [null]; + + #nullable disable + MyCollection? + #nullable enable + x3 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (8,29): warning CS8625: Cannot convert null literal to non-nullable reference type. + // MyCollection? x2 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(8, 29) + ); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_NonNullableString() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection x1 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) // non-nullable string + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (7,20): warning CS8625: Cannot convert null literal to non-nullable reference type. + // MyCollection x1 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(7, 20) + ); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_NullableString() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection x1 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) // nullable string + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics(); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_ConstrainedCreate() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection x1 = [null]; + MyCollection x2 = [null]; + + #nullable disable + MyCollection + #nullable enable + x3 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) where T : notnull + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + // Note: we warn on x3 because the of type substitution rules for oblivious type argument + // into Create method with MyCollection return type + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (7,28): warning CS8714: The type 'string?' cannot be used as type parameter 'T' in the generic type or method 'MyCollectionBuilder.Create(ReadOnlySpan)'. Nullability of type argument 'string?' doesn't match 'notnull' constraint. + // MyCollection x1 = [null]; + Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterNotNullConstraint, "[null]").WithArguments("MyCollectionBuilder.Create(System.ReadOnlySpan)", "T", "string?").WithLocation(7, 28), + // (8,28): warning CS8625: Cannot convert null literal to non-nullable reference type. + // MyCollection x2 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(8, 28), + // (13,11): warning CS8625: Cannot convert null literal to non-nullable reference type. + // x3 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(13, 11) + ); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_NotNullCreate() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection x1 = [null]; + MyCollection x2 = [null]; + + #nullable disable + MyCollection + #nullable enable + x3 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) where T : notnull + { + return new MyCollection(new List(items.ToArray()!)); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (7,28): warning CS8714: The type 'string?' cannot be used as type parameter 'T' in the generic type or method 'MyCollectionBuilder.Create(ReadOnlySpan)'. Nullability of type argument 'string?' doesn't match 'notnull' constraint. + // MyCollection x1 = [null]; + Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterNotNullConstraint, "[null]").WithArguments("MyCollectionBuilder.Create(System.ReadOnlySpan)", "T", "string?").WithLocation(7, 28) + ); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_InInference() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + M("hi", [null]); // 1 + M((string?)null, [null]); + + void M(T t, MyCollection mc) { } + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + var comp = CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics(); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + var invocations = tree.GetRoot().DescendantNodes().OfType().ToArray(); + + // https://github.com/dotnet/roslyn/issues/68786: Incorrect symbols (nullability wasn't updated) + Assert.Equal("""M("hi", [null])""", invocations[0].ToString()); + Assert.Equal("void M(System.String t, MyCollection mc)", + model.GetSymbolInfo(invocations[0]).Symbol.ToTestDisplayString(includeNonNullable: true)); + + Assert.Equal("M((string?)null, [null])", invocations[1].ToString()); + Assert.Equal("void M(System.String? t, MyCollection mc)", + model.GetSymbolInfo(invocations[1]).Symbol.ToTestDisplayString(includeNonNullable: true)); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_InInference_WithExactBounds() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + string notNull = "hi"; + M(ref notNull, [null]); // 1 + + string? maybeNull = null; + M(ref maybeNull, [null]); + + void M(ref T t, MyCollection mc) { } + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + // https://github.com/dotnet/roslyn/issues/68786: missing diagnostics + var comp = CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics(); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + var invocations = tree.GetRoot().DescendantNodes().OfType().ToArray(); + + Assert.Equal("M(ref notNull, [null])", invocations[0].ToString()); + Assert.Equal("void M(ref System.String! t, MyCollection mc)", + model.GetSymbolInfo(invocations[0]).Symbol.ToTestDisplayString(includeNonNullable: true)); + + Assert.Equal("M(ref maybeNull, [null])", invocations[1].ToString()); + Assert.Equal("void M(ref System.String? t, MyCollection mc)", + model.GetSymbolInfo(invocations[1]).Symbol.ToTestDisplayString(includeNonNullable: true)); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_Var() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + string notNullString = "hi"; + var notNull = M(notNullString); + notNull = [null]; // 1 + + string? maybeNullString = null; + var maybeNull = M(maybeNullString); + maybeNull = [null]; + + #nullable disable + string obliviousString = null; + #nullable enable + + var oblivious = M(obliviousString); + oblivious = [null]; + + MyCollection M(T t) => throw null!; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + // https://github.com/dotnet/roslyn/issues/68786: Missing diagnostic for 1 + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics(); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_InInference_SingleParameter() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + M([(string?)null]); + M(["hi"]); + M(["hi", null]); + + void M(MyCollection mc) { } + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + var comp = CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics(); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + var invocations = tree.GetRoot().DescendantNodes().OfType().ToArray(); + + // https://github.com/dotnet/roslyn/issues/68786: Incorrect symbols (nullability wasn't updated) + Assert.Equal("M([(string?)null])", invocations[0].ToString()); + Assert.Equal("void M(MyCollection mc)", + model.GetSymbolInfo(invocations[0]).Symbol.ToTestDisplayString(includeNonNullable: true)); + + Assert.Equal("""M(["hi"])""", invocations[1].ToString()); + Assert.Equal("void M(MyCollection mc)", + model.GetSymbolInfo(invocations[1]).Symbol.ToTestDisplayString(includeNonNullable: true)); + + Assert.Equal("""M(["hi", null])""", invocations[2].ToString()); + Assert.Equal("void M(MyCollection mc)", + model.GetSymbolInfo(invocations[2]).Symbol.ToTestDisplayString(includeNonNullable: true)); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_NullReturningCreate() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection x1 = [null]; + MyCollection x2 = [null]; + + #nullable disable + MyCollection + #nullable enable + x3 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public class MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection? Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray()!)); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (7,28): warning CS8600: Converting null literal or possible null value to non-nullable type. + // MyCollection x1 = [null]; + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "[null]").WithLocation(7, 28), + // (8,27): warning CS8600: Converting null literal or possible null value to non-nullable type. + // MyCollection x2 = [null]; + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "[null]").WithLocation(8, 27), + // (8,28): warning CS8625: Cannot convert null literal to non-nullable reference type. + // MyCollection x2 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(8, 28) + ); + } + + [Fact] + public void ElementNullability_CollectionBuilderCollection_NullReturningCreate_WithAttribute() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection x1 = [null]; + MyCollection x2 = [null]; + + #nullable disable + MyCollection + #nullable enable + x3 = [null]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public class MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + [return: System.Diagnostics.CodeAnalysis.NotNull] + public static MyCollection? Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray()!)); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (8,28): warning CS8625: Cannot convert null literal to non-nullable reference type. + // MyCollection x2 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(8, 28) + ); + } + + [Fact] + public void ElementNullability_CollectionInitializerCollection() + { + string src = """ + using System.Collections; + + #nullable enable + + CNotNull x1 = [null]; // 1 + CNullable x2 = [null]; + COblivious x3 = [null]; + + class CNotNull : IEnumerable + { + public CNotNull() { } + IEnumerator IEnumerable.GetEnumerator() => throw null!; + public void Add(string s) { } + } + + class CNullable : IEnumerable + { + public CNullable() { } + IEnumerator IEnumerable.GetEnumerator() => throw null!; + public void Add(string? s) { } + } + + class COblivious : IEnumerable + { + public COblivious() { } + IEnumerator IEnumerable.GetEnumerator() => throw null!; + #nullable disable + public void Add(string s) { } + } + """; + + CreateCompilation(src).VerifyEmitDiagnostics( + // (5,16): warning CS8625: Cannot convert null literal to non-nullable reference type. + // CNotNull x1 = [null]; // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(5, 16) + ); + } + + [Fact] + public void ElementNullability_InterfaceTypeCollection() + { + string src = """ + using System.Collections.Generic; + + #nullable enable + IEnumerable x1 = [null]; + IEnumerable x2 = [null]; + + #nullable disable + IEnumerable + #nullable enable + x3 = [null]; + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics( + // (5,27): warning CS8625: Cannot convert null literal to non-nullable reference type. + // IEnumerable x2 = [null]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(5, 27) + ); + } + + [Fact] + public void ElementNullability_NoConversion() + { + string src = """ + #nullable enable + int[] x = [null]; + int y = null; + """; + + // Note: the nullability diagnostic is superfluous and results from the bound tree + // not containing element conversions in the error case. + CreateCompilation(src).VerifyEmitDiagnostics( + // (2,12): error CS0037: Cannot convert null to 'int' because it is a non-nullable value type + // int[] x = [null]; + Diagnostic(ErrorCode.ERR_ValueCantBeNull, "null").WithArguments("int").WithLocation(2, 12), + // (2,12): warning CS8619: Nullability of reference types in value of type '' doesn't match target type 'int'. + // int[] x = [null]; + Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "null").WithArguments("", "int").WithLocation(2, 12), + // (3,9): error CS0037: Cannot convert null to 'int' because it is a non-nullable value type + // int y = null; + Diagnostic(ErrorCode.ERR_ValueCantBeNull, "null").WithArguments("int").WithLocation(3, 9) + ); + } + + [Fact] + public void ElementNullability_CheckExpressions() + { + string src = """ + #nullable enable + object? o = null; + string[] x = [o.ToString()]; + """; + + CreateCompilation(src).VerifyEmitDiagnostics( + // (3,15): warning CS8602: Dereference of a possibly null reference. + // string[] x = [o.ToString()]; + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "o").WithLocation(3, 15) + ); + } + + [Fact] + public void SpreadNullability() + { + string src = """ + #nullable enable + string[]? x1 = null; + string[] x2 = new string[0]; + + string[] y1 = [.. x1]; + string[] y2 = [.. x2]; + """; + + // https://github.com/dotnet/roslyn/issues/68786: We should check the spreads + CreateCompilation(src).VerifyEmitDiagnostics(); + } + + [Fact] + public void SpreadNullability_CheckExpressions() + { + string src = """ + #nullable enable + string[] y1 = [.. M(null)]; + string[] y2 = [.. M(new object())]; + + string[] M(object o) => throw null!; + """; + + CreateCompilation(src).VerifyEmitDiagnostics( + // (2,21): warning CS8625: Cannot convert null literal to non-nullable reference type. + // string[] y1 = [.. M(null)]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(2, 21) + ); + } } } From a08a9a2b6274984a81a669177427c8aa34ed36ed Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 29 Sep 2023 22:30:46 -0700 Subject: [PATCH 02/11] Add nested test --- .../Semantics/CollectionExpressionTests.cs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs index 6b05b485a5c29..69d21be0271dc 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs @@ -17918,7 +17918,7 @@ public void ElementNullability_ArrayCollection() string?[] x2 = [null]; #nullable disable - string[] + string[] #nullable enable x3 = [null]; """; @@ -17930,6 +17930,27 @@ public void ElementNullability_ArrayCollection() ); } + [Fact] + public void ElementNullability_ArrayCollection_Nested() + { + string src = """ + #nullable enable + string[][] x1 = [[null]]; + string?[][] x2 = [[null]]; + + #nullable disable + string[][] + #nullable enable + x3 = [[null]]; + """; + + CreateCompilation(src).VerifyEmitDiagnostics( + // (2,19): warning CS8625: Cannot convert null literal to non-nullable reference type. + // string[][] x1 = [[null]]; + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(2, 19) + ); + } + [Fact] public void ElementNullability_ArrayCollection_NullableValueTypes() { From c3b3a74306bf5fceb261de28187db7e994223114 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Wed, 4 Oct 2023 15:55:56 -0700 Subject: [PATCH 03/11] Test updates --- .../Semantics/CollectionExpressionTests.cs | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs index 69d21be0271dc..758dc2218e4b7 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs @@ -18090,6 +18090,40 @@ public static MyCollection Create(ReadOnlySpan items) ); } + [Fact] + public void ElementNullability_CollectionBuilderCollection_NullableElements() + { + string src = """ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Runtime.CompilerServices; + + #nullable enable + MyCollection? x1 = [null]; + MyCollection? x2 = [0]; + + [CollectionBuilder(typeof(MyCollectionBuilder), nameof(MyCollectionBuilder.Create))] + public struct MyCollection : IEnumerable + { + private readonly List _list; + public MyCollection(List list) { _list = list; } + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class MyCollectionBuilder + { + public static MyCollection Create(ReadOnlySpan items) + { + return new MyCollection(new List(items.ToArray())); + } + } + """; + + CreateCompilation(src, targetFramework: TargetFramework.Net80).VerifyEmitDiagnostics(); + } + [Fact] public void ElementNullability_CollectionBuilderCollection_NonNullableString() { @@ -18264,7 +18298,7 @@ public void ElementNullability_CollectionBuilderCollection_InInference() using System.Runtime.CompilerServices; #nullable enable - M("hi", [null]); // 1 + M("hi", [null]); M((string?)null, [null]); void M(T t, MyCollection mc) { } @@ -18688,5 +18722,24 @@ public void SpreadNullability_CheckExpressions() Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(2, 21) ); } + + [Fact] + public void SpreadNullability_SplitExpression() + { + // https://github.com/dotnet/roslyn/issues/68786: We should check the spreads without asserting in DebugVerifier +#if RELEASE + string src = """ + #nullable enable + object x = ""; + object[] y1 = [..(x is null)]; + """; + + CreateCompilation(src).VerifyEmitDiagnostics( + // (3,18): error CS1579: foreach statement cannot operate on variables of type 'bool' because 'bool' does not contain a public instance or extension definition for 'GetEnumerator' + // object[] y1 = [..(x is null)]; + Diagnostic(ErrorCode.ERR_ForEachMissingMember, "(x is null)").WithArguments("bool", "GetEnumerator").WithLocation(3, 18) + ); +#endif + } } } From 9389b5abedbb9db6d86c8d3958976853075eb2a2 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 09:20:32 -0700 Subject: [PATCH 04/11] Use ConditionalFact --- .../CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs index 758dc2218e4b7..72e2e3c29b6d1 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs @@ -18723,11 +18723,10 @@ public void SpreadNullability_CheckExpressions() ); } - [Fact] + [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] public void SpreadNullability_SplitExpression() { // https://github.com/dotnet/roslyn/issues/68786: We should check the spreads without asserting in DebugVerifier -#if RELEASE string src = """ #nullable enable object x = ""; @@ -18739,7 +18738,6 @@ public void SpreadNullability_SplitExpression() // object[] y1 = [..(x is null)]; Diagnostic(ErrorCode.ERR_ForEachMissingMember, "(x is null)").WithArguments("bool", "GetEnumerator").WithLocation(3, 18) ); -#endif } } } From d320641f6c082bf85df8c23e1f3cb9077fccf6b0 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 12:14:44 -0700 Subject: [PATCH 05/11] Fix conflicts --- .../Portable/Binder/Binder_Conversions.cs | 2 +- .../Binder/Semantics/Conversions/Conversions.cs | 8 +++++--- .../Semantics/CollectionExpressionTests.cs | 17 +++++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs index f82623d134309..c395deae00871 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs @@ -793,6 +793,7 @@ private void GenerateImplicitConversionErrorForCollectionExpression( Debug.Assert(elementTypeWithAnnotations.HasType); } + var elementType = elementTypeWithAnnotations.Type; if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerableT && findSingleIEnumerableTImplementation(targetType, Compilation) is { } implementation) { @@ -801,7 +802,6 @@ private void GenerateImplicitConversionErrorForCollectionExpression( } bool reportedErrors = false; - var elementType = elementTypeWithAnnotations.Type; if (collectionTypeKind != CollectionExpressionTypeKind.None && elementType is { }) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs index 83110facc303c..c290eb7c0628c 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs @@ -155,7 +155,7 @@ protected override Conversion GetCollectionExpressionConversion( { var syntax = node.Syntax; var collectionTypeKind = GetCollectionExpressionTypeKind(Compilation, targetType, out TypeWithAnnotations elementTypeWithAnnotations); - + var elementType = elementTypeWithAnnotations.Type; switch (collectionTypeKind) { case CollectionExpressionTypeKind.None: @@ -163,7 +163,7 @@ protected override Conversion GetCollectionExpressionConversion( case CollectionExpressionTypeKind.CollectionBuilder: { - _binder.TryGetCollectionIterationType((Syntax.ExpressionSyntax)syntax, targetType, out TypeWithAnnotations elementTypeWithAnnotations); + _binder.TryGetCollectionIterationType((Syntax.ExpressionSyntax)syntax, targetType, out elementTypeWithAnnotations); elementType = elementTypeWithAnnotations.Type; if (elementType is null) { @@ -173,7 +173,9 @@ protected override Conversion GetCollectionExpressionConversion( break; } - Debug.Assert(collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerable || elementType is { }); + Debug.Assert(collectionTypeKind is CollectionExpressionTypeKind.ImplementsIEnumerable or CollectionExpressionTypeKind.ImplementsIEnumerableT + || elementType is { }); + var elements = node.Elements; if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerable) { diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs index 0b49d6ec2d69e..91dab6815ff1b 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs @@ -2818,7 +2818,7 @@ static void Main() expectedOutput: "(System.Object[]) [2, 3], (System.Object[]) [1, 2, 3], "); } - [Fact] + [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] public void TypeInference_Spread_07() { string source = """ @@ -2840,7 +2840,7 @@ static void Main() comp.VerifyEmitDiagnostics(); } - [Fact] + [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] public void TypeInference_Spread_08() { string source = """ @@ -8360,11 +8360,16 @@ static void Main() } """; var comp = CreateCompilation(source); - // https://github.com/dotnet/roslyn/issues/68786: // 2 and 3 should be reported as warnings. comp.VerifyEmitDiagnostics( // (8,9): warning CS8602: Dereference of a possibly null reference. // x[0].ToString(); // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x[0]").WithLocation(8, 9)); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x[0]").WithLocation(8, 9), + // (9,27): warning CS8625: Cannot convert null literal to non-nullable reference type. + // List y = [null]; // 2 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(9, 27), + // (11,17): warning CS8625: Cannot convert null literal to non-nullable reference type. + // y = [2, null]; // 3 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(11, 17)); } [Fact] @@ -21977,7 +21982,7 @@ public void ElementNullability_CheckExpressions() ); } - [Fact] + [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] public void SpreadNullability() { string src = """ @@ -21993,7 +21998,7 @@ public void SpreadNullability() CreateCompilation(src).VerifyEmitDiagnostics(); } - [Fact] + [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] public void SpreadNullability_CheckExpressions() { string src = """ From fad602549a49a3952ea41c08f45d9405220e4259 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 12:19:05 -0700 Subject: [PATCH 06/11] Tweaks --- src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs | 4 ++-- .../Portable/Binder/Semantics/Conversions/Conversions.cs | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs index c395deae00871..2abbd2ea9c56d 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs @@ -782,9 +782,10 @@ private void GenerateImplicitConversionErrorForCollectionExpression( BindingDiagnosticBag diagnostics) { var collectionTypeKind = ConversionsBase.GetCollectionExpressionTypeKind(Compilation, targetType, out TypeWithAnnotations elementTypeWithAnnotations); + var elementType = elementTypeWithAnnotations.Type; if (collectionTypeKind == CollectionExpressionTypeKind.CollectionBuilder) { - Debug.Assert(elementTypeWithAnnotations.Type is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases. + Debug.Assert(elementType is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases. if (!TryGetCollectionIterationType((ExpressionSyntax)node.Syntax, targetType, out elementTypeWithAnnotations)) { Error(diagnostics, ErrorCode.ERR_CollectionBuilderNoElementType, node.Syntax, targetType); @@ -793,7 +794,6 @@ private void GenerateImplicitConversionErrorForCollectionExpression( Debug.Assert(elementTypeWithAnnotations.HasType); } - var elementType = elementTypeWithAnnotations.Type; if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerableT && findSingleIEnumerableTImplementation(targetType, Compilation) is { } implementation) { diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs index c290eb7c0628c..ad6495bac900a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs @@ -173,9 +173,6 @@ protected override Conversion GetCollectionExpressionConversion( break; } - Debug.Assert(collectionTypeKind is CollectionExpressionTypeKind.ImplementsIEnumerable or CollectionExpressionTypeKind.ImplementsIEnumerableT - || elementType is { }); - var elements = node.Elements; if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerable) { From a9afc31134412f2769df6591613b1018dd37decf Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 12:38:57 -0700 Subject: [PATCH 07/11] Revert tweak --- src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs index 2abbd2ea9c56d..c395deae00871 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs @@ -782,10 +782,9 @@ private void GenerateImplicitConversionErrorForCollectionExpression( BindingDiagnosticBag diagnostics) { var collectionTypeKind = ConversionsBase.GetCollectionExpressionTypeKind(Compilation, targetType, out TypeWithAnnotations elementTypeWithAnnotations); - var elementType = elementTypeWithAnnotations.Type; if (collectionTypeKind == CollectionExpressionTypeKind.CollectionBuilder) { - Debug.Assert(elementType is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases. + Debug.Assert(elementTypeWithAnnotations.Type is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases. if (!TryGetCollectionIterationType((ExpressionSyntax)node.Syntax, targetType, out elementTypeWithAnnotations)) { Error(diagnostics, ErrorCode.ERR_CollectionBuilderNoElementType, node.Syntax, targetType); @@ -794,6 +793,7 @@ private void GenerateImplicitConversionErrorForCollectionExpression( Debug.Assert(elementTypeWithAnnotations.HasType); } + var elementType = elementTypeWithAnnotations.Type; if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerableT && findSingleIEnumerableTImplementation(targetType, Compilation) is { } implementation) { From 9521597a2c0c167f94c63100bb423d06280279ad Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 14:33:23 -0700 Subject: [PATCH 08/11] Resolve conflict --- .../Portable/Binder/Semantics/Conversions/ConversionsBase.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs index eb917d2f15fd0..5db56047e7634 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs @@ -1673,9 +1673,8 @@ internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSh elementType = default; return CollectionExpressionTypeKind.ImplementsIEnumerable; } - else if (destination.IsArrayInterface(out TypeWithAnnotations typeArg)) + else if (destination.IsArrayInterface(out elementType)) { - elementType = typeArg.Type; return CollectionExpressionTypeKind.ArrayInterface; } From 6dd8cab3e49abaa0df38cec760f5e6bdadb4b059 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 14:38:24 -0700 Subject: [PATCH 09/11] Remove unused local function --- .../Semantics/Conversions/ConversionsBase.cs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs index 5db56047e7634..b1c74ac04d692 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs @@ -1699,26 +1699,6 @@ static bool implementsSpecialInterface(CSharpCompilation compilation, TypeSymbol var specialType = compilation.GetSpecialType(specialInterface); return allInterfaces.Any(static (a, b) => ReferenceEquals(a.OriginalDefinition, b), specialType); } - - static bool isListInterface(CSharpCompilation compilation, TypeSymbol targetType, [NotNullWhen(true)] out TypeSymbol? elementType) - { - if (targetType is NamedTypeSymbol - { - OriginalDefinition.SpecialType: - SpecialType.System_Collections_Generic_IEnumerable_T or - SpecialType.System_Collections_Generic_IReadOnlyCollection_T or - SpecialType.System_Collections_Generic_IReadOnlyList_T or - SpecialType.System_Collections_Generic_ICollection_T or - SpecialType.System_Collections_Generic_IList_T, - TypeArgumentsWithAnnotationsNoUseSiteDiagnostics: [var typeArg] - }) - { - elementType = typeArg.Type; - return true; - } - elementType = null; - return false; - } } #nullable disable From 8fac2d0ab288241ec38ee3f08107abbc176b9f3d Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 15:11:12 -0700 Subject: [PATCH 10/11] Don't conditionally skip spread tests --- .../CSharp/Portable/FlowAnalysis/NullableWalker.cs | 3 +-- .../Test/Emit2/Semantics/CollectionExpressionTests.cs | 10 +++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index fcd7ec823f319..d2d34198416a2 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -246,8 +246,7 @@ private PooledDictionary s_skippedExpressions = ImmutableArray.Create(BoundKind.ArrayInitialization, BoundKind.ObjectInitializerExpression, BoundKind.CollectionInitializerExpression, - BoundKind.DynamicCollectionElementInitializer, - BoundKind.CollectionExpressionSpreadElement); // https://github.com/dotnet/roslyn/issues/68786: We should handle spreads + BoundKind.DynamicCollectionElementInitializer); #endif diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs index 34da2855f23f5..e4507e6d1b16f 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs @@ -3008,7 +3008,7 @@ static void Main() expectedOutput: "(System.Object[]) [2, 3], (System.Object[]) [1, 2, 3], "); } - [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] + [Fact] public void TypeInference_Spread_07() { string source = """ @@ -3030,7 +3030,7 @@ static void Main() comp.VerifyEmitDiagnostics(); } - [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] + [Fact] public void TypeInference_Spread_08() { string source = """ @@ -22517,7 +22517,7 @@ public void ElementNullability_CheckExpressions() ); } - [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] + [Fact] public void SpreadNullability() { string src = """ @@ -22533,7 +22533,7 @@ public void SpreadNullability() CreateCompilation(src).VerifyEmitDiagnostics(); } - [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] + [Fact] public void SpreadNullability_CheckExpressions() { string src = """ @@ -22551,7 +22551,7 @@ public void SpreadNullability_CheckExpressions() ); } - [ConditionalFact(typeof(IsRelease), Reason = "https://github.com/dotnet/roslyn/issues/68786")] + [Fact] public void SpreadNullability_SplitExpression() { // https://github.com/dotnet/roslyn/issues/68786: We should check the spreads without asserting in DebugVerifier From 33320c4edbc51a75504c54c0ec753acb47c06d3e Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Oct 2023 15:12:40 -0700 Subject: [PATCH 11/11] space --- src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index d2d34198416a2..f4f212600e9b6 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -247,7 +247,6 @@ private PooledDictionary