Skip to content

Commit 2ffde78

Browse files
doma17doma17
authored
Fix csv.writer QUOTE_NONE with quotechar=None (#8201)
* Prevent csv QUOTE_NONE writer panics without quotechar Handle csv.writer rows with QUOTE_NONE through a RustPython-owned unquoted writer path so quotechar=None no longer reaches the unfinished csv_core configuration branch. Escape delimiter, newline, quotechar, and escapechar bytes according to the active dialect, and preserve CPython's single-empty-field error behavior. Constraint: Match CPython csv.writer behavior without changing Lib/ copied stdlib files. Rejected: Relying on csv_core QuoteStyle::Never | it does not model CPython QUOTE_NONE escaping with quotechar=None. Confidence: high Scope-risk: narrow Directive: Keep QUOTE_NONE writer behavior separate unless csv_core gains matching CPython semantics. Tested: prek run --all-files; cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher; cargo build --release --features sqlite; pytest -v in extra_tests Assisted-by: Codex:gpt-5.5 * Keep csv parity tests aligned with QUOTE_NONE support Remove the stale RustPython expected-failure marker for the CPython escaped-field writer test that now passes, and cover CR/LF escaping in the snippet regression requested during review. Constraint: RustPython test policy allows removing expectedFailure markers only when the upstream test passes. Rejected: Refactoring writer row boilerplate in this follow-up | review marked it as a heavy-lift nitpick and it would broaden the CI fix. Confidence: high Scope-risk: narrow Tested: cargo run --release --features sqlite -- -m test test_csv; cargo run -- extra_tests/snippets/stdlib_csv.py; PATH=/tmp/pyshim:$PATH prek run --all-files; git diff --check; cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher; pytest -v in extra_tests Assisted-by: Codex:gpt-5.5 * Keep csv quote-style dispatch coherent Review feedback pointed out that adjacent branches selected behavior from the same quoting discriminator. Use one match so future quote-style additions are routed in a single place. Constraint: RustPython review requested merging adjacent quote-style checks. Confidence: high Scope-risk: narrow Tested: cargo fmt --check Tested: target/release/rustpython extra_tests/snippets/stdlib_csv.py Tested: target/release/rustpython -m test test_csv Tested: PATH=/tmp/pyshim:$PATH prek run --all-files Tested: RUST_TEST_THREADS=1 cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher -- --test-threads=1 Tested: cargo clippy --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --all-targets Assisted-by: Codex:gpt-5.5
1 parent cdcb13d commit 2ffde78

3 files changed

Lines changed: 143 additions & 12 deletions

File tree

‎Lib/test/test_csv.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,6 @@ class EscapedExcel(csv.excel):
854854
class TestEscapedExcel(TestCsvBase):
855855
dialect = EscapedExcel()
856856

857-
@unittest.expectedFailure # TODO: RUSTPYTHON
858857
def test_escape_fieldsep(self):
859858
self.writerAssertEqual([['abc,def']], 'abc\\,def\r\n')
860859

‎crates/stdlib/src/csv.rs‎

Lines changed: 81 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -910,12 +910,8 @@ mod _csv {
910910
writer = writer.delimiter(t);
911911
}
912912

913-
if let Some(t) = self.quotechar {
914-
if let Some(u) = t {
915-
writer = writer.quote(u);
916-
} else {
917-
todo!()
918-
}
913+
if let Some(Some(t)) = self.quotechar {
914+
writer = writer.quote(t);
919915
}
920916

921917
if let Some(t) = self.doublequote {
@@ -1148,6 +1144,24 @@ mod _csv {
11481144
Ok(())
11491145
}
11501146

1147+
fn write_unquoted_field(
1148+
output: &mut Vec<u8>,
1149+
data: &[u8],
1150+
dialect: PyDialect,
1151+
vm: &VirtualMachine,
1152+
) -> PyResult<()> {
1153+
for &byte in data {
1154+
if field_needs_escape(byte, dialect) {
1155+
let escapechar = dialect
1156+
.escapechar
1157+
.ok_or_else(|| new_csv_error(vm, "need to escape, but no escapechar set"))?;
1158+
output.push(escapechar);
1159+
}
1160+
output.push(byte);
1161+
}
1162+
Ok(())
1163+
}
1164+
11511165
fn field_needs_quotes(data: &[u8], dialect: PyDialect) -> bool {
11521166
data.iter().any(|&byte| {
11531167
byte == dialect.delimiter
@@ -1157,6 +1171,14 @@ mod _csv {
11571171
})
11581172
}
11591173

1174+
fn field_needs_escape(byte: u8, dialect: PyDialect) -> bool {
1175+
byte == dialect.delimiter
1176+
|| dialect.quotechar == Some(byte)
1177+
|| dialect.escapechar == Some(byte)
1178+
|| matches!(byte, b'\r' | b'\n')
1179+
|| matches!(dialect.lineterminator, Terminator::Any(t) if byte == t)
1180+
}
1181+
11601182
fn write_lineterminator(output: &mut Vec<u8>, terminator: Terminator) {
11611183
match terminator {
11621184
Terminator::CRLF => output.extend_from_slice(b"\r\n"),
@@ -1222,13 +1244,61 @@ mod _csv {
12221244
self.write.call((s,), vm)
12231245
}
12241246

1247+
fn writerow_quote_none(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult {
1248+
let _state = self.state.lock();
1249+
1250+
let row: ArgIterable = ArgIterable::try_from_object(vm, row.clone()).map_err(|_e| {
1251+
new_csv_error(
1252+
vm,
1253+
format!("'{}' object is not iterable", row.class().name()),
1254+
)
1255+
})?;
1256+
1257+
let fields = row.iter(vm)?.collect::<PyResult<Vec<_>>>()?;
1258+
let single_field = fields.len() == 1;
1259+
let mut output = Vec::new();
1260+
1261+
for (index, field) in fields.into_iter().enumerate() {
1262+
if index > 0 {
1263+
output.push(self.dialect.delimiter);
1264+
}
1265+
1266+
let stringified;
1267+
let data: &[u8] = match_class!(match field {
1268+
ref s @ PyStr => s.as_bytes(),
1269+
crate::builtins::PyNone => b"",
1270+
ref obj => {
1271+
stringified = obj.str(vm)?;
1272+
stringified.as_bytes()
1273+
}
1274+
});
1275+
1276+
if single_field && data.is_empty() {
1277+
return Err(new_csv_error(
1278+
vm,
1279+
"single empty field record must be quoted",
1280+
));
1281+
}
1282+
1283+
write_unquoted_field(&mut output, data, self.dialect, vm)?;
1284+
}
1285+
1286+
write_lineterminator(&mut output, self.dialect.lineterminator);
1287+
1288+
let s = core::str::from_utf8(&output)
1289+
.map_err(|_| vm.new_unicode_decode_error("csv not utf8"))?;
1290+
1291+
self.write.call((s,), vm)
1292+
}
1293+
12251294
#[pymethod]
12261295
fn writerow(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult {
1227-
if matches!(
1228-
self.dialect.quoting,
1229-
QuoteStyle::Strings | QuoteStyle::Notnull
1230-
) {
1231-
return self.writerow_quoted_strings(row, vm);
1296+
match self.dialect.quoting {
1297+
QuoteStyle::None => return self.writerow_quote_none(row, vm),
1298+
QuoteStyle::Strings | QuoteStyle::Notnull => {
1299+
return self.writerow_quoted_strings(row, vm);
1300+
}
1301+
_ => {}
12321302
}
12331303

12341304
let mut state = self.state.lock();

‎extra_tests/snippets/stdlib_csv.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,65 @@ def test_quote_strings_and_notnull_writer():
7272

7373

7474
test_quote_strings_and_notnull_writer()
75+
76+
77+
def test_quote_none_writer_without_quotechar():
78+
no_quotechar_buf = io.StringIO()
79+
csv.writer(
80+
no_quotechar_buf,
81+
quoting=csv.QUOTE_NONE,
82+
quotechar=None,
83+
escapechar="\\",
84+
).writerow(["a,b", 'x"y'])
85+
assert no_quotechar_buf.getvalue() == 'a\\,b,x"y\r\n'
86+
87+
default_quotechar_buf = io.StringIO()
88+
csv.writer(
89+
default_quotechar_buf,
90+
quoting=csv.QUOTE_NONE,
91+
escapechar="\\",
92+
).writerow(["a,b", 'x"y'])
93+
assert default_quotechar_buf.getvalue() == 'a\\,b,x\\"y\r\n'
94+
95+
escapechar_buf = io.StringIO()
96+
csv.writer(
97+
escapechar_buf,
98+
quoting=csv.QUOTE_NONE,
99+
quotechar=None,
100+
escapechar="\\",
101+
).writerow(["a\\b"])
102+
assert escapechar_buf.getvalue() == "a\\\\b\r\n"
103+
104+
linebreak_buf = io.StringIO()
105+
csv.writer(
106+
linebreak_buf,
107+
quoting=csv.QUOTE_NONE,
108+
quotechar=None,
109+
escapechar="\\",
110+
).writerow(["a\rb", "c\nd"])
111+
assert linebreak_buf.getvalue() == "a\\\rb,c\\\nd\r\n"
112+
113+
with assert_raises(csv.Error):
114+
csv.writer(io.StringIO(), quoting=csv.QUOTE_NONE, quotechar=None).writerow(
115+
["a,b"]
116+
)
117+
118+
with assert_raises(csv.Error):
119+
csv.writer(
120+
io.StringIO(),
121+
quoting=csv.QUOTE_NONE,
122+
quotechar=None,
123+
escapechar="\\",
124+
).writerow([None])
125+
126+
two_empty_buf = io.StringIO()
127+
csv.writer(
128+
two_empty_buf,
129+
quoting=csv.QUOTE_NONE,
130+
quotechar=None,
131+
escapechar="\\",
132+
).writerow([None, ""])
133+
assert two_empty_buf.getvalue() == ",\r\n"
134+
135+
136+
test_quote_none_writer_without_quotechar()

0 commit comments

Comments
 (0)