Fix named_parameters when string literals contain comment markers (#2783)

named_parameters stripped SQL comments before string literals in
separate passes. A string literal such as '-- TODO' would be treated
as the start of a line comment, swallowing the rest of the line and
hiding any named parameters that followed it. For example:

    select * from t where note = '-- TODO' and id = :id

returned [] instead of ['id'], so the query parameter input form
would be missing the :id field.

Match comments and string literals in a single left-to-right pass so
that whichever construct starts first wins, matching how SQL is
actually tokenized.

Co-authored-by: JSap0914 <JSap0914@users.noreply.github.com>

datasette/utils/__init__.py

@@ -1288,10 +1288,18 @@ class StartupError(Exception):
     pass
 
 
-_single_line_comment_re = re.compile(r"--.*")
-_multi_line_comment_re = re.compile(r"/\*.*?\*/", re.DOTALL)
-_single_quote_re = re.compile(r"'(?:''|[^'])*'")
-_double_quote_re = re.compile(r'"(?:\"\"|[^"])*"')
+# Comments and string literals, matched in a single pass so that whichever
+# construct starts first "wins" - this ensures a comment marker inside a string
+# literal (or a quote inside a comment) does not confuse the parameter scan.
+_comments_and_strings_re = re.compile(
+    r"""
+    --[^\n]*            # single line comment
+    | /\*.*?\*/         # multi line comment
+    | '(?:''|[^'])*'    # single quoted string ('' escapes a quote)
+    | "(?:""|[^"])*"    # double quoted identifier ("" escapes a quote)
+    """,
+    re.DOTALL | re.VERBOSE,
+)
 _named_param_re = re.compile(r":(\w+)")
 
 
@@ -1302,10 +1310,9 @@ def named_parameters(sql: str) -> List[str]:
 
     e.g. for ``select * from foo where id=:id`` this would return ``["id"]``
     """
-    sql = _single_line_comment_re.sub("", sql)
-    sql = _multi_line_comment_re.sub("", sql)
-    sql = _single_quote_re.sub("", sql)
-    sql = _double_quote_re.sub("", sql)
+    # Strip comments and string literals first so that any ":name" sequences
+    # inside them are not mistaken for named parameters
+    sql = _comments_and_strings_re.sub("", sql)
     # Extract parameters from what is left
     return _named_param_re.findall(sql)
 

tests/test_utils.py

@@ -756,6 +756,16 @@ def test_parse_metadata(content, expected):
         ("select 1 + :one + :two", ["one", "two"]),
         ("select 'bob' || '0:00' || :cat", ["cat"]),
         ("select this is invalid :one, :two, :three", ["one", "two", "three"]),
+        # A string literal containing a comment marker should not hide
+        # parameters that come after it
+        ("select * from t where note = '-- TODO' and id = :id", ["id"]),
+        ("select '--' || :y", ["y"]),
+        ("select * from t where note = '/* x */' and id = :id", ["id"]),
+        # Parameters that live inside a comment should be ignored
+        ("select :x -- and :ignored", ["x"]),
+        ("select :x /* and :ignored */ from t", ["x"]),
+        # Parameters inside a string literal should be ignored
+        ("select ':ignored' || :real", ["real"]),
     ),
 )
 @pytest.mark.parametrize("use_async_version", (False, True))