Move starred expression checks out of parser (#1158)

Author: slozier

Src/IronPython/Compiler/Ast/PythonAst.cs

@@ -149,6 +149,7 @@ public void ParsingFinished(int[] lineLocations, Statement body, ModuleOptions l
         /// </summary>
         public void Bind() {
             PythonNameBinder.BindAst(this, _compilerContext);
+            StarredExpressionChecker.Check(this, _compilerContext);
         }
 
         public override string Name {

Src/IronPython/Compiler/Ast/StarredExpression.cs

@@ -2,24 +2,29 @@
 // The .NET Foundation licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information.
 
-using System;
+#nullable enable
 
-using Microsoft.Scripting;
+using System;
+using System.Collections.Generic;
 
+using IronPython.Runtime;
 using IronPython.Runtime.Binding;
 
+using Microsoft.Scripting;
+using Microsoft.Scripting.Runtime;
+
 using MSAst = System.Linq.Expressions;
 
 namespace IronPython.Compiler.Ast {
-
     public class StarredExpression : Expression {
         public StarredExpression(Expression value) {
+            if (value is null) throw new ArgumentNullException(nameof(value));
             Value = value;
         }
 
         public Expression Value { get; }
 
-        public override MSAst.Expression Reduce() => Value;
+        public override bool CanReduce => false;
 
         internal override MSAst.Expression TransformSet(SourceSpan span, MSAst.Expression right, PythonOperationKind op)
             => Value.TransformSet(span, right, op);
@@ -34,11 +39,74 @@ internal override MSAst.Expression TransformSet(SourceSpan span, MSAst.Expressio
 
         public override void Walk(PythonWalker walker) {
             if (walker.Walk(this)) {
-                Value?.Walk(walker);
+                Value.Walk(walker);
             }
             walker.PostWalk(this);
         }
 
         internal override bool CanThrow => Value.CanThrow;
     }
+
+    internal class StarredExpressionChecker : PythonWalker {
+        private readonly CompilerContext context;
+
+        private StarredExpressionChecker(CompilerContext context) {
+            this.context = context;
+        }
+
+        public static void Check(PythonAst ast, CompilerContext context) {
+            var finder = new StarredExpressionChecker(context);
+            ast.Walk(finder);
+        }
+
+        public override bool Walk(AssignmentStatement node) {
+            foreach (var expr in node.Left) {
+                WalkAssignmentTarget(expr);
+            }
+            node.Right?.Walk(this);
+            return false;
+        }
+
+        public override bool Walk(ForStatement node) {
+            WalkAssignmentTarget(node.Left);
+            node.List?.Walk(this);
+            node.Body?.Walk(this);
+            node.Else?.Walk(this);
+            return false;
+        }
+
+        public override bool Walk(StarredExpression node) {
+            ReportSyntaxError("can use starred expression only as assignment target", node);
+            return base.Walk(node);
+        }
+
+        private void ReportSyntaxError(string message, Node node) {
+            context.Errors.Add(context.SourceUnit, message, node.Span, ErrorCodes.SyntaxError, Severity.FatalError);
+        }
+
+        private void WalkAssignmentTarget(Expression expr) {
+            switch (expr) {
+                case StarredExpression starred:
+                    ReportSyntaxError("starred assignment target must be in a list or tuple", starred);
+                    break;
+                case SequenceExpression sequenceExpression:
+                    WalkItems(sequenceExpression.Items);
+                    break;
+                default:
+                    expr?.Walk(this);
+                    break;
+            }
+        }
+
+        private bool WalkItems(IList<Expression> items) {
+            foreach (var item in items) {
+                if (item is StarredExpression starred) {
+                    starred.Value.Walk(this);
+                } else {
+                    item.Walk(this);
+                }
+            }
+            return false;
+        }
+    }
 }

Src/IronPython/Compiler/Parser.cs

@@ -627,10 +627,6 @@ private Statement FinishAssignments(Expression right) {
                     ReportSyntaxError(right.StartIndex, right.EndIndex, assignError, ErrorCodes.SyntaxError | ErrorCodes.NoCaret);
                 }
 
-                if (right is StarredExpression) {
-                    ReportSyntaxError(right.StartIndex, right.EndIndex, "starred assignment target must be in a list or tuple");
-                }
-
                 if (singleLeft == null) {
                     singleLeft = right;
                 } else {
@@ -644,8 +640,6 @@ private Statement FinishAssignments(Expression right) {
                 right = MaybeEat(TokenKind.KeywordYield) ? ParseYieldExpression() : ParseTestListStarExpr();
             }
 
-            CheckNotAssignmentTargetOnly(right);
-
             var target = left?.ToArray() ?? new[] { singleLeft };
 
             Debug.Assert(target.Length > 0);
@@ -689,37 +683,11 @@ private Statement ParseExprStmt() {
                 return aug;
             }
 
-            CheckNotAssignmentTargetOnly(ret);
-
             Statement stmt = new ExpressionStatement(ret);
             stmt.SetLoc(_globalParent, ret.IndexSpan);
             return stmt;
         }
 
-        private void CheckNotAssignmentTargetOnly(Expression expr) {
-            switch (expr) {
-                case SequenceExpression sequence: {
-                        foreach (var expression in sequence.Items) {
-                            if (expression is StarredExpression starred) {
-                                ReportSyntaxError(
-                                    starred.StartIndex,
-                                    starred.EndIndex,
-                                    "can use starred expression only as assignment target");
-                            }
-                        }
-
-                        break;
-                    }
-                case StarredExpression starred:
-                    ReportSyntaxError(
-                        starred.StartIndex,
-                        starred.EndIndex,
-                        "can use starred expression only as assignment target");
-
-                    break;
-            }
-        }
-
         private PythonOperator GetAssignOperator(Token t) {
             switch (t.Kind) {
                 case TokenKind.AddEqual: return PythonOperator.Add;