Skip to content

Commit 8fbef8b

Browse files
authored
fix: safe annotation quotes (#7028)
## 📝 Summary fixed #7026 If annotations without top-level resolution were quoted and containing quotes themselves, syntax breaking serialization occurred rendering a notebook un-openable. This fixes this particular issue and additionally adds a defense guard such that invalid code should never be serialized.
1 parent 837ba05 commit 8fbef8b

2 files changed

Lines changed: 105 additions & 2 deletions

File tree

marimo/_ast/codegen.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,9 @@ def to_annotated_string(
246246
annotation = variable.annotation_data
247247
if annotation:
248248
if annotation.refs - allowed_refs:
249-
response[name] = f'"{annotation.repr}"'
249+
# replace unescaped quotes with escaped quotes
250+
safe_repr = re.sub(r'(?<!\\)"', r'\\"', annotation.repr)
251+
response[name] = f'"{safe_repr}"'
250252
else:
251253
response[name] = annotation.repr
252254
return response
@@ -419,6 +421,25 @@ def serialize_cell(
419421
raise ValueError("Unknown cell status, please report this issue.")
420422

421423

424+
def safe_serialize_cell(
425+
extraction: TopLevelExtraction, status: TopLevelStatus
426+
) -> str:
427+
"""Additional defensive layer- we should _never_ generate invalid code."""
428+
code = serialize_cell(extraction, status)
429+
try:
430+
ast_parse(code)
431+
except SyntaxError as e:
432+
LOGGER.warning(
433+
f"Generated code for cell {status.name} is invalid, "
434+
"falling back to unparsable cell. Please report this error. "
435+
f"Error: {e}"
436+
)
437+
return generate_unparsable_cell(
438+
code=status.code, config=status.cell_config, name=status.name
439+
)
440+
return code
441+
442+
422443
def generate_app_constructor(config: Optional[_AppConfig]) -> str:
423444
updates = {}
424445
# only include a config setting if it's not a default setting, to
@@ -461,7 +482,9 @@ def generate_filecontents(
461482
if setup_cell:
462483
toplevel_defs = set(setup_cell.defs)
463484
extraction = TopLevelExtraction(codes, names, cell_configs, toplevel_defs)
464-
cell_blocks = [serialize_cell(extraction, status) for status in extraction]
485+
cell_blocks = [
486+
safe_serialize_cell(extraction, status) for status in extraction
487+
]
465488

466489
filecontents = []
467490
if header_comments is not None:

tests/_ast/test_codegen.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from pathlib import Path
99
from textwrap import dedent
1010
from typing import Any, Optional
11+
from unittest.mock import patch
1112

1213
import codegen_data.test_main as mod
1314
import pytest
@@ -449,6 +450,85 @@ def test_quote_standardization_edge_cases(self) -> None:
449450
)
450451
assert fndef == expected
451452

453+
def test_quote_nested_edge_cases(self) -> None:
454+
"""Test edge cases for quote standardization in type annotations."""
455+
# Test mixed quotes where double quotes are preserved
456+
referring = 'x: tuple[tuple[Literal["foo", \'bar\']]] = "((foo,),)"'
457+
ref_vars = compile_cell(referring).init_variable_data
458+
459+
code = "z = x"
460+
cell = compile_cell(code)
461+
fndef = codegen.to_functiondef(
462+
cell, "foo", allowed_refs={"tuple"}, variable_data=ref_vars
463+
)
464+
expected = "\n".join(
465+
[
466+
"@app.cell",
467+
'def foo(x: "tuple[tuple[Literal[\\"foo\\", \\"bar\\"]]]"):',
468+
" z = x",
469+
" return (z,)",
470+
]
471+
)
472+
assert fndef == expected
473+
474+
def test_quote_nested_esscaped_edge_cases(self) -> None:
475+
referring = "x: Literal['say \"hello\"'] = 'say \"hello\"'"
476+
ref_vars = compile_cell(referring).init_variable_data
477+
478+
code = "z = x"
479+
cell = compile_cell(code)
480+
fndef = codegen.to_functiondef(
481+
cell, "foo", allowed_refs=set(), variable_data=ref_vars
482+
)
483+
expected = "\n".join(
484+
[
485+
"@app.cell",
486+
'def foo(x: "Literal[\'say \\"hello\\"\']"):',
487+
" z = x",
488+
" return (z,)",
489+
]
490+
)
491+
assert fndef == expected
492+
493+
def test_safe_serialize_cell_handles_syntax_error(self) -> None:
494+
"""Test that safe_serialize_cell falls back when ast_parse fails.
495+
496+
This test mocks ast_parse to fail, exercising the except block in
497+
safe_serialize_cell that falls back to generate_unparsable_cell.
498+
Goes through the full codegen path via generate_filecontents.
499+
"""
500+
# Create a simple cell that would normally serialize fine
501+
code = "x = 1"
502+
name = "test_cell"
503+
504+
# Mock ast_parse to raise SyntaxError and mock the logger
505+
with (
506+
patch("marimo._ast.codegen.ast_parse") as mock_ast_parse,
507+
patch("marimo._ast.codegen.LOGGER.warning") as mock_warning,
508+
):
509+
mock_ast_parse.side_effect = SyntaxError("Mock syntax error")
510+
511+
# Go through the full codegen path
512+
result = wrap_generate_filecontents([code], [name])
513+
514+
# Verify ast_parse was called
515+
assert mock_ast_parse.called
516+
517+
# Verify the result contains an unparsable cell format
518+
# (it should contain the original code)
519+
assert "x = 1" in result
520+
# Unparsable cells use app._unparsable_cell wrapper
521+
assert "app._unparsable_cell" in result
522+
# Verify it has the cell name
523+
assert name in result
524+
525+
# Verify warning was logged with the correct message
526+
assert mock_warning.called
527+
warning_message = mock_warning.call_args[0][0]
528+
assert "falling back to unparsable cell" in warning_message
529+
assert name in warning_message
530+
assert "Mock syntax error" in warning_message
531+
452532
@staticmethod
453533
def test_generate_app_constructor_with_auto_download() -> None:
454534
config = _AppConfig(

0 commit comments

Comments
 (0)