Skip to content

Commit 2bb6fd4

Browse files
youknowoneyouknowone
committed
fix syntaxerror ast end_offset
1 parent 6cc103b commit 2bb6fd4

File tree

6 files changed

+104
-40
lines changed

6 files changed

+104
-40
lines changed

‎Lib/test/test_cmd_line_script.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -611,8 +611,6 @@ def test_issue20500_exit_with_exception_value(self):
611611
text = stderr.decode('ascii')
612612
self.assertEqual(text.rstrip(), "some text")
613613

614-
# TODO: RUSTPYTHON
615-
@unittest.expectedFailure
616614
def test_syntaxerror_unindented_caret_position(self):
617615
script = "1 + 1 = 2\n"
618616
with os_helper.temp_dir() as script_dir:
@@ -622,8 +620,7 @@ def test_syntaxerror_unindented_caret_position(self):
622620
# Confirm that the caret is located under the '=' sign
623621
self.assertIn("\n ^^^^^\n", text)
624622

625-
# TODO: RUSTPYTHON
626-
@unittest.expectedFailure
623+
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON")
627624
def test_syntaxerror_indented_caret_position(self):
628625
script = textwrap.dedent("""\
629626
if True:

‎crates/compiler/src/lib.rs‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub struct ParseError {
2525
pub error: parser::ParseErrorType,
2626
pub raw_location: ruff_text_size::TextRange,
2727
pub location: SourceLocation,
28+
pub end_location: SourceLocation,
2829
pub source_path: String,
2930
}
3031

@@ -44,13 +45,26 @@ pub enum CompileError {
4445

4546
impl CompileError {
4647
pub fn from_ruff_parse_error(error: parser::ParseError, source_file: &SourceFile) -> Self {
47-
let location = source_file
48-
.to_source_code()
49-
.source_location(error.location.start(), PositionEncoding::Utf8);
48+
let source_code = source_file.to_source_code();
49+
let location = source_code.source_location(error.location.start(), PositionEncoding::Utf8);
50+
let mut end_location =
51+
source_code.source_location(error.location.end(), PositionEncoding::Utf8);
52+
53+
// If the error range ends at the start of a new line (column 1),
54+
// adjust it to the end of the previous line
55+
if end_location.character_offset.get() == 1 && end_location.line > location.line {
56+
// Get the end of the previous line
57+
let prev_line_end = error.location.end() - ruff_text_size::TextSize::from(1);
58+
end_location = source_code.source_location(prev_line_end, PositionEncoding::Utf8);
59+
// Adjust column to be after the last character
60+
end_location.character_offset = end_location.character_offset.saturating_add(1);
61+
}
62+
5063
Self::Parse(ParseError {
5164
error: error.error,
5265
raw_location: error.location,
5366
location,
67+
end_location,
5468
source_path: source_file.name().to_owned(),
5569
})
5670
}
@@ -70,6 +84,16 @@ impl CompileError {
7084
}
7185
}
7286

87+
pub fn python_end_location(&self) -> Option<(usize, usize)> {
88+
match self {
89+
CompileError::Codegen(_) => None,
90+
CompileError::Parse(parse_error) => Some((
91+
parse_error.end_location.line.get(),
92+
parse_error.end_location.character_offset.get(),
93+
)),
94+
}
95+
}
96+
7397
pub fn source_path(&self) -> &str {
7498
match self {
7599
Self::Codegen(codegen_error) => &codegen_error.source_path,

‎crates/vm/src/builtins/type.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,13 +1997,28 @@ fn calculate_meta_class(
19971997
let mut winner = metatype;
19981998
for base in bases {
19991999
let base_type = base.class();
2000+
2001+
// First try fast_issubclass for PyType instances
20002002
if winner.fast_issubclass(base_type) {
20012003
continue;
20022004
} else if base_type.fast_issubclass(&winner) {
20032005
winner = base_type.to_owned();
20042006
continue;
20052007
}
20062008

2009+
// If fast_issubclass didn't work, fall back to general is_subclass
2010+
// This handles cases where metaclasses are not PyType subclasses
2011+
let winner_is_subclass = winner.as_object().is_subclass(base_type.as_object(), vm)?;
2012+
if winner_is_subclass {
2013+
continue;
2014+
}
2015+
2016+
let base_type_is_subclass = base_type.as_object().is_subclass(winner.as_object(), vm)?;
2017+
if base_type_is_subclass {
2018+
winner = base_type.to_owned();
2019+
continue;
2020+
}
2021+
20072022
return Err(vm.new_type_error(
20082023
"metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass \
20092024
of the metaclasses of all its bases",

‎crates/vm/src/exceptions.rs‎

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -223,37 +223,49 @@ impl VirtualMachine {
223223
if let Some(offset) = maybe_offset {
224224
let maybe_end_offset: Option<isize> =
225225
getattr("end_offset").and_then(|obj| obj.try_to_value::<isize>(vm).ok());
226-
227-
let mut end_offset = match maybe_end_offset {
228-
Some(0) | None => offset,
229-
Some(end_offset) => end_offset,
226+
let maybe_end_lineno: Option<isize> =
227+
getattr("end_lineno").and_then(|obj| obj.try_to_value::<isize>(vm).ok());
228+
let maybe_lineno_int: Option<isize> =
229+
getattr("lineno").and_then(|obj| obj.try_to_value::<isize>(vm).ok());
230+
231+
// Only show caret if end_lineno is same as lineno (or not set)
232+
let same_line = match (maybe_lineno_int, maybe_end_lineno) {
233+
(Some(lineno), Some(end_lineno)) => lineno == end_lineno,
234+
_ => true,
230235
};
231236

232-
if offset == end_offset || end_offset == -1 {
233-
end_offset = offset + 1;
234-
}
237+
if same_line {
238+
let mut end_offset = match maybe_end_offset {
239+
Some(0) | None => offset,
240+
Some(end_offset) => end_offset,
241+
};
235242

236-
// Convert 1-based column offset to 0-based index into stripped text
237-
let colno = offset - 1 - spaces;
238-
let end_colno = end_offset - 1 - spaces;
239-
if colno >= 0 {
240-
let caret_space = l_text
241-
.chars()
242-
.take(colno as usize)
243-
.map(|c| if c.is_whitespace() { c } else { ' ' })
244-
.collect::<String>();
245-
246-
let mut error_width = end_colno - colno;
247-
if error_width < 1 {
248-
error_width = 1;
243+
if offset == end_offset || end_offset == -1 {
244+
end_offset = offset + 1;
249245
}
250246

251-
writeln!(
252-
output,
253-
" {}{}",
254-
caret_space,
255-
"^".repeat(error_width as usize)
256-
)?;
247+
// Convert 1-based column offset to 0-based index into stripped text
248+
let colno = offset - 1 - spaces;
249+
let end_colno = end_offset - 1 - spaces;
250+
if colno >= 0 {
251+
let caret_space = l_text
252+
.chars()
253+
.take(colno as usize)
254+
.map(|c| if c.is_whitespace() { c } else { ' ' })
255+
.collect::<String>();
256+
257+
let mut error_width = end_colno - colno;
258+
if error_width < 1 {
259+
error_width = 1;
260+
}
261+
262+
writeln!(
263+
output,
264+
" {}{}",
265+
caret_space,
266+
"^".repeat(error_width as usize)
267+
)?;
268+
}
257269
}
258270
}
259271
}

‎crates/vm/src/stdlib/ast.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,15 @@ pub(crate) fn parse(
271271
) -> Result<PyObjectRef, CompileError> {
272272
let source_file = SourceFileBuilder::new("".to_owned(), source.to_owned()).finish();
273273
let top = parser::parse(source, mode.into())
274-
.map_err(|parse_error| ParseError {
275-
error: parse_error.error,
276-
raw_location: parse_error.location,
277-
location: text_range_to_source_range(&source_file, parse_error.location)
278-
.start
279-
.to_source_location(),
280-
source_path: "<unknown>".to_string(),
274+
.map_err(|parse_error| {
275+
let range = text_range_to_source_range(&source_file, parse_error.location);
276+
ParseError {
277+
error: parse_error.error,
278+
raw_location: parse_error.location,
279+
location: range.start.to_source_location(),
280+
end_location: range.end.to_source_location(),
281+
source_path: "<unknown>".to_string(),
282+
}
281283
})?
282284
.into_syntax();
283285
let top = match top {

‎crates/vm/src/vm/vm_new.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,20 @@ impl VirtualMachine {
499499
.set_attr("offset", offset, self)
500500
.unwrap();
501501

502+
// Set end_lineno and end_offset if available
503+
if let Some((end_lineno, end_offset)) = error.python_end_location() {
504+
let end_lineno = self.ctx.new_int(end_lineno);
505+
let end_offset = self.ctx.new_int(end_offset);
506+
syntax_error
507+
.as_object()
508+
.set_attr("end_lineno", end_lineno, self)
509+
.unwrap();
510+
syntax_error
511+
.as_object()
512+
.set_attr("end_offset", end_offset, self)
513+
.unwrap();
514+
}
515+
502516
syntax_error
503517
.as_object()
504518
.set_attr("text", statement.to_pyobject(self), self)

0 commit comments

Comments
 (0)