Correct @flow types. Refactor some Parser code for stricter typing. (#896)

* Correct @flow types in defineFunction and types.

* Parser: Split parseFunction into two for stricter typing.

src/Parser.js

@@ -259,7 +259,7 @@ class Parser {
             // greediness
             const funcGreediness = functions[group.result].greediness;
             if (funcGreediness > Parser.SUPSUB_GREEDINESS) {
-                return this.parseFunction(group);
+                return this.parseGivenFunction(group);
             } else {
                 throw new ParseError(
                     "Got function '" + group.result + "' with no arguments " +
@@ -438,7 +438,7 @@ class Parser {
         if (func === "\\left") {
             // If we see a left:
             // Parse the entire left function (including the delimiter)
-            const left = this.parseFunction(start);
+            const left = this.parseGivenFunction(start);
             // Parse out the implicit body
             ++this.leftrightDepth;
             const body = this.parseExpression(false);
@@ -453,7 +453,7 @@ class Parser {
             }, this.mode);
         } else if (func === "\\begin") {
             // begin...end is similar to left...right
-            const begin = this.parseFunction(start);
+            const begin = this.parseGivenFunction(start);
             const envName = begin.value.name;
             if (!environments.has(envName)) {
                 throw new ParseError(
@@ -543,47 +543,49 @@ class Parser {
             }, "math");
         } else {
             // Defer to parseFunction if it's not a function we handle
-            return this.parseFunction(start);
+            return this.parseGivenFunction(start);
         }
     }
 
     /**
      * Parses an entire function, including its base and all of its arguments.
-     * The base might either have been parsed already, in which case
-     * it is provided as an argument, or it's the next group in the input.
+     * It also handles the case where the parsed node is not a function.
      *
-     * @param {ParseFuncOrArgument=} baseGroup optional as described above
      * @return {?ParseNode}
      */
-    parseFunction(baseGroup) {
-        if (!baseGroup) {
-            baseGroup = this.parseGroup();
-        }
+    parseFunction() {
+        const baseGroup = this.parseGroup();
+        return baseGroup ? this.parseGivenFunction(baseGroup) : null;
+    }
 
-        if (baseGroup) {
-            if (baseGroup.isFunction) {
-                const func = baseGroup.result;
-                const funcData = functions[func];
-                if (this.mode === "text" && !funcData.allowedInText) {
-                    throw new ParseError(
-                        "Can't use function '" + func + "' in text mode",
-                        baseGroup.token);
-                } else if (this.mode === "math" &&
-                    funcData.allowedInMath === false) {
-                    throw new ParseError(
-                        "Can't use function '" + func + "' in math mode",
-                        baseGroup.token);
-                }
-
-                const args = this.parseArguments(func, funcData);
-                const token = baseGroup.token;
-                const result = this.callFunction(func, args, token);
-                return new ParseNode(result.type, result, this.mode);
-            } else {
-                return baseGroup.result;
+    /**
+     * Same as parseFunction(), except that the base is provided, guaranteeing a
+     * non-nullable result.
+     *
+     * @param {ParseFuncOrArgument} baseGroup
+     * @return {ParseNode}
+     */
+    parseGivenFunction(baseGroup) {
+        if (baseGroup.isFunction) {
+            const func = baseGroup.result;
+            const funcData = functions[func];
+            if (this.mode === "text" && !funcData.allowedInText) {
+                throw new ParseError(
+                    "Can't use function '" + func + "' in text mode",
+                    baseGroup.token);
+            } else if (this.mode === "math" &&
+                funcData.allowedInMath === false) {
+                throw new ParseError(
+                    "Can't use function '" + func + "' in math mode",
+                    baseGroup.token);
             }
+
+            const args = this.parseArguments(func, funcData);
+            const token = baseGroup.token;
+            const result = this.callFunction(func, args, token);
+            return new ParseNode(result.type, result, this.mode);
         } else {
-            return null;
+            return baseGroup.result;
         }
     }
 
@@ -652,7 +654,7 @@ class Parser {
                 const argGreediness =
                     functions[arg.result].greediness;
                 if (argGreediness > baseGreediness) {
-                    argNode = this.parseFunction(arg);
+                    argNode = this.parseGivenFunction(arg);
                 } else {
                     throw new ParseError(
                         "Got function '" + arg.result + "' as " +

src/defineFunction.js

@@ -2,10 +2,10 @@
 import {groupTypes as htmlGroupTypes} from "./buildHTML";
 import {groupTypes as mathmlGroupTypes} from "./buildMathML";
 
+import type Parser from "./Parser" ;
 import type ParseNode from "./ParseNode" ;
 import type Options from "./Options";
 import type {ArgType} from "./types" ;
-import type {Parser} from "./Parser" ;
 import type {Token} from "./Token" ;
 
 /** Context provided to function handlers for error messages. */
@@ -16,6 +16,12 @@ export type FunctionContext = {|
 |};
 
 // TODO: Enumerate all allowed output types.
+
+// TODO: The real type of `args` is `(?ParseNode)[]`, but doing that breaks
+//       compilation since some of these arguments are passed to `ordargument`
+//       below which requires a non-nullable argument.
+//       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 = {
@@ -105,7 +111,7 @@ type FunctionDefSpec = {|
  * 2. requires all arguments except argTypes.
  * It is generated by `defineFunction()` below.
  */
-type FunctionSpec = {|
+export type FunctionSpec = {|
     numArgs: number,
     argTypes?: ArgType[],
     greediness: number,
@@ -160,7 +166,7 @@ export default function defineFunction({
 
 // Since the corresponding buildHTML/buildMathML function expects a
 // list of elements, we normalize for different kinds of arguments
-export const ordargument = function(arg: ParseNode) {
+export const ordargument = function(arg: ParseNode): ParseNode[] {
     if (arg.type === "ordgroup") {
         return arg.value;
     } else {

src/types.js

@@ -15,8 +15,8 @@ export type Mode = "math" | "text";
 //                 bodies of functions like \textcolor where the
 //                 first argument is special and the second
 //                 argument is parsed normally)
-//   - "text": Node group parsed as in text mode.
-export type ArgType = "color" | "size" | "original" | "text";
+//   - Mode: Node group parsed in given mode.
+export type ArgType = "color" | "size" | "original" | Mode;
 
 // LaTeX display style.
 export type StyleStr = "text" | "display";