Fix incorrect stripping of literal dollar sign in regexps (#6116)

* Fix incorrect stripping of '$' from the regexp '\$' (literal dollar sign)

* Add a workaround to allow querying the test program for '--help'
without requiring test data. The issue was raised at
mirage/alcotest#358

* Update changelog

* Add link to alcotest issue

* Clarify comment

* Rephrase

changelog.d/gh-5987.fixed

@@ -0,0 +1,2 @@
+Fixed incorrect stripping of '\$' (literal dollar sign) in regexps used in the
+context of `metavariable-regex`.

semgrep-core/Makefile

@@ -36,6 +36,11 @@ clean:
 #
 .PHONY: test
 test: all
+ # The test executable has a few options that can be useful
+ # in some contexts.
+ # The following command ensures that we can call 'test.exe --help'
+ # without having to chdir into the test data folder.
+ ./_build/default/tests/test.exe --help 2>&1 >/dev/null
  $(MAKE) -C src/spacegrep test
  dune runtest -f --no-buffer
 

semgrep-core/src/utils/Regexp_engine.ml

@@ -129,8 +129,9 @@ let remove_end_of_string_assertions_from_string src : string option =
  match (a0, a1) with
  | '^', '$' -> Some ""
  | '^', c -> String.make 1 c |> finish
- | c, '$' -> String.make 1 c |> finish
  | '\\', ('A' | 'Z' | 'z') -> Some ""
+ | '\\', _ -> Some src
+ | c, '$' -> String.make 1 c |> finish
  | _, _ -> src |> finish
  else
  (* "XXX" or longer *)
@@ -140,23 +141,21 @@ let remove_end_of_string_assertions_from_string src : string option =
  | '\\', 'A' -> String.sub src 2 (len - 2)
  | _ -> src
  in
- (* "X" or longer *)
+ (* remaining string: "X" or longer *)
  let len = String.length src in
  let z1 = src.[len - 1] in
  if len = 1 then
  match z1 with
  | '$' -> Some ""
  | _ -> src |> finish
  else
- (* "XX" or longer *)
+ (* remaining string: "XX" or longer *)
  let z0 = src.[len - 2] in
- let src =
- match (z0, z1) with
- | _, '$' -> String.sub src 0 (len - 1)
- | '\\', ('Z' | 'z') -> String.sub src 0 (len - 2)
- | _ -> src
- in
- finish src
+ match (z0, z1) with
+ | '\\', ('Z' | 'z') -> Some (String.sub src 0 (len - 2))
+ | '\\', _ -> Some src
+ | _, '$' -> String.sub src 0 (len - 1) |> finish
+ | _ -> src |> finish
 
 let remove_end_of_string_assertions (src_pat, _old) =
  match remove_end_of_string_assertions_from_string src_pat with

semgrep-core/src/utils/Unit_regexp_engine.ml

@@ -16,6 +16,11 @@ let test_remove_eos_assertions () =
  ("AA", Some "AA");
  ("AAA", Some "AAA");
  ("AAAA", Some "AAAA");
+ ({|\$|}, Some {|\$|});
+ ({|A\$|}, Some {|A\$|});
+ ({|AA\$|}, Some {|AA\$|});
+ ({|AAA\$|}, Some {|AAA\$|});
+ ({|AAAA\$|}, Some {|AAAA\$|});
  ("^", Some "");
  ("$", Some "");
  ({|\A|}, Some "");
@@ -30,10 +35,10 @@ let test_remove_eos_assertions () =
  ("^AAA$", Some "AAA");
  ("^^", None);
  ("$$", None);
- ({|A\A|}, None);
+ ({|A\A|}, Some {|A\A|});
  ("[$]*", None);
  ("(?:^)", None);
- ({|\\A|}, None);
+ ({|\\A|}, Some {|\\A|});
  ({|(?<!.|\n)|}, None);
  (* DIY beginning-of-string assertion = \A *)
  ({|(?!.|\n)|}, None) (* DIY end-of-string assertion = \z *);

semgrep-core/tests/Test.ml

@@ -74,8 +74,20 @@ let tests () = List.flatten [
 (* Entry point *)
 (*****************************************************************************)
 
+(*
+ This allows running the test program with '--help' from any folder
+ without getting an error due to not being able to load test data.
+
+ See https://github.com/mirage/alcotest/issues/358 for a request
+ to allow what we want without this workaround.
+*)
+let tests_with_delayed_error () =
+ try tests ()
+ with e ->
+ ["cannot load test data - not a real test", (fun () -> raise e)]
+
 let main () =
- let alcotest_tests = Testutil.to_alcotest (tests ()) in
+ let alcotest_tests = Testutil.to_alcotest (tests_with_delayed_error ()) in
  Alcotest.run "semgrep-core" alcotest_tests
 
 let () = main ()