diff --git a/crates/vm/src/stdlib/_ast.rs b/crates/vm/src/stdlib/_ast.rs index 241ff35eaef..497dec48b49 100644 --- a/crates/vm/src/stdlib/_ast.rs +++ b/crates/vm/src/stdlib/_ast.rs @@ -1788,7 +1788,7 @@ fn empty_arguments_object(vm: &VirtualMachine) -> PyObjectRef { #[cfg(feature = "parser")] #[allow(clippy::too_many_arguments)] -pub(crate) fn parse( +pub(crate) fn parse>( vm: &VirtualMachine, source: &str, filename: &str, @@ -1800,12 +1800,13 @@ pub(crate) fn parse( interactive: bool, explicit_future_features: crate::bytecode::CodeFlags, dont_imply_dedent: bool, -) -> Result { + mut emit_warning: impl FnMut(usize, char) -> Result<(), E>, +) -> Result { let source_file = SourceFileBuilder::new(filename.to_owned(), source.to_owned()).finish(); let mut options = parser::ParseOptions::from(mode); let target_version = target_version.unwrap_or(ast::PythonVersion::PY314); if let Some(error) = feature_version_syntax_error(source, &source_file, target_version) { - return Err(error); + return Err(error.into()); } options = options.with_target_version(target_version); let barry_source = rustpython_compiler::prepare_barry_as_flufl_source( @@ -1819,22 +1820,22 @@ pub(crate) fn parse( && let Some(error) = rustpython_compiler::single_mode_multiple_statements_error(&source_file, &parsed) { - return Err(error); + return Err(error.into()); } let type_comment_source = type_comments.then(|| TypeCommentSource::new(source, parsed.tokens())); if let Some(lines) = &type_comment_source && let Some(error) = invalid_type_comment_syntax_error(&source_file, lines) { - return Err(error); + return Err(error.into()); } if let Err(errors) = parsed.as_result() { let parse_error = errors[0].clone(); if let Some(error) = barry_source.diagnostic(Some(&parse_error), &source_file) { - return Err(error); + return Err(error.into()); } let range = text_range_to_source_range(&source_file, parse_error.location); - return Err(ParseError { + return Err(CompileError::from(ParseError { error: parse_error.error, raw_location: parse_error.location, location: range.start.to_source_location(), @@ -1842,17 +1843,17 @@ pub(crate) fn parse( source_path: source_file.name().to_owned(), is_unclosed_bracket: false, is_unclosed_string: false, - } + }) .into()); } if let Some(error) = barry_source.diagnostic(None, &source_file) { - return Err(error); + return Err(error.into()); } if dont_imply_dedent && interactive && let Some(error) = rustpython_compiler::dont_imply_dedent_source_error(&source_file) { - return Err(error); + return Err(error.into()); } if let Some(error) = parsed @@ -1861,10 +1862,10 @@ pub(crate) fn parse( .find(|error| should_report_unsupported_syntax_error(error)) { if let Some(error) = cpython_unsupported_syntax_error(error, source, &source_file) { - return Err(error); + return Err(error.into()); } let range = text_range_to_source_range(&source_file, error.range()); - return Err(ParseError { + return Err(CompileError::from(ParseError { error: parser::ParseErrorType::OtherError(error.to_string()), raw_location: error.range(), location: range.start.to_source_location(), @@ -1872,7 +1873,7 @@ pub(crate) fn parse( source_path: source_file.name().to_owned(), is_unclosed_bracket: false, is_unclosed_string: false, - } + }) .into()); } @@ -1881,11 +1882,11 @@ pub(crate) fn parse( parsed.tokens(), vm.state.int_max_str_digits.load(), ) { - return Err(error); + return Err(error.into()); } if let Some(error) = rustpython_compiler::leading_byte_order_mark_error(&source_file) { - return Err(error); + return Err(error.into()); } let mut top = parsed.into_syntax(); rustpython_compiler::too_deeply_nested_error( @@ -1894,18 +1895,34 @@ pub(crate) fn parse( rustpython_compiler::CompileOpts::default().recursion_limit, )?; if let Some(error) = rustpython_compiler::unsupported_grammar_error(&top, &source_file) { - return Err(error); + return Err(error.into()); } if let Some(error) = ipython_escape_command_syntax_error(&top, &source_file) { - return Err(error); + return Err(error.into()); } if let Some(error) = feature_version_ast_syntax_error(&top, &source_file, target_version) { - return Err(error); + return Err(error.into()); + } + #[cfg(feature = "codegen")] + let future_features = codegen::preprocess::checked_future_features(&top) + .map_err(|err| future_feature_compile_error(&source_file, err))?; + // Keep original literal spans: preprocessing can fold multiple source + // literals into one node, but each has its own lexical diagnostic. + match &top { + ast::Mod::Module(module) => { + string::emit_format_spec_warnings(&module.body, &[], &source_file, &mut emit_warning)?; + } + ast::Mod::Expression(expression) => { + string::emit_format_spec_warnings( + &[], + core::slice::from_ref(expression.body.as_ref()), + &source_file, + &mut emit_warning, + )?; + } } #[cfg(feature = "codegen")] { - let future_features = codegen::preprocess::checked_future_features(&top) - .map_err(|err| future_feature_compile_error(&source_file, err))?; let future_annotations = explicit_future_features .contains(crate::bytecode::CodeFlags::FUTURE_ANNOTATIONS) || future_features.contains(crate::bytecode::CodeFlags::FUTURE_ANNOTATIONS); @@ -1969,14 +1986,16 @@ pub(crate) fn wrap_interactive(vm: &VirtualMachine, module_obj: &PyObject) -> Py } #[cfg(feature = "parser")] -pub(crate) fn parse_func_type( +pub(crate) fn parse_func_type>( vm: &VirtualMachine, source: &str, filename: &str, optimize: u8, target_version: Option, -) -> Result { + mut emit_warning: impl FnMut(usize, char) -> Result<(), E>, +) -> Result { let _ = optimize; + let leading_space = source.len() - source.trim_start().len(); let source = source.trim(); let invalid_func_type = || -> CompileError { ParseError { @@ -2008,7 +2027,7 @@ pub(crate) fn parse_func_type( } let Some(split_at) = split_at else { - return Err(ParseError { + return Err(CompileError::from(ParseError { error: parser::ParseErrorType::OtherError("invalid func_type".to_owned()), raw_location: TextRange::default(), location: SourceLocation::default(), @@ -2016,7 +2035,7 @@ pub(crate) fn parse_func_type( source_path: filename.to_owned(), is_unclosed_bracket: false, is_unclosed_string: false, - } + }) .into()); }; @@ -2029,7 +2048,7 @@ pub(crate) fn parse_func_type( .with_target_version(target_version.unwrap_or(ast::PythonVersion::PY314)); let parsed = parser::parse(expr_src, options).map_err(|parse_error| { let range = text_range_to_source_range(&source_file, parse_error.location); - ParseError { + CompileError::from(ParseError { error: parse_error.error, raw_location: parse_error.location, location: range.start.to_source_location(), @@ -2037,7 +2056,7 @@ pub(crate) fn parse_func_type( source_path: source_file.name().to_owned(), is_unclosed_bracket: false, is_unclosed_string: false, - } + }) })?; let ast::Mod::Expression(expression) = parsed.into_syntax() else { unreachable!(); @@ -2046,22 +2065,25 @@ pub(crate) fn parse_func_type( }; if !left.starts_with('(') || !left.ends_with(')') { - return Err(invalid_func_type()); + return Err(invalid_func_type().into()); } let inner = left[1..left.len() - 1].trim(); + let inner_start = left.len() - 1 - left[1..left.len() - 1].trim_start().len(); + const ARG_PREFIX: &str = "__rustpython_func_type__("; + let mut arg_source_file = None; let argtypes = if inner.is_empty() { Vec::new() } else { if inner.ends_with(',') { - return Err(invalid_func_type()); + return Err(invalid_func_type().into()); } - let call_source = format!("__rustpython_func_type__({inner})"); + let call_source = format!("{ARG_PREFIX}{inner})"); let source_file = SourceFileBuilder::new(filename.to_owned(), call_source.clone()).finish(); let options = parser::ParseOptions::from(parser::Mode::Expression) .with_target_version(target_version.unwrap_or(ast::PythonVersion::PY314)); let parsed = parser::parse(&call_source, options).map_err(|parse_error| { let range = text_range_to_source_range(&source_file, parse_error.location); - ParseError { + CompileError::from(ParseError { error: parse_error.error, raw_location: parse_error.location, location: range.start.to_source_location(), @@ -2069,13 +2091,13 @@ pub(crate) fn parse_func_type( source_path: source_file.name().to_owned(), is_unclosed_bracket: false, is_unclosed_string: false, - } + }) })?; let ast::Mod::Expression(expression) = parsed.into_syntax() else { unreachable!(); }; let ast::Expr::Call(call) = *expression.body else { - return Err(invalid_func_type()); + return Err(invalid_func_type().into()); }; let mut args = Vec::new(); let positional_len = call.arguments.args.len(); @@ -2084,7 +2106,7 @@ pub(crate) fn parse_func_type( match arg { ast::Expr::Starred(starred) => { if seen_star || index + 1 != positional_len { - return Err(invalid_func_type()); + return Err(invalid_func_type().into()); } seen_star = true; args.push(*starred.value); @@ -2095,22 +2117,38 @@ pub(crate) fn parse_func_type( let mut seen_kw_star = false; for keyword in call.arguments.keywords { if keyword.arg.is_some() || seen_kw_star { - return Err(invalid_func_type()); + return Err(invalid_func_type().into()); } seen_kw_star = true; args.push(keyword.value); } + arg_source_file = Some(source_file); args }; let returns = parse_expr(right)?; + // Func-type nodes are parsed from separate fragments. Keep warning offsets + // in those fragments until the boundary, then map them to the original + // source without changing the AST's existing location representation. + if let Some(source_file) = arg_source_file { + string::emit_format_spec_warnings(&[], &argtypes, &source_file, &mut |offset, ch| { + emit_warning(leading_space + inner_start + offset - ARG_PREFIX.len(), ch) + })?; + } + let return_source_file = SourceFileBuilder::new(filename.to_owned(), right.to_owned()).finish(); + string::emit_format_spec_warnings( + &[], + core::slice::from_ref(&returns), + &return_source_file, + &mut |offset, ch| emit_warning(leading_space + source.len() - right.len() + offset, ch), + )?; let func_type = ModFunctionType { argtypes: argtypes.into_boxed_slice(), returns, runtime_argtypes: None, }; - let source_file = SourceFileBuilder::new("".to_owned(), source.to_owned()).finish(); + let source_file = SourceFileBuilder::new(filename.to_owned(), source.to_owned()).finish(); Ok(func_type.ast_to_object(vm, &source_file)) } @@ -2498,7 +2536,10 @@ pub(crate) fn compile( filename: &str, mode: crate::compiler::Mode, mut opts: codegen::CompileOpts, + module: Option<&Py>, ) -> PyResult { + #[cfg(not(feature = "parser"))] + let _ = module; let text = synthetic_source_from_ast_object(vm, &object)?; let source_file = SourceFileBuilder::new(filename.to_owned(), text.clone()).finish(); let ast = Node::ast_from_object(vm, &source_file, object)?; @@ -2535,7 +2576,7 @@ pub(crate) fn compile( message.into(), fname, location.line.get(), - None, + module.map(|module| module.to_owned().into()), vm.ctx.none(), None, None, diff --git a/crates/vm/src/stdlib/_ast/string.rs b/crates/vm/src/stdlib/_ast/string.rs index e3b1c94754e..c3f0a10514f 100644 --- a/crates/vm/src/stdlib/_ast/string.rs +++ b/crates/vm/src/stdlib/_ast/string.rs @@ -1,6 +1,5 @@ use super::constant::{Constant, ConstantLiteral}; use super::*; -use crate::warn; use ast::str_prefix::StringLiteralPrefix; use rustpython_common::wtf8::Wtf8Buf; @@ -175,42 +174,41 @@ fn normalize_template_str_parts(values: Vec) -> Vec( source_file: &SourceFile, range: TextRange, -) { + emit_warning: &mut impl FnMut(usize, char) -> Result<(), E>, +) -> Result<(), E> { let source = source_file.source_text(); let start = range.start().to_usize(); let end = range.end().to_usize(); if start >= end || end > source.len() { - return; - } - let mut raw = &source[start..end]; - if raw.starts_with(':') { - raw = &raw[1..]; + return Ok(()); } - - let mut chars = raw.chars().peekable(); - while let Some(ch) = chars.next() { + let Some(raw) = source.get(start..end) else { + return Ok(()); + }; + let mut chars = raw.char_indices().peekable(); + while let Some((offset, ch)) = chars.next() { if ch != '\\' { continue; } - let Some(next) = chars.next() else { + let Some((_, next)) = chars.next() else { break; }; let valid = match next { '\\' | '\'' | '"' | 'a' | 'b' | 'f' | 'n' | 'r' | 't' | 'v' => true, '\n' => true, '\r' => { - if let Some('\n') = chars.peek().copied() { + if let Some((_, '\n')) = chars.peek().copied() { chars.next(); } true } '0'..='7' => { for _ in 0..2 { - if let Some('0'..='7') = chars.peek().copied() { + if let Some((_, '0'..='7')) = chars.peek().copied() { chars.next(); } else { break; @@ -220,7 +218,7 @@ fn warn_invalid_escape_sequences_in_format_spec( } 'x' => { for _ in 0..2 { - if chars.peek().is_some_and(|c| c.is_ascii_hexdigit()) { + if chars.peek().is_some_and(|(_, c)| c.is_ascii_hexdigit()) { chars.next(); } else { break; @@ -230,7 +228,7 @@ fn warn_invalid_escape_sequences_in_format_spec( } 'u' => { for _ in 0..4 { - if chars.peek().is_some_and(|c| c.is_ascii_hexdigit()) { + if chars.peek().is_some_and(|(_, c)| c.is_ascii_hexdigit()) { chars.next(); } else { break; @@ -240,7 +238,7 @@ fn warn_invalid_escape_sequences_in_format_spec( } 'U' => { for _ in 0..8 { - if chars.peek().is_some_and(|c| c.is_ascii_hexdigit()) { + if chars.peek().is_some_and(|(_, c)| c.is_ascii_hexdigit()) { chars.next(); } else { break; @@ -249,9 +247,9 @@ fn warn_invalid_escape_sequences_in_format_spec( true } 'N' => { - if let Some('{') = chars.peek().copied() { + if let Some((_, '{')) = chars.peek().copied() { chars.next(); - for c in chars.by_ref() { + for (_, c) in chars.by_ref() { if c == '}' { break; } @@ -262,18 +260,196 @@ fn warn_invalid_escape_sequences_in_format_spec( _ => false, }; if !valid { - let message = vm.ctx.new_str(format!( - "\"\\{next}\" is an invalid escape sequence. Such sequences will not work in the future. Did you mean \"\\\\{next}\"? A raw string is also an option." - )); - let _ = warn::warn( - message.into(), - Some(vm.ctx.exceptions.syntax_warning.to_owned()), - 1, - None, - vm, + return emit_warning(start + offset, next); + } + } + Ok(()) +} + +/// Keep AST parsing's existing format-spec diagnostics at a fallible boundary, +/// rather than emitting warnings from the infallible Python AST conversion. +#[cfg(feature = "parser")] +pub(super) fn emit_format_spec_warnings( + statements: &[ast::Stmt], + expressions: &[ast::Expr], + source_file: &SourceFile, + emit_warning: &mut impl FnMut(usize, char) -> Result<(), E>, +) -> Result<(), E> { + use ast::visitor::{self, Visitor}; + use std::collections::HashSet; + + struct LiteralVisitor<'a, F, E> { + source_file: &'a SourceFile, + emit_warning: F, + emitted: HashSet, + error: Option, + } + + impl LiteralVisitor<'_, F, E> + where + F: FnMut(usize, char) -> Result<(), E>, + { + fn check_literal(&mut self, mut range: TextRange, interpolated: bool) { + if self.error.is_some() { + return; + } + // Ruff leaves the interpolation delimiter outside the literal range. + // Include it so a trailing backslash can still diagnose \{ or \}. + if interpolated + && self + .source_file + .source_text() + .as_bytes() + .get(range.end().to_usize()) + .is_some_and(|byte| matches!(byte, b'{' | b'}')) + { + range = TextRange::new(range.start(), range.end() + TextSize::from(1)); + } + let result = warn_invalid_escape_sequences_in_format_spec( + self.source_file, + range, + &mut |offset, ch| { + if !self.emitted.contains(&offset) { + (self.emit_warning)(offset, ch)?; + self.emitted.insert(offset); + } + Ok(()) + }, ); + if let Err(error) = result { + self.error = Some(error); + } + } + + fn visit_elements<'a>(&mut self, elements: &'a ast::InterpolatedStringElements, raw: bool) + where + Self: Visitor<'a>, + { + for element in elements { + match element { + ast::InterpolatedStringElement::Literal(literal) => { + if !raw { + self.check_literal(literal.range, true); + } + } + ast::InterpolatedStringElement::Interpolation(interpolation) => { + self.visit_expr(&interpolation.expression); + if let Some(spec) = &interpolation.format_spec { + self.visit_elements(&spec.elements, raw); + } + } + } + } } } + + // Visit only literal spans within an existing format-spec warning region. + // An enclosing spec can include nested raw strings, nonraw expressions in a + // raw f-string, and comments; scanning its entire source range conflates them. + impl<'a, F, E> Visitor<'a> for LiteralVisitor<'_, F, E> + where + F: FnMut(usize, char) -> Result<(), E>, + { + fn visit_expr(&mut self, expr: &'a ast::Expr) { + if self.error.is_none() { + visitor::walk_expr(self, expr); + } + } + + fn visit_string_literal(&mut self, literal: &'a ast::StringLiteral) { + if !matches!( + literal.flags.prefix(), + ast::str_prefix::StringLiteralPrefix::Raw { .. } + ) { + self.check_literal(literal.range, false); + } + } + + fn visit_bytes_literal(&mut self, literal: &'a ast::BytesLiteral) { + if !literal.flags.prefix().is_raw() { + self.check_literal(literal.range, false); + } + } + + fn visit_f_string(&mut self, fstring: &'a ast::FString) { + self.visit_elements(&fstring.elements, fstring.flags.prefix().is_raw()); + } + + fn visit_t_string(&mut self, tstring: &'a ast::TString) { + self.visit_elements(&tstring.elements, tstring.flags.prefix().is_raw()); + } + } + + struct FormatSpecVisitor<'a, F, E> { + literals: LiteralVisitor<'a, F, E>, + } + + impl<'a, F, E> Visitor<'a> for FormatSpecVisitor<'_, F, E> + where + F: FnMut(usize, char) -> Result<(), E>, + { + fn visit_expr(&mut self, expr: &'a ast::Expr) { + if self.literals.error.is_some() { + return; + } + if let ast::Expr::FString(fstring) = expr { + // Conversion checked all enclosing specs before converting their + // expressions, including across concatenated f-string parts. + for part in &fstring.value { + if let ast::FStringPartRef::FString(part) = part { + self.visit_format_specs(&part.elements, part.flags.prefix().is_raw()); + } + } + } + visitor::walk_expr(self, expr); + } + + fn visit_t_string(&mut self, tstring: &'a ast::TString) { + let raw = tstring.flags.prefix().is_raw(); + for element in &tstring.elements { + if let ast::InterpolatedStringElement::Interpolation(interpolation) = element { + self.visit_expr(&interpolation.expression); + if let Some(spec) = &interpolation.format_spec { + // Template specs become JoinedStr nodes during conversion; + // their nested format specs already emitted warnings too. + self.visit_format_specs(&spec.elements, raw); + for element in &spec.elements { + self.visit_interpolated_string_element(element); + } + } + } + } + } + } + + impl FormatSpecVisitor<'_, F, E> + where + F: FnMut(usize, char) -> Result<(), E>, + { + fn visit_format_specs(&mut self, elements: &ast::InterpolatedStringElements, raw: bool) { + for element in elements { + if let ast::InterpolatedStringElement::Interpolation(interpolation) = element + && let Some(spec) = &interpolation.format_spec + { + self.literals.visit_elements(&spec.elements, raw); + } + } + } + } + + let mut visitor = FormatSpecVisitor { + literals: LiteralVisitor { + source_file, + emit_warning, + emitted: HashSet::new(), + error: None, + }, + }; + visitor.visit_body(statements); + for expr in expressions { + visitor.visit_expr(expr); + } + visitor.literals.error.map_or(Ok(()), Err) } fn ruff_format_spec_to_joined_str( @@ -763,13 +939,6 @@ pub(super) fn fstring_to_object( } } let values = normalize_joined_str_parts(values); - for part in &values { - if let JoinedStrPart::FormattedValue(value) = part - && let Some(format_spec) = &value.format_spec - { - warn_invalid_escape_sequences_in_format_spec(vm, source_file, format_spec.range()); - } - } let c = JoinedStr { range, values: values.into_boxed_slice(), diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs index 65f64299443..23e4c7ea7d7 100644 --- a/crates/vm/src/stdlib/builtins.rs +++ b/crates/vm/src/stdlib/builtins.rs @@ -118,6 +118,8 @@ mod builtins { dont_inherit: ArgIntoBool, #[pyarg(any, default = -1)] optimize: i32, + #[pyarg(named, optional)] + module: Option, #[pyarg(named, default = -1)] _feature_version: i32, } @@ -208,6 +210,17 @@ mod builtins { 0..=2 => optimize as u8, _ => return Err(vm.new_value_error("compile(): invalid optimize value")), }; + let module = args.module.filter(|module| !vm.is_none(module)); + let module = module + .map(|module| { + PyStrRef::try_from_object(vm, module.clone()).map_err(|_| { + vm.new_type_error(format!( + "compile() argument 'module' must be str or None, not {}", + module.class().name() + )) + }) + }) + .transpose()?; let dont_inherit = args.dont_inherit.into_bool(); let is_ast_only = cf.contains(CompilerFlags::ONLY_AST); let future_features = merge_compile_future_features(flags, dont_inherit, vm); @@ -297,7 +310,14 @@ mod builtins { opts.optimize = optimize; opts.allow_top_level_await = cf.contains(CompilerFlags::ALLOW_TOP_LEVEL_AWAIT); opts.future_features = future_features; - return _ast::compile(vm, args.source, &filename.to_string_lossy(), mode, opts); + return _ast::compile( + vm, + args.source, + &filename.to_string_lossy(), + mode, + opts, + module.as_deref(), + ); } } @@ -314,13 +334,14 @@ mod builtins { let mut compile_flags = flags | future_features.bits() as i32; #[cfg(feature = "rustpython-compiler")] let compile_source = |source: &[u8], compile_flags: i32| { - vm.compile_string_object_with_flags( + vm.compile_string_object_with_flags_and_module( source, &filename.to_string_lossy(), start.as_i32(), compile_flags, feature_version, optimize as i32, + module.as_deref(), ) }; match &source { diff --git a/crates/vm/src/vm/compile.rs b/crates/vm/src/vm/compile.rs index b5f7c77c772..2f13fb63a4c 100644 --- a/crates/vm/src/vm/compile.rs +++ b/crates/vm/src/vm/compile.rs @@ -5,8 +5,8 @@ use core::fmt; use crate::{ - AsObject, PyObjectRef, PyRef, PyResult, VirtualMachine, - builtins::{PyBaseExceptionRef, PyCode}, + AsObject, Py, PyObjectRef, PyRef, PyResult, VirtualMachine, + builtins::{PyBaseExceptionRef, PyCode, PyStr}, compiler::{self, CompileError, CompileOpts}, vm::compile_mode::{CompileStart, CompilerFlags, compile_future_features_from_flags}, }; @@ -381,7 +381,29 @@ impl VirtualMachine { feature_version: i32, optimize: i32, ) -> PyResult { - use crate::convert::ToPyException; + self.compile_string_object_with_flags_and_module( + source, + filename, + start, + flags, + feature_version, + optimize, + None, + ) + } + + #[cfg(feature = "parser")] + #[allow(clippy::too_many_arguments)] + pub(crate) fn compile_string_object_with_flags_and_module( + &self, + source: &[u8], + filename: &str, + start: i32, + flags: i32, + feature_version: i32, + optimize: i32, + module: Option<&Py>, + ) -> PyResult { use crate::stdlib::_ast; let cf = CompilerFlags::from_bits_retain(flags); @@ -411,10 +433,29 @@ impl VirtualMachine { None }; + let mut emit_ast_warning = |offset, ch| { + escape_warnings::warn_invalid_escape_sequence( + source, + escape_warnings::InvalidEscape::Char { ch, offset }, + filename, + module, + self, + ) + .map_err(VmCompileError::Warning) + }; if is_ast_only { if start == CompileStart::FuncType { - return _ast::parse_func_type(self, source, filename, optimize, target_version) - .map_err(|e| (e, Some(source), allow_incomplete).to_pyexception(self)); + return _ast::parse_func_type( + self, + source, + filename, + optimize, + target_version, + &mut emit_ast_warning, + ) + .map_err(|e| { + e.into_pyexception_maybe_incomplete(self, Some(source), allow_incomplete) + }); } let (parser_mode, interactive) = match start { CompileStart::Single => (ruff_python_parser::Mode::Module, true), @@ -434,14 +475,18 @@ impl VirtualMachine { interactive, future_features, dont_imply_dedent, + &mut emit_ast_warning, ) - .map_err(|e| (e, Some(source), allow_incomplete).to_pyexception(self))?; + .map_err(|e| { + e.into_pyexception_maybe_incomplete(self, Some(source), allow_incomplete) + })?; if start == CompileStart::Single { return _ast::wrap_interactive(self, &parsed); } return Ok(parsed); } + let mut emitted_escapes = Vec::new(); if type_comments { let parser_mode = match start { CompileStart::Single | CompileStart::File => ruff_python_parser::Mode::Module, @@ -460,8 +505,20 @@ impl VirtualMachine { start == CompileStart::Single, future_features, dont_imply_dedent, + |offset, ch| { + emit_ast_warning(offset, ch)?; + // The ordinary compiler must not emit this same escape again. + emitted_escapes.push(offset); + Ok(()) + }, ) - .map_err(|e| (e, Some(source), allow_incomplete).to_pyexception(self))?; + .map_err(|e: VmCompileError| { + e.into_pyexception_maybe_incomplete(self, Some(source), allow_incomplete) + })?; + // AST traversal need not follow source order. The later compiler + // uses binary search to skip these already-considered offsets. + emitted_escapes.sort_unstable(); + emitted_escapes.dedup(); } let mode = match start { @@ -476,7 +533,7 @@ impl VirtualMachine { opts.future_features = future_features; opts.dont_imply_dedent = dont_imply_dedent; let code = self - .compile_with_opts(source, mode, filename, opts) + .compile_with_opts_and_module(source, mode, filename, opts, module, &emitted_escapes) .map_err(|err| { err.into_pyexception_maybe_incomplete(self, Some(source), allow_incomplete) })?; @@ -498,13 +555,27 @@ impl VirtualMachine { mode: compiler::Mode, source_path: impl Into, opts: CompileOpts, + ) -> Result, VmCompileError> { + self.compile_with_opts_and_module(source, mode, source_path, opts, None, &[]) + } + + pub(crate) fn compile_with_opts_and_module( + &self, + source: &str, + mode: compiler::Mode, + source_path: impl Into, + opts: CompileOpts, + module: Option<&Py>, + emitted_escapes: &[usize], ) -> Result, VmCompileError> { let source_path = source_path.into(); + #[cfg(not(feature = "parser"))] + let _ = (module, emitted_escapes); #[cfg(feature = "parser")] { - self.emit_tokenizer_syntax_warnings(source, &source_path) + self.emit_tokenizer_syntax_warnings(source, &source_path, module) .map_err(VmCompileError::Warning)?; - self.emit_string_escape_warnings(source, &source_path) + self.emit_string_escape_warnings(source, &source_path, module, emitted_escapes) .map_err(VmCompileError::Warning)?; } #[cfg(feature = "parser")] @@ -514,19 +585,25 @@ impl VirtualMachine { let escalated: core::cell::Cell> = core::cell::Cell::new(None); let mut syntax_warning_handler = |location, message| { - escape_warnings::warn_syntax_at_location(&source_path, location, message, self) - .map_err(|warning| { - escalated.set(Some(warning)); - // Recovered below via `escalated`, so this is never surfaced. - compiler::codegen::error::CodegenError { - location: Some(location), - end_location: None, - error: compiler::codegen::error::CodegenErrorType::SyntaxError( - String::new(), - ), - source_path: source_path.clone(), - } - }) + escape_warnings::warn_syntax_at_location( + &source_path, + module, + location, + message, + self, + ) + .map_err(|warning| { + escalated.set(Some(warning)); + // Recovered below via `escalated`, so this is never surfaced. + compiler::codegen::error::CodegenError { + location: Some(location), + end_location: None, + error: compiler::codegen::error::CodegenErrorType::SyntaxError( + String::new(), + ), + source_path: source_path.clone(), + } + }) }; let result = compiler::compile_with_syntax_warning_handler( source, @@ -618,7 +695,7 @@ mod escape_warnings { if cs <= ce { Some((cs, ce)) } else { None } } - enum InvalidEscape { + pub(super) enum InvalidEscape { Char { ch: char, offset: usize }, Octal { digits: [u8; 3], offset: usize }, } @@ -773,10 +850,11 @@ mod escape_warnings { /// Emit `SyntaxWarning` for an invalid escape sequence. /// /// `warn_invalid_escape_sequence()` in `Parser/string_parser.c` - fn warn_invalid_escape_sequence( + pub(super) fn warn_invalid_escape_sequence( source: &str, escape: InvalidEscape, filename: &str, + module: Option<&Py>, vm: &VirtualMachine, ) -> Result<(), CompileWarningError> { let (lineno, column) = line_offset_at(source, escape.offset()); @@ -788,7 +866,7 @@ mod escape_warnings { vm.ctx.new_str(warning).into(), fname, lineno, - None, + module.map(|module| module.to_owned().into()), vm.ctx.none(), None, None, @@ -809,6 +887,7 @@ mod escape_warnings { fn warn_syntax_at_offset( source: &str, filename: &str, + module: Option<&Py>, offset: usize, message: String, vm: &VirtualMachine, @@ -821,7 +900,7 @@ mod escape_warnings { message.into(), fname, lineno, - None, + module.map(|module| module.to_owned().into()), vm.ctx.none(), None, None, @@ -832,6 +911,7 @@ mod escape_warnings { pub(super) fn warn_syntax_at_location( filename: &str, + module: Option<&Py>, location: compiler::core::SourceLocation, message: String, vm: &VirtualMachine, @@ -843,7 +923,7 @@ mod escape_warnings { message.into(), fname, location.line.get(), - None, + module.map(|module| module.to_owned().into()), vm.ctx.none(), None, None, @@ -1084,13 +1164,14 @@ mod escape_warnings { end: usize, is_bytes: bool, filename: &str, + module: Option<&Py>, vm: &VirtualMachine, ) -> Result<(), CompileWarningError> { if let Some((start, content_end)) = content_bounds(source, text_range_from_bounds(quote_index, end)) && let Some(escape) = first_invalid_escape(source, start, content_end, is_bytes) { - warn_invalid_escape_sequence(source, escape, filename, vm)?; + warn_invalid_escape_sequence(source, escape, filename, module, vm)?; } Ok(()) } @@ -1100,13 +1181,14 @@ mod escape_warnings { start: usize, end: usize, filename: &str, + module: Option<&Py>, vm: &VirtualMachine, ) -> Result<(), CompileWarningError> { if start >= end || end > source.len() { return Ok(()); } if let Some(escape) = first_invalid_escape(source, start, end, false) { - return warn_invalid_escape_sequence(source, escape, filename, vm); + return warn_invalid_escape_sequence(source, escape, filename, module, vm); } let trailing_bs = source.as_bytes()[start..end] .iter() @@ -1124,6 +1206,7 @@ mod escape_warnings { offset: end - 1, }, filename, + module, vm, )?; } @@ -1185,6 +1268,7 @@ mod escape_warnings { end: usize, is_raw: bool, filename: &str, + module: Option<&Py>, vm: &VirtualMachine, ) -> Result<(), CompileWarningError> { let bytes = source.as_bytes(); @@ -1197,7 +1281,14 @@ mod escape_warnings { } b'{' => { if !is_raw { - warn_fstring_literal_part(source, literal_start, index, filename, vm)?; + warn_fstring_literal_part( + source, + literal_start, + index, + filename, + module, + vm, + )?; } let close = find_interpolation_end(bytes, index, end); let body_end = if close > index + 1 { close - 1 } else { close }; @@ -1207,6 +1298,7 @@ mod escape_warnings { body_end, Some(is_raw), filename, + module, vm, )?; index = close.max(index + 1); @@ -1220,7 +1312,7 @@ mod escape_warnings { } } if !is_raw { - warn_fstring_literal_part(source, literal_start, end, filename, vm)?; + warn_fstring_literal_part(source, literal_start, end, filename, module, vm)?; } Ok(()) } @@ -1230,9 +1322,10 @@ mod escape_warnings { fn emit_string_escape_warnings_unparsed( source: &str, filename: &str, + module: Option<&Py>, vm: &VirtualMachine, ) -> Result<(), CompileWarningError> { - emit_string_escape_warnings_in_range(source, 0, source.len(), None, filename, vm) + emit_string_escape_warnings_in_range(source, 0, source.len(), None, filename, module, vm) } fn emit_string_escape_warnings_in_range( @@ -1241,6 +1334,7 @@ mod escape_warnings { end: usize, format_spec_raw: Option, filename: &str, + module: Option<&Py>, vm: &VirtualMachine, ) -> Result<(), CompileWarningError> { let bytes = source.as_bytes(); @@ -1275,17 +1369,20 @@ mod escape_warnings { content_end.min(end), is_raw, filename, + module, vm, )?; } } else if !is_raw { warn_quoted_literal( - source, index, quote_end, is_bytes, filename, vm, + source, index, quote_end, is_bytes, filename, module, vm, )?; } } StringPrefix::None => { - warn_quoted_literal(source, index, quote_end, false, filename, vm)?; + warn_quoted_literal( + source, index, quote_end, false, filename, module, vm, + )?; } StringPrefix::Invalid => {} } @@ -1296,7 +1393,15 @@ mod escape_warnings { && bracket == 0 && brace == 0 => { - scan_interpolated_content(source, index + 1, end, is_raw, filename, vm)?; + scan_interpolated_content( + source, + index + 1, + end, + is_raw, + filename, + module, + vm, + )?; return Ok(()); } b'(' if format_spec_raw.is_some() => { @@ -1332,6 +1437,7 @@ mod escape_warnings { fn emit_numeric_literal_warnings( source: &str, filename: &str, + module: Option<&Py>, vm: &VirtualMachine, ) -> Result<(), CompileWarningError> { let bytes = source.as_bytes(); @@ -1363,6 +1469,7 @@ mod escape_warnings { warn_syntax_at_offset( source, filename, + module, index, format!("invalid {kind} literal"), vm, @@ -1379,8 +1486,10 @@ mod escape_warnings { struct EscapeWarningVisitor<'a> { source: &'a str, filename: &'a str, + module: Option<&'a Py>, vm: &'a VirtualMachine, error: Option, + emitted_escapes: &'a [usize], /// This pass runs before the compile that rejects an over-nested /// tree, so it has to stop itself. depth: usize, @@ -1401,9 +1510,18 @@ mod escape_warnings { fn check_quoted_literal(&mut self, range: TextRange, is_bytes: bool) { if let Some((start, end)) = content_bounds(self.source, range) && let Some(escape) = first_invalid_escape(self.source, start, end, is_bytes) + && self + .emitted_escapes + .binary_search(&escape.offset()) + .is_err() { - let result = - warn_invalid_escape_sequence(self.source, escape, self.filename, self.vm); + let result = warn_invalid_escape_sequence( + self.source, + escape, + self.filename, + self.module, + self.vm, + ); self.record_warning(result); } } @@ -1421,8 +1539,16 @@ mod escape_warnings { return; } if let Some(escape) = first_invalid_escape(self.source, start, end, false) { - let result = - warn_invalid_escape_sequence(self.source, escape, self.filename, self.vm); + if self.emitted_escapes.binary_search(&escape.offset()).is_ok() { + return; + } + let result = warn_invalid_escape_sequence( + self.source, + escape, + self.filename, + self.module, + self.vm, + ); self.record_warning(result); return; } @@ -1440,6 +1566,7 @@ mod escape_warnings { if trailing_bs % 2 == 1 && let Some(&after) = self.source.as_bytes().get(end) && (after == b'{' || after == b'}') + && self.emitted_escapes.binary_search(&(end - 1)).is_err() { let result = warn_invalid_escape_sequence( self.source, @@ -1448,6 +1575,7 @@ mod escape_warnings { offset: end - 1, }, self.filename, + self.module, self.vm, ); self.record_warning(result); @@ -1547,8 +1675,9 @@ mod escape_warnings { &self, source: &str, filename: &str, + module: Option<&Py>, ) -> Result<(), CompileWarningError> { - emit_numeric_literal_warnings(source, filename, self) + emit_numeric_literal_warnings(source, filename, module, self) } /// Walk all string literals in `source` and emit `SyntaxWarning` for @@ -1557,6 +1686,8 @@ mod escape_warnings { &self, source: &str, filename: &str, + module: Option<&Py>, + emitted_escapes: &[usize], ) -> Result<(), CompileWarningError> { // The compile that follows rejects this source; parsing it here // would build a tree that exhausts the stack when dropped. @@ -1567,14 +1698,16 @@ mod escape_warnings { let Ok(parsed) = ruff_python_parser::parse(source, ruff_python_parser::Mode::Module.into()) else { - return emit_string_escape_warnings_unparsed(source, filename, self); + return emit_string_escape_warnings_unparsed(source, filename, module, self); }; let ast = parsed.into_syntax(); let mut visitor = EscapeWarningVisitor { source, filename, + module, vm: self, error: None, + emitted_escapes, depth: 0, depth_limit: compiler::CompileOpts::default().recursion_limit, }; diff --git a/extra_tests/snippets/builtin_compile_module.py b/extra_tests/snippets/builtin_compile_module.py new file mode 100644 index 00000000000..8ff58583b41 --- /dev/null +++ b/extra_tests/snippets/builtin_compile_module.py @@ -0,0 +1,330 @@ +import ast +import warnings + +from testutils import assert_raises, skip_if_unsupported + + +def test_compile_module(): + class ModuleName(str): + pass + + for module in (None, "", "package.module", ModuleName("package.module"), "\ud800"): + code = compile("value = 42", "", "exec", module=module) + assert code.co_filename == "" + namespace = {} + exec(code, namespace) + assert namespace["value"] == 42 + + for module in (1, False, b"package.module", [], {}, object()): + with assert_raises(TypeError) as caught: + compile("", "", "exec", module=module) + assert "argument 'module' must be str or None" in str(caught.exception) + + # module is keyword-only; _feature_version stays keyword-only too. + with assert_raises(TypeError): + compile("", "", "exec", 0, False, -1, "package.module") + + lexical_sources = [ + "x = 1or 0", + r"'\z'", + r"'\400'", + r'f"{1:\z}"', + r'f"\{1}"', + ] + codegen_sources = [ + "assert (1, 2)", + "1 is 1", + "def f():\n try:\n pass\n finally:\n return 1\n", + ] + + # Default source compilation uses module for matching, while diagnostics retain + # the real source filename. Test both text and bytes compilation. + for source in lexical_sources + codegen_sources: + for input_source in (source, source.encode()): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("error", SyntaxWarning) + warnings.filterwarnings( + "always", category=SyntaxWarning, module=r"^package\.module$" + ) + compile( + input_source, "", "exec", module="package.module" + ) + assert len(caught) == 1, (source, caught) + assert caught[0].filename == "" + assert caught[0].category is SyntaxWarning + with warnings.catch_warnings(): + warnings.simplefilter("ignore", SyntaxWarning) + warnings.filterwarnings( + "error", category=SyntaxWarning, module=r"^package\.module$" + ) + with assert_raises(SyntaxError): + compile( + input_source, + "", + "exec", + module="package.module", + ) + + # Compiling a pre-existing AST must use the module too. + for source in codegen_sources: + tree = ast.parse(source) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("error", SyntaxWarning) + warnings.filterwarnings( + "always", category=SyntaxWarning, module=r"^package\.module$" + ) + compile(tree, "", "exec", module="package.module") + assert len(caught) == 1, (source, caught) + assert caught[0].filename == "" + + # A following compile, with omitted or None module, must not inherit the override. + for kwargs in ({}, {"module": None}): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("error", SyntaxWarning) + warnings.filterwarnings( + "always", category=SyntaxWarning, module=r"^$" + ) + compile("1 is 1", "", "exec", **kwargs) + assert len(caught) == 1 + + # Failed parses use the fallback string scanner, which must retain the module. + for source in (r"b'\z' )", r'f"{1:\z}" )'): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("error", SyntaxWarning) + warnings.filterwarnings( + "always", category=SyntaxWarning, module=r"^package\.module$" + ) + with assert_raises(SyntaxError): + compile(source, "", "exec", module="package.module") + assert len(caught) == 1, (source, caught) + assert caught[0].filename == "" + + # AST conversion must use the same fallible, source-located warning boundary. + # TYPE_COMMENTS validates through the AST parser before compiling the source; + # the later compiler must not filter or emit the same escape a second time. + for flags in ( + ast.PyCF_ONLY_AST, + ast.PyCF_TYPE_COMMENTS, + ast.PyCF_ONLY_AST | ast.PyCF_TYPE_COMMENTS, + ast.PyCF_ONLY_AST | ast.PyCF_OPTIMIZED_AST, + ): + source = '\nπ = f"{1:\\z}"\n' + for kwargs, expected_module in ( + ({}, ""), + ({"module": None}, ""), + ({"module": "package.module"}, "package.module"), + ): + modules = [] + + class ModuleMatcher: + def match(self, module): + modules.append(module) + return True + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", SyntaxWarning) + warnings.filters.insert( + 0, ("always", None, SyntaxWarning, ModuleMatcher(), 0) + ) + compile(source, "", "exec", flags, **kwargs) + assert modules == [expected_module], (flags, kwargs, modules) + assert len(caught) == 1, (flags, kwargs, caught) + assert (caught[0].filename, caught[0].lineno) == ("", 2) + + with warnings.catch_warnings(): + warnings.simplefilter("ignore", SyntaxWarning) + warnings.filterwarnings( + "error", category=SyntaxWarning, module=r"^package\.module$" + ) + with assert_raises(SyntaxError) as caught: + compile( + source, "", "exec", flags, module="package.module" + ) + error = caught.exception + assert error.filename == "" + assert (error.lineno, error.offset, error.end_lineno, error.end_offset) == ( + 2, + 10, + 2, + 12, + ) + assert error.text == 'π = f"{1:\\z}"\n' + assert "Such sequences will not work in the future" not in error.msg + + # Warning handlers may raise arbitrary exceptions, which must survive intact. + failure = RuntimeError("warning handler failed") + + def showwarning(*args, **kwargs): + raise failure + + with warnings.catch_warnings(): + warnings.simplefilter("always", SyntaxWarning) + warnings.showwarning = showwarning + with assert_raises(RuntimeError) as caught: + compile( + source, "", "exec", flags, module="package.module" + ) + assert caught.exception is failure + + # Nested compile calls must not share module overrides or duplicate tracking. + modules = [] + locations = [] + + class ModuleMatcher: + def match(self, module): + modules.append(module) + return True + + def showwarning(message, category, filename, lineno, *args, **kwargs): + locations.append((filename, lineno)) + if len(locations) == 1: + compile(r'f"{1:\z}"', "", "exec", flags, module="package.inner") + + with warnings.catch_warnings(): + warnings.simplefilter("always", SyntaxWarning) + warnings.filters.insert( + 0, ("always", None, SyntaxWarning, ModuleMatcher(), 0) + ) + warnings.showwarning = showwarning + compile( + 'f"{1:\\z}"\nf"{2:\\q}"', + "", + "exec", + flags, + module="package.outer", + ) + assert modules == ["package.outer", "package.inner", "package.outer"], modules + assert locations == [("", 1), ("", 1), ("", 2)], locations + + # These warnings already came from AST conversion. Retain them instead of + # replacing that path with a broader scanner before every AST parse. + for source in ('''rf"{1:{'\\z'}}"''', '''f"{1:{t'\\z'}}"'''): + for flags in ( + ast.PyCF_ONLY_AST, + ast.PyCF_TYPE_COMMENTS, + ast.PyCF_ONLY_AST | ast.PyCF_TYPE_COMMENTS, + ): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("error", SyntaxWarning) + warnings.filterwarnings( + "always", category=SyntaxWarning, module=r"^package\.module$" + ) + compile( + source, "", "exec", flags, module="package.module" + ) + assert len(caught) == 1, (source, flags, caught) + assert (caught[0].filename, caught[0].lineno) == ("", 1) + + for flags in (ast.PyCF_ONLY_AST, ast.PyCF_ONLY_AST | ast.PyCF_TYPE_COMMENTS): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", SyntaxWarning) + with assert_raises(SyntaxError): + compile( + '!!!\nf"{1:\\z}"', + "", + "exec", + flags, + module="package.module", + ) + assert not caught + + # A filter changed by showwarning must affect the next distinct diagnostic, + # rather than treating the already-considered format escape as a new warning. + with warnings.catch_warnings(): + warnings.simplefilter("always", SyntaxWarning) + + def showwarning(*args, **kwargs): + warnings.simplefilter("error", SyntaxWarning) + + warnings.showwarning = showwarning + with assert_raises(SyntaxError) as caught: + compile( + 'f"{1:\\z}"\n"\\q"', + "", + "exec", + ast.PyCF_TYPE_COMMENTS, + module="package.module", + ) + assert caught.exception.lineno == 2 + assert "\\q" in caught.exception.msg + + # Raw literal text is not an invalid escape. Nested nonraw expressions still + # retain the diagnostics previously emitted during AST conversion. + for flags in ( + ast.PyCF_ONLY_AST, + ast.PyCF_TYPE_COMMENTS, + ast.PyCF_ONLY_AST | ast.PyCF_TYPE_COMMENTS, + ): + for source in ( + r'rf"{1:\z}"', + r'rf"{1:{2:\z}}"', + r'tr"{1:{2:\z}}"', + "f\"{1:{r'\\z'}}\"", + ): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("error", SyntaxWarning) + compile( + source, "", "exec", flags, module="package.module" + ) + assert not caught, (source, flags, caught) + for source in ("rf\"{f'{2:\\z}'}\"", "rf\"{1:{'\\z'}}\""): + with warnings.catch_warnings(): + warnings.simplefilter("error", SyntaxWarning) + with assert_raises(SyntaxError): + compile( + source, + "", + "exec", + flags, + module="package.module", + ) + + # Func-type parsing trims its source before converting the parsed nodes. + # Already-detected escapes must still point into the original source. + for prefix in ("", "\n\n"): + source = prefix + '(int) -> f"{1:aaaaaaaaaaaa\\zbbbbbbbbbbbb}"' + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", SyntaxWarning) + compile( + source, + "", + "func_type", + ast.PyCF_ONLY_AST, + module="package.module", + ) + assert len(caught) == 1, (source, caught) + assert caught[0].filename == "" + assert caught[0].lineno == prefix.count("\n") + 1 + + # Warn for the first invalid escape in each original literal, including + # literals later folded together. Comments are not literal source text. + for source, count in ( + (r'f"{1:\z\q}"', 1), + (r'f"{1:\{2}}"', 1), + (r'f"{1:\\{2}}"', 0), + ('f"{1:{(\n1 # \\z\n)}}"', 0), + ("f\"{1:{'\\z' + '\\q'}}\"", 2), + ("f\"{1:{'\\z' '\\q'}}\"", 2), + ("f\"{1:{('\\z', '\\q')[0]}}\"", 2), + ): + for flags in ( + ast.PyCF_ONLY_AST, + ast.PyCF_TYPE_COMMENTS, + ast.PyCF_ONLY_AST | ast.PyCF_OPTIMIZED_AST, + ): + for optimize in (0, 2): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", SyntaxWarning) + compile( + source, + "", + "exec", + flags, + optimize=optimize, + module="package.module", + ) + assert len(caught) == count, (source, flags, optimize, caught) + assert all(w.filename == "" for w in caught) + + +skip_if_unsupported(3, 15, test_compile_module)