Parser: Separate mandatory and optional arguments in parseArguments (#903)

* Parser: Make parseArguments() more DRY.

* Parser: Separate args and optional args for type strictness.

src/Parser.js

@@ -62,7 +62,7 @@ class ParseFuncOrArgument {
     }
 }
 
-class Parser {
+export default class Parser {
     constructor(input, settings) {
         // Create a new macro expander (gullet) and (indirectly via that) also a
         // new lexer (mouth) for this parser (stomach, in the language of TeX)
@@ -226,7 +226,7 @@ class Parser {
                 denomNode = new ParseNode("ordgroup", denomBody, this.mode);
             }
 
-            const value = this.callFunction(funcName, [numerNode, denomNode]);
+            const value = this.callFunction(funcName, [numerNode, denomNode], []);
             return [new ParseNode(value.type, value, this.mode)];
         } else {
             return body;
@@ -462,13 +462,14 @@ class Parser {
             // Build the environment object. Arguments and other information will
             // be made available to the begin and end methods using properties.
             const env = environments.get(envName);
-            const args = this.parseArguments("\\begin{" + envName + "}", env);
+            const {args, optArgs} =
+                this.parseArguments("\\begin{" + envName + "}", env);
             const context = {
                 mode: this.mode,
                 envName: envName,
                 parser: this,
             };
-            const result = env.handler(context, args);
+            const result = env.handler(context, args, optArgs);
             this.expect("\\end", false);
             const endNameToken = this.nextToken;
             const end = this.parseFunction();
@@ -580,9 +581,9 @@ class Parser {
                     baseGroup.token);
             }
 
-            const args = this.parseArguments(func, funcData);
+            const {args, optArgs} = this.parseArguments(func, funcData);
             const token = baseGroup.token;
-            const result = this.callFunction(func, args, token);
+            const result = this.callFunction(func, args, optArgs, token);
             return new ParseNode(result.type, result, this.mode);
         } else {
             return baseGroup.result;
@@ -591,62 +592,63 @@ class Parser {
 
     /**
      * Call a function handler with a suitable context and arguments.
+     * @param {string} name
+     * @param {Array<ParseNode>} args
+     * @param {Array<?ParseNode>} optArgs
+     * @param {Token=} token
      */
-    callFunction(name, args, token) {
+    callFunction(name, args, optArgs, token) {
         const context = {
             funcName: name,
             parser: this,
             token,
         };
-        return functions[name].handler(context, args);
+        return functions[name].handler(context, args, optArgs);
     }
 
     /**
      * Parses the arguments of a function or environment
      *
      * @param {string} func  "\name" or "\begin{name}"
-     * @param {{numArgs:number,numOptionalArgs:number|undefined}} funcData
+     * @param {{
-     * @return the array of arguments
+     *   numArgs: number,
+     *   numOptionalArgs: (number|undefined),
+     * }} funcData
+     * @return {{
+     *   args: Array<ParseNode>,
+     *   optArgs: Array<?ParseNode>,
+     * }}
      */
     parseArguments(func, funcData) {
         const totalArgs = funcData.numArgs + funcData.numOptionalArgs;
         if (totalArgs === 0) {
-            return [];
+            return {args: [], optArgs: []};
         }
 
         const baseGreediness = funcData.greediness;
         const args = [];
+        const optArgs = [];
 
         for (let i = 0; i < totalArgs; i++) {
             const nextToken = this.nextToken;
             const argType = funcData.argTypes && funcData.argTypes[i];
-            let arg;
+            const isOptional = i < funcData.numOptionalArgs;
-            if (i < funcData.numOptionalArgs) {
+            let arg = argType ?
-                if (argType) {
+                this.parseGroupOfType(argType, isOptional) :
-                    arg = this.parseGroupOfType(argType, true);
+                this.parseGroup(isOptional);
-                } else {
+            if (!arg) {
-                    arg = this.parseGroup(true);
+                if (isOptional) {
-                }
+                    optArgs.push(null);
-                if (!arg) {
-                    args.push(null);
                     continue;
                 }
-            } else {
+                if (!this.settings.throwOnError &&
-                if (argType) {
+                    this.nextToken.text[0] === "\\") {
-                    arg = this.parseGroupOfType(argType);
+                    arg = new ParseFuncOrArgument(
+                        this.handleUnsupportedCmd(),
+                        false);
                 } else {
-                    arg = this.parseGroup();
+                    throw new ParseError(
-                }
+                        "Expected group after '" + func + "'", nextToken);
-                if (!arg) {
-                    if (!this.settings.throwOnError &&
-                        this.nextToken.text[0] === "\\") {
-                        arg = new ParseFuncOrArgument(
-                            this.handleUnsupportedCmd(),
-                            false);
-                    } else {
-                        throw new ParseError(
-                            "Expected group after '" + func + "'", nextToken);
-                    }
                 }
             }
             let argNode;
@@ -663,10 +665,10 @@ class Parser {
             } else {
                 argNode = arg.result;
             }
-            args.push(argNode);
+            (isOptional ? optArgs : args).push(argNode);
         }
 
-        return args;
+        return {args, optArgs};
     }
 
     /**
@@ -940,7 +942,3 @@ class Parser {
         }
     }
 }
-
-Parser.prototype.ParseNode = ParseNode;
-
-module.exports = Parser;

src/defineEnvironment.js

@@ -21,11 +21,15 @@ type EnvContext = {|
 |};
 
 /**
- * The handler function receives two arguments
  *  - context: information and references provided by the parser
  *  - args: an array of arguments passed to \begin{name}
+ *  - optArgs: an array of optional arguments passed to \begin{name}
  */
-type EnvHandler = (context: EnvContext, args: ParseNode[]) => ParseNode;
+type EnvHandler = (
+    context: EnvContext,
+    args: ParseNode[],
+    optArgs: (?ParseNode)[],
+) => ParseNode;
 
 /**
  *  - numArgs: (default 0) The number of arguments after the \begin{name} function.

src/defineFunction.js

@@ -16,13 +16,11 @@ export type FunctionContext = {|
 |};
 
 // TODO: Enumerate all allowed output types.
-
+export type FunctionHandler = (
-// TODO: The real type of `args` is `(?ParseNode)[]`, but doing that breaks
+    context: FunctionContext,
-//       compilation since some of these arguments are passed to `ordargument`
+    args: ParseNode[],
-//       below which requires a non-nullable argument.
+    optArgs: (?ParseNode)[],
-//       Separate optional (nullable) `args` from mandatory (non-nullable) args
+) => *;
-//       for better type safety and correct the type here.
-export type FunctionHandler = (context: FunctionContext, args: ParseNode[]) => *;
 
 export type FunctionPropSpec = {
     // The number of arguments the function takes.
@@ -30,7 +28,8 @@ export type FunctionPropSpec = {
 
     // An array corresponding to each argument of the function, giving the
     // type of argument that should be parsed. Its length should be equal
-    // to `numArgs + numOptionalArgs`.
+    // to `numOptionalArgs + numArgs`, and types for optional arguments
+    // should appear before types for mandatory arguments.
     argTypes?: ArgType[],
 
     // The greediness of the function to use ungrouped arguments.

src/functions.js

@@ -30,9 +30,9 @@ const defineFunction = function(
 defineFunction(["\\sqrt"], {
     numArgs: 1,
     numOptionalArgs: 1,
-}, function(context, args) {
+}, function(context, args, optArgs) {
-    const index = args[0];
+    const index = optArgs[0];
-    const body = args[1];
+    const body = args[0];
     return {
         type: "sqrt",
         body: body,
@@ -151,10 +151,10 @@ defineFunction(["\\rule"], {
     numArgs: 2,
     numOptionalArgs: 1,
     argTypes: ["size", "size", "size"],
-}, function(context, args) {
+}, function(context, args, optArgs) {
-    const shift = args[0];
+    const shift = optArgs[0];
-    const width = args[1];
+    const width = args[0];
-    const height = args[2];
+    const height = args[1];
     return {
         type: "rule",
         shift: shift && shift.value,
@@ -443,10 +443,10 @@ defineFunction(["\\smash"], {
     numArgs: 1,
     numOptionalArgs: 1,
     allowedInText: true,
-}, function(context, args) {
+}, function(context, args, optArgs) {
     let smashHeight = false;
     let smashDepth = false;
-    const tbArg = args[0];
+    const tbArg = optArgs[0];
     if (tbArg) {
         // Optional [tb] argument is engaged.
         // ref: amsmath: \renewcommand{\smash}[1][tb]{%
@@ -469,7 +469,7 @@ defineFunction(["\\smash"], {
         smashDepth = true;
     }
 
-    const body = args[1];
+    const body = args[0];
     return {
         type: "smash",
         body: body,
@@ -616,9 +616,9 @@ defineFunction([
 ], {
     numArgs: 1,
     numOptionalArgs: 1,
-}, function(context, args) {
+}, function(context, args, optArgs) {
-    const below = args[0];
+    const below = optArgs[0];
-    const body = args[1];
+    const body = args[0];
     return {
         type: "xArrow",   // x for extensible
         label: context.funcName,
@@ -670,8 +670,8 @@ defineFunction(["\\\\", "\\cr"], {
     numArgs: 0,
     numOptionalArgs: 1,
     argTypes: ["size"],
-}, function(context, args) {
+}, function(context, args, optArgs) {
-    const size = args[0];
+    const size = optArgs[0];
     return {
         type: "cr",
         size: size,