Do not panic in parser

parser/src/error.rs

@@ -20,6 +20,7 @@ pub enum LexicalErrorType {
     StringError,
     UnicodeError,
     NestingError,
+    DefaultArgumentError,
     PositionalArgumentError,
     DuplicateKeywordArgumentError,
     UnrecognizedToken { tok: char },
@@ -34,6 +35,9 @@ impl fmt::Display for LexicalErrorType {
             LexicalErrorType::FStringError(error) => write!(f, "Got error in f-string: {}", error),
             LexicalErrorType::UnicodeError => write!(f, "Got unexpected unicode"),
             LexicalErrorType::NestingError => write!(f, "Got unexpected nesting"),
+            LexicalErrorType::DefaultArgumentError => {
+                write!(f, "non-default argument follows default argument")
+            }
             LexicalErrorType::DuplicateKeywordArgumentError => {
                 write!(f, "keyword argument repeated")
             }

parser/src/function.rs

@@ -3,6 +3,32 @@ use std::collections::HashSet;
 use crate::ast;
 use crate::error::{LexicalError, LexicalErrorType};
 
+type ParameterDefs = (Vec<ast::Parameter>, Vec<ast::Expression>);
+type ParameterDef = (ast::Parameter, Option<ast::Expression>);
+
+pub fn parse_params(params: Vec<ParameterDef>) -> Result<ParameterDefs, LexicalError> {
+    let mut names = vec![];
+    let mut defaults = vec![];
+
+    for (name, default) in params {
+        if let Some(default) = default {
+            defaults.push(default);
+        } else {
+            if !defaults.is_empty() {
+                // Once we have started with defaults, all remaining arguments must
+                // have defaults
+                return Err(LexicalError {
+                    error: LexicalErrorType::DefaultArgumentError,
+                    location: name.location.clone(),
+                });
+            }
+        }
+        names.push(name);
+    }
+
+    Ok((names, defaults))
+}
+
 type FunctionArgument = (Option<Option<String>>, ast::Expression);
 
 pub fn parse_args(func_args: Vec<FunctionArgument>) -> Result<ast::ArgumentList, LexicalError> {

parser/src/python.lalrpop

@@ -7,7 +7,7 @@ use std::iter::FromIterator;
 
 use crate::ast;
 use crate::fstring::parse_located_fstring;
-use crate::function::parse_args;
+use crate::function::{parse_args, parse_params};
 use crate::error::LexicalError;
 use crate::lexer;
 use crate::location;
@@ -455,23 +455,23 @@ Parameters: ast::Parameters = {
 // Note that this is a macro which is used once for function defs, and
 // once for lambda defs.
 ParameterList<ArgType>: ast::Parameters = {
-    <param1:ParameterDefs<ArgType>> <args2:("," ParameterListStarArgs<ArgType>)?> ","? => {
-        let (names, default_elements) = param1;
+    <param1:ParameterDefs<ArgType>> <args2:("," ParameterListStarArgs<ArgType>)?> ","? =>? {
+        let (names, default_elements) = parse_params(param1)?;
 
         // Now gather rest of parameters:
         let (vararg, kwonlyargs, kw_defaults, kwarg) = args2.map_or((None, vec![], vec![], None), |x| x.1);
 
-        ast::Parameters {
+        Ok(ast::Parameters {
             args: names,
             kwonlyargs,
             vararg: vararg.into(),
             kwarg: kwarg.into(),
             defaults: default_elements,
             kw_defaults: kw_defaults,
-        }
+        })
     },
-    <param1:ParameterDefs<ArgType>> <kw:("," KwargParameter<ArgType>)> ","? => {
-        let (names, default_elements) = param1;
+    <param1:ParameterDefs<ArgType>> <kw:("," KwargParameter<ArgType>)> ","? =>? {
+        let (names, default_elements) = parse_params(param1)?;
 
         // Now gather rest of parameters:
         let vararg = None;
@@ -479,14 +479,14 @@ ParameterList<ArgType>: ast::Parameters = {
         let kw_defaults = vec![];
         let kwarg = Some(kw.1);
 
-        ast::Parameters {
+        Ok(ast::Parameters {
             args: names,
             kwonlyargs,
             vararg: vararg.into(),
             kwarg: kwarg.into(),
             defaults: default_elements,
             kw_defaults: kw_defaults,
-        }
+        })
     },
     <params:ParameterListStarArgs<ArgType>> ","? => {
         let (vararg, kwonlyargs, kw_defaults, kwarg) = params;
@@ -513,27 +513,9 @@ ParameterList<ArgType>: ast::Parameters = {
 
 // Use inline here to make sure the "," is not creating an ambiguity.
 #[inline]
-ParameterDefs<ArgType>: (Vec<ast::Parameter>, Vec<ast::Expression>) = {
+ParameterDefs<ArgType>: Vec<(ast::Parameter, Option<ast::Expression>)> = {
     <args:OneOrMore<ParameterDef<ArgType>>> => {
-        let mut names = vec![];
-        let mut default_elements = vec![];
-
-        for (name, default) in args.into_iter() {
-            if let Some(default) = default {
-                default_elements.push(default);
-            } else {
-                if default_elements.len() > 0 {
-                    // Once we have started with defaults, all remaining arguments must
-                    // have defaults
-                    panic!("non-default argument follows default argument: {}", &name.arg);
-                }
-            }
-            names.push(name);
-        }
-
-        //Ok(
-            (names, default_elements)
-        //)
+        args
     }
 };
 

tests/snippets/function_args.py

@@ -102,3 +102,6 @@ def inc(n):
 
 with assert_raises(SyntaxError):
     exec("inc(n=1, n=2)")
+
+with assert_raises(SyntaxError):
+    exec("def f(a=1, b): pass")