Align ReScript string escapes with JS strict-mode

[kevinbarabash]

This fixes the issue raised in #469. I added more test cases to verify that the behaviour is also consistent across string literal constants and string literals that appear in other places such as patterns and object keys. This means the following:

• octal escapes using \o are ignored since since that's how they're handled in JavaScript strings
• numeric escapes, e.g. \123 are not allow in .res(i) files except for \0
• numeric escapes are allowed in .ml(i) files and converted from decimal to hexadecimal escapes which JavaScript strings support

Test Plan:

• make
• make roundtrip-test

TODO:

• investigate warnings for .re files in roundtrip-test (I see the same warnings on master)
• [ ] special case \000 to \0 instead of \x00? It's not possible to do this consistently since \0004 can't be converted to \04 since \04 is not allowed in strings in JavaScript's strict mode. It's easier to use \x00 even if \0 is more succinct.

[kevinbarabash]

I converted tests strings using the playground and copy/pasting the string from the JS output.

[kevinbarabash]

I just ran make roundtrip-test and am seeing a bunch of test failures. This is because the printer still outputs decimal escapes. This was something I was planning on fixing anyways in order to address #468.

[kevinbarabash]

make roundtrip-test is still failing, but this appears limited to the sexpr tests. This is because we store the string literal as parsed in the parse tree so the first time we parse an .ml file we get the ml escapes and then when we parse the .res output we get the js escapes.

We could change the parse tree to store the js escapes. This will mean we'd have to update the ml printer to convert those escapes back to ml, but given the focus of this project moving forward appears to the res syntax, I think that makes sense.

[kevinbarabash]

make rounder-trip test is now passing:

$  make roundtrip-test                                                        
ocamlopt.opt -g -w +a-4-42-40-9-48 -warn-error +a -bin-annot -I +compiler-libs -I src -I tests -absname -c tests/res_test.ml
ocamlopt.opt -g -w +a-4-42-40-9-48 -warn-error +a -bin-annot -I +compiler-libs -I src -I tests -absname -O2 -o ./lib/test.exe -bin-annot -I +compiler-libs ocamlcommon.cmxa str.cmxa -I src -I tests src/reactjs_jsx_ppx_v3.cmx src/res_io.cmx src/res_minibuffer.cmx src/res_doc.cmx src/res_comment.cmx src/res_token.cmx src/res_grammar.cmx src/res_reporting.cmx src/res_diagnostics_printing_utils.cmx src/res_diagnostics.cmx src/res_parsetree_viewer.cmx src/res_parens.cmx src/res_comments_table.cmx src/res_utf8.cmx src/res_printer.cmx src/res_scanner.cmx src/res_js_ffi.cmx src/res_parser.cmx src/res_core.cmx src/res_driver.cmx src/res_ast_conversion.cmx src/res_driver_ml_parser.cmx src/res_driver_reason_binary.cmx src/res_driver_binary.cmx src/res_ast_debugger.cmx src/res_outcome_printer.cmx src/res_multi_printer.cmx tests/res_utf8_test.cmx tests/res_test.cmx
./node_modules/.bin/reanalyze -all-cmt . -suppress tests

  Exception Analysis
  File "./src/res_printer.ml", line 496, characters 4-20
  convertDecEscape might raise Failure (res_printer.ml:498:15) Invalid_argument (res_printer.ml:499:11 res_printer.ml:500:11 res_printer.ml:501:29 res_printer.ml:501:47) and is not annotated with @raises Failure Invalid_argument

  Exception Analysis
  File "./src/res_ast_debugger.ml", line 67, characters 6-22
  convertDecEscape might raise Failure (res_ast_debugger.ml:69:17) Invalid_argument (res_ast_debugger.ml:70:13 res_ast_debugger.ml:71:13 res_ast_debugger.ml:72:31 res_ast_debugger.ml:72:49) and is not annotated with @raises Failure Invalid_argument
  
  Analysis reported 2 issues (Exception Analysis:2)
./lib/test.exe
✅ multi printer api tests
File "tests/oprint/oprint.res", line 342, characters 9-13:
Warning 16: this optional argument cannot be erased.
✅ Parser make: initializes parser and checking offsets
✅ Parser handles LF correct
✅ Parser handles CRLF correct
✅ utf8 tests
ROUNDTRIP_TEST=1 ./test.sh
✅ No unstaged tests difference.
Running roundtrip tests…
File "tests/idempotency/nook-exchange/App.re", line 79, characters 55-57:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/nook-exchange/App.re", line 79, characters 62-64:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/nook-exchange/ImportPage.re", line 765, characters 52-54:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/nook-exchange/ImportPage.re", line 765, characters 59-61:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/nook-exchange/ItemFilters.re", line 230, characters 38-40:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/nook-exchange/ItemFilters.re", line 230, characters 45-47:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/nook-exchange/ItemFilters.re", line 927, characters 52-54:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/nook-exchange/ItemFilters.re", line 927, characters 59-61:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/reasonml.org/common/BeltData.re", line 2, characters 49-51:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/reasonml.org/common/BeltData.re", line 2, characters 51-53:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/reasonml.org/common/BeltData.re", line 2, characters 55-57:
Warning 14: illegal backslash escape in string.
File "tests/idempotency/wildcards-world-ui/Helper.re", line 6, characters 27-29:
Warning 14: illegal backslash escape in string.
✅ Roundtrip tests succeeded.

[cristianoc]

@kevinbarabash this is probably a good time to revisit this task, if still interested.
A bunch of things have changed, but the scanStringEscapeSequence function still exists.

[kevinbarabash]

@cristianoc thanks for the ping. I don't have time to push this forward right now. Given that things have changed it's probably best for someone to look at this with fresh eyes.