Merge remote-tracking branch 'dotnet/main' into diagnostics

docs/compilers/CSharp/Compiler Breaking Changes - DotNet 6.md

@@ -1,14 +1,15 @@
 ## This document lists known breaking changes in Roslyn in C# 10.0 which will be introduced with .NET 6.
 
-1. Beginning with C# 10.0, null suppression operator is no longer allowed in patterns.
+1. <a name="1"></a>Beginning with C# 10.0, null suppression operator is no longer allowed in patterns.
+
     ```csharp
     void M(object o)
     {
         if (o is null!) {} // error
     }
     ```
 
-2. In C# 10, lambda expressions and method groups with inferred type are implicitly convertible to `System.MulticastDelegate`, and bases classes and interfaces of `System.MulticastDelegate` including `object`,
+2. <a name="2"></a>In C# 10, lambda expressions and method groups with inferred type are implicitly convertible to `System.MulticastDelegate`, and bases classes and interfaces of `System.MulticastDelegate` including `object`,
 and lambda expressions and method groups are implicitly convertible to `System.Linq.Expressions.Expression` and `System.Linq.Expressions.LambdaExpression`.
 These are _function_type_conversions_.
 
@@ -92,7 +93,7 @@ These are _function_type_conversions_.
     }
     ```
 
-3. In C#10, a lambda expression with inferred type may contribute an argument type that affects overload resolution.
+3. <a name="3"></a>In C#10, a lambda expression with inferred type may contribute an argument type that affects overload resolution.
 
     ```csharp
     using System;
@@ -109,18 +110,38 @@ These are _function_type_conversions_.
     }
     ```
 
-4. In Visual Studio 17.1, `struct` type declarations with field initializers must include an explicitly declared constructor. Additionally, all fields must be definitely assigned in `struct` instance constructors that do not have a `: this()` initializer so any previously unassigned fields must be assigned from the added constructor or from field initializers.
+4. <a name="4"></a><a name="roslyn-58339"></a>In Visual Studio 17.0 servicing, an error is reported in a `record struct` with a primary constructor if an explicit constructor has a `this()` initializer that invokes the implicit parameterless constructor. See [roslyn#58339](https://github.com/dotnet/roslyn/pull/58339).
 
-    For instance, the following results in an error in 17.1:
+    For instance, the following results in an error:
     ```csharp
-    struct S
+    record struct R(int X, int Y)
+    {
+        // error CS8982: A constructor declared in a 'record struct' with parameter list must have a 'this'
+        // initializer that calls the primary constructor or an explicitly declared constructor.
+        public R(int x) : this() { X = x; Y = 0; }
+    }
+    ```
+
+    The error could be resolved by invoking the primary constructor (as below) from the `this()` initializer, or by declaring a parameterless constructor that invokes the primary constructor.
+    ```csharp
+    record struct R(int X, int Y)
     {
-        int X = 1; // error: struct with field initializers must include an explicitly declared constructor
+        public R(int x) : this(x, 0) { } // ok
+    }
+    ```
+
+5. <a name="5"></a><a name="roslyn-57925"></a>In Visual Studio 17.0 servicing, if a `struct` type declaration with no constructors includes initializers for some but not all fields, the compiler will report an error that all fields must be assigned. See [roslyn#57925](https://github.com/dotnet/roslyn/pull/57925).
+
+    For instance, the following results in an error:
+    ```csharp
+    struct S // error CS0171: Field 'S.Y' must be fully assigned before control is returned to the caller
+    {
+        int X = 1;
         int Y;
     }
     ```
 
-    The error could be resolved by adding a constructor and assigning the other field.
+    For compatibility with 17.1 (see [#6](#6)), the error should be resolved by adding a constructor and assigning the other field.
     ```csharp
     struct S
     {
@@ -129,3 +150,4 @@ These are _function_type_conversions_.
         public S() { Y = 0; } // ok
     }
     ```
+

docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md

@@ -1,6 +1,6 @@
 ## This document lists known breaking changes in Roslyn after .NET 6 all the way to .NET 7.
 
-1. In Visual Studio 17.1, the contextual keyword `var` cannot be used as an explicit lambda return type.
+1. <a name="1"></a>In Visual Studio 17.1, the contextual keyword `var` cannot be used as an explicit lambda return type.
 
     ```csharp
     using System;
@@ -13,7 +13,7 @@
     class var { }
     ```
 
-2. In Visual Studio 17.1, indexers that take an interpolated string handler and require the receiver as an input for the constructor cannot be used in an object initializer.
+2. <a name="2"></a>In Visual Studio 17.1, indexers that take an interpolated string handler and require the receiver as an input for the constructor cannot be used in an object initializer.
 
     ```cs
     using System.Runtime.CompilerServices;
@@ -35,7 +35,7 @@
     }
     ```
 
-3. In Visual Studio 17.1, `ref`/`ref readonly`/`in`/`out` are not allowed to be used on return/parameters of a method attributed with `UnmanagedCallersOnly`.  
+3. <a name="3"></a>In Visual Studio 17.1, `ref`/`ref readonly`/`in`/`out` are not allowed to be used on return/parameters of a method attributed with `UnmanagedCallersOnly`.  
 https://github.com/dotnet/roslyn/issues/57025
 
     ```cs
@@ -56,7 +56,7 @@ https://github.com/dotnet/roslyn/issues/57025
     static void M5(out int o) => throw null; // error CS8977: Cannot use 'ref', 'in', or 'out' in a method attributed with 'UnmanagedCallersOnly'.
     ```
 
-4. Beginning with C# 11.0, `Length` and `Count` properties on countable and indexable types
+4. <a name="4"></a>Beginning with C# 11.0, `Length` and `Count` properties on countable and indexable types
 are assumed to be non-negative for purpose of subsumption and exhaustiveness analysis of patterns and switches.
 Those types can be used with implicit Index indexer and list patterns.
 
@@ -67,7 +67,7 @@ Those types can be used with implicit Index indexer and list patterns.
     }
     ```
 
-5. Starting with Visual Studio 17.1, format specifiers in interpolated strings can not contain curly braces (either `{` or `}`). In previous versions `{{` was interpreted as an escaped `{` and `}}` was interpreted as an escaped `}` char in the format specifier. Now the first `}` char in a format specifier ends the interpolation, and any `{` char is an error.
+5.  <a name="5"></a>Starting with Visual Studio 17.1, format specifiers in interpolated strings can not contain curly braces (either `{` or `}`). In previous versions `{{` was interpreted as an escaped `{` and `}}` was interpreted as an escaped `}` char in the format specifier. Now the first `}` char in a format specifier ends the interpolation, and any `{` char is an error.
 https://github.com/dotnet/roslyn/issues/57750
 
     ```csharp
@@ -77,3 +77,24 @@ https://github.com/dotnet/roslyn/issues/57750
 
     //prints now: "{C}" - not "{X}}"
     ```
+
+6. <a name="6"></a><a name="roslyn-58581"></a>In Visual Studio 17.1, `struct` type declarations with field initializers must include an explicitly declared constructor. Additionally, all fields must be definitely assigned in `struct` instance constructors that do not have a `: this()` initializer so any previously unassigned fields must be assigned from the added constructor or from field initializers. See [csharplang#5552](https://github.com/dotnet/csharplang/issues/5552), [roslyn#58581](https://github.com/dotnet/roslyn/pull/58581).
+
+    For instance, the following results in an error in 17.1:
+    ```csharp
+    struct S
+    {
+        int X = 1; // error CS8983: A 'struct' with field initializers must include an explicitly declared constructor.
+        int Y;
+    }
+    ```
+
+    The error could be resolved by adding a constructor and assigning the other field.
+    ```csharp
+    struct S
+    {
+        int X = 1;
+        int Y;
+        public S() { Y = 0; } // ok
+    }
+    ```

src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs

@@ -289,7 +289,12 @@ private BoundListPattern BindListPattern(
             BoundListPatternReceiverPlaceholder? receiverPlaceholder;
             BoundListPatternIndexPlaceholder? argumentPlaceholder;
 
-            if (inputType.IsErrorType())
+            if (inputType.IsDynamic())
+            {
+                Error(diagnostics, ErrorCode.ERR_UnsupportedTypeForListPattern, node, inputType);
+            }
+
+            if (inputType.IsErrorType() || inputType.IsDynamic())
             {
                 hasErrors = true;
                 elementType = inputType;
@@ -337,13 +342,9 @@ private bool IsCountableAndIndexable(SyntaxNode node, TypeSymbol inputType, out
         private bool BindLengthAndIndexerForListPattern(SyntaxNode node, TypeSymbol inputType, uint inputValEscape, BindingDiagnosticBag diagnostics,
             out BoundExpression indexerAccess, out BoundExpression lengthAccess, out BoundListPatternReceiverPlaceholder? receiverPlaceholder, out BoundListPatternIndexPlaceholder argumentPlaceholder)
         {
-            bool hasErrors = false;
-            if (inputType.IsDynamic())
-            {
-                hasErrors |= true;
-                Error(diagnostics, ErrorCode.ERR_UnsupportedTypeForListPattern, node, inputType);
-            }
+            Debug.Assert(!inputType.IsDynamic());
 
+            bool hasErrors = false;
             receiverPlaceholder = new BoundListPatternReceiverPlaceholder(node, GetValEscape(inputType, inputValEscape), inputType) { WasCompilerGenerated = true };
             if (inputType.IsSZArray())
             {
@@ -359,7 +360,11 @@ private bool BindLengthAndIndexerForListPattern(SyntaxNode node, TypeSymbol inpu
             }
             else
             {
-                hasErrors |= !TryBindLengthOrCount(node, receiverPlaceholder, out lengthAccess, diagnostics);
+                if (!TryBindLengthOrCount(node, receiverPlaceholder, out lengthAccess, diagnostics))
+                {
+                    hasErrors = true;
+                    Error(diagnostics, ErrorCode.ERR_ListPatternRequiresLength, node, inputType);
+                }
             }
 
             var analyzedArguments = AnalyzedArguments.GetInstance();

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -6760,6 +6760,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_UnsupportedTypeForListPattern" xml:space="preserve">
     <value>List patterns may not be used for a value of type '{0}'.</value>
   </data>
+  <data name="ERR_ListPatternRequiresLength" xml:space="preserve">
+    <value>List patterns may not be used for a value of type '{0}'. No suitable 'Length' or 'Count' property was found.</value>
+  </data>
   <data name="ERR_UnsupportedTypeForSlicePattern" xml:space="preserve">
     <value>Slice patterns may not be used for a value of type '{0}'.</value>
   </data>

src/Compilers/CSharp/Portable/Declarations/SingleTypeDeclaration.cs

@@ -26,7 +26,7 @@ internal sealed class SingleTypeDeclaration : SingleNamespaceOrTypeDeclaration
         /// through a using alias in the file. For example
         /// <c>using X = System.Runtime.CompilerServices.TypeForwardedToAttribute</c> or
         /// <c>[TypeForwardedToAttribute]</c>.  Can be used to avoid having to go back to source
-        /// to retrieve attributes whtn there is no chance they would bind to attribute of interest.
+        /// to retrieve attributes when there is no chance they would bind to attribute of interest.
         /// </summary>
         public QuickAttributes QuickAttributes { get; }
 

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -2020,6 +2020,7 @@ internal enum ErrorCode
         ERR_StructHasInitializersAndNoDeclaredConstructor = 8983,
         ERR_EncUpdateFailedDelegateTypeChanged = 8984,
 
+        ERR_ListPatternRequiresLength = 8985,
         ERR_DiscardCannotBeNullChecked = 8990,
         ERR_MustNullCheckInImplementation = 8991,
         ERR_NonNullableValueTypeIsNullChecked = 8992,

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -2067,7 +2067,7 @@ internal static bool AreParameterAnnotationsCompatible(
                     overriddenType,
                     overriddenAnnotations,
                     // We don't consider '!!' when deciding whether 'overridden' is compatible with 'override'
-                    isNullChecked: false);
+                    applyParameterNullCheck: false);
                 if (isBadAssignment(valueState, overridingType, overridingAnnotations))
                 {
                     return false;
@@ -2527,9 +2527,9 @@ private void EnterParameter(ParameterSymbol parameter, TypeWithAnnotations param
             return null;
         }
 
-        internal static TypeWithState GetParameterState(TypeWithAnnotations parameterType, FlowAnalysisAnnotations parameterAnnotations, bool isNullChecked)
+        internal static TypeWithState GetParameterState(TypeWithAnnotations parameterType, FlowAnalysisAnnotations parameterAnnotations, bool applyParameterNullCheck)
         {
-            if (isNullChecked)
+            if (applyParameterNullCheck)
             {
                 return TypeWithState.Create(parameterType.Type, NullableFlowState.NotNull);
             }

src/Compilers/CSharp/Portable/Parser/Lexer_StringLiteral.cs

@@ -708,7 +708,7 @@ private bool IsAtEndOfMultiLineRawLiteral(InterpolatedStringKind kind, int start
 
             /// <summary>
             /// Returns <see langword="true"/> if the quote was an end delimiter and lexing of the contents of the
-            /// interpolated string literal should stop.  If it was an end delimeter it will not be consumed.  If it is
+            /// interpolated string literal should stop.  If it was an end delimiter it will not be consumed.  If it is
             /// content and should not terminate the string then it will be consumed by this method.
             /// </summary>
             private bool IsEndDelimiterOtherwiseConsume(InterpolatedStringKind kind, int startingQuoteCount)

src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs

@@ -809,15 +809,43 @@ internal static void ReportParameterNullCheckingErrors(DiagnosticBag diagnostics
             {
                 diagnostics.Add(ErrorCode.ERR_DiscardCannotBeNullChecked, location);
             }
-            if (parameter.TypeWithAnnotations.NullableAnnotation.IsAnnotated()
-                || parameter.Type.IsNullableTypeOrTypeParameter())
+
+            var annotations = parameter.FlowAnalysisAnnotations;
+            if ((annotations & FlowAnalysisAnnotations.NotNull) == 0
+                && NullableWalker.GetParameterState(parameter.TypeWithAnnotations, annotations, applyParameterNullCheck: false).State.MayBeNull()
+                && !isTypeParameterWithPossiblyNonNullableType(parameter.TypeWithAnnotations, annotations))
             {
                 diagnostics.Add(ErrorCode.WRN_NullCheckingOnNullableType, location, parameter);
             }
-            else if (parameter.Type.IsValueType && !parameter.Type.IsPointerOrFunctionPointer())
+
+            if (parameter.Type.IsNonNullableValueType() && !parameter.Type.IsPointerOrFunctionPointer())
             {
                 diagnostics.Add(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, location, parameter);
             }
+
+            // For type parameters, we only want to give the warning if no type argument would result in a non-nullable type.
+            static bool isTypeParameterWithPossiblyNonNullableType(TypeWithAnnotations typeWithAnnotations, FlowAnalysisAnnotations annotations)
+            {
+                if (!typeWithAnnotations.Type.IsTypeParameter())
+                {
+                    return false;
+                }
+
+                // We avoid checking the nullable annotations, etc. of constraints due to implementation complexity,
+                // and consider it acceptable to miss "!! on nullable type" warnings in scenarios like `void M<T, U>(U u!!) where U : T?`.
+                if (typeWithAnnotations.NullableAnnotation.IsAnnotated())
+                {
+                    return false;
+                }
+
+                // `void M<T>([AllowNull] T t!!)`
+                if ((annotations & FlowAnalysisAnnotations.AllowNull) != 0)
+                {
+                    return false;
+                }
+
+                return true;
+            }
         }
 
         internal static ImmutableArray<CustomModifier> ConditionallyCreateInModifiers(RefKind refKind, bool addRefReadOnlyModifier, Binder binder, BindingDiagnosticBag diagnostics, SyntaxNode syntax)