Deconstruction-assignment: PR feedback (2)

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

@@ -1723,19 +1723,11 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax
             var boundRHS = BindValue(node.Right, diagnostics, BindValueKind.RValue);
 
             int numElements = arguments.Count;
-            if (numElements < 2)
-            {
-                // this should be a parse error already.
-                BoundExpression[] args = numElements == 1 ?
-                    new[] { BindValue(arguments[0].Expression, diagnostics, BindValueKind.Assignment), boundRHS } :
-                    new[] { boundRHS };
-
-                return BadExpression(node, args);
-            }
+            Debug.Assert(numElements >= 2); // this should not have parsed as a tuple.
 
             // bind the variables and check they can be assigned to
             var checkedVariablesBuilder = ArrayBuilder<BoundExpression>.GetInstance(numElements);
-            for (int i = 0, l = numElements; i < l; i++)
+            for (int i = 0; i < numElements; i++)
             {
                 var boundVariable = BindExpression(arguments[i].Expression, diagnostics, invoked: false, indexed: false);
                 var checkedVariable = CheckValue(boundVariable, BindValueKind.Assignment, diagnostics);
@@ -1746,16 +1738,17 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax
             var checkedVariables = checkedVariablesBuilder.ToImmutableAndFree();
 
             // symbol and parameters for Deconstruct
-            MethodSymbol deconstructMethod;
-            ImmutableArray<TypeSymbol> parameterTypes;
-            if (!FindDeconstruct(checkedVariables, boundRHS, out deconstructMethod, out parameterTypes, node, diagnostics))
+            MethodSymbol deconstructMethod = FindDeconstruct(checkedVariables, boundRHS, node, diagnostics);
+            if ((object)deconstructMethod == null)
             {
                 return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, ErrorMethodSymbol.UnknownMethod,
                                                                 ImmutableArray<BoundDeconstructionAssignmentOperator.AssignmentInfo>.Empty,
                                                                 ErrorTypeSymbol.UnknownResultType,
                                                                 hasErrors: true);
             }
 
+            var parameterTypes = deconstructMethod.Parameters.SelectAsArray(p => p.Type);
+
             // figure out the pairwise conversions
             var assignmentsBuilder = ArrayBuilder<BoundDeconstructionAssignmentOperator.AssignmentInfo>.GetInstance(numElements);
             for (int i = 0; i < checkedVariables.Length; i++)
@@ -1777,23 +1770,22 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax
         /// <summary>
         /// Find the Deconstruct method for the expression on the right, that will fit the assignable bound expressions on the left.
         /// Returns true if the Deconstruct method is found.
-        ///     - If so, it outputs the method and all of its parameter types.
-        ///     - Otherwise, it outputs null/default out parameters.
+        ///     If so, it outputs the method.
         /// </summary>
-        private static bool FindDeconstruct(ImmutableArray<BoundExpression> checkedVariables, BoundExpression boundRHS, out MethodSymbol deconstructMethod, out ImmutableArray<TypeSymbol> parameterTypes, SyntaxNode node, DiagnosticBag diagnostics)
+        private static MethodSymbol FindDeconstruct(ImmutableArray<BoundExpression> checkedVariables, BoundExpression boundRHS, SyntaxNode node, DiagnosticBag diagnostics)
         {
             // find symbol for Deconstruct member
             ImmutableArray<Symbol> candidates = boundRHS.Type.GetMembers("Deconstruct");
             switch (candidates.Length)
             {
                 case 0:
                     Error(diagnostics, ErrorCode.ERR_MissingDeconstruct, boundRHS.Syntax, boundRHS.Type);
-                    goto error;
+                    return null;
                 case 1:
                     break;
                 default:
                     Error(diagnostics, ErrorCode.ERR_AmbiguousDeconstruct, boundRHS.Syntax, boundRHS.Type);
-                    goto error;
+                    return null;
             }
 
             Symbol deconstructMember = candidates[0];
@@ -1802,46 +1794,29 @@ private static bool FindDeconstruct(ImmutableArray<BoundExpression> checkedVaria
             if (deconstructMember.Kind != SymbolKind.Method)
             {
                 Error(diagnostics, ErrorCode.ERR_MissingDeconstruct, boundRHS.Syntax, boundRHS.Type);
-                goto error;
+                return null;
             }
 
-            deconstructMethod = (MethodSymbol)deconstructMember;
+            MethodSymbol deconstructMethod = (MethodSymbol)deconstructMember;
             if (deconstructMethod.MethodKind != MethodKind.Ordinary)
             {
                 Error(diagnostics, ErrorCode.ERR_MissingDeconstruct, boundRHS.Syntax, boundRHS.Type);
-                goto error;
+                return null;
             }
 
             if (deconstructMethod.ParameterCount != checkedVariables.Length)
             {
-                Error(diagnostics, ErrorCode.ERR_DeconstructWrongParams, boundRHS.Syntax, boundRHS.Type);
-                goto error;
+                Error(diagnostics, ErrorCode.ERR_DeconstructWrongParams, boundRHS.Syntax, boundRHS.Type, checkedVariables.Length);
+                return null;
             }
 
-            // get the parameter types
-            var parameters = deconstructMethod.Parameters;
-            var parameterTypesBuilder = ArrayBuilder<TypeSymbol>.GetInstance(parameters.Length);
-            foreach (var parameter in parameters)
+            if (deconstructMethod.Parameters.Any(p => p.RefKind != RefKind.Out))
             {
-                if (parameter.RefKind != RefKind.Out)
-                {
-                    Error(diagnostics, ErrorCode.ERR_DeconstructRequiresOutParams, boundRHS.Syntax, boundRHS.Type);
-                    goto error;
-                }
-
-                parameterTypesBuilder.Add(parameter.Type);
+                Error(diagnostics, ErrorCode.ERR_DeconstructRequiresOutParams, boundRHS.Syntax, boundRHS.Type);
+                return null;
             }
 
-            parameterTypes = parameterTypesBuilder.ToImmutableAndFree();
-
-            return true;
-
-            error:
-
-            deconstructMethod = null;
-            parameterTypes = default(ImmutableArray<TypeSymbol>);
-
-            return false;
+            return deconstructMethod;
         }
 
         private BoundAssignmentOperator BindAssignment(AssignmentExpressionSyntax node, BoundExpression op1, BoundExpression op2, DiagnosticBag diagnostics)

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -396,7 +396,7 @@
 
     <Field Name="LeftVariables" Type="ImmutableArray&lt;BoundExpression&gt;"/>
 
-    <Field Name="DeconstructReceiver" Type="BoundExpression"/>
+    <Field Name="Right" Type="BoundExpression"/>
     <Field Name="DeconstructMember" Type="MethodSymbol" Null="disallow"/>
 
     <!-- The assignments have placeholders for the left and right part of the assignment -->

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -242,7 +242,7 @@
   </data>
   <data name="IDS_FeatureReplace" xml:space="preserve">
     <value>replaced members</value>
-  </data>  
+  </data>
   <data name="IDS_SK_METHOD" xml:space="preserve">
     <value>method</value>
   </data>
@@ -4879,9 +4879,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>The Deconstruct method for type '{0}' must have only out parameters.</value>
   </data>
   <data name="ERR_DeconstructWrongParams" xml:space="preserve">
-    <value>The Deconstruct method for type '{0}' has the wrong number of parameters for this deconstruction.</value>
+    <value>The Deconstruct method for type '{0}' doesn't have the number of parameters ({1}) needed for this deconstruction.</value>
   </data>
   <data name="ERR_MissingDeconstruct" xml:space="preserve">
     <value>No Deconstruct instance or extension method was found for type '{0}'.</value>
   </data>
-</root>
+</root>

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

@@ -1521,7 +1521,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct
                 VisitLvalue(variable);
             }
 
-            VisitRvalue(node.DeconstructReceiver);
+            VisitRvalue(node.Right);
 
             return null;
         }

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs

@@ -23,11 +23,11 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct
                 lhsReceivers.Add(TransformCompoundAssignmentLHS(variable, stores, temps, isDynamicAssignment: false));
             }
 
-            BoundExpression loweredRight = VisitExpression(node.DeconstructReceiver);
+            BoundExpression loweredRight = VisitExpression(node.Right);
 
             // prepare out parameters for Deconstruct
-            var outParametersBuilder = ArrayBuilder<BoundExpression>.GetInstance();
             var deconstructParameters = node.DeconstructMember.Parameters;
+            var outParametersBuilder = ArrayBuilder<BoundExpression>.GetInstance(deconstructParameters.Length);
             Debug.Assert(deconstructParameters.Length == node.LeftVariables.Length);
 
             for (int i = 0; i < numVariables; i++)

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

@@ -6264,9 +6264,12 @@ private ScanTypeFlags ScanNonArrayType(out SyntaxToken lastTokenOfType)
         /// <summary>
         /// Returns TupleType when a possible tuple type is found.
         /// Note that this is not MustBeType, so that the caller can consider deconstruction syntaxes.
+        /// The caller is expected to have consumed the opening paren.
         /// </summary>
         private ScanTypeFlags ScanTupleType(out SyntaxToken lastTokenOfType)
         {
+            bool mustBeType = false;
+
             var tupleElementType = ScanType(out lastTokenOfType);
             if (tupleElementType != ScanTypeFlags.NotType)
             {
@@ -6287,6 +6290,10 @@ private ScanTypeFlags ScanTupleType(out SyntaxToken lastTokenOfType)
                             lastTokenOfType = this.EatToken();
                             return ScanTypeFlags.NotType;
                         }
+                        else if (tupleElementType == ScanTypeFlags.MustBeType)
+                        {
+                            mustBeType = true;
+                        }
 
                         if (this.CurrentToken.Kind == SyntaxKind.IdentifierToken)
                         {
@@ -6298,7 +6305,15 @@ private ScanTypeFlags ScanTupleType(out SyntaxToken lastTokenOfType)
                     if (this.CurrentToken.Kind == SyntaxKind.CloseParenToken)
                     {
                         lastTokenOfType = this.EatToken();
-                        return ScanTypeFlags.TupleType;
+
+                        if (mustBeType)
+                        {
+                            return ScanTypeFlags.MustBeType;
+                        }
+                        else
+                        {
+                            return ScanTypeFlags.TupleType;
+                        }
                     }
                 }
             }
@@ -9858,8 +9873,7 @@ private bool ScanCast()
             if (type == ScanTypeFlags.PointerOrMultiplication ||
                 type == ScanTypeFlags.NullableType ||
                 type == ScanTypeFlags.MustBeType ||
-                type == ScanTypeFlags.AliasQualifiedName ||
-                type == ScanTypeFlags.TupleType)
+                type == ScanTypeFlags.AliasQualifiedName)
             {
                 return true;
             }
@@ -9869,7 +9883,7 @@ private bool ScanCast()
             // check for ambiguous type or expression followed by disambiguating token.  i.e.
             //
             // "(A)b" is a cast.  But "(A)+b" is not a cast.  
-            return (type == ScanTypeFlags.GenericTypeOrMethod || type == ScanTypeFlags.GenericTypeOrExpression || type == ScanTypeFlags.NonGenericTypeOrExpression) && CanFollowCast(this.CurrentToken.Kind);
+            return (type == ScanTypeFlags.GenericTypeOrMethod || type == ScanTypeFlags.GenericTypeOrExpression || type == ScanTypeFlags.NonGenericTypeOrExpression || type == ScanTypeFlags.TupleType) && CanFollowCast(this.CurrentToken.Kind);
         }
 
         private bool ScanAsyncLambda(Precedence precedence)

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs

@@ -110,9 +110,9 @@ public void Deconstruct(out int a)
 }";
             var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.Regular.WithTuplesFeature());
             comp.VerifyDiagnostics(
-                // (8,18): error CS8208: The Deconstruct method for type 'C' has the wrong number of parameters for this deconstruction.
+                // (8,18): error CS8209: The Deconstruct method for type 'C' doesn't have the number of parameters (2) needed for this deconstruction.
                 //         (x, y) = new C();
-                Diagnostic(ErrorCode.ERR_DeconstructWrongParams, "new C()").WithArguments("C").WithLocation(8, 18)
+                Diagnostic(ErrorCode.ERR_DeconstructWrongParams, "new C()").WithArguments("C", "2").WithLocation(8, 18)
                 );
         }
 
@@ -139,9 +139,9 @@ static void Main()
                 // (8,17): error CS1061: 'string' does not contain a definition for 'g' and no extension method 'g' accepting a first argument of type 'string' could be found (are you missing a using directive or an assembly reference?)
                 //         (x.f, y.g) = new C();
                 Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "g").WithArguments("string", "g").WithLocation(8, 17),
-                // (8,22): error CS8208: The Deconstruct method for type 'C' has the wrong number of parameters for this deconstruction.
+                // (8,22): error CS8209: The Deconstruct method for type 'C' doesn't have the number of parameters (2) needed for this deconstruction.
                 //         (x.f, y.g) = new C();
-                Diagnostic(ErrorCode.ERR_DeconstructWrongParams, "new C()").WithArguments("C").WithLocation(8, 22)
+                Diagnostic(ErrorCode.ERR_DeconstructWrongParams, "new C()").WithArguments("C", "2").WithLocation(8, 22)
                 );
         }
 
@@ -540,5 +540,27 @@ static void Main()
                 Diagnostic(ErrorCode.ERR_IllegalStatement, "(a, b) => { }").WithLocation(6, 9)
                 );
         }
+
+        [Fact]
+        public void CastButNotCast()
+        {
+            // int and string must be types, so (int, string) must be type and ((int, string)) a cast, but then .String() cannot follow a cast...
+            string source = @"
+class C
+{
+    static void Main()
+    {
+        ((int, string)).ToString();
+    }
+}
+";
+
+            var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature());
+            comp.VerifyDiagnostics(
+                // (6,24): error CS1525: Invalid expression term '.'
+                //         ((int, string)).ToString();
+                Diagnostic(ErrorCode.ERR_InvalidExprTerm, ".").WithArguments(".").WithLocation(6, 24)
+                );
+        }
     }
 }

src/Compilers/CSharp/Test/Syntax/Parsing/DeconstructionTest.cs

@@ -697,6 +697,50 @@ public void CastWithTupleType()
             Assert.Equal(SyntaxKind.CastExpression, lhsContent.Expression.Kind());
         }
 
+        [Fact]
+        public void NotACast()
+        {
+            var text = "((Int32.MaxValue, Int32.MaxValue)).ToString();";
+            var statement = SyntaxFactory.ParseStatement(text, offset: 0, options: TestOptions.Regular.WithTuplesFeature());
+            Assert.False(statement.HasErrors);
+
+            var expression = ((ExpressionStatementSyntax)statement).Expression;
+            var invocation = (InvocationExpressionSyntax)expression;
+            var lhs = (MemberAccessExpressionSyntax)invocation.Expression;
+            var paren = (ParenthesizedExpressionSyntax)lhs.Expression;
+            Assert.Equal(SyntaxKind.TupleExpression, paren.Expression.Kind());
+        }
+
+        [Fact]
+        public void AlsoNotACast()
+        {
+            var text = "((x, y)).ToString();";
+            var statement = SyntaxFactory.ParseStatement(text, offset: 0, options: TestOptions.Regular.WithTuplesFeature());
+            Assert.False(statement.HasErrors);
+
+            var expression = ((ExpressionStatementSyntax)statement).Expression;
+            var invocation = (InvocationExpressionSyntax)expression;
+            var lhs = (MemberAccessExpressionSyntax)invocation.Expression;
+            var paren = (ParenthesizedExpressionSyntax)lhs.Expression;
+            Assert.Equal(SyntaxKind.TupleExpression, paren.Expression.Kind());
+        }
+
+        [Fact]
+        public void StillNotACast()
+        {
+            var text = "((((x, y)))).ToString();";
+            var statement = SyntaxFactory.ParseStatement(text, offset: 0, options: TestOptions.Regular.WithTuplesFeature());
+            Assert.False(statement.HasErrors);
+
+            var expression = ((ExpressionStatementSyntax)statement).Expression;
+            var invocation = (InvocationExpressionSyntax)expression;
+            var lhs = (MemberAccessExpressionSyntax)invocation.Expression;
+            var paren1 = (ParenthesizedExpressionSyntax)lhs.Expression;
+            var paren2 = (ParenthesizedExpressionSyntax)paren1.Expression;
+            var paren3 = (ParenthesizedExpressionSyntax)paren2.Expression;
+            Assert.Equal(SyntaxKind.TupleExpression, paren3.Expression.Kind());
+        }
+
         [Fact]
         public void LambdaInExpressionStatement()
         {
@@ -726,5 +770,6 @@ public void InvalidStatement()
             var statement = SyntaxFactory.ParseStatement(text, offset: 0, options: TestOptions.Regular.WithTuplesFeature());
             Assert.True(statement.HasErrors);
         }
+
     }
 }