Merge master to features/module-initializers

docs/compilers/API Notes/6-11-20.md

@@ -0,0 +1,61 @@
+### `BaseTypeSyntax` Design
+
+In master, we currently have added an optional argument list to `SimpleBaseTypeSyntax` for record primary constructors. However, there is an argument to be made that we should add a new subtype of `BaseTypeSyntax`, something like `PrimaryConstructorTypeSyntax`, to represent these new base type clauses. This was an explicit extension point added in the original design for this very purpose. For either of these designs, GetSymbolInfo will behave consistently: if the `BaseTypeSyntax` has an argument list (either because it's a separate type or because it's a `PrimaryConstructorTypeSyntax`), it will return the constructor symbol or candidates as appropriate. If there are no arguments, then it will return nothing.
+
+Pros for keeping the optional argument list on `SimpleBaseTypeSyntax`:
+* The primary constructor arguments feel like an extension to existing nodes, not a totally new node type
+* Existing analyzers/fixers might pick things up right without much modification
+
+Pros for extending `BaseTypeSyntax`:
+* This extension point was put in explicitly for this
+* Existing code might be less likely to erroneously use it
+* There was some feeling that it would serve as a more clear delineation for future additions to this syntax (if such C# features ever come to pass)
+
+#### Conclusion
+
+We lean towards extending `BaseTypeSyntax` in the next preview.
+
+### `CompilationOutputFilePaths`
+
+The plural when this struct only has one path in it currently felt wrong. Some options for renaming:
+
+* `CompilationOutputFileInfo`
+* `CompilationOutputInfo`
+* `CompilationOutputFiles`
+
+We leave it up to whoever does the rename to choose among these.
+
+### `SolutionInfo.Create`
+
+Add `EditorBrowsable(Never)` to the backcompat overload.
+
+### `SymbolKind.FunctionPointer`
+
+Suffix with `Type` for consistency with the other kinds that are also `TypeKind`s.
+
+### `SyntaxFactory.ParseType`
+
+The new overload should use `ParseOptions` instead of the specific C#/VB type in both factories.
+Most of the Parse overloads that take a `ParseOptions` here use the non-specific one, and it can be very convenient for IDE APIs here.
+While it is marginally less type-safe, the ergonomic and consistency benefits are a deciding factor.
+
+### `WithExpressionSyntax.Receiver`
+
+This should be renamed to `Expression` for consistency with other `SyntaxNode`s.
+
+### `SymbolFinder.Find(Derived(Classes|Interfaces)|Interfaces)Async`
+
+We should reconsider adding multiple overloads, and instead remove the default parameters from the existing overload and introduce a new overload with `transitive` set to `true` to match current behavior.
+This is our standard modus operandi for these scenarios.
+
+### `INegatedPatternOperation.NegatedPattern`
+
+Rename to just `Pattern`.
+
+### `IWithOperation.Value`
+
+Rename to `Operand`.
+
+### `Renamer.RenameDocumentAsync`
+
+Make this and the related structs internal unless we have a partner team ready to consume the API in 16.7. We can make the API public again when someone is ready to consume and validate the API shape.

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

@@ -666,7 +666,7 @@ private ImmutableArray<TypedConstant> GetRewrittenAttributeConstructorArguments(
                     }
                     else if (reorderedArgument.Kind == TypedConstantKind.Array &&
                         parameter.Type.TypeKind == TypeKind.Array &&
-                        !((TypeSymbol)reorderedArgument.TypeInternal).Equals(parameter.Type, TypeCompareKind.AllIgnoreOptions))
+                        !((TypeSymbol)reorderedArgument.TypeInternal!).Equals(parameter.Type, TypeCompareKind.AllIgnoreOptions))
                     {
                         // NOTE: As in dev11, we don't allow array covariance conversions (presumably, we don't have a way to
                         // represent the conversion in metadata).
@@ -922,6 +922,7 @@ private static bool TryGetNormalParamValue(ParameterSymbol parameter, ImmutableA
             }
 
             HashSet<DiagnosticInfo>? useSiteDiagnostics = null; // ignoring, since already bound argument and parameter
+            Debug.Assert(argument.TypeInternal is object);
             Conversion conversion = conversions.ClassifyBuiltInConversion((TypeSymbol)argument.TypeInternal, parameter.Type, ref useSiteDiagnostics);
 
             // NOTE: Won't always succeed, even though we've performed overload resolution.

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

@@ -3777,9 +3777,8 @@ private static bool IsNegativeConstantForArraySize(BoundExpression expression)
         /// </summary>
         /// <param name="initializerArgumentListOpt">
         /// Null for implicit, 
-        /// BaseConstructorInitializerSyntax.ArgumentList, or 
-        /// ThisConstructorInitializerSyntax.ArgumentList, or 
-        /// SimpleBaseTypeSyntax.ArgumentList for explicit.</param>
+        /// <see cref="ConstructorInitializerSyntax.ArgumentList"/>, or 
+        /// <see cref="PrimaryConstructorBaseTypeSyntax.ArgumentList"/> for explicit.</param>
         /// <param name="constructor">Constructor containing the initializer.</param>
         /// <param name="diagnostics">Accumulates errors (e.g. unable to find constructor to invoke).</param>
         /// <returns>A bound expression for the constructor initializer call.</returns>
@@ -3921,9 +3920,8 @@ private BoundExpression BindConstructorInitializerCore(
                         errorLocation = initializerSyntax.ThisOrBaseKeyword.GetLocation();
                         break;
 
-                    case SimpleBaseTypeSyntax baseWithArguments:
-                        Debug.Assert(baseWithArguments.Parent?.Parent is RecordDeclarationSyntax recordDecl && recordDecl.BaseList.Types.FirstOrDefault() == baseWithArguments);
-                        nonNullSyntax = initializerArgumentListOpt;
+                    case PrimaryConstructorBaseTypeSyntax baseWithArguments:
+                        nonNullSyntax = baseWithArguments;
                         errorLocation = initializerArgumentListOpt.GetLocation();
                         break;
 
@@ -6241,9 +6239,9 @@ private BoundExpression BindInstanceMemberAccess(
                         lookupResult,
                         flags);
 
-                    if (!boundMethodGroup.HasErrors && boundMethodGroup.ResultKind == LookupResultKind.Empty && typeArgumentsSyntax.Any(SyntaxKind.OmittedTypeArgument))
+                    if (!boundMethodGroup.HasErrors && typeArgumentsSyntax.Any(SyntaxKind.OmittedTypeArgument))
                     {
-                        Error(diagnostics, ErrorCode.ERR_BadArity, node, rightName, MessageID.IDS_MethodGroup.Localize(), typeArgumentsSyntax.Count);
+                        Error(diagnostics, ErrorCode.ERR_OmittedTypeArgument, node);
                     }
 
                     return boundMethodGroup;

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

@@ -180,7 +180,7 @@ internal BoundPattern BindConstantPatternWithFallbackToTypePattern(
             bool hasErrors,
             DiagnosticBag diagnostics)
         {
-            ExpressionSyntax innerExpression = expression.SkipParens();
+            ExpressionSyntax innerExpression = SkipParensAndNullSuppressions(expression);
             if (innerExpression.Kind() == SyntaxKind.DefaultLiteralExpression)
             {
                 diagnostics.Add(ErrorCode.ERR_DefaultPattern, innerExpression.Location);
@@ -204,6 +204,24 @@ internal BoundPattern BindConstantPatternWithFallbackToTypePattern(
             }
         }
 
+        private ExpressionSyntax SkipParensAndNullSuppressions(ExpressionSyntax e)
+        {
+            while (true)
+            {
+                switch (e)
+                {
+                    case ParenthesizedExpressionSyntax p:
+                        e = p.Expression;
+                        break;
+                    case PostfixUnaryExpressionSyntax { RawKind: (int)SyntaxKind.SuppressNullableWarningExpression } p:
+                        e = p.Operand;
+                        break;
+                    default:
+                        return e;
+                }
+            }
+        }
+
         /// <summary>
         /// Binds the expression for a pattern.  Sets <paramref name="wasExpression"/> if it was a type rather than an expression,
         /// and in that case it returns a <see cref="BoundTypeExpression"/>.
@@ -1239,6 +1257,12 @@ private BoundPattern BindRelationalPattern(
             DiagnosticBag diagnostics)
         {
             BoundExpression value = BindExpressionForPattern(inputType, node.Expression, ref hasErrors, diagnostics, out var constantValueOpt, out _);
+            ExpressionSyntax innerExpression = SkipParensAndNullSuppressions(node.Expression);
+            if (innerExpression.Kind() == SyntaxKind.DefaultLiteralExpression)
+            {
+                diagnostics.Add(ErrorCode.ERR_DefaultPattern, innerExpression.Location);
+                hasErrors = true;
+            }
             RoslynDebug.Assert(value.Type is { });
             BinaryOperatorKind operation = tokenKindToBinaryOperatorKind(node.OperatorToken.Kind());
             if (operation == BinaryOperatorKind.Equal)

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

@@ -3322,7 +3322,7 @@ private BoundNode BindRecordConstructorBody(RecordDeclarationSyntax recordDecl,
             Debug.Assert(bodyBinder != null);
 
             BoundExpressionStatement initializer = null;
-            if (recordDecl.BaseWithArguments is SimpleBaseTypeSyntax baseWithArguments)
+            if (recordDecl.PrimaryConstructorBaseType is PrimaryConstructorBaseTypeSyntax baseWithArguments)
             {
                 initializer = bodyBinder.BindConstructorInitializer(baseWithArguments, diagnostics);
             }
@@ -3334,10 +3334,8 @@ private BoundNode BindRecordConstructorBody(RecordDeclarationSyntax recordDecl,
                                                   expressionBody: null);
         }
 
-        internal BoundExpressionStatement BindConstructorInitializer(SimpleBaseTypeSyntax initializer, DiagnosticBag diagnostics)
+        internal virtual BoundExpressionStatement BindConstructorInitializer(PrimaryConstructorBaseTypeSyntax initializer, DiagnosticBag diagnostics)
         {
-            Debug.Assert(initializer.Parent?.Parent is RecordDeclarationSyntax recordDecl && recordDecl.ParameterList is object && recordDecl.BaseWithArguments == initializer);
-
             BoundExpression initializerInvocation = GetBinder(initializer).BindConstructorInitializer(initializer.ArgumentList, (MethodSymbol)this.ContainingMember(), diagnostics);
             var constructorInitializer = new BoundExpressionStatement(initializer, initializerInvocation);
             Debug.Assert(initializerInvocation.HasAnyErrors || constructorInitializer.IsConstructorInitializer(), "Please keep this bound node in sync with BoundNodeExtensions.IsConstructorInitializer.");

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

@@ -49,7 +49,7 @@ protected void FindExpressionVariables(
                 case SyntaxKind.GotoCaseStatement:
                     break;
                 case SyntaxKind.ArgumentList:
-                    Debug.Assert(node.Parent is ConstructorInitializerSyntax);
+                    Debug.Assert(node.Parent is ConstructorInitializerSyntax || node.Parent is PrimaryConstructorBaseTypeSyntax);
                     break;
                 case SyntaxKind.RecordDeclaration:
                     Debug.Assert(((RecordDeclarationSyntax)node).ParameterList is object);
@@ -397,7 +397,7 @@ public override void VisitRecordDeclaration(RecordDeclarationSyntax node)
         {
             Debug.Assert(node.ParameterList is object);
 
-            if (node.BaseWithArguments is SimpleBaseTypeSyntax baseWithArguments)
+            if (node.PrimaryConstructorBaseType is PrimaryConstructorBaseTypeSyntax baseWithArguments)
             {
                 VisitNodeToBind(baseWithArguments);
             }

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

@@ -162,13 +162,14 @@ public override void VisitRecordDeclaration(RecordDeclarationSyntax node)
 
             Binder enclosing = new ExpressionVariableBinder(node, _enclosing);
             AddToMap(node, enclosing);
+            Visit(node.PrimaryConstructorBaseType, enclosing);
+        }
 
-            if (node.BaseWithArguments is SimpleBaseTypeSyntax baseWithArguments)
-            {
-                enclosing = enclosing.WithAdditionalFlags(BinderFlags.ConstructorInitializer);
-                AddToMap(baseWithArguments, enclosing);
-                Visit(baseWithArguments.ArgumentList, enclosing);
-            }
+        public override void VisitPrimaryConstructorBaseType(PrimaryConstructorBaseTypeSyntax node)
+        {
+            Binder enclosing = _enclosing.WithAdditionalFlags(BinderFlags.ConstructorInitializer);
+            AddToMap(node, enclosing);
+            VisitConstructorInitializerArgumentList(node, node.ArgumentList, enclosing);
         }
 
         public override void VisitDestructorDeclaration(DestructorDeclarationSyntax node)
@@ -317,16 +318,20 @@ public override void VisitConstructorInitializer(ConstructorInitializerSyntax no
         {
             var binder = _enclosing.WithAdditionalFlags(BinderFlags.ConstructorInitializer);
             AddToMap(node, binder);
+            VisitConstructorInitializerArgumentList(node, node.ArgumentList, binder);
+        }
 
-            if (node.ArgumentList != null)
+        private void VisitConstructorInitializerArgumentList(CSharpSyntaxNode node, ArgumentListSyntax argumentList, Binder binder)
+        {
+            if (argumentList != null)
             {
                 if (_root == node)
                 {
-                    binder = new ExpressionVariableBinder(node.ArgumentList, binder);
-                    AddToMap(node.ArgumentList, binder);
+                    binder = new ExpressionVariableBinder(argumentList, binder);
+                    AddToMap(argumentList, binder);
                 }
 
-                Visit(node.ArgumentList, binder);
+                Visit(argumentList, binder);
             }
         }