Change parse to return an error instead of throwing.

Author: harrisont

server/src/parseDataProvider.ts

@@ -54,21 +54,16 @@ export class ParseDataProvider {
     //
     // Returns |ParseError| on failing to parse the file contents.
     updateParseDataWithContent(uri: vscodeUri.URI, content: string): Maybe<ParseData>  {
-        try {
-            const parseData = parse(content, uri.toString(), this.parseOptions);
-            this.data.set(uri.toString(), {
-                isStale: false,
-                data: parseData,
-                fileContent: content,
-            });
-            return Maybe.ok(parseData);
-        } catch (error) {
+        const maybeParseData = parse(content, uri.toString(), this.parseOptions);
+        if (maybeParseData.hasError) {
             const existingData = this.data.get(uri.toString());
             if (existingData !== undefined) {
                 // Mark the existing data as stale instead of deleting it, so that we can still do
                 // useful things like auto-completion while in the middle of editing text.
                 existingData.isStale = true;
             }
+            
+            const error = maybeParseData.getError();
             if (error instanceof Error) {
                 return Maybe.error(error);
             } else {
@@ -77,6 +72,14 @@ export class ParseDataProvider {
                 const typedError = new Error(String(error));
                 return Maybe.error(typedError);
             }
+        } else {
+            const parseData = maybeParseData.getValue();
+            this.data.set(uri.toString(), {
+                isStale: false,
+                data: parseData,
+                fileContent: content,
+            });
+            return Maybe.ok(parseData);
         }
     }
 

server/src/parser.ts

@@ -1,4 +1,5 @@
 import * as nearley from 'nearley';
+import { Maybe } from './coreTypes';
 import fbuildGrammar from './fbuild-grammar';
 import { UriStr } from './parseDataProvider';
 
@@ -304,8 +305,8 @@ function createParseErrorFromNearlyParseError(
 
 // Parse the input and return the statements.
 //
-// Throws |ParseError| on a parse error, or |Error| on unknown errors.
-export function parse(input: string, fileUri: UriStr, options: ParseOptions): ParseData {
+// Returns |ParseError| on a parse error, or |Error| on unknown errors.
+export function parse(input: string, fileUri: UriStr, options: ParseOptions): Maybe<ParseData> {
     // Pre-process the input:
     //  * Remove comments.
     //  * Add a newline in order to make parsing easier.
@@ -330,11 +331,16 @@ export function parse(input: string, fileUri: UriStr, options: ParseOptions): Pa
         }
 
         if (nearlyParseError instanceof Error) {
-            throw createParseErrorFromNearlyParseError(nearlyParseError, modifiedInput, fileUri, options.includeCodeLocationInError);
+            const parseError = createParseErrorFromNearlyParseError(
+                nearlyParseError,
+                modifiedInput,
+                fileUri,
+                options.includeCodeLocationInError);
+            return Maybe.error(parseError);
         } else {
             // We should only throw `Error` instances, but handle other types as a fallback.
             // `error` could be anything. Try to get a useful message out of it.
-            throw new Error(String(nearlyParseError));
+            return Maybe.error(new Error(String(nearlyParseError)));
         }
     }
 
@@ -343,12 +349,13 @@ export function parse(input: string, fileUri: UriStr, options: ParseOptions): Pa
         if (options.enableDiagnostics) {
             console.log(getParseTable(parser));
         }
-        throw new ParseNumParsesError(`Should parse to exactly 1 result, but parsed to ${numResults}`, fileUri);
+        return Maybe.error(new ParseNumParsesError(`Should parse to exactly 1 result, but parsed to ${numResults}`, fileUri));
     }
     const statements = parser.results[0];
-    return {
+    const result: ParseData = {
         statements
     };
+    return Maybe.ok(result);
 }
 
 // TODO: make this more efficient by moving it into the grammar.

server/src/test/1-parser.test.ts

@@ -5,17 +5,36 @@ import {
     createRange,
     isPositionInRange,
     parse,
+    ParseError,
     ParseSourceRange,
 } from '../parser';
 
 function assertParseResultsEqual(input: string, expectedResult: any[]): void {
-    const result = parse(input, 'file:///dummy.bff', { enableDiagnostics: true, includeCodeLocationInError: true} );
+    const maybeResult = parse(input, 'file:///dummy.bff', { enableDiagnostics: true, includeCodeLocationInError: true} );
+    if (maybeResult.hasError) {
+        const error = maybeResult.getError();
+        throw error;
+    }
+    const result = maybeResult.getValue();
     assert.deepStrictEqual(result.statements, expectedResult);
 }
 
 function assertInputsGenerateSameParseResult(input1: string, input2: string): void {
-    const result1 = parse(input1, 'file:///dummy.bff', { enableDiagnostics: true, includeCodeLocationInError: true} );
-    const result2 = parse(input2, 'file:///dummy.bff', { enableDiagnostics: true, includeCodeLocationInError: true} );
+    const maybeResult1 = parse(input1, 'file:///dummy.bff', { enableDiagnostics: true, includeCodeLocationInError: true} );
+    const maybeResult2 = parse(input2, 'file:///dummy.bff', { enableDiagnostics: true, includeCodeLocationInError: true} );
+    
+    if (maybeResult1.hasError) {
+        const error = maybeResult1.getError();
+        throw error;
+    }
+    const result1 = maybeResult1.getValue();
+
+    if (maybeResult2.hasError) {
+        const error = maybeResult2.getError();
+        throw error;
+    }
+    const result2 = maybeResult2.getValue();
+
     assert.deepStrictEqual(result1.statements, result2.statements);
 }
 
@@ -24,15 +43,13 @@ function getParseSourceRangeString(range: ParseSourceRange): string {
 }
 
 function assertParseSyntaxError(input: string, expectedErrorMessage: string, expectedRange: ParseSourceRange): void {
-    assert.throws(
-        () => parse(input, 'file:///dummy.bff', { enableDiagnostics: false, includeCodeLocationInError: true } ),
-        actualError => {
-            assert.strictEqual(actualError.name, 'ParseSyntaxError', `Expected a ParseSyntaxError exception but got ${actualError}:\n\n${actualError.stack}`);
-            assert(actualError.message === expectedErrorMessage, `Got error message <${actualError.message}> but expected <${expectedErrorMessage}>`);
-            assert.deepStrictEqual(actualError.range, expectedRange, `Expected the error range to be ${getParseSourceRangeString(expectedRange)} but it is ${getParseSourceRangeString(actualError.range)}`);
-            return true;
-        }
-    );
+    const maybeResult = parse(input, 'file:///dummy.bff', { enableDiagnostics: false, includeCodeLocationInError: true } );
+    assert.strictEqual(maybeResult.hasError, true);
+    const actualError = maybeResult.getError();
+    assert.strictEqual(actualError.name, 'ParseSyntaxError', `Expected a ParseSyntaxError exception but got ${actualError}:\n\n${actualError.stack}`);
+    const actualParseError = actualError as ParseError;
+    assert(actualParseError.message === expectedErrorMessage, `Got error message <${actualParseError.message}> but expected <${expectedErrorMessage}>`);
+    assert.deepStrictEqual(actualParseError.range, expectedRange, `Expected the error range to be ${getParseSourceRangeString(expectedRange)} but it is ${getParseSourceRangeString(actualParseError.range)}`);
 }
 
 describe('parser', () => {