PR feedback (2)

dotnet · Nov 8, 2016 · d5bede6 · d5bede6
1 parent 7400a9f
commit d5bede6
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 14 deletions.
diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/VariablesDeclaredWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/VariablesDeclaredWalker.cs
@@ -173,7 +173,10 @@ private void CheckOutVarDeclaration(BoundLocal node)
                 !node.WasCompilerGenerated && node.Syntax.Kind() == SyntaxKind.DeclarationExpression)
             {
                 var declaration = (DeclarationExpressionSyntax)node.Syntax;
-                if (((SingleVariableDesignationSyntax)declaration.Designation).Identifier == node.LocalSymbol.IdentifierToken &&
+                if (declaration.Designation.Kind() == SyntaxKind.SingleVariableDesignation &&
+                    ((SingleVariableDesignationSyntax)declaration.Designation).Identifier == node.LocalSymbol.IdentifierToken &&
+                    declaration.Parent != null &&
+                    declaration.Parent.Kind() == SyntaxKind.Argument &&
                     ((ArgumentSyntax)declaration.Parent).RefOrOutKeyword.Kind() == SyntaxKind.OutKeyword)
                 {
                     _variablesDeclared.Add(node.LocalSymbol);

diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
@@ -11,7 +11,6 @@
 namespace Microsoft.CodeAnalysis.CSharp.Syntax.InternalSyntax
 {
     using Microsoft.CodeAnalysis.Syntax.InternalSyntax;
-    using System.Linq;
 
     internal partial class LanguageParser : SyntaxParser
     {
@@ -7542,7 +7541,7 @@ private StatementSyntax ParseEmbeddedStatement(bool complexCheck)
                     if (expression.Kind == SyntaxKind.SimpleAssignmentExpression)
                     {
                         var assignment = (AssignmentExpressionSyntax)expression;
-                        if (assignment.Left.EnumerateNodes().Any(x => x.RawKind == (int)SyntaxKind.DeclarationExpression))
+                        if (IsDeconstructionDeclarationLeft(assignment.Left))
                         {
                             statement = this.AddError(statement, ErrorCode.ERR_BadEmbeddedStmt);
                         }
@@ -8668,6 +8667,28 @@ private static bool TypeFoundInDeconstructionDeclarationVariables(ExpressionSynt
             }
         }
 
+        /// <summary>
+        /// Returns true if the expression is composed only of nested tuple and declaration expressions.
+        /// </summary>
+        private static bool IsDeconstructionDeclarationLeft(ExpressionSyntax node)
+        {
+            switch (node.Kind)
+            {
+                case SyntaxKind.TupleExpression:
+                    var arguments = ((TupleExpressionSyntax)node).Arguments;
+                    for (int i = 0; i < arguments.Count; i++)
+                    {
+                        if (!IsDeconstructionDeclarationLeft(arguments[i].Expression)) return false;
+                    }
+
+                    return true;
+                case SyntaxKind.DeclarationExpression:
+                    return true;
+                default:
+                    return false;
+            }
+        }
+
         private VariableDesignationSyntax ParseDeconstructionDesignation(bool topLevel = false)
         {
             // the two forms of designation are

diff --git a/src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt b/src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt
@@ -263,7 +263,6 @@ static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.ConstructorDeclaration(Micros
 static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.ConstructorDeclaration(Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.AttributeListSyntax> attributeLists, Microsoft.CodeAnalysis.SyntaxTokenList modifiers, Microsoft.CodeAnalysis.SyntaxToken identifier, Microsoft.CodeAnalysis.CSharp.Syntax.ParameterListSyntax parameterList, Microsoft.CodeAnalysis.CSharp.Syntax.ConstructorInitializerSyntax initializer, Microsoft.CodeAnalysis.CSharp.Syntax.BlockSyntax body, Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax expressionBody) -> Microsoft.CodeAnalysis.CSharp.Syntax.ConstructorDeclarationSyntax
 static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.ConstructorDeclaration(Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.AttributeListSyntax> attributeLists, Microsoft.CodeAnalysis.SyntaxTokenList modifiers, Microsoft.CodeAnalysis.SyntaxToken identifier, Microsoft.CodeAnalysis.CSharp.Syntax.ParameterListSyntax parameterList, Microsoft.CodeAnalysis.CSharp.Syntax.ConstructorInitializerSyntax initializer, Microsoft.CodeAnalysis.CSharp.Syntax.BlockSyntax body, Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax expressionBody, Microsoft.CodeAnalysis.SyntaxToken semicolonToken) -> Microsoft.CodeAnalysis.CSharp.Syntax.ConstructorDeclarationSyntax
 static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.DeclarationExpression(Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax type, Microsoft.CodeAnalysis.CSharp.Syntax.VariableDesignationSyntax designation) -> Microsoft.CodeAnalysis.CSharp.Syntax.DeclarationExpressionSyntax
-static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.DeclarationExpression(Microsoft.CodeAnalysis.CSharp.Syntax.VariableDesignationSyntax designation) -> Microsoft.CodeAnalysis.CSharp.Syntax.DeclarationExpressionSyntax
 static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.DeclarationPattern(Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax type, Microsoft.CodeAnalysis.SyntaxToken identifier) -> Microsoft.CodeAnalysis.CSharp.Syntax.DeclarationPatternSyntax
 static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.DestructorDeclaration(Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.AttributeListSyntax> attributeLists, Microsoft.CodeAnalysis.SyntaxTokenList modifiers, Microsoft.CodeAnalysis.SyntaxToken identifier, Microsoft.CodeAnalysis.CSharp.Syntax.ParameterListSyntax parameterList, Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax expressionBody) -> Microsoft.CodeAnalysis.CSharp.Syntax.DestructorDeclarationSyntax
 static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.DestructorDeclaration(Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.AttributeListSyntax> attributeLists, Microsoft.CodeAnalysis.SyntaxTokenList modifiers, Microsoft.CodeAnalysis.SyntaxToken identifier, Microsoft.CodeAnalysis.CSharp.Syntax.ParameterListSyntax parameterList, Microsoft.CodeAnalysis.CSharp.Syntax.BlockSyntax body, Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax expressionBody) -> Microsoft.CodeAnalysis.CSharp.Syntax.DestructorDeclarationSyntax

diff --git a/src/Compilers/CSharp/Portable/Syntax/Syntax.xml b/src/Compilers/CSharp/Portable/Syntax/Syntax.xml
@@ -1131,7 +1131,7 @@
   </Node>
   <Node Name="DeclarationExpressionSyntax" Base="ExpressionSyntax">
     <Kind Name="DeclarationExpression"/>
-    <Field Name="Type" Type="TypeSyntax" Optional="true"/>
+    <Field Name="Type" Type="TypeSyntax"/>
     <Field Name="Designation" Type="VariableDesignationSyntax">
       <PropertyComment>
         <summary>Declaration representing the variable declared in an out parameter or deconstruction.</summary>

diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/DeconstructionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/DeconstructionTests.cs
@@ -2454,6 +2454,24 @@ void M()
                 );
         }
 
+        [Fact]
+        public void AssignmentExpressionCanBeUsedInEmbeddedStatement()
+        {
+            var source = @"
+class C1
+{
+    void M()
+    {
+        int x, y;
+        if (true)
+            (x, y) = (1, 2);
+    }
+}
+";
+            var comp = CreateCompilationWithMscorlib(source, references: s_valueTupleRefs);
+            comp.VerifyDiagnostics();
+        }
+
         [Fact]
         public void DeconstructObsoleteWarning()
         {

diff --git a/...o/CSharp/Impl/Debugging/CSharpProximityExpressionsService.RelevantExpressionsCollector.cs b/...o/CSharp/Impl/Debugging/CSharpProximityExpressionsService.RelevantExpressionsCollector.cs
@@ -136,7 +136,6 @@ public override void VisitSwitchStatement(SwitchStatementSyntax node)
                         {
                             var t = (TupleExpressionSyntax)component;
                             foreach (ArgumentSyntax a in t.Arguments) AddVariableExpressions(a.Expression, expressions);
-                            // TODO REVIEW I think there was a bug there. Are we missing tests?
                             break;
                         }
                     case SyntaxKind.DeclarationExpression:

diff --git a/src/Workspaces/CSharp/Portable/Extensions/ExpressionSyntaxExtensions.cs b/src/Workspaces/CSharp/Portable/Extensions/ExpressionSyntaxExtensions.cs
@@ -2161,14 +2161,6 @@ private static bool IsThisOrTypeOrNamespace(MemberAccessExpressionSyntax memberA
                 return true;
             }
 
-            //if (simpleName.IsParentKind(SyntaxKind.TypedVariableComponent))
-            //{
-            //    replacementNode = candidateReplacementNode;
-            //    issueSpan = candidateIssueSpan;
-            //    return true;
-            //}
-            // TODO REVIEW Are we lacking tests? It seems this code should be needed for DeclarationExpression
-
             return false;
         }