Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions Lib/test/test_eof.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ def test_EOF_single_quote(self):
self.assertEqual(str(cm.exception), expect)
self.assertEqual(cm.exception.offset, 1)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_EOFS(self):
expect = ("unterminated triple-quoted string literal (detected at line 3) (<string>, line 1)")
with self.assertRaises(SyntaxError) as cm:
Expand All @@ -45,7 +44,6 @@ def test_EOFS(self):
self.assertEqual(cm.exception.text, "ä = '''thîs is ")
self.assertEqual(cm.exception.offset, 5)

@unittest.expectedFailure # TODO: RUSTPYTHON
@force_not_colorized
def test_EOFS_with_file(self):
expect = ("(<string>, line 1)")
Expand Down Expand Up @@ -86,15 +84,13 @@ def test_EOFS_with_file(self):
' ^',
'SyntaxError: unterminated triple-quoted string literal (detected at line 4)'])

@unittest.expectedFailure # TODO: RUSTPYTHON
@warnings_helper.ignore_warnings(category=SyntaxWarning)
def test_eof_with_line_continuation(self):
expect = "unexpected EOF while parsing (<string>, line 1)"
with self.assertRaises(SyntaxError) as cm:
compile('"\\Xhh" \\', '<string>', 'exec')
self.assertEqual(str(cm.exception), expect)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_line_continuation_EOF(self):
"""A continuation at the end of input must be an error; bpo2180."""
expect = 'unexpected EOF while parsing (<string>, line 1)'
Expand Down Expand Up @@ -127,7 +123,6 @@ def test_line_continuation_EOF(self):
exec('\\')
self.assertEqual(str(cm.exception), expect)

@unittest.expectedFailure # TODO: RUSTPYTHON
@unittest.skipIf(not sys.executable, "sys.executable required")
@force_not_colorized
def test_line_continuation_EOF_from_file_bpo2180(self):
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2145,7 +2145,6 @@ class AssertionErrorTests(unittest.TestCase):
def tearDown(self):
unlink(TESTFN)

@unittest.expectedFailure # TODO: RUSTPYTHON
@force_not_colorized
def test_assertion_error_location(self):
cases = [
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_tokenize.py
Original file line number Diff line number Diff line change
Expand Up @@ -1922,7 +1922,6 @@ def test_newline_and_space_at_the_end_of_the_source_without_newline(self):
tokens = list(tokenize.tokenize(BytesIO(source.encode('utf-8')).readline))
self.assertEqual(tokens, expected_tokens)

@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: b'SyntaxError' not found in b'OSError: stream did not contain valid UTF-8\n'
def test_invalid_character_in_fstring_middle(self):
# See gh-103824
script = b'''F"""
Expand Down
61 changes: 55 additions & 6 deletions crates/compiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ pub enum CompileError {

impl CompileError {
#[must_use]
pub fn from_ruff_parse_error(error: parser::ParseError, source_file: &SourceFile) -> Self {
pub fn from_ruff_parse_error(
error: parser::ParseError,
source_file: &SourceFile,
mode: Mode,
) -> Self {
let raw_location = error.location;
let diagnostic = match cpython_parse_diagnostic_override(&error, source_file) {
let diagnostic = match cpython_parse_diagnostic_override(&error, source_file, mode) {
Some(diagnostic) => diagnostic,
None => default_parse_diagnostic(error, source_file),
};
Expand Down Expand Up @@ -129,6 +133,13 @@ fn source_location(source_file: &SourceFile, offset: TextSize) -> SourceLocation
.source_location(offset, PositionEncoding::Utf8)
}

// Call only with UTF-8 character boundaries for Python-facing offsets.
fn source_location_in_code_points(source_file: &SourceFile, offset: TextSize) -> SourceLocation {
source_file
.to_source_code()
.source_location(offset, PositionEncoding::Utf32)
Comment thread
chestnut1717 marked this conversation as resolved.
}

fn source_locations(
source_file: &SourceFile,
start: TextSize,
Expand Down Expand Up @@ -175,6 +186,21 @@ impl NormalizedParseDiagnostic {
)
}

fn other_in_code_points(
source_file: &SourceFile,
message: String,
start: usize,
end: usize,
) -> Self {
let start = TextSize::new(start as u32);
let end = TextSize::new(end as u32);
Self::new(
parser::ParseErrorType::OtherError(message),
source_location_in_code_points(source_file, start),
source_location_in_code_points(source_file, end),
)
}

const fn with_unclosed_bracket(mut self, is_unclosed_bracket: bool) -> Self {
self.is_unclosed_bracket = is_unclosed_bracket;
self
Expand All @@ -184,6 +210,7 @@ impl NormalizedParseDiagnostic {
fn cpython_parse_diagnostic_override(
error: &parser::ParseError,
source_file: &SourceFile,
mode: Mode,
) -> Option<NormalizedParseDiagnostic> {
let source_text = source_file.source_text();

Expand Down Expand Up @@ -223,6 +250,18 @@ fn cpython_parse_diagnostic_override(
&error.error,
parser::ParseErrorType::Lexical(parser::LexicalErrorType::LineContinuationError)
) {
// Only a backslash at the end of the source is an EOF error.
let terminal_backslash = source_text.len().checked_sub(1);
if !matches!(mode, Mode::Eval)
&& terminal_backslash == Some(error.location.start().to_usize())
{
let loc = source_line_end_location(source_file, error.location.start());
return Some(NormalizedParseDiagnostic::new(
parser::ParseErrorType::OtherError("unexpected EOF while parsing".to_owned()),
loc,
loc,
));
}
let loc = source_location(source_file, error.location.start() + TextSize::from(1));
return Some(NormalizedParseDiagnostic::new(
error.error.clone(),
Expand All @@ -231,7 +270,15 @@ fn cpython_parse_diagnostic_override(
));
}

source_error!(unterminated_string_error(source_text));
if let Some((message, start, end)) = unterminated_string_error(source_text) {
// The scanner reports quote positions, which are UTF-8 character boundaries.
return Some(NormalizedParseDiagnostic::other_in_code_points(
source_file,
message,
start,
end,
));
}
source_error!(expected_indented_block_error(error, source_text));

if matches!(
Expand Down Expand Up @@ -5176,7 +5223,7 @@ fn _compile_with_syntax_warning_handler<'a>(
};
let parser_options = parser::ParseOptions::from(parser_mode);
let parsed = parser::parse(source_file.source_text(), parser_options)
.map_err(|err| CompileError::from_ruff_parse_error(err, &source_file))?;
.map_err(|err| CompileError::from_ruff_parse_error(err, &source_file, mode))?;
if opts.dont_imply_dedent
&& matches!(mode, Mode::Single)
&& let Some(error) = dont_imply_dedent_source_error(&source_file)
Expand Down Expand Up @@ -5235,10 +5282,12 @@ pub fn _compile_symtable(
let res = match mode {
Mode::Exec | Mode::Single | Mode::BlockExpr => {
let ast = ruff_python_parser::parse_module(source_file.source_text())
.map_err(|e| CompileError::from_ruff_parse_error(e, &source_file))?;
.map_err(|e| CompileError::from_ruff_parse_error(e, &source_file, mode))?;

if let Some(error) =
post_parse_source_error(&source_file, ast.tokens(), &CompileOpts::default())
{

return Err(error);
}
let ast = ast.into_syntax();
Expand All @@ -5254,7 +5303,7 @@ pub fn _compile_symtable(
source_file.source_text(),
parser::Mode::Expression.into(),
)
.map_err(|e| CompileError::from_ruff_parse_error(e, &source_file))?;
.map_err(|e| CompileError::from_ruff_parse_error(e, &source_file, mode))?;
if let Some(error) =
post_parse_source_error(&source_file, ast.tokens(), &CompileOpts::default())
{
Expand Down
6 changes: 5 additions & 1 deletion crates/vm/src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,11 @@ impl VirtualMachine {
_ => true,
};

if same_line {
// A lone continuation at EOF has no highlighted source span.
let lone_line_continuation =
maybe_end_offset == Some(-1) && l_text.to_string_lossy() == "\\";

if same_line && !lone_line_continuation {
let mut end_offset = match maybe_end_offset {
Some(0) | None => offset,
Some(end_offset) => end_offset,
Expand Down
19 changes: 19 additions & 0 deletions crates/vm/src/stdlib/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -818,8 +818,27 @@ pub mod sys {
vm: &VirtualMachine,
) -> PyResult<()> {
let stderr = super::get_stderr(vm)?;
// Keep runtime SyntaxErrors on the normal traceback path.
let has_traceback = !vm.is_none(&exc_tb);
match vm.normalize_exception(exc_type, exc_val.clone(), exc_tb) {
Ok(exc) => {
let native_syntax_error_display = !has_traceback
&& exc.fast_isinstance(vm.ctx.exceptions.syntax_error)
&& exc
.as_object()
.get_attr("msg", vm)
.ok()
.and_then(|msg| msg.downcast::<PyStr>().ok())
.is_some_and(|msg| msg.to_string_lossy() == "unexpected EOF while parsing")
&& exc
.as_object()
.get_attr("text", vm)
.ok()
.and_then(|text| text.downcast::<PyStr>().ok())
.is_some_and(|text| text.to_string_lossy().trim_end() == "\\");
if native_syntax_error_display {
return vm.write_exception(&mut crate::py_io::PyWriter(stderr, vm), &exc);
}
// PyErr_Display: try traceback._print_exception_bltin first
if let Ok(tb_mod) = vm.import("traceback", 0)
&& let Ok(print_exc_builtin) = tb_mod.get_attr("_print_exception_bltin", vm)
Expand Down
4 changes: 4 additions & 0 deletions crates/vm/src/vm/python_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ mod file_run {
"source code cannot contain null bytes".into(),
));
}
#[cfg(feature = "parser")]
// Match compile() by honoring BOMs and encoding cookies in files.
let source = self.decode_source_bytes(&source_bytes, path, false)?;
Comment thread
chestnut1717 marked this conversation as resolved.
#[cfg(not(feature = "parser"))]
let source = String::from_utf8(source_bytes)
.map_err(|err| self.new_os_error(err.to_string()))?;
let code_obj = self
Expand Down
26 changes: 19 additions & 7 deletions crates/vm/src/vm/vm_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ impl VirtualMachine {
Some(line + "\n")
}

let statement = source.and_then(|src| get_statement(src, error.location()));
let mut statement = source.and_then(|src| get_statement(src, error.location()));

let mut msg = error.to_string();
if !msg.starts_with("Exceeds the limit ")
Expand Down Expand Up @@ -799,6 +799,16 @@ impl VirtualMachine {
}

let SyntaxErrorInfo { msg, narrow_caret } = syntax_error_info;
let unterminated_triple_quoted_string =
msg.starts_with("unterminated triple-quoted string literal");
let unexpected_eof_error = msg == "unexpected EOF while parsing";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShaharNaveh is this currently the best way to detect specific kind of error?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

atm, yes:(

Either ruff don't expose the exact reason, or we need to do a major refactor of how we propagate the errors from the compiler

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how could it be detected after the major refactor? we can't do this message matching forever 😂

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ig you're right...

maybe we can't escape it and just have our own ruff fork/have a rustpython-ast crate

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry if the last comment was confusing. I'd like to ask what kind of refactor do you have in mind?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry if the last comment was confusing. I'd like to ask what kind of refactor do you have in mind?

Looking at it again, I don't believe there's a refactor that would save us from doing a match over ruff error messages. the real issue is that ruff does detect invalid syntax and choose to expose it to the user (us, in this case) in a way that is more convenient for ruff and not for someone who's working on a python implementation.

I've got a response for this issue in the past astral-sh/ruff#23334 (comment) (although I haven't asked to provide something like ErrorKind::UnterminatedTripleQuote)

if unterminated_triple_quoted_string
&& let Some(statement) = statement.as_mut()
&& statement.ends_with('\n')
{
// CPython omits the parser-added final newline from SyntaxError.text.
statement.pop();
}
let check_version_suite_error = msg.starts_with("Async functions are")
|| msg.starts_with("Async for loops are")
|| msg.starts_with("Async with statements are")
Expand All @@ -820,12 +830,14 @@ impl VirtualMachine {

// Set end_lineno and end_offset if available
if let Some((end_lineno, end_offset)) = error.python_end_location() {
let (end_lineno, end_offset) = if check_version_suite_error
&& statement
.as_deref()
.and_then(|line| line.chars().next())
.is_some_and(|ch| ch.is_ascii_whitespace())
{
// EOF errors have no source span in CPython.
let no_end_offset = unexpected_eof_error
|| (check_version_suite_error
&& statement
.as_deref()
.and_then(|line| line.chars().next())
.is_some_and(|ch| ch.is_ascii_whitespace()));
let (end_lineno, end_offset) = if no_end_offset {
(end_lineno, -1)
} else if line_end_binary_operator_error && end_offset == offset_raw {
(end_lineno, (end_offset + 1) as isize)
Expand Down
Loading