Merge pull request #111 from rgrinberg/longident-bug

Add tests for parsing longident's with .

CHANGES.md

@@ -1,6 +1,9 @@
 unreleased
 -------------------
 
+- Fix `Longident.parse` so it properly handles unparenthesized dotted operators
+  such as `+.` or `*.`. (#111, @rgrinberg, @NathanReb)
+
 - raising an exception does no longer cancel the whole context free phase(#453, @burnleydev1)
 
 - Sort embedded errors that are appended to the AST by location so the compiler

src/longident.ml

@@ -51,10 +51,14 @@ let parse s =
   let invalid () =
     invalid_arg (Printf.sprintf "Ppxlib.Longident.parse: %S" s)
   in
-  match (String.index_opt s '(', String.rindex_opt s ')') with
-  | None, None -> parse_simple s
-  | None, _ | _, None -> invalid ()
-  | Some l, Some r -> (
+  if String.length s < 1 then invalid ();
+  let open_par = String.index_opt s '(' in
+  let close_par = String.index_opt s ')' in
+  match (s.[0], open_par, close_par) with
+  | ('A' .. 'Z' | 'a' .. 'z' | '_'), None, None -> parse_simple s
+  | _, None, None -> Lident s (* This is a raw operator, no module path *)
+  | _, None, _ | _, _, None -> invalid ()
+  | _, Some l, Some r -> (
       if Int.(r <> String.length s - 1) then invalid ();
       let group =
         if Int.(r = l + 1) then "()"

test/base/test.ml

@@ -114,6 +114,28 @@ let _ = convert_longident ")"
 Exception: Invalid_argument "Ppxlib.Longident.parse: \")\"".
 |}]
 
+let _ = convert_longident "+."
+[%%expect{|
+- : string * longident = ("( +. )", Ppxlib.Longident.Lident "+.")
+|}]
+
+let _ = convert_longident "(+.)"
+[%%expect{|
+- : string * longident = ("( +. )", Ppxlib.Longident.Lident "+.")
+|}]
+
+let _ = convert_longident "Foo.(+.)"
+[%%expect{|
+- : string * longident =
+("Foo.( +. )", Ppxlib.Longident.Ldot (Ppxlib.Longident.Lident "Foo", "+."))
+|}]
+
+let _ = convert_longident "Foo.( *. )"
+[%%expect{|
+- : string * longident =
+("Foo.( *. )", Ppxlib.Longident.Ldot (Ppxlib.Longident.Lident "Foo", "*."))
+|}]
+
 let _ = Ppxlib.Code_path.(file_path @@ top_level ~file_path:"dir/main.ml")
 [%%expect{|
 - : string = "dir/main.ml"