Merge pull request #7 from mbarbin/range-to-end-of-file

Range to end of file

CHANGES.md

@@ -1,3 +1,9 @@
+## 0.2.2 (2024-11-04)
+
+### Fixed
+
+- Fix `Offset.to_position` to allow the end-of-file offset instead of raising (#7, @mbarbin).
+
 ## 0.2.1 (2024-11-04)
 
 ### Added

src/loc.ml

@@ -132,7 +132,7 @@ module File_cache = struct
             ; pos_bol
             }))
     in
-    if pos_cnum < 0 || pos_cnum >= t.length
+    if pos_cnum < 0 || pos_cnum > t.length
     then raise (Invalid_argument "Loc.File_cache.position")
     else binary_search ~from:0 ~to_:(Array.length t.bols - 1)
   ;;

src/loc.mli

@@ -138,14 +138,19 @@ module Offset : sig
   (** Reading the [pos_cnum] of a lexing position. *)
   val of_position : Lexing.position -> t
 
-  (** Rebuild the position from a file at the given offset. *)
+  (** Rebuild the position from a file at the given offset. Raises
+      [Invalid_argument] if [offset < 0] or if [offset > file.length]. The
+      end-of-file position [file.length] is purposely allowed in order for the
+      resulting position to be used as [stop] value for a range covering the
+      file until its last character (which may occasionally be useful). *)
   val to_position : t -> file_cache:File_cache.t -> Lexing.position
 end
 
 val start_offset : t -> Offset.t
 val stop_offset : t -> Offset.t
 
-(** A convenient wrapper to build a loc from the position at a given offset. *)
+(** A convenient wrapper to build a loc from the position at a given offset. See
+    {!val:Offset.to_position}. *)
 val of_file_offset : file_cache:File_cache.t -> offset:Offset.t -> t
 
 module Range : sig
@@ -166,7 +171,9 @@ end
 
 val range : t -> Range.t
 
-(** A convenient wrapper to build a loc from a file range. *)
+(** A convenient wrapper to build a loc from a file range. Raises
+    [Invalid_argument] if the range is not valid for that file. See
+    {!val:Offset.to_position}. *)
 val of_file_range : file_cache:File_cache.t -> range:Range.t -> t
 
 module Txt : sig

test/test__file_cache.ml

@@ -180,7 +180,7 @@ let%expect_test "of_file_offset" =
   in
   require_does_raise [%here] (fun () -> test (-1));
   [%expect {| (Invalid_argument Loc.File_cache.position) |}];
-  require_does_raise [%here] (fun () -> test (String.length file_contents));
+  require_does_raise [%here] (fun () -> test (String.length file_contents + 1));
   [%expect {| (Invalid_argument Loc.File_cache.position) |}];
   test 0;
   [%expect {| File "foo.txt", line 1, characters 0-0: |}];
@@ -190,6 +190,8 @@ let%expect_test "of_file_offset" =
   [%expect {| File "foo.txt", line 1, characters 10-10: |}];
   test 11;
   [%expect {| File "foo.txt", line 2, characters 0-0: |}];
+  test (String.length file_contents);
+  [%expect {| File "foo.txt", line 11, characters 0-0: |}];
   ()
 ;;
 
@@ -209,8 +211,10 @@ let%expect_test "of_file_offset more" =
     let loc = Loc.of_file_offset ~file_cache ~offset in
     print_endline (Loc.to_string loc)
   in
-  require_does_raise [%here] (fun () -> test (String.length file_contents));
+  require_does_raise [%here] (fun () -> test (String.length file_contents + 1));
   [%expect {| (Invalid_argument Loc.File_cache.position) |}];
+  test (String.length file_contents);
+  [%expect {| File "foo.txt", line 3, characters 0-0: |}];
   test (String.length file_contents - 1);
   [%expect {| File "foo.txt", line 2, characters 4-4: |}];
   let file_contents = "Hello\nFriendly\nWorld\n" in
@@ -228,8 +232,10 @@ let%expect_test "of_file_offset more" =
     let loc = Loc.of_file_offset ~file_cache ~offset in
     print_endline (Loc.to_string loc)
   in
-  require_does_raise [%here] (fun () -> test (String.length file_contents));
+  require_does_raise [%here] (fun () -> test (String.length file_contents + 1));
   [%expect {| (Invalid_argument Loc.File_cache.position) |}];
+  test (String.length file_contents);
+  [%expect {| File "foo.txt", line 4, characters 0-0: |}];
   test (String.length file_contents - 1);
   [%expect {| File "foo.txt", line 3, characters 5-5: |}];
   let file_contents = "Hello\nFriendly\nWorld" in
@@ -247,8 +253,10 @@ let%expect_test "of_file_offset more" =
     let loc = Loc.of_file_offset ~file_cache ~offset in
     print_endline (Loc.to_string loc)
   in
-  require_does_raise [%here] (fun () -> test (String.length file_contents));
+  require_does_raise [%here] (fun () -> test (String.length file_contents + 1));
   [%expect {| (Invalid_argument Loc.File_cache.position) |}];
+  test (String.length file_contents);
+  [%expect {| File "foo.txt", line 4, characters 0-0: |}];
   test (String.length file_contents - 1);
   [%expect {| File "foo.txt", line 3, characters 4-4: |}];
   ()
@@ -263,7 +271,7 @@ let%expect_test "of_file_range" =
   in
   require_does_raise [%here] (fun () -> test (-1) 0);
   [%expect {| (Invalid_argument Loc.File_cache.position) |}];
-  require_does_raise [%here] (fun () -> test 0 (String.length file_contents));
+  require_does_raise [%here] (fun () -> test 0 (String.length file_contents + 1));
   [%expect {| (Invalid_argument Loc.File_cache.position) |}];
   require_does_raise [%here] (fun () -> test 1 0);
   [%expect {| (Invalid_argument Loc.of_file_range) |}];
@@ -275,5 +283,9 @@ let%expect_test "of_file_range" =
   [%expect {| File "foo.txt", lines 1-2, characters 10-11: |}];
   test 11 15;
   [%expect {| File "foo.txt", line 2, characters 0-4: |}];
+  test 0 (String.length file_contents);
+  [%expect {| File "foo.txt", lines 1-11, characters 0-109: |}];
+  test 45 (String.length file_contents);
+  [%expect {| File "foo.txt", lines 5-11, characters 1-65: |}];
   ()
 ;;