Repository: geldata/gel
Pull request: enable multiple string prefixes
fixes https://github.com/edgedb/edgedb/issues/2332

Problem statement:
Feature request: multiple string prefixes at once
<!-- Please search existing issues to avoid creating duplicates. -->

<!-- Describe the feature you'd like to see implemented in EdgeDB. -->
Currently you must use either `r` or `b`, not both:
```
edgedb> br"";
error: prefix "br" is not allowed for strings, allowed: `b`, `r`
  ┌─ query:1:1
  │
1 │ br"";
  │ ^ error
```
Prefixes can be combined in python, would be nice if EdgeDB could support this too.

Hints from issue discussion:
Yeah, it's an oversight we don't have them.

Commit message: add functional test

Patch to review:
diff --git a/edb/edgeql-parser/src/tokenizer.rs b/edb/edgeql-parser/src/tokenizer.rs
index 15e4a280725..23f8af6f0b0 100644
--- a/edb/edgeql-parser/src/tokenizer.rs
+++ b/edb/edgeql-parser/src/tokenizer.rs
@@ -339,6 +339,8 @@ impl<'a> TokenStream<'a> {
                             let (raw, binary) = match prefix {
                                 "r" => (true, false),
                                 "b" => (false, true),
+                                "rb" => (true, true),
+                                "br" => (true, true),
                                 _ => return Err(Error::unexpected_format(
                                     format_args!("prefix {:?} \
                                     is not allowed for strings, \
diff --git a/edb/edgeql-parser/tests/tokenizer.rs b/edb/edgeql-parser/tests/tokenizer.rs
index 2d691e212d9..8fe37efd5bd 100644
--- a/edb/edgeql-parser/tests/tokenizer.rs
+++ b/edb/edgeql-parser/tests/tokenizer.rs
@@ -557,6 +557,10 @@ fn strings() {
     assert_eq!(tok_typ(r#" b""  "#), [BinStr]);
     assert_eq!(tok_str(r#" b''  "#), [r#"b''"#]);
     assert_eq!(tok_typ(r#" b''  "#), [BinStr]);
+    assert_eq!(tok_str(r#" br""  "#), [r#"br"""#]);
+    assert_eq!(tok_typ(r#" br""  "#), [BinStr]);
+    assert_eq!(tok_str(r#" br''  "#), [r#"br''"#]);
+    assert_eq!(tok_typ(r#" br''  "#), [BinStr]);
     assert_eq!(tok_err(r#" ``  "#),
         "Unexpected `backtick quotes cannot be empty`");
 
@@ -572,6 +576,10 @@ fn strings() {
     assert_eq!(tok_typ(r#" b"hello"  "#), [BinStr]);
     assert_eq!(tok_str(r#" b'hello'  "#), [r#"b'hello'"#]);
     assert_eq!(tok_typ(r#" b'hello'  "#), [BinStr]);
+    assert_eq!(tok_str(r#" rb"hello"  "#), [r#"rb"hello""#]);
+    assert_eq!(tok_typ(r#" rb"hello"  "#), [BinStr]);
+    assert_eq!(tok_str(r#" rb'hello'  "#), [r#"rb'hello'"#]);
+    assert_eq!(tok_typ(r#" rb'hello'  "#), [BinStr]);
     assert_eq!(tok_str(r#" `hello`  "#), [r#"`hello`"#]);
     assert_eq!(tok_typ(r#" `hello`  "#), [BacktickName]);
 
@@ -587,6 +595,10 @@ fn strings() {
     assert_eq!(tok_typ(r#" b"hello""#), [BinStr]);
     assert_eq!(tok_str(r#" b'hello'"#), [r#"b'hello'"#]);
     assert_eq!(tok_typ(r#" b'hello'"#), [BinStr]);
+    assert_eq!(tok_str(r#" rb"hello""#), [r#"rb"hello""#]);
+    assert_eq!(tok_typ(r#" rb"hello""#), [BinStr]);
+    assert_eq!(tok_str(r#" rb'hello'"#), [r#"rb'hello'"#]);
+    assert_eq!(tok_typ(r#" rb'hello'"#), [BinStr]);
     assert_eq!(tok_str(r#" `hello`"#), [r#"`hello`"#]);
     assert_eq!(tok_typ(r#" `hello`"#), [BacktickName]);
 
@@ -602,6 +614,10 @@ fn strings() {
     assert_eq!(tok_typ(r#" b"h\"ello" "#), [BinStr]);
     assert_eq!(tok_str(r#" b'h\'ello' "#), [r#"b'h\'ello'"#]);
     assert_eq!(tok_typ(r#" b'h\'ello' "#), [BinStr]);
+    assert_eq!(tok_str(r#" rb"hello\" "#), [r#"rb"hello\""#]);
+    assert_eq!(tok_typ(r#" rb"hello\" "#), [BinStr]);
+    assert_eq!(tok_str(r#" rb'hello\' "#), [r#"rb'hello\'"#]);
+    assert_eq!(tok_typ(r#" rb'hello\' "#), [BinStr]);
     assert_eq!(tok_str(r#" `hello\` "#), [r#"`hello\`"#]);
     assert_eq!(tok_typ(r#" `hello\` "#), [BacktickName]);
     assert_eq!(tok_str(r#" `hel``lo` "#), [r#"`hel``lo`"#]);
@@ -619,6 +635,10 @@ fn strings() {
     assert_eq!(tok_typ(r#" b"h'el`lo" "#), [BinStr]);
     assert_eq!(tok_str(r#" b'h"el`lo' "#), [r#"b'h"el`lo'"#]);
     assert_eq!(tok_typ(r#" b'h"el`lo' "#), [BinStr]);
+    assert_eq!(tok_str(r#" rb"h'el`lo" "#), [r#"rb"h'el`lo""#]);
+    assert_eq!(tok_typ(r#" rb"h'el`lo" "#), [BinStr]);
+    assert_eq!(tok_str(r#" rb'h"el`lo' "#), [r#"rb'h"el`lo'"#]);
+    assert_eq!(tok_typ(r#" rb'h"el`lo' "#), [BinStr]);
     assert_eq!(tok_str(r#" `h'el"lo` "#), [r#"`h'el"lo`"#]);
     assert_eq!(tok_typ(r#" `h'el"lo\` "#), [BacktickName]);
 
@@ -634,6 +654,10 @@ fn strings() {
     assert_eq!(tok_typ(" b\"hel\nlo\" "), [BinStr]);
     assert_eq!(tok_str(" b'hel\nlo' "), ["b'hel\nlo'"]);
     assert_eq!(tok_typ(" b'hel\nlo' "), [BinStr]);
+    assert_eq!(tok_typ(" rb'hel\nlo' "), [BinStr]);
+    assert_eq!(tok_typ(" br'hel\nlo' "), [BinStr]);
+    assert_eq!(tok_str(" rb'hel\nlo' "), ["rb'hel\nlo'"]);
+    assert_eq!(tok_str(" br'hel\nlo' "), ["br'hel\nlo'"]);
     assert_eq!(tok_str(" `hel\nlo` "), ["`hel\nlo`"]);
     assert_eq!(tok_typ(" `hel\nlo` "), [BacktickName]);
 
diff --git a/edb/edgeql-rust/src/tokenizer.rs b/edb/edgeql-rust/src/tokenizer.rs
index 3ba505e2722..ca530419bfb 100644
--- a/edb/edgeql-rust/src/tokenizer.rs
+++ b/edb/edgeql-rust/src/tokenizer.rs
@@ -551,7 +551,7 @@ fn convert(py: Python, tokens: &Tokens, cache: &mut Cache,
             Ok((tokens.bconst.clone_ref(py),
                 PyString::new(py, value),
                 PyBytes::new(py,
-                    &unquote_bytes(&value[2..value.len()-1])
+                    &unquote_bytes(value)
                     .map_err(|s| TokenizerError::new(py,
                         (s, py_pos(py, &token.start))))?)
                    .into_object()))
@@ -654,7 +654,15 @@ impl<'a> From<SpannedToken<'a>> for CowToken<'a> {
     }
 }
 
-fn unquote_bytes<'a>(s: &'a str) -> Result<Vec<u8>, String> {
+fn unquote_bytes<'a>(value: &'a str) -> Result<Vec<u8>, String> {
+    if value.starts_with("rb") || value.starts_with("br") {
+        Ok(value[3..value.len() -1].as_bytes().to_vec())
+    } else {
+        Ok(_unquote_bytes(&value[2..value.len()-1])?.into())
+    }
+}
+
+fn _unquote_bytes<'a>(s: &'a str) -> Result<Vec<u8>, String> {
     let mut res = Vec::with_capacity(s.len());
     let mut bytes = s.as_bytes().iter();
     while let Some(&c) = bytes.next() {
@@ -712,32 +720,32 @@ fn unquote_bytes<'a>(s: &'a str) -> Result<Vec<u8>, String> {
 
 #[test]
 fn simple_bytes() {
-    assert_eq!(unquote_bytes(r#"\x09"#).unwrap(), b"\x09");
-    assert_eq!(unquote_bytes(r#"\x0A"#).unwrap(), b"\x0A");
-    assert_eq!(unquote_bytes(r#"\x0D"#).unwrap(), b"\x0D");
-    assert_eq!(unquote_bytes(r#"\x20"#).unwrap(), b"\x20");
+    assert_eq!(_unquote_bytes(r#"\x09"#).unwrap(), b"\x09");
+    assert_eq!(_unquote_bytes(r#"\x0A"#).unwrap(), b"\x0A");
+    assert_eq!(_unquote_bytes(r#"\x0D"#).unwrap(), b"\x0D");
+    assert_eq!(_unquote_bytes(r#"\x20"#).unwrap(), b"\x20");
 }
 
 #[test]
 fn newline_escaping_bytes() {
-    assert_eq!(unquote_bytes(r"hello \
+    assert_eq!(_unquote_bytes(r"hello \
                                 world").unwrap(), b"hello world");
 
-    assert_eq!(unquote_bytes(r"bb\
+    assert_eq!(_unquote_bytes(r"bb\
 aa \
             bb").unwrap(), b"bbaa bb");
-    assert_eq!(unquote_bytes(r"bb\
+    assert_eq!(_unquote_bytes(r"bb\
 
         aa").unwrap(), b"bbaa");
-    assert_eq!(unquote_bytes(r"bb\
+    assert_eq!(_unquote_bytes(r"bb\
         \
         aa").unwrap(), b"bbaa");
-    assert_eq!(unquote_bytes("bb\\\r   aa").unwrap(), b"bbaa");
-    assert_eq!(unquote_bytes("bb\\\r\n   aa").unwrap(), b"bbaa");
+    assert_eq!(_unquote_bytes("bb\\\r   aa").unwrap(), b"bbaa");
+    assert_eq!(_unquote_bytes("bb\\\r\n   aa").unwrap(), b"bbaa");
 }
 
 #[test]
 fn complex_bytes() {
-    assert_eq!(unquote_bytes(r#"\x09 hello \x0A there"#).unwrap(),
+    assert_eq!(_unquote_bytes(r#"\x09 hello \x0A there"#).unwrap(),
         b"\x09 hello \x0A there");
 }
diff --git a/tests/test_edgeql_expressions.py b/tests/test_edgeql_expressions.py
index 1bfe87b5577..be18d9d0e07 100644
--- a/tests/test_edgeql_expressions.py
+++ b/tests/test_edgeql_expressions.py
@@ -2216,6 +2216,15 @@ async def test_edgeql_expr_bytes_op_02(self):
             [True],
         )
 
+    async def test_edgeql_expr_bytes_op_03(self):
+        await self.assert_query_result(
+            r'''
+                WITH x := rb'test\raw\x01' ++ br'\now\x02' ++ b'\x03\x04',
+                SELECT x = b"test\\raw\\x01\\now\\x02\x03\x04";
+            ''',
+            [True],
+        )
+
     async def test_edgeql_expr_paths_01(self):
         cases = [
             "Issue.owner.name",


Code review comments:
diff_hunk: @@ -654,7 +654,15 @@ impl<'a> From<SpannedToken<'a>> for CowToken<'a> {
     }
 }
 
-fn unquote_bytes<'a>(s: &'a str) -> Result<Vec<u8>, String> {
+fn unquote_bytes<'a>(value: &'a str) -> Result<Vec<u8>, String> {
+    if value.starts_with("rb") || value.starts_with("br") {
path: edb/edgeql-rust/src/tokenizer.rs
text: @user1:
Let's do something like this:
```
let idx = value.find(|c| c == '\'' || c == '"').ok_or_else(...)?;
let prefix = value[..idx]
match prefix {
    "rb" | "br" => 
    "b" => 
    _ => Err(format!("unimplemented prefix {:?}", prefix)),
}
```
I think this would make potential new syntaxes safer.

@author:
changed