Return a result type in Parser.parse

Summary:
This function use to throw an exception on parse errors. To force error handling at the call site, let's return a result type.
This will be used in a follow-up to convert parse error in pysa models into normal validation errors, which is necessary for editor plugins.

Reviewed By: grievejia

Differential Revision: D28948404

fbshipit-source-id: 32b1d6d16c02d39e98ca25c17aa2aa3beabe1bbb

Author: arthaud

source/analysis/astEnvironment.ml

@@ -117,12 +117,9 @@ type parse_result =
 let parse_source ~configuration ({ SourcePath.relative; qualifier; _ } as source_path) =
   let parse_lines lines =
     let metadata = Source.Metadata.parse ~qualifier lines in
-    try
-      let statements = Parser.parse ~relative lines in
-      Success (Source.create_from_source_path ~metadata ~source_path statements)
-    with
-    | Parser.Error error
-    | Failure error ->
+    match Parser.parse ~relative lines with
+    | Ok statements -> Success (Source.create_from_source_path ~metadata ~source_path statements)
+    | Error error ->
         let is_suppressed =
           let { Source.Metadata.local_mode; ignore_codes; _ } = metadata in
           match Source.mode ~configuration ~local_mode with
@@ -131,7 +128,7 @@ let parse_source ~configuration ({ SourcePath.relative; qualifier; _ } as source
               (* NOTE: The number needs to be updated when the error code changes. *)
               List.exists ignore_codes ~f:(Int.equal 404)
         in
-        Error { message = error; is_suppressed }
+        Error { message = Parser.Error.show error; is_suppressed }
   in
   let path = SourcePath.full_path ~configuration source_path in
   try File.lines_exn (File.create path) |> parse_lines with
@@ -282,7 +279,7 @@ let get_and_preprocess_source
       (* Files that have parser errors fall back into getattr-any. *)
       let fallback_source = ["import typing"; "def __getattr__(name: str) -> typing.Any: ..."] in
       let metadata = Source.Metadata.parse ~qualifier fallback_source in
-      let statements = Parser.parse ~relative fallback_source in
+      let statements = Parser.parse_exn ~relative fallback_source in
       Source.create_from_source_path ~metadata ~source_path statements |> preprocessing
 
 

source/analysis/preprocessing.ml

@@ -88,24 +88,19 @@ let transform_string_annotation_expression ~relative =
               arguments = variable_name :: List.map ~f:transform_argument remaining_arguments;
             }
       | String { StringLiteral.value = string_value; _ } -> (
-          try
-            let parsed =
-              (* Start at column + 1 since parsing begins after the opening quote of the string
-                 literal. *)
-              Parser.parse
-                ~start_line
-                ~start_column:(start_column + 1)
-                [string_value ^ "\n"]
-                ~relative
-            in
-            match parsed with
-            | [{ Node.value = Expression { Node.value = Name _ as expression; _ }; _ }]
-            | [{ Node.value = Expression { Node.value = Call _ as expression; _ }; _ }] ->
-                expression
-            | _ -> failwith "Invalid annotation"
+          (* Start at column + 1 since parsing begins after the opening quote of the string literal. *)
+          match
+            Parser.parse
+              ~start_line
+              ~start_column:(start_column + 1)
+              [string_value ^ "\n"]
+              ~relative
           with
-          | Parser.Error _
-          | Failure _ ->
+          | Ok [{ Node.value = Expression { Node.value = Name _ as expression; _ }; _ }]
+          | Ok [{ Node.value = Expression { Node.value = Call _ as expression; _ }; _ }] ->
+              expression
+          | Ok _
+          | Error _ ->
               Log.debug
                 "Invalid string annotation `%s` at %s:%a"
                 string_value
@@ -283,14 +278,11 @@ let expand_format_string ({ Source.source_path = { SourcePath.relative; _ }; _ }
             List.fold substrings ~init:[] ~f:gather_expressions_in_substring |> List.rev
           in
           let parse ((start_line, start_column), input_string) =
-            try
-              let string = input_string ^ "\n" in
-              match Parser.parse [string ^ "\n"] ~start_line ~start_column ~relative with
-              | [{ Node.value = Expression expression; _ }] -> [expression]
-              | _ -> failwith "Not an expression"
-            with
-            | Parser.Error _
-            | Failure _ ->
+            let string = input_string ^ "\n" in
+            match Parser.parse [string ^ "\n"] ~start_line ~start_column ~relative with
+            | Ok [{ Node.value = Expression expression; _ }] -> [expression]
+            | Ok _
+            | Error _ ->
                 Log.debug
                   "Pyre could not parse format string `%s` at %s:%a"
                   input_string
@@ -1142,16 +1134,13 @@ let qualify
             | _ -> kind
           in
           if qualify_strings then (
-            try
-              match Parser.parse [value ^ "\n"] ~relative with
-              | [{ Node.value = Expression expression; _ }] ->
-                  qualify_expression ~qualify_strings ~scope expression
-                  |> Expression.show
-                  |> fun value -> Expression.String { StringLiteral.value; kind }
-              | _ -> failwith "Not an expression"
-            with
-            | Parser.Error _
-            | Failure _ ->
+            match Parser.parse [value ^ "\n"] ~relative with
+            | Ok [{ Node.value = Expression expression; _ }] ->
+                qualify_expression ~qualify_strings ~scope expression
+                |> Expression.show
+                |> fun value -> Expression.String { StringLiteral.value; kind }
+            | Ok _
+            | Error _ ->
                 Log.debug
                   "Invalid string annotation `%s` at %s:%a"
                   value

source/analysis/type.ml

@@ -3262,7 +3262,10 @@ let rec create_logic ~resolve_aliases ~variable_aliases { Node.value = expressio
         let expression =
           try
             let parsed =
-              Parser.parse [value] |> Source.create |> Preprocessing.preprocess |> Source.statements
+              Parser.parse_exn [value]
+              |> Source.create
+              |> Preprocessing.preprocess
+              |> Source.statements
             in
             match parsed with
             | [{ Node.value = Expression { Node.value; _ }; _ }] -> Some value

source/ast/test/statementTest.ml

@@ -384,7 +384,7 @@ let test_pp _ =
     let pretty_print_of_source =
       source
       |> String.split_on_chars ~on:['\n']
-      |> Parser.parse
+      |> Parser.parse_exn
       |> List.map ~f:show
       |> String.concat ~sep:"\n"
       |> String.rstrip ~drop:(Char.equal '\n')

source/interprocedural_analyses/taint/modelParser.ml

@@ -2376,7 +2376,7 @@ let create ~resolution ~path ~configuration ~rule_filter source =
   in
   let signatures_and_queries, errors =
     String.split ~on:'\n' source
-    |> Parser.parse
+    |> Parser.parse_exn
     |> Source.create
     |> Source.statements
     |> List.map ~f:(parse_statement ~resolution ~path ~configuration)

source/parser/parser.ml

@@ -10,6 +10,31 @@ open Ast
 
 exception Error of string
 
+module Error = struct
+  type t = {
+    location: Location.t;
+    file_name: string;
+    content: string option;
+  }
+
+  let pp formatter { location; file_name; content } =
+    let column = location.Location.start.Location.column in
+    let header = Format.asprintf "Could not parse file at %s:%a" file_name Location.pp location in
+    match content with
+    | Some content ->
+        let indicator =
+          if column > 0 then
+            String.make (column - 1) ' ' ^ "^"
+          else
+            "^"
+        in
+        Format.fprintf formatter "%s\n  %s\n  %s" header content indicator
+    | None -> Format.fprintf formatter "%s" header
+
+
+  let show = Format.asprintf "%a" pp
+end
+
 let sanitize_input lines =
   (* Remove byte order mark from first line if it exists. *)
   let lines =
@@ -44,27 +69,19 @@ let parse ?start_line ?start_column ?relative lines =
     buffer
   in
   let state = Lexer.State.initial () in
-  try Generator.parse (Lexer.read state) buffer with
+  try Ok (Generator.parse (Lexer.read state) buffer) with
   | Pyre.ParserError _
   | Generator.Error
   | Failure _ ->
       let location =
         Location.create ~start:buffer.Lexing.lex_curr_p ~stop:buffer.Lexing.lex_curr_p
       in
-      let line = location.Location.start.Location.line - 1
-      and column = location.Location.start.Location.column in
-      let error =
-        let header =
-          Format.asprintf "Could not parse file at %s:%a" file_name Location.pp location
-        in
-        let indicator =
-          if column > 0 then
-            String.make (column - 1) ' ' ^ "^"
-          else
-            "^"
-        in
-        match List.nth (String.split ~on:'\n' input) line with
-        | Some line -> Format.sprintf "%s\n  %s\n  %s" header line indicator
-        | None -> Format.sprintf "%s" header
-      in
-      raise (Error error)
+      let line_number = location.Location.start.Location.line - 1 in
+      let content = List.nth (String.split ~on:'\n' input) line_number in
+      Error { Error.location; file_name; content }
+
+
+let parse_exn ?start_line ?start_column ?relative lines =
+  match parse ?start_line ?start_column ?relative lines with
+  | Ok statements -> statements
+  | Error error -> raise (Error (Error.show error))

source/parser/parser.mli

@@ -9,10 +9,26 @@ open Ast
 
 exception Error of string
 
+module Error : sig
+  type t = {
+    location: Location.t;
+    file_name: string;
+    content: string option;
+  }
+  [@@deriving show]
+end
+
 val parse
   :  ?start_line:int ->
   ?start_column:int ->
   ?relative:string ->
   string list ->
-  Statement.t list
+  (Statement.t list, Error.t) Result.t
 (** Parse python source. ?handle is path relative to the file's source root, if any. *)
+
+val parse_exn
+  :  ?start_line:int ->
+  ?start_column:int ->
+  ?relative:string ->
+  string list ->
+  Statement.t list

source/parser/test/generatorTest.ml

@@ -5464,12 +5464,13 @@ let test_setitem _ =
 
 
 let test_byte_order_mark _ =
+  let parse lines = PyreParser.Parser.parse_exn lines |> ignore in
   let byte_order_mark = [0xEF; 0xBB; 0xBF] |> List.map ~f:Char.of_int_exn |> String.of_char_list in
   (* Ensure that we can parse UTF-8 files with byte order marks properly. *)
-  PyreParser.Parser.parse [byte_order_mark ^ "1"] |> ignore;
+  parse [byte_order_mark ^ "1"];
   assert_raises
     (PyreParser.Parser.Error "Could not parse file at $invalid_path:2:0-2:0\n  \239\187\1912\n  ^")
-    (fun () -> PyreParser.Parser.parse ["1"; byte_order_mark ^ "2"])
+    (fun () -> parse ["1"; byte_order_mark ^ "2"])
 
 
 let test_walrus_operator _ =

source/server/query.ml

@@ -294,17 +294,19 @@ let help () =
 let rec parse_request_exn query =
   let open Expression in
   match PyreParser.Parser.parse [query] with
-  | [
-   {
-     Node.value =
-       Expression
-         {
-           Node.value = Call { callee = { Node.value = Name (Name.Identifier name); _ }; arguments };
-           _;
-         };
-     _;
-   };
-  ] -> (
+  | Ok
+      [
+        {
+          Node.value =
+            Expression
+              {
+                Node.value =
+                  Call { callee = { Node.value = Name (Name.Identifier name); _ }; arguments };
+                _;
+              };
+          _;
+        };
+      ] -> (
       let expression { Call.Argument.value; _ } = value in
       let access = function
         | { Call.Argument.value; _ } when has_identifier_base value -> value
@@ -352,9 +354,9 @@ let rec parse_request_exn query =
       | "validate_taint_models", [] -> ValidateTaintModels None
       | "validate_taint_models", [argument] -> Request.ValidateTaintModels (Some (string argument))
       | _ -> raise (InvalidQuery "unexpected query") )
-  | _ when String.equal query "help" -> Help (help ())
-  | _ -> raise (InvalidQuery "unexpected query")
-  | exception PyreParser.Parser.Error _ -> raise (InvalidQuery "failed to parse query")
+  | Ok _ when String.equal query "help" -> Help (help ())
+  | Ok _ -> raise (InvalidQuery "unexpected query")
+  | Error _ -> raise (InvalidQuery "failed to parse query")
 
 
 let parse_request query =