From 6ba7166f2a2add3a162f7d53ce64db43c6c0e258 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 12:29:50 -0500 Subject: [PATCH 01/36] add unused Parser::syntax_errors field --- crates/ruff_python_parser/src/error.rs | 9 +++++++++ crates/ruff_python_parser/src/lib.rs | 7 ++++++- crates/ruff_python_parser/src/parser/mod.rs | 8 ++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 98efdf52e2e48..3f1c23dc66342 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -426,6 +426,15 @@ impl std::fmt::Display for LexicalErrorType { } } +#[derive(Debug, PartialEq, Clone)] +pub struct SyntaxError { + pub error: SyntaxErrorType, + pub location: TextRange, +} + +#[derive(Debug, PartialEq, Clone)] +pub enum SyntaxErrorType {} + #[cfg(target_pointer_width = "64")] mod sizes { use crate::error::{LexicalError, LexicalErrorType}; diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 61db67ddad4f4..4eb872f54e24f 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -67,7 +67,9 @@ use std::iter::FusedIterator; use std::ops::Deref; -pub use crate::error::{FStringErrorType, LexicalErrorType, ParseError, ParseErrorType}; +pub use crate::error::{ + FStringErrorType, LexicalErrorType, ParseError, ParseErrorType, SyntaxError, SyntaxErrorType, +}; pub use crate::parser::ParseOptions; pub use crate::token::{Token, TokenKind}; @@ -305,6 +307,7 @@ pub struct Parsed { syntax: T, tokens: Tokens, errors: Vec, + syntax_errors: Vec, } impl Parsed { @@ -373,6 +376,7 @@ impl Parsed { syntax: module, tokens: self.tokens, errors: self.errors, + syntax_errors: self.syntax_errors, }), Mod::Expression(_) => None, } @@ -392,6 +396,7 @@ impl Parsed { syntax: expression, tokens: self.tokens, errors: self.errors, + syntax_errors: self.syntax_errors, }), } } diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 951727667f0ce..67a68eaca2273 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -5,6 +5,7 @@ use bitflags::bitflags; use ruff_python_ast::{Mod, ModExpression, ModModule}; use ruff_text_size::{Ranged, TextRange, TextSize}; +use crate::error::SyntaxError; use crate::parser::expression::ExpressionContext; use crate::parser::progress::{ParserProgress, TokenId}; use crate::token::TokenValue; @@ -35,6 +36,10 @@ pub(crate) struct Parser<'src> { /// Stores all the syntax errors found during the parsing. errors: Vec, + /// Stores non-fatal syntax errors found during parsing, such as version-related errors and + /// errors detected by the Python compiler. + syntax_errors: Vec, + /// Options for how the code will be parsed. options: ParseOptions, @@ -70,6 +75,7 @@ impl<'src> Parser<'src> { options, source, errors: Vec::new(), + syntax_errors: Vec::new(), tokens, recovery_context: RecoveryContext::empty(), prev_token_end: TextSize::new(0), @@ -166,6 +172,7 @@ impl<'src> Parser<'src> { syntax, tokens: Tokens::new(tokens), errors: parse_errors, + syntax_errors: self.syntax_errors, }; } @@ -197,6 +204,7 @@ impl<'src> Parser<'src> { syntax, tokens: Tokens::new(tokens), errors: merged, + syntax_errors: self.syntax_errors, } } From cab545f206e699e165b608d09f4c2a59f2803b60 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 13:15:07 -0500 Subject: [PATCH 02/36] add hard-coded python version and detect `match` --- crates/ruff_python_parser/src/error.rs | 4 +++- crates/ruff_python_parser/src/parser/mod.rs | 7 +++++++ crates/ruff_python_parser/src/parser/statement.rs | 14 ++++++++++++-- .../ruff_python_parser/src/parser/version/mod.rs | 13 +++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 crates/ruff_python_parser/src/parser/version/mod.rs diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 3f1c23dc66342..42423a5296f8c 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -433,7 +433,9 @@ pub struct SyntaxError { } #[derive(Debug, PartialEq, Clone)] -pub enum SyntaxErrorType {} +pub enum SyntaxErrorType { + MatchBeforePy310, +} #[cfg(target_pointer_width = "64")] mod sizes { diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 67a68eaca2273..74948a01573dc 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -8,6 +8,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::error::SyntaxError; use crate::parser::expression::ExpressionContext; use crate::parser::progress::{ParserProgress, TokenId}; +use crate::parser::version::PythonVersion; use crate::token::TokenValue; use crate::token_set::TokenSet; use crate::token_source::{TokenSource, TokenSourceCheckpoint}; @@ -23,6 +24,8 @@ mod pattern; mod progress; mod recovery; mod statement; +mod version; + #[cfg(test)] mod tests; @@ -40,6 +43,9 @@ pub(crate) struct Parser<'src> { /// errors detected by the Python compiler. syntax_errors: Vec, + /// The Python version to use for checking version-related syntax errors. + python_version: PythonVersion, + /// Options for how the code will be parsed. options: ParseOptions, @@ -76,6 +82,7 @@ impl<'src> Parser<'src> { source, errors: Vec::new(), syntax_errors: Vec::new(), + python_version: PythonVersion::PY38, // TODO this needs to be passed from outside tokens, recovery_context: RecoveryContext::empty(), prev_token_end: TextSize::new(0), diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index e76fc08915e1f..8972ceafd4118 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -16,9 +16,10 @@ use crate::parser::{ }; use crate::token::{TokenKind, TokenValue}; use crate::token_set::TokenSet; -use crate::{Mode, ParseErrorType}; +use crate::{Mode, ParseErrorType, SyntaxError, SyntaxErrorType}; use super::expression::ExpressionContext; +use super::version::PythonVersion; use super::Parenthesized; /// Tokens that represent compound statements. @@ -2262,10 +2263,19 @@ impl<'src> Parser<'src> { let cases = self.parse_match_body(); + let range = self.node_range(start); + + if self.python_version < PythonVersion::PY310 { + self.syntax_errors.push(SyntaxError { + error: SyntaxErrorType::MatchBeforePy310, + location: range, + }); + } + ast::StmtMatch { subject: Box::new(subject), cases, - range: self.node_range(start), + range, } } diff --git a/crates/ruff_python_parser/src/parser/version/mod.rs b/crates/ruff_python_parser/src/parser/version/mod.rs new file mode 100644 index 0000000000000..454199c73c1ac --- /dev/null +++ b/crates/ruff_python_parser/src/parser/version/mod.rs @@ -0,0 +1,13 @@ +#[derive(Debug, Eq, PartialOrd, Ord, PartialEq)] +pub(super) struct PythonVersion { + major: u8, + minor: u8, +} + +impl PythonVersion { + pub(super) const PY38: PythonVersion = PythonVersion { major: 3, minor: 8 }; + pub(super) const PY310: PythonVersion = PythonVersion { + major: 3, + minor: 10, + }; +} From 8f0bd4c87241310efc8814d46be20d1a852a6f8f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 13:43:04 -0500 Subject: [PATCH 03/36] make PythonVersion more public, convert SyntaxError in red-knot --- crates/red_knot_project/src/lib.rs | 9 +++ crates/red_knot_python_semantic/src/lib.rs | 1 + crates/red_knot_python_semantic/src/syntax.rs | 58 +++++++++++++++++++ crates/ruff_db/src/diagnostic.rs | 22 +++++-- crates/ruff_python_parser/src/error.rs | 31 ++++++++-- crates/ruff_python_parser/src/lib.rs | 7 ++- crates/ruff_python_parser/src/parser/mod.rs | 4 +- .../src/parser/statement.rs | 9 +-- .../src/parser/version/mod.rs | 13 ----- crates/ruff_python_parser/src/version.rs | 13 +++++ 10 files changed, 136 insertions(+), 31 deletions(-) create mode 100644 crates/red_knot_python_semantic/src/syntax.rs delete mode 100644 crates/ruff_python_parser/src/parser/version/mod.rs create mode 100644 crates/ruff_python_parser/src/version.rs diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index 2126f36a841e9..3eb3faeacc0af 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -7,6 +7,7 @@ use metadata::settings::Settings; pub use metadata::{ProjectDiscoveryError, ProjectMetadata}; use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection}; use red_knot_python_semantic::register_lints; +use red_knot_python_semantic::syntax::SyntaxDiagnostic; use red_knot_python_semantic::types::check_types; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity, Span}; use ruff_db::files::{system_path_to_file, File}; @@ -339,6 +340,14 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec> { diagnostic })); + if parsed.is_valid() { + diagnostics.extend(parsed.syntax_errors().iter().map(|error| { + let diagnostic: Box = + Box::new(SyntaxDiagnostic::from_syntax_error(error, file)); + diagnostic + })); + } + diagnostics.extend(check_types(db.upcast(), file).iter().map(|diagnostic| { let boxed: Box = Box::new(diagnostic.clone()); boxed diff --git a/crates/red_knot_python_semantic/src/lib.rs b/crates/red_knot_python_semantic/src/lib.rs index ba8cf29d8fd72..0625fdd741bd6 100644 --- a/crates/red_knot_python_semantic/src/lib.rs +++ b/crates/red_knot_python_semantic/src/lib.rs @@ -24,6 +24,7 @@ mod semantic_model; pub(crate) mod site_packages; mod suppression; pub(crate) mod symbol; +pub mod syntax; pub mod types; mod unpack; mod util; diff --git a/crates/red_knot_python_semantic/src/syntax.rs b/crates/red_knot_python_semantic/src/syntax.rs new file mode 100644 index 0000000000000..fd3ea61fb9a9b --- /dev/null +++ b/crates/red_knot_python_semantic/src/syntax.rs @@ -0,0 +1,58 @@ +use std::borrow::Cow; + +use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity}; +use ruff_db::files::File; +use ruff_python_parser::SyntaxError; +use ruff_text_size::TextRange; + +use crate::PythonVersion; + +#[derive(Debug, Eq, PartialEq, Clone)] +pub struct SyntaxDiagnostic { + id: DiagnosticId, + message: String, + file: File, + range: TextRange, +} + +impl Diagnostic for SyntaxDiagnostic { + fn id(&self) -> ruff_db::diagnostic::DiagnosticId { + self.id + } + + fn message(&self) -> Cow { + Cow::from(&self.message) + } + + fn file(&self) -> Option { + Some(self.file) + } + + fn range(&self) -> Option { + Some(self.range) + } + + fn severity(&self) -> ruff_db::diagnostic::Severity { + Severity::Error + } +} + +impl From for ruff_python_parser::version::PythonVersion { + fn from(value: PythonVersion) -> Self { + Self { + major: value.major, + minor: value.minor, + } + } +} + +impl SyntaxDiagnostic { + pub fn from_syntax_error(value: &SyntaxError, file: File) -> Self { + Self { + id: DiagnosticId::invalid_syntax(Some(value.kind.as_str())), + message: value.message(), + file, + range: value.range, + } + } +} diff --git a/crates/ruff_db/src/diagnostic.rs b/crates/ruff_db/src/diagnostic.rs index 7b47735843bb8..1fb68a1c2f096 100644 --- a/crates/ruff_db/src/diagnostic.rs +++ b/crates/ruff_db/src/diagnostic.rs @@ -68,8 +68,12 @@ pub enum DiagnosticId { /// Some I/O operation failed Io, - /// Some code contains a syntax error - InvalidSyntax, + /// Some code contains a syntax error. + /// + /// Contains `Some` for syntax errors that are individually documented (as opposed to those + /// emitted by the parser). An example of an individually documented syntax error might be use of + /// the `match` statement on a Python version before 3.10. + InvalidSyntax(Option), /// A lint violation. /// @@ -89,6 +93,14 @@ impl DiagnosticId { Self::Lint(LintName::of(name)) } + /// Creates a new `DiagnosticId` for a syntax error with an optional name. + pub const fn invalid_syntax(name: Option<&'static str>) -> Self { + Self::InvalidSyntax(match name { + Some(name) => Some(LintName::of(name)), + None => None, + }) + } + /// Returns `true` if this `DiagnosticId` represents a lint. pub fn is_lint(&self) -> bool { matches!(self, DiagnosticId::Lint(_)) @@ -126,8 +138,8 @@ impl DiagnosticId { pub fn as_str(&self) -> Result<&str, DiagnosticAsStrError> { Ok(match self { DiagnosticId::Io => "io", - DiagnosticId::InvalidSyntax => "invalid-syntax", - DiagnosticId::Lint(name) => { + DiagnosticId::InvalidSyntax(None) => "invalid-syntax", + DiagnosticId::InvalidSyntax(Some(name)) | DiagnosticId::Lint(name) => { return Err(DiagnosticAsStrError::Category { category: "lint", name: name.as_str(), @@ -591,7 +603,7 @@ impl ParseDiagnostic { impl Diagnostic for ParseDiagnostic { fn id(&self) -> DiagnosticId { - DiagnosticId::InvalidSyntax + DiagnosticId::InvalidSyntax(None) } fn message(&self) -> Cow { diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 42423a5296f8c..24a6ad854d13b 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -2,7 +2,7 @@ use std::fmt; use ruff_text_size::TextRange; -use crate::TokenKind; +use crate::{version::PythonVersion, TokenKind}; /// Represents represent errors that occur during parsing and are /// returned by the `parse_*` functions. @@ -428,15 +428,36 @@ impl std::fmt::Display for LexicalErrorType { #[derive(Debug, PartialEq, Clone)] pub struct SyntaxError { - pub error: SyntaxErrorType, - pub location: TextRange, + pub kind: SyntaxErrorKind, + pub range: TextRange, + pub target_version: PythonVersion, } -#[derive(Debug, PartialEq, Clone)] -pub enum SyntaxErrorType { +impl SyntaxError { + pub fn message(&self) -> String { + match self.kind { + SyntaxErrorKind::MatchBeforePy310 => format!( + "Cannot use `match` statement on Python {major}.{minor} (syntax was new in Python 3.10)", + major = self.target_version.major, + minor = self.target_version.minor, + ), + } + } +} + +#[derive(Debug, PartialEq, Clone, Copy)] +pub enum SyntaxErrorKind { MatchBeforePy310, } +impl SyntaxErrorKind { + pub const fn as_str(self) -> &'static str { + match self { + SyntaxErrorKind::MatchBeforePy310 => "match-before-python-310", + } + } +} + #[cfg(target_pointer_width = "64")] mod sizes { use crate::error::{LexicalError, LexicalErrorType}; diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 4eb872f54e24f..df4c9e20a436f 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -68,7 +68,7 @@ use std::iter::FusedIterator; use std::ops::Deref; pub use crate::error::{ - FStringErrorType, LexicalErrorType, ParseError, ParseErrorType, SyntaxError, SyntaxErrorType, + FStringErrorType, LexicalErrorType, ParseError, ParseErrorType, SyntaxError, SyntaxErrorKind, }; pub use crate::parser::ParseOptions; pub use crate::token::{Token, TokenKind}; @@ -89,6 +89,7 @@ mod token; mod token_set; mod token_source; pub mod typing; +pub mod version; /// Parse a full Python module usually consisting of multiple lines. /// @@ -326,6 +327,10 @@ impl Parsed { &self.errors } + pub fn syntax_errors(&self) -> &[SyntaxError] { + &self.syntax_errors + } + /// Consumes the [`Parsed`] output and returns the contained syntax node. pub fn into_syntax(self) -> T { self.syntax diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 74948a01573dc..41c40735f289a 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -8,10 +8,10 @@ use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::error::SyntaxError; use crate::parser::expression::ExpressionContext; use crate::parser::progress::{ParserProgress, TokenId}; -use crate::parser::version::PythonVersion; use crate::token::TokenValue; use crate::token_set::TokenSet; use crate::token_source::{TokenSource, TokenSourceCheckpoint}; +use crate::version::PythonVersion; use crate::{Mode, ParseError, ParseErrorType, TokenKind}; use crate::{Parsed, Tokens}; @@ -24,8 +24,6 @@ mod pattern; mod progress; mod recovery; mod statement; -mod version; - #[cfg(test)] mod tests; diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 8972ceafd4118..305863f74796c 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -16,10 +16,10 @@ use crate::parser::{ }; use crate::token::{TokenKind, TokenValue}; use crate::token_set::TokenSet; -use crate::{Mode, ParseErrorType, SyntaxError, SyntaxErrorType}; +use crate::version::PythonVersion; +use crate::{Mode, ParseErrorType, SyntaxError, SyntaxErrorKind}; use super::expression::ExpressionContext; -use super::version::PythonVersion; use super::Parenthesized; /// Tokens that represent compound statements. @@ -2267,8 +2267,9 @@ impl<'src> Parser<'src> { if self.python_version < PythonVersion::PY310 { self.syntax_errors.push(SyntaxError { - error: SyntaxErrorType::MatchBeforePy310, - location: range, + kind: SyntaxErrorKind::MatchBeforePy310, + range, + target_version: self.python_version, }); } diff --git a/crates/ruff_python_parser/src/parser/version/mod.rs b/crates/ruff_python_parser/src/parser/version/mod.rs deleted file mode 100644 index 454199c73c1ac..0000000000000 --- a/crates/ruff_python_parser/src/parser/version/mod.rs +++ /dev/null @@ -1,13 +0,0 @@ -#[derive(Debug, Eq, PartialOrd, Ord, PartialEq)] -pub(super) struct PythonVersion { - major: u8, - minor: u8, -} - -impl PythonVersion { - pub(super) const PY38: PythonVersion = PythonVersion { major: 3, minor: 8 }; - pub(super) const PY310: PythonVersion = PythonVersion { - major: 3, - minor: 10, - }; -} diff --git a/crates/ruff_python_parser/src/version.rs b/crates/ruff_python_parser/src/version.rs new file mode 100644 index 0000000000000..55d5155575465 --- /dev/null +++ b/crates/ruff_python_parser/src/version.rs @@ -0,0 +1,13 @@ +#[derive(Clone, Copy, Debug, Eq, PartialOrd, Ord, PartialEq)] +pub struct PythonVersion { + pub major: u8, + pub minor: u8, +} + +impl PythonVersion { + pub const PY38: PythonVersion = PythonVersion { major: 3, minor: 8 }; + pub const PY310: PythonVersion = PythonVersion { + major: 3, + minor: 10, + }; +} From 846314f7969bb2140a4533575f3d73657b7f6adc Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 14:04:18 -0500 Subject: [PATCH 04/36] process SyntaxErrors in ruff --- crates/ruff_linter/src/linter.rs | 10 +++++++++- crates/ruff_linter/src/message/mod.rs | 11 ++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index ed27a6fdfd456..f4bfaf4db2df4 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -13,7 +13,7 @@ use ruff_notebook::Notebook; use ruff_python_ast::{ModModule, PySourceType}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; -use ruff_python_parser::{ParseError, Parsed}; +use ruff_python_parser::{ParseError, Parsed, SyntaxError}; use ruff_source_file::SourceFileBuilder; use ruff_text_size::Ranged; @@ -426,6 +426,7 @@ pub fn lint_only( messages: diagnostics_to_messages( diagnostics, parsed.errors(), + parsed.syntax_errors(), path, &locator, &directives, @@ -438,6 +439,7 @@ pub fn lint_only( fn diagnostics_to_messages( diagnostics: Vec, parse_errors: &[ParseError], + syntax_errors: &[SyntaxError], path: &Path, locator: &Locator, directives: &Directives, @@ -456,6 +458,11 @@ fn diagnostics_to_messages( parse_errors .iter() .map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone())) + .chain( + syntax_errors + .iter() + .map(|syntax_error| Message::from_syntax_error(syntax_error, file.deref().clone())), + ) .chain(diagnostics.into_iter().map(|diagnostic| { let noqa_offset = directives.noqa_line_for.resolve(diagnostic.start()); Message::from_diagnostic(diagnostic, file.deref().clone(), noqa_offset) @@ -573,6 +580,7 @@ pub fn lint_fix<'a>( messages: diagnostics_to_messages( diagnostics, parsed.errors(), + parsed.syntax_errors(), path, &locator, &directives, diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 52de250c4c32d..e958069b97531 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -16,7 +16,7 @@ pub use pylint::PylintEmitter; pub use rdjson::RdjsonEmitter; use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix}; use ruff_notebook::NotebookIndex; -use ruff_python_parser::ParseError; +use ruff_python_parser::{ParseError, SyntaxError}; use ruff_source_file::{SourceFile, SourceLocation}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; pub use sarif::SarifEmitter; @@ -121,6 +121,15 @@ impl Message { }) } + /// Create a [`Message`] from the given [`SyntaxError`]. + pub fn from_syntax_error(syntax_error: &SyntaxError, file: SourceFile) -> Message { + Message::SyntaxError(SyntaxErrorMessage { + message: format!("SyntaxError: {}", syntax_error.message()), + range: syntax_error.range, + file, + }) + } + pub const fn as_diagnostic_message(&self) -> Option<&DiagnosticMessage> { match self { Message::Diagnostic(m) => Some(m), From 4712ff4d140e7ee01060aabca093ffbcd5645470 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 15:28:14 -0500 Subject: [PATCH 05/36] pass target version at the very end I initially tried passing the target version to all of the parser functions, but this required a huge number of changes. The downside of the current approach is that we're likely to accumulate quite a large Vec in the parser, only to filter it down later. The upside is that we don't have to fix every single call site of every parser function right now --- crates/red_knot_project/src/lib.rs | 7 ++++--- crates/red_knot_python_semantic/src/syntax.rs | 8 ++++++-- crates/ruff_linter/src/linter.rs | 20 ++++++++++--------- crates/ruff_linter/src/message/mod.rs | 9 +++++++-- crates/ruff_python_parser/src/error.rs | 14 +++++++++---- crates/ruff_python_parser/src/lib.rs | 11 ++++++++-- crates/ruff_python_parser/src/parser/mod.rs | 5 ----- .../src/parser/statement.rs | 12 ++++------- 8 files changed, 51 insertions(+), 35 deletions(-) diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index 3eb3faeacc0af..89fa847e90521 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -6,9 +6,9 @@ use files::{Index, Indexed, IndexedFiles}; use metadata::settings::Settings; pub use metadata::{ProjectDiscoveryError, ProjectMetadata}; use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection}; -use red_knot_python_semantic::register_lints; use red_knot_python_semantic::syntax::SyntaxDiagnostic; use red_knot_python_semantic::types::check_types; +use red_knot_python_semantic::{register_lints, Program}; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity, Span}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; @@ -341,9 +341,10 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec> { })); if parsed.is_valid() { - diagnostics.extend(parsed.syntax_errors().iter().map(|error| { + let version = Program::get(db).python_version(db); + diagnostics.extend(parsed.syntax_errors(version.into()).map(|error| { let diagnostic: Box = - Box::new(SyntaxDiagnostic::from_syntax_error(error, file)); + Box::new(SyntaxDiagnostic::from_syntax_error(error, file, version)); diagnostic })); } diff --git a/crates/red_knot_python_semantic/src/syntax.rs b/crates/red_knot_python_semantic/src/syntax.rs index fd3ea61fb9a9b..debf5fcfe30e3 100644 --- a/crates/red_knot_python_semantic/src/syntax.rs +++ b/crates/red_knot_python_semantic/src/syntax.rs @@ -47,10 +47,14 @@ impl From for ruff_python_parser::version::PythonVersion { } impl SyntaxDiagnostic { - pub fn from_syntax_error(value: &SyntaxError, file: File) -> Self { + pub fn from_syntax_error( + value: &SyntaxError, + file: File, + target_version: PythonVersion, + ) -> Self { Self { id: DiagnosticId::invalid_syntax(Some(value.kind.as_str())), - message: value.message(), + message: value.message(target_version.into()), file, range: value.range, } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index f4bfaf4db2df4..5e77867df4292 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -6,6 +6,7 @@ use std::path::Path; use anyhow::{anyhow, Result}; use colored::Colorize; use itertools::Itertools; +use ruff_python_parser::version::PythonVersion; use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; @@ -426,23 +427,25 @@ pub fn lint_only( messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(), + parsed.syntax_errors(settings.target_version.into()), path, &locator, &directives, + settings.target_version.into(), ), has_syntax_error: !parsed.is_valid(), } } /// Convert from diagnostics to messages. -fn diagnostics_to_messages( +fn diagnostics_to_messages<'a>( diagnostics: Vec, parse_errors: &[ParseError], - syntax_errors: &[SyntaxError], + syntax_errors: impl Iterator, path: &Path, locator: &Locator, directives: &Directives, + target_version: PythonVersion, ) -> Vec { let file = LazyCell::new(|| { let mut builder = @@ -458,11 +461,9 @@ fn diagnostics_to_messages( parse_errors .iter() .map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone())) - .chain( - syntax_errors - .iter() - .map(|syntax_error| Message::from_syntax_error(syntax_error, file.deref().clone())), - ) + .chain(syntax_errors.map(|syntax_error| { + Message::from_syntax_error(syntax_error, file.deref().clone(), target_version) + })) .chain(diagnostics.into_iter().map(|diagnostic| { let noqa_offset = directives.noqa_line_for.resolve(diagnostic.start()); Message::from_diagnostic(diagnostic, file.deref().clone(), noqa_offset) @@ -580,10 +581,11 @@ pub fn lint_fix<'a>( messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(), + parsed.syntax_errors(settings.target_version.into()), path, &locator, &directives, + settings.target_version.into(), ), has_syntax_error: !is_valid_syntax, }, diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index e958069b97531..65962d9836651 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -3,6 +3,7 @@ use std::collections::BTreeMap; use std::io::Write; use std::ops::Deref; +use ruff_python_parser::version::PythonVersion; use rustc_hash::FxHashMap; pub use azure::AzureEmitter; @@ -122,9 +123,13 @@ impl Message { } /// Create a [`Message`] from the given [`SyntaxError`]. - pub fn from_syntax_error(syntax_error: &SyntaxError, file: SourceFile) -> Message { + pub fn from_syntax_error( + syntax_error: &SyntaxError, + file: SourceFile, + target_version: PythonVersion, + ) -> Message { Message::SyntaxError(SyntaxErrorMessage { - message: format!("SyntaxError: {}", syntax_error.message()), + message: format!("SyntaxError: {}", syntax_error.message(target_version)), range: syntax_error.range, file, }) diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 24a6ad854d13b..97b763d790c65 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -430,19 +430,25 @@ impl std::fmt::Display for LexicalErrorType { pub struct SyntaxError { pub kind: SyntaxErrorKind, pub range: TextRange, - pub target_version: PythonVersion, } impl SyntaxError { - pub fn message(&self) -> String { + pub fn message(&self, target_version: PythonVersion) -> String { match self.kind { SyntaxErrorKind::MatchBeforePy310 => format!( "Cannot use `match` statement on Python {major}.{minor} (syntax was new in Python 3.10)", - major = self.target_version.major, - minor = self.target_version.minor, + major = target_version.major, + minor = target_version.minor, ), } } + + /// The earliest allowed version for the syntax associated with this error. + pub const fn version(&self) -> PythonVersion { + match self.kind { + SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, + } + } } #[derive(Debug, PartialEq, Clone, Copy)] diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index df4c9e20a436f..1864f931b792b 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -80,6 +80,7 @@ use ruff_python_ast::{ }; use ruff_python_trivia::CommentRanges; use ruff_text_size::{Ranged, TextRange, TextSize}; +use version::PythonVersion; mod error; pub mod lexer; @@ -327,8 +328,14 @@ impl Parsed { &self.errors } - pub fn syntax_errors(&self) -> &[SyntaxError] { - &self.syntax_errors + /// Returns the syntax errors for `target_version`. + pub fn syntax_errors( + &self, + target_version: PythonVersion, + ) -> impl Iterator { + self.syntax_errors + .iter() + .filter(move |error| target_version < error.version()) } /// Consumes the [`Parsed`] output and returns the contained syntax node. diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 41c40735f289a..67a68eaca2273 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -11,7 +11,6 @@ use crate::parser::progress::{ParserProgress, TokenId}; use crate::token::TokenValue; use crate::token_set::TokenSet; use crate::token_source::{TokenSource, TokenSourceCheckpoint}; -use crate::version::PythonVersion; use crate::{Mode, ParseError, ParseErrorType, TokenKind}; use crate::{Parsed, Tokens}; @@ -41,9 +40,6 @@ pub(crate) struct Parser<'src> { /// errors detected by the Python compiler. syntax_errors: Vec, - /// The Python version to use for checking version-related syntax errors. - python_version: PythonVersion, - /// Options for how the code will be parsed. options: ParseOptions, @@ -80,7 +76,6 @@ impl<'src> Parser<'src> { source, errors: Vec::new(), syntax_errors: Vec::new(), - python_version: PythonVersion::PY38, // TODO this needs to be passed from outside tokens, recovery_context: RecoveryContext::empty(), prev_token_end: TextSize::new(0), diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 305863f74796c..582668a75f15e 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -16,7 +16,6 @@ use crate::parser::{ }; use crate::token::{TokenKind, TokenValue}; use crate::token_set::TokenSet; -use crate::version::PythonVersion; use crate::{Mode, ParseErrorType, SyntaxError, SyntaxErrorKind}; use super::expression::ExpressionContext; @@ -2265,13 +2264,10 @@ impl<'src> Parser<'src> { let range = self.node_range(start); - if self.python_version < PythonVersion::PY310 { - self.syntax_errors.push(SyntaxError { - kind: SyntaxErrorKind::MatchBeforePy310, - range, - target_version: self.python_version, - }); - } + self.syntax_errors.push(SyntaxError { + kind: SyntaxErrorKind::MatchBeforePy310, + range, + }); ast::StmtMatch { subject: Box::new(subject), From 27e9461b51891acd958edab146a693f60060bd92 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 15:36:43 -0500 Subject: [PATCH 06/36] pass tests --- crates/red_knot_project/src/lib.rs | 6 +++++- crates/ruff/src/cache.rs | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index 89fa847e90521..389572b13a88c 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -341,7 +341,11 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec> { })); if parsed.is_valid() { - let version = Program::get(db).python_version(db); + // TODO should just be `get` but panics in + // tests::check_file_skips_type_checking_when_file_cant_be_read + let version = Program::try_get(db) + .map(|p| p.python_version(db)) + .unwrap_or_default(); diagnostics.extend(parsed.syntax_errors(version.into()).map(|error| { let diagnostic: Box = Box::new(SyntaxDiagnostic::from_syntax_error(error, file, version)); diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 40b495189afbc..91beb5c3c9d82 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -586,6 +586,8 @@ mod tests { use anyhow::Result; use filetime::{set_file_mtime, FileTime}; use itertools::Itertools; + use ruff_linter::settings::types::PythonVersion; + use ruff_linter::settings::LinterSettings; use test_case::test_case; use ruff_cache::CACHE_DIR_NAME; @@ -611,6 +613,10 @@ mod tests { let settings = Settings { cache_dir, + linter: LinterSettings { + target_version: PythonVersion::Py310, + ..Default::default() + }, ..Settings::default() }; From 625682ece1c8c6e54ad56fc3f1d768dccee3129c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 16:04:07 -0500 Subject: [PATCH 07/36] add ruff test case --- crates/ruff/tests/lint.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index a082ab7cbb6c0..6515102f56bdd 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2567,3 +2567,29 @@ fn a005_module_shadowing_strict_default() -> Result<()> { }); Ok(()) } + +#[test] +fn match_before_py310() { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py39") + .arg("-") + .pass_stdin( + r#" +match 2: + case 1: + print("it's one") +"# + ), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:2:1: SyntaxError: Cannot use `match` statement on Python 3.9 (syntax was new in Python 3.10) + Found 1 error. + + ----- stderr ----- + " + ); +} From e2614be95b4e126f3c023cd0640836013cf97248 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 16:29:08 -0500 Subject: [PATCH 08/36] detect late future imports in the parser --- crates/ruff_python_parser/src/error.rs | 6 ++++++ crates/ruff_python_parser/src/parser/mod.rs | 9 +++++++++ .../src/parser/statement.rs | 19 ++++++++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 97b763d790c65..9472b3b39b8e6 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -440,6 +440,9 @@ impl SyntaxError { major = target_version.major, minor = target_version.minor, ), + SyntaxErrorKind::LateFutureImport => { + "__future__ imports must be located at the beginning of a file".to_string() + } } } @@ -447,12 +450,14 @@ impl SyntaxError { pub const fn version(&self) -> PythonVersion { match self.kind { SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, + SyntaxErrorKind::LateFutureImport => PythonVersion { major: 0, minor: 0 }, } } } #[derive(Debug, PartialEq, Clone, Copy)] pub enum SyntaxErrorKind { + LateFutureImport, MatchBeforePy310, } @@ -460,6 +465,7 @@ impl SyntaxErrorKind { pub const fn as_str(self) -> &'static str { match self { SyntaxErrorKind::MatchBeforePy310 => "match-before-python-310", + SyntaxErrorKind::LateFutureImport => "late-future-import", } } } diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 67a68eaca2273..c60d76ed4fead 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -26,6 +26,12 @@ mod statement; #[cfg(test)] mod tests; +#[derive(Debug, Default)] +struct SyntaxErrorState { + /// Whether or not the [`Parser`] has traversed past the "top-of-file" import boundary. + seen_futures_boundary: bool, +} + #[derive(Debug)] pub(crate) struct Parser<'src> { source: &'src str, @@ -40,6 +46,8 @@ pub(crate) struct Parser<'src> { /// errors detected by the Python compiler. syntax_errors: Vec, + syntax_error_state: SyntaxErrorState, + /// Options for how the code will be parsed. options: ParseOptions, @@ -76,6 +84,7 @@ impl<'src> Parser<'src> { source, errors: Vec::new(), syntax_errors: Vec::new(), + syntax_error_state: SyntaxErrorState::default(), tokens, recovery_context: RecoveryContext::empty(), prev_token_end: TextSize::new(0), diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 582668a75f15e..49447be9e30b0 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -510,6 +510,8 @@ impl<'src> Parser<'src> { self.add_error(ParseErrorType::EmptyImportNames, self.current_token_range()); } + self.syntax_error_state.seen_futures_boundary = true; + ast::StmtImport { range: self.node_range(start), names, @@ -619,11 +621,26 @@ impl<'src> Parser<'src> { self.expect(TokenKind::Rpar); } + let range = self.node_range(start); + + // Allow __future__ imports until we see a non-__future__ import. + match module.as_deref() { + Some("__future__") => { + if self.syntax_error_state.seen_futures_boundary { + self.syntax_errors.push(SyntaxError { + kind: SyntaxErrorKind::LateFutureImport, + range, + }); + } + } + _ => self.syntax_error_state.seen_futures_boundary = true, + } + ast::StmtImportFrom { module, names, level: leading_dots, - range: self.node_range(start), + range, } } From 4423bd4a8eb126ed94c893e25d8b0c1f918193c2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 17:57:55 -0500 Subject: [PATCH 09/36] pass f404 tests --- .../src/checkers/ast/analyze/statement.rs | 8 ------- crates/ruff_linter/src/linter.rs | 24 ++++++++++++++++++- crates/ruff_linter/src/message/mod.rs | 21 ++++++++++++---- crates/ruff_python_parser/src/error.rs | 5 +++- crates/ruff_python_parser/src/parser/mod.rs | 1 + .../src/parser/statement.rs | 23 +++++++++++++++--- 6 files changed, 64 insertions(+), 18 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 4b5594e47f801..8b0a7b57d06bf 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -855,14 +855,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::FutureFeatureNotDefined) { pyflakes::rules::future_feature_not_defined(checker, alias); } - if checker.enabled(Rule::LateFutureImport) { - if checker.semantic.seen_futures_boundary() { - checker.report_diagnostic(Diagnostic::new( - pyflakes::rules::LateFutureImport, - stmt.range(), - )); - } - } } else if &alias.name == "*" { if checker.enabled(Rule::UndefinedLocalWithNestedImportStarUsage) { if !matches!(checker.semantic.current_scope().kind, ScopeKind::Module) { diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 5e77867df4292..1376b2790c6ef 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -292,6 +292,12 @@ pub fn check_path( } if parsed.is_valid() { + diagnostics.extend( + parsed + .syntax_errors(settings.target_version.into()) + .flat_map(try_diagnostic_from_syntax_error), + ); + // Remove fixes for any rules marked as unfixable. for diagnostic in &mut diagnostics { if !settings.rules.should_fix(diagnostic.kind.rule()) { @@ -320,6 +326,16 @@ pub fn check_path( diagnostics } +fn try_diagnostic_from_syntax_error(syntax_error: &SyntaxError) -> Option { + match syntax_error.kind { + ruff_python_parser::SyntaxErrorKind::LateFutureImport => Some(Diagnostic::new( + crate::rules::pyflakes::rules::LateFutureImport, + syntax_error.range, + )), + _ => None, + } +} + const MAX_ITERATIONS: usize = 100; /// Add any missing `# noqa` pragmas to the source code at the given `Path`. @@ -462,7 +478,13 @@ fn diagnostics_to_messages<'a>( .iter() .map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone())) .chain(syntax_errors.map(|syntax_error| { - Message::from_syntax_error(syntax_error, file.deref().clone(), target_version) + let noqa_offset = directives.noqa_line_for.resolve(syntax_error.range.start()); + Message::from_syntax_error( + syntax_error, + file.deref().clone(), + target_version, + noqa_offset, + ) })) .chain(diagnostics.into_iter().map(|diagnostic| { let noqa_offset = directives.noqa_line_for.resolve(diagnostic.start()); diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 65962d9836651..01df85d785cd2 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -127,12 +127,23 @@ impl Message { syntax_error: &SyntaxError, file: SourceFile, target_version: PythonVersion, + noqa_offset: TextSize, ) -> Message { - Message::SyntaxError(SyntaxErrorMessage { - message: format!("SyntaxError: {}", syntax_error.message(target_version)), - range: syntax_error.range, - file, - }) + match syntax_error.kind { + ruff_python_parser::SyntaxErrorKind::LateFutureImport => Message::from_diagnostic( + Diagnostic::new( + crate::rules::pyflakes::rules::LateFutureImport, + syntax_error.range, + ), + file, + noqa_offset, + ), + _ => Message::SyntaxError(SyntaxErrorMessage { + message: format!("SyntaxError: {}", syntax_error.message(target_version)), + range: syntax_error.range, + file, + }), + } } pub const fn as_diagnostic_message(&self) -> Option<&DiagnosticMessage> { diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 9472b3b39b8e6..c318a6456c894 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -450,7 +450,10 @@ impl SyntaxError { pub const fn version(&self) -> PythonVersion { match self.kind { SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, - SyntaxErrorKind::LateFutureImport => PythonVersion { major: 0, minor: 0 }, + SyntaxErrorKind::LateFutureImport => PythonVersion { + major: std::u8::MAX, + minor: std::u8::MAX, + }, } } } diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index c60d76ed4fead..73a4b2c51e2c3 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -30,6 +30,7 @@ mod tests; struct SyntaxErrorState { /// Whether or not the [`Parser`] has traversed past the "top-of-file" import boundary. seen_futures_boundary: bool, + seen_docstring_boundary: bool, } #[derive(Debug)] diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 49447be9e30b0..498a9a6d05c6b 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -260,7 +260,7 @@ impl<'src> Parser<'src> { /// /// See: fn parse_simple_statement(&mut self) -> Stmt { - match self.current_token_kind() { + let stmt = match self.current_token_kind() { TokenKind::Return => Stmt::Return(self.parse_return_statement()), TokenKind::Import => Stmt::Import(self.parse_import_statement()), TokenKind::From => Stmt::ImportFrom(self.parse_from_import_statement()), @@ -315,7 +315,26 @@ impl<'src> Parser<'src> { }) } } + }; + + let is_string_literal = stmt + .as_expr_stmt() + .is_some_and(|expr| expr.value.is_string_literal_expr()); + + let is_future_import = stmt + .as_import_from_stmt() + .is_some_and(|import| matches!(import.module.as_deref(), Some("__future__"))); + + if !self.syntax_error_state.seen_docstring_boundary && is_string_literal { + // do nothing, this is the docstring + } else if !is_future_import { + self.syntax_error_state.seen_futures_boundary = true; } + + // anything we see sets the docstring boundary + self.syntax_error_state.seen_docstring_boundary = true; + + stmt } /// Parses a delete statement. @@ -510,8 +529,6 @@ impl<'src> Parser<'src> { self.add_error(ParseErrorType::EmptyImportNames, self.current_token_range()); } - self.syntax_error_state.seen_futures_boundary = true; - ast::StmtImport { range: self.node_range(start), names, From 6ee2eb8947a8a917e97f006d1a2c50c41ff31c16 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 18:02:25 -0500 Subject: [PATCH 10/36] check if rules are enabled before converting to diagnostics --- crates/ruff_linter/src/linter.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 1376b2790c6ef..b7584ef7d9aab 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -14,7 +14,7 @@ use ruff_notebook::Notebook; use ruff_python_ast::{ModModule, PySourceType}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; -use ruff_python_parser::{ParseError, Parsed, SyntaxError}; +use ruff_python_parser::{ParseError, Parsed, SyntaxError, SyntaxErrorKind}; use ruff_source_file::SourceFileBuilder; use ruff_text_size::Ranged; @@ -33,6 +33,7 @@ use crate::package::PackageRoot; use crate::registry::{AsRule, Rule, RuleSet}; #[cfg(any(feature = "test-rules", test))] use crate::rules::ruff::rules::test_rules::{self, TestRule, TEST_RULES}; +use crate::settings::rule_table::RuleTable; use crate::settings::types::UnsafeFixes; use crate::settings::{flags, LinterSettings}; use crate::source_kind::SourceKind; @@ -295,7 +296,7 @@ pub fn check_path( diagnostics.extend( parsed .syntax_errors(settings.target_version.into()) - .flat_map(try_diagnostic_from_syntax_error), + .flat_map(|error| try_diagnostic_from_syntax_error(error, &settings.rules)), ); // Remove fixes for any rules marked as unfixable. @@ -326,12 +327,17 @@ pub fn check_path( diagnostics } -fn try_diagnostic_from_syntax_error(syntax_error: &SyntaxError) -> Option { +fn try_diagnostic_from_syntax_error( + syntax_error: &SyntaxError, + rules: &RuleTable, +) -> Option { match syntax_error.kind { - ruff_python_parser::SyntaxErrorKind::LateFutureImport => Some(Diagnostic::new( - crate::rules::pyflakes::rules::LateFutureImport, - syntax_error.range, - )), + SyntaxErrorKind::LateFutureImport if rules.enabled(Rule::LateFutureImport) => { + Some(Diagnostic::new( + crate::rules::pyflakes::rules::LateFutureImport, + syntax_error.range, + )) + } _ => None, } } From a1b0aa6c672d2a8734666617ad0bd883c2175806 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 18:17:09 -0500 Subject: [PATCH 11/36] add a todo about duplicate diagnostics --- crates/ruff_linter/src/linter.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index b7584ef7d9aab..c0a31af7bda26 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -293,6 +293,10 @@ pub fn check_path( } if parsed.is_valid() { + // TODO(brent) I think this might cause duplicates when calling `diagnostics_to_messages` + // because that function chains `syntax_errors` with `diagnostics` after we've added some + // `syntax_errors` directly into `diagnostics` here. maybe `Message`s get deduplicated and + // it doesn't matter? this does not appear to be the case, though. diagnostics.extend( parsed .syntax_errors(settings.target_version.into()) From 5c2465539628b764de0e4ea8c253cea2bf10c216 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 10 Feb 2025 18:20:07 -0500 Subject: [PATCH 12/36] clippy --- crates/ruff_linter/src/linter.rs | 2 +- crates/ruff_linter/src/message/mod.rs | 6 +++--- crates/ruff_python_parser/src/error.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index c0a31af7bda26..bd1a8def32ec2 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -300,7 +300,7 @@ pub fn check_path( diagnostics.extend( parsed .syntax_errors(settings.target_version.into()) - .flat_map(|error| try_diagnostic_from_syntax_error(error, &settings.rules)), + .filter_map(|error| try_diagnostic_from_syntax_error(error, &settings.rules)), ); // Remove fixes for any rules marked as unfixable. diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 01df85d785cd2..e34114de052e3 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -17,7 +17,7 @@ pub use pylint::PylintEmitter; pub use rdjson::RdjsonEmitter; use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix}; use ruff_notebook::NotebookIndex; -use ruff_python_parser::{ParseError, SyntaxError}; +use ruff_python_parser::{ParseError, SyntaxError, SyntaxErrorKind}; use ruff_source_file::{SourceFile, SourceLocation}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; pub use sarif::SarifEmitter; @@ -130,7 +130,7 @@ impl Message { noqa_offset: TextSize, ) -> Message { match syntax_error.kind { - ruff_python_parser::SyntaxErrorKind::LateFutureImport => Message::from_diagnostic( + SyntaxErrorKind::LateFutureImport => Message::from_diagnostic( Diagnostic::new( crate::rules::pyflakes::rules::LateFutureImport, syntax_error.range, @@ -138,7 +138,7 @@ impl Message { file, noqa_offset, ), - _ => Message::SyntaxError(SyntaxErrorMessage { + SyntaxErrorKind::MatchBeforePy310 => Message::SyntaxError(SyntaxErrorMessage { message: format!("SyntaxError: {}", syntax_error.message(target_version)), range: syntax_error.range, file, diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index c318a6456c894..f1a51bf5c95de 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -451,8 +451,8 @@ impl SyntaxError { match self.kind { SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, SyntaxErrorKind::LateFutureImport => PythonVersion { - major: std::u8::MAX, - minor: std::u8::MAX, + major: u8::MAX, + minor: u8::MAX, }, } } From 44a6c662dd6502e9c37ada20f05496060118f0ba Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Feb 2025 18:41:41 -0500 Subject: [PATCH 13/36] update to use ast::PythonVersion and Span --- crates/red_knot_project/src/lib.rs | 2 +- crates/red_knot_python_semantic/src/syntax.rs | 24 ++++--------------- crates/ruff_linter/src/linter.rs | 2 +- crates/ruff_linter/src/message/mod.rs | 2 +- crates/ruff_linter/src/settings/types.rs | 7 ++++++ crates/ruff_python_parser/src/error.rs | 3 ++- crates/ruff_python_parser/src/lib.rs | 3 +-- 7 files changed, 18 insertions(+), 25 deletions(-) diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index 389572b13a88c..b0ed8d695c7d5 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -346,7 +346,7 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec> { let version = Program::try_get(db) .map(|p| p.python_version(db)) .unwrap_or_default(); - diagnostics.extend(parsed.syntax_errors(version.into()).map(|error| { + diagnostics.extend(parsed.syntax_errors(version).map(|error| { let diagnostic: Box = Box::new(SyntaxDiagnostic::from_syntax_error(error, file, version)); diagnostic diff --git a/crates/red_knot_python_semantic/src/syntax.rs b/crates/red_knot_python_semantic/src/syntax.rs index debf5fcfe30e3..d6fac145fa1ea 100644 --- a/crates/red_knot_python_semantic/src/syntax.rs +++ b/crates/red_knot_python_semantic/src/syntax.rs @@ -1,12 +1,11 @@ use std::borrow::Cow; -use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity}; +use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span}; use ruff_db::files::File; +use ruff_python_ast::python_version::PythonVersion; use ruff_python_parser::SyntaxError; use ruff_text_size::TextRange; -use crate::PythonVersion; - #[derive(Debug, Eq, PartialEq, Clone)] pub struct SyntaxDiagnostic { id: DiagnosticId, @@ -24,25 +23,12 @@ impl Diagnostic for SyntaxDiagnostic { Cow::from(&self.message) } - fn file(&self) -> Option { - Some(self.file) - } - - fn range(&self) -> Option { - Some(self.range) - } - fn severity(&self) -> ruff_db::diagnostic::Severity { Severity::Error } -} -impl From for ruff_python_parser::version::PythonVersion { - fn from(value: PythonVersion) -> Self { - Self { - major: value.major, - minor: value.minor, - } + fn span(&self) -> Option { + Some(Span::from(self.file).with_range(self.range)) } } @@ -54,7 +40,7 @@ impl SyntaxDiagnostic { ) -> Self { Self { id: DiagnosticId::invalid_syntax(Some(value.kind.as_str())), - message: value.message(target_version.into()), + message: value.message(target_version), file, range: value.range, } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index bd1a8def32ec2..40084c275463e 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -6,11 +6,11 @@ use std::path::Path; use anyhow::{anyhow, Result}; use colored::Colorize; use itertools::Itertools; -use ruff_python_parser::version::PythonVersion; use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; use ruff_notebook::Notebook; +use ruff_python_ast::python_version::PythonVersion; use ruff_python_ast::{ModModule, PySourceType}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index e34114de052e3..8f7c10a7f5401 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use std::io::Write; use std::ops::Deref; -use ruff_python_parser::version::PythonVersion; +use ruff_python_ast::python_version::PythonVersion; use rustc_hash::FxHashMap; pub use azure::AzureEmitter; diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 007a4a4e2bc24..749214ae48153 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -73,6 +73,13 @@ impl From for pep440_rs::Version { } } +impl From for ruff_python_ast::python_version::PythonVersion { + fn from(value: PythonVersion) -> Self { + let (major, minor) = value.as_tuple(); + Self { major, minor } + } +} + impl PythonVersion { pub const fn as_tuple(&self) -> (u8, u8) { match self { diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index f1a51bf5c95de..b5cfa8039f473 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -1,8 +1,9 @@ use std::fmt; +use ruff_python_ast::python_version::PythonVersion; use ruff_text_size::TextRange; -use crate::{version::PythonVersion, TokenKind}; +use crate::TokenKind; /// Represents represent errors that occur during parsing and are /// returned by the `parse_*` functions. diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 1864f931b792b..8142536b7a265 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -75,12 +75,12 @@ pub use crate::token::{Token, TokenKind}; use crate::parser::Parser; +use ruff_python_ast::python_version::PythonVersion; use ruff_python_ast::{ Expr, Mod, ModExpression, ModModule, PySourceType, StringFlags, StringLiteral, Suite, }; use ruff_python_trivia::CommentRanges; use ruff_text_size::{Ranged, TextRange, TextSize}; -use version::PythonVersion; mod error; pub mod lexer; @@ -90,7 +90,6 @@ mod token; mod token_set; mod token_source; pub mod typing; -pub mod version; /// Parse a full Python module usually consisting of multiple lines. /// From 4bb914cb00c486cd09ead7fda8382cbd09a6b739 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Feb 2025 22:37:54 -0500 Subject: [PATCH 14/36] remove LateFutureImport detection from the parser --- .../src/checkers/ast/analyze/statement.rs | 8 +++++ crates/ruff_linter/src/linter.rs | 36 ++----------------- crates/ruff_linter/src/message/mod.rs | 9 ----- crates/ruff_python_parser/src/error.rs | 9 ----- crates/ruff_python_parser/src/parser/mod.rs | 10 ------ .../src/parser/statement.rs | 34 +----------------- 6 files changed, 11 insertions(+), 95 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 8b0a7b57d06bf..4b5594e47f801 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -855,6 +855,14 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::FutureFeatureNotDefined) { pyflakes::rules::future_feature_not_defined(checker, alias); } + if checker.enabled(Rule::LateFutureImport) { + if checker.semantic.seen_futures_boundary() { + checker.report_diagnostic(Diagnostic::new( + pyflakes::rules::LateFutureImport, + stmt.range(), + )); + } + } } else if &alias.name == "*" { if checker.enabled(Rule::UndefinedLocalWithNestedImportStarUsage) { if !matches!(checker.semantic.current_scope().kind, ScopeKind::Module) { diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 40084c275463e..259e5784790fc 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -14,7 +14,7 @@ use ruff_python_ast::python_version::PythonVersion; use ruff_python_ast::{ModModule, PySourceType}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; -use ruff_python_parser::{ParseError, Parsed, SyntaxError, SyntaxErrorKind}; +use ruff_python_parser::{ParseError, Parsed, SyntaxError}; use ruff_source_file::SourceFileBuilder; use ruff_text_size::Ranged; @@ -33,7 +33,6 @@ use crate::package::PackageRoot; use crate::registry::{AsRule, Rule, RuleSet}; #[cfg(any(feature = "test-rules", test))] use crate::rules::ruff::rules::test_rules::{self, TestRule, TEST_RULES}; -use crate::settings::rule_table::RuleTable; use crate::settings::types::UnsafeFixes; use crate::settings::{flags, LinterSettings}; use crate::source_kind::SourceKind; @@ -293,16 +292,6 @@ pub fn check_path( } if parsed.is_valid() { - // TODO(brent) I think this might cause duplicates when calling `diagnostics_to_messages` - // because that function chains `syntax_errors` with `diagnostics` after we've added some - // `syntax_errors` directly into `diagnostics` here. maybe `Message`s get deduplicated and - // it doesn't matter? this does not appear to be the case, though. - diagnostics.extend( - parsed - .syntax_errors(settings.target_version.into()) - .filter_map(|error| try_diagnostic_from_syntax_error(error, &settings.rules)), - ); - // Remove fixes for any rules marked as unfixable. for diagnostic in &mut diagnostics { if !settings.rules.should_fix(diagnostic.kind.rule()) { @@ -331,21 +320,6 @@ pub fn check_path( diagnostics } -fn try_diagnostic_from_syntax_error( - syntax_error: &SyntaxError, - rules: &RuleTable, -) -> Option { - match syntax_error.kind { - SyntaxErrorKind::LateFutureImport if rules.enabled(Rule::LateFutureImport) => { - Some(Diagnostic::new( - crate::rules::pyflakes::rules::LateFutureImport, - syntax_error.range, - )) - } - _ => None, - } -} - const MAX_ITERATIONS: usize = 100; /// Add any missing `# noqa` pragmas to the source code at the given `Path`. @@ -488,13 +462,7 @@ fn diagnostics_to_messages<'a>( .iter() .map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone())) .chain(syntax_errors.map(|syntax_error| { - let noqa_offset = directives.noqa_line_for.resolve(syntax_error.range.start()); - Message::from_syntax_error( - syntax_error, - file.deref().clone(), - target_version, - noqa_offset, - ) + Message::from_syntax_error(syntax_error, file.deref().clone(), target_version) })) .chain(diagnostics.into_iter().map(|diagnostic| { let noqa_offset = directives.noqa_line_for.resolve(diagnostic.start()); diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 8f7c10a7f5401..4c777da6d4301 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -127,17 +127,8 @@ impl Message { syntax_error: &SyntaxError, file: SourceFile, target_version: PythonVersion, - noqa_offset: TextSize, ) -> Message { match syntax_error.kind { - SyntaxErrorKind::LateFutureImport => Message::from_diagnostic( - Diagnostic::new( - crate::rules::pyflakes::rules::LateFutureImport, - syntax_error.range, - ), - file, - noqa_offset, - ), SyntaxErrorKind::MatchBeforePy310 => Message::SyntaxError(SyntaxErrorMessage { message: format!("SyntaxError: {}", syntax_error.message(target_version)), range: syntax_error.range, diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index b5cfa8039f473..d7cf4df56a164 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -441,9 +441,6 @@ impl SyntaxError { major = target_version.major, minor = target_version.minor, ), - SyntaxErrorKind::LateFutureImport => { - "__future__ imports must be located at the beginning of a file".to_string() - } } } @@ -451,17 +448,12 @@ impl SyntaxError { pub const fn version(&self) -> PythonVersion { match self.kind { SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, - SyntaxErrorKind::LateFutureImport => PythonVersion { - major: u8::MAX, - minor: u8::MAX, - }, } } } #[derive(Debug, PartialEq, Clone, Copy)] pub enum SyntaxErrorKind { - LateFutureImport, MatchBeforePy310, } @@ -469,7 +461,6 @@ impl SyntaxErrorKind { pub const fn as_str(self) -> &'static str { match self { SyntaxErrorKind::MatchBeforePy310 => "match-before-python-310", - SyntaxErrorKind::LateFutureImport => "late-future-import", } } } diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 73a4b2c51e2c3..67a68eaca2273 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -26,13 +26,6 @@ mod statement; #[cfg(test)] mod tests; -#[derive(Debug, Default)] -struct SyntaxErrorState { - /// Whether or not the [`Parser`] has traversed past the "top-of-file" import boundary. - seen_futures_boundary: bool, - seen_docstring_boundary: bool, -} - #[derive(Debug)] pub(crate) struct Parser<'src> { source: &'src str, @@ -47,8 +40,6 @@ pub(crate) struct Parser<'src> { /// errors detected by the Python compiler. syntax_errors: Vec, - syntax_error_state: SyntaxErrorState, - /// Options for how the code will be parsed. options: ParseOptions, @@ -85,7 +76,6 @@ impl<'src> Parser<'src> { source, errors: Vec::new(), syntax_errors: Vec::new(), - syntax_error_state: SyntaxErrorState::default(), tokens, recovery_context: RecoveryContext::empty(), prev_token_end: TextSize::new(0), diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 498a9a6d05c6b..569b1435d7fc7 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -260,7 +260,7 @@ impl<'src> Parser<'src> { /// /// See: fn parse_simple_statement(&mut self) -> Stmt { - let stmt = match self.current_token_kind() { + match self.current_token_kind() { TokenKind::Return => Stmt::Return(self.parse_return_statement()), TokenKind::Import => Stmt::Import(self.parse_import_statement()), TokenKind::From => Stmt::ImportFrom(self.parse_from_import_statement()), @@ -315,26 +315,7 @@ impl<'src> Parser<'src> { }) } } - }; - - let is_string_literal = stmt - .as_expr_stmt() - .is_some_and(|expr| expr.value.is_string_literal_expr()); - - let is_future_import = stmt - .as_import_from_stmt() - .is_some_and(|import| matches!(import.module.as_deref(), Some("__future__"))); - - if !self.syntax_error_state.seen_docstring_boundary && is_string_literal { - // do nothing, this is the docstring - } else if !is_future_import { - self.syntax_error_state.seen_futures_boundary = true; } - - // anything we see sets the docstring boundary - self.syntax_error_state.seen_docstring_boundary = true; - - stmt } /// Parses a delete statement. @@ -640,19 +621,6 @@ impl<'src> Parser<'src> { let range = self.node_range(start); - // Allow __future__ imports until we see a non-__future__ import. - match module.as_deref() { - Some("__future__") => { - if self.syntax_error_state.seen_futures_boundary { - self.syntax_errors.push(SyntaxError { - kind: SyntaxErrorKind::LateFutureImport, - range, - }); - } - } - _ => self.syntax_error_state.seen_futures_boundary = true, - } - ast::StmtImportFrom { module, names, From 1a1b2ad22ffdb72ba8c457562b3d2f8b2995f34b Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Feb 2025 22:53:56 -0500 Subject: [PATCH 15/36] tidy up --- crates/ruff_linter/src/linter.rs | 12 ++++++++---- crates/ruff_python_parser/src/parser/mod.rs | 3 +-- crates/ruff_python_parser/src/parser/statement.rs | 4 +--- crates/ruff_python_parser/src/version.rs | 13 ------------- 4 files changed, 10 insertions(+), 22 deletions(-) delete mode 100644 crates/ruff_python_parser/src/version.rs diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 259e5784790fc..387dee0d7c6a5 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -423,15 +423,17 @@ pub fn lint_only( &parsed, ); + let target_version = settings.target_version.into(); + LinterResult { messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(settings.target_version.into()), + parsed.syntax_errors(target_version), path, &locator, &directives, - settings.target_version.into(), + target_version, ), has_syntax_error: !parsed.is_valid(), } @@ -576,16 +578,18 @@ pub fn lint_fix<'a>( report_failed_to_converge_error(path, transformed.source_code(), &diagnostics); } + let target_version = settings.target_version.into(); + return Ok(FixerResult { result: LinterResult { messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(settings.target_version.into()), + parsed.syntax_errors(target_version), path, &locator, &directives, - settings.target_version.into(), + target_version, ), has_syntax_error: !is_valid_syntax, }, diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 67a68eaca2273..4840ae2ad2f6e 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -36,8 +36,7 @@ pub(crate) struct Parser<'src> { /// Stores all the syntax errors found during the parsing. errors: Vec, - /// Stores non-fatal syntax errors found during parsing, such as version-related errors and - /// errors detected by the Python compiler. + /// Stores non-fatal syntax errors found during parsing, such as version-related errors. syntax_errors: Vec, /// Options for how the code will be parsed. diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 569b1435d7fc7..582668a75f15e 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -619,13 +619,11 @@ impl<'src> Parser<'src> { self.expect(TokenKind::Rpar); } - let range = self.node_range(start); - ast::StmtImportFrom { module, names, level: leading_dots, - range, + range: self.node_range(start), } } diff --git a/crates/ruff_python_parser/src/version.rs b/crates/ruff_python_parser/src/version.rs deleted file mode 100644 index 55d5155575465..0000000000000 --- a/crates/ruff_python_parser/src/version.rs +++ /dev/null @@ -1,13 +0,0 @@ -#[derive(Clone, Copy, Debug, Eq, PartialOrd, Ord, PartialEq)] -pub struct PythonVersion { - pub major: u8, - pub minor: u8, -} - -impl PythonVersion { - pub const PY38: PythonVersion = PythonVersion { major: 3, minor: 8 }; - pub const PY310: PythonVersion = PythonVersion { - major: 3, - minor: 10, - }; -} From 723cf7e23fca2f87026c75c42b097f1e06654c60 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 19 Feb 2025 11:28:01 -0500 Subject: [PATCH 16/36] clean up after rebase --- crates/red_knot_python_semantic/src/syntax.rs | 2 +- crates/ruff/src/cache.rs | 5 ++--- crates/ruff_linter/src/linter.rs | 14 +++++--------- crates/ruff_linter/src/message/mod.rs | 2 +- crates/ruff_linter/src/settings/types.rs | 7 ------- crates/ruff_python_parser/src/error.rs | 2 +- crates/ruff_python_parser/src/lib.rs | 4 ++-- 7 files changed, 12 insertions(+), 24 deletions(-) diff --git a/crates/red_knot_python_semantic/src/syntax.rs b/crates/red_knot_python_semantic/src/syntax.rs index d6fac145fa1ea..8762207d38a01 100644 --- a/crates/red_knot_python_semantic/src/syntax.rs +++ b/crates/red_knot_python_semantic/src/syntax.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span}; use ruff_db::files::File; -use ruff_python_ast::python_version::PythonVersion; +use ruff_python_ast::PythonVersion; use ruff_python_parser::SyntaxError; use ruff_text_size::TextRange; diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 91beb5c3c9d82..2059d308ac657 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -586,7 +586,6 @@ mod tests { use anyhow::Result; use filetime::{set_file_mtime, FileTime}; use itertools::Itertools; - use ruff_linter::settings::types::PythonVersion; use ruff_linter::settings::LinterSettings; use test_case::test_case; @@ -595,7 +594,7 @@ mod tests { use ruff_linter::package::PackageRoot; use ruff_linter::settings::flags; use ruff_linter::settings::types::UnsafeFixes; - use ruff_python_ast::PySourceType; + use ruff_python_ast::{PySourceType, PythonVersion}; use ruff_workspace::Settings; use crate::cache::{self, FileCache, FileCacheData, FileCacheKey}; @@ -614,7 +613,7 @@ mod tests { let settings = Settings { cache_dir, linter: LinterSettings { - target_version: PythonVersion::Py310, + target_version: PythonVersion::PY310, ..Default::default() }, ..Settings::default() diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 387dee0d7c6a5..6d081f48af835 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -10,7 +10,7 @@ use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; use ruff_notebook::Notebook; -use ruff_python_ast::python_version::PythonVersion; +use ruff_python_ast::PythonVersion; use ruff_python_ast::{ModModule, PySourceType}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; @@ -423,17 +423,15 @@ pub fn lint_only( &parsed, ); - let target_version = settings.target_version.into(); - LinterResult { messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(target_version), + parsed.syntax_errors(settings.target_version), path, &locator, &directives, - target_version, + settings.target_version, ), has_syntax_error: !parsed.is_valid(), } @@ -578,18 +576,16 @@ pub fn lint_fix<'a>( report_failed_to_converge_error(path, transformed.source_code(), &diagnostics); } - let target_version = settings.target_version.into(); - return Ok(FixerResult { result: LinterResult { messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(target_version), + parsed.syntax_errors(settings.target_version), path, &locator, &directives, - target_version, + settings.target_version, ), has_syntax_error: !is_valid_syntax, }, diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 4c777da6d4301..f9a54fb2cb7d0 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use std::io::Write; use std::ops::Deref; -use ruff_python_ast::python_version::PythonVersion; +use ruff_python_ast::PythonVersion; use rustc_hash::FxHashMap; pub use azure::AzureEmitter; diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 749214ae48153..007a4a4e2bc24 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -73,13 +73,6 @@ impl From for pep440_rs::Version { } } -impl From for ruff_python_ast::python_version::PythonVersion { - fn from(value: PythonVersion) -> Self { - let (major, minor) = value.as_tuple(); - Self { major, minor } - } -} - impl PythonVersion { pub const fn as_tuple(&self) -> (u8, u8) { match self { diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index d7cf4df56a164..d5d5c0c3fb94c 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -1,6 +1,6 @@ use std::fmt; -use ruff_python_ast::python_version::PythonVersion; +use ruff_python_ast::PythonVersion; use ruff_text_size::TextRange; use crate::TokenKind; diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 8142536b7a265..02b0e6c3c9b7f 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -75,9 +75,9 @@ pub use crate::token::{Token, TokenKind}; use crate::parser::Parser; -use ruff_python_ast::python_version::PythonVersion; use ruff_python_ast::{ - Expr, Mod, ModExpression, ModModule, PySourceType, StringFlags, StringLiteral, Suite, + Expr, Mod, ModExpression, ModModule, PySourceType, PythonVersion, StringFlags, StringLiteral, + Suite, }; use ruff_python_trivia::CommentRanges; use ruff_text_size::{Ranged, TextRange, TextSize}; From 832e0a7ac14497f2f53c398c37109b1179d6b829 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 19 Feb 2025 15:05:38 -0500 Subject: [PATCH 17/36] add ParseOptions::target_version --- .../ruff_python_parser/src/parser/options.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_parser/src/parser/options.rs b/crates/ruff_python_parser/src/parser/options.rs index 27a87a32ba4fb..3da27847bb385 100644 --- a/crates/ruff_python_parser/src/parser/options.rs +++ b/crates/ruff_python_parser/src/parser/options.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::PySourceType; +use ruff_python_ast::{PySourceType, PythonVersion}; use crate::{AsMode, Mode}; @@ -24,11 +24,24 @@ use crate::{AsMode, Mode}; pub struct ParseOptions { /// Specify the mode in which the code will be parsed. pub(crate) mode: Mode, + /// Target version for detecting version-related syntax errors. + pub(crate) target_version: PythonVersion, +} + +impl ParseOptions { + #[must_use] + pub fn with_target_version(mut self, target_version: PythonVersion) -> Self { + self.target_version = target_version; + self + } } impl From for ParseOptions { fn from(mode: Mode) -> Self { - Self { mode } + Self { + mode, + target_version: PythonVersion::default(), + } } } @@ -36,6 +49,7 @@ impl From for ParseOptions { fn from(source_type: PySourceType) -> Self { Self { mode: source_type.as_mode(), + target_version: PythonVersion::default(), } } } From d1daafa8f8ff0d0034bdd5bcc06636e6df599a18 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 19 Feb 2025 15:09:50 -0500 Subject: [PATCH 18/36] filter syntax errors when parsing, store version on SyntaxError --- crates/red_knot_project/src/lib.rs | 11 +++------- crates/red_knot_python_semantic/src/syntax.rs | 9 ++------- crates/ruff_linter/src/linter.rs | 20 +++++++++---------- crates/ruff_linter/src/message/mod.rs | 9 ++------- crates/ruff_python_parser/src/error.rs | 7 ++++--- crates/ruff_python_parser/src/lib.rs | 14 ++++--------- .../src/parser/statement.rs | 14 ++++++++----- 7 files changed, 33 insertions(+), 51 deletions(-) diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index b0ed8d695c7d5..3eb3faeacc0af 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -6,9 +6,9 @@ use files::{Index, Indexed, IndexedFiles}; use metadata::settings::Settings; pub use metadata::{ProjectDiscoveryError, ProjectMetadata}; use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection}; +use red_knot_python_semantic::register_lints; use red_knot_python_semantic::syntax::SyntaxDiagnostic; use red_knot_python_semantic::types::check_types; -use red_knot_python_semantic::{register_lints, Program}; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity, Span}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; @@ -341,14 +341,9 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec> { })); if parsed.is_valid() { - // TODO should just be `get` but panics in - // tests::check_file_skips_type_checking_when_file_cant_be_read - let version = Program::try_get(db) - .map(|p| p.python_version(db)) - .unwrap_or_default(); - diagnostics.extend(parsed.syntax_errors(version).map(|error| { + diagnostics.extend(parsed.syntax_errors().iter().map(|error| { let diagnostic: Box = - Box::new(SyntaxDiagnostic::from_syntax_error(error, file, version)); + Box::new(SyntaxDiagnostic::from_syntax_error(error, file)); diagnostic })); } diff --git a/crates/red_knot_python_semantic/src/syntax.rs b/crates/red_knot_python_semantic/src/syntax.rs index 8762207d38a01..25ef97acd8755 100644 --- a/crates/red_knot_python_semantic/src/syntax.rs +++ b/crates/red_knot_python_semantic/src/syntax.rs @@ -2,7 +2,6 @@ use std::borrow::Cow; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span}; use ruff_db::files::File; -use ruff_python_ast::PythonVersion; use ruff_python_parser::SyntaxError; use ruff_text_size::TextRange; @@ -33,14 +32,10 @@ impl Diagnostic for SyntaxDiagnostic { } impl SyntaxDiagnostic { - pub fn from_syntax_error( - value: &SyntaxError, - file: File, - target_version: PythonVersion, - ) -> Self { + pub fn from_syntax_error(value: &SyntaxError, file: File) -> Self { Self { id: DiagnosticId::invalid_syntax(Some(value.kind.as_str())), - message: value.message(target_version), + message: value.message(), file, range: value.range, } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 6d081f48af835..f4bfaf4db2df4 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -10,7 +10,6 @@ use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; use ruff_notebook::Notebook; -use ruff_python_ast::PythonVersion; use ruff_python_ast::{ModModule, PySourceType}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; @@ -427,25 +426,23 @@ pub fn lint_only( messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(settings.target_version), + parsed.syntax_errors(), path, &locator, &directives, - settings.target_version, ), has_syntax_error: !parsed.is_valid(), } } /// Convert from diagnostics to messages. -fn diagnostics_to_messages<'a>( +fn diagnostics_to_messages( diagnostics: Vec, parse_errors: &[ParseError], - syntax_errors: impl Iterator, + syntax_errors: &[SyntaxError], path: &Path, locator: &Locator, directives: &Directives, - target_version: PythonVersion, ) -> Vec { let file = LazyCell::new(|| { let mut builder = @@ -461,9 +458,11 @@ fn diagnostics_to_messages<'a>( parse_errors .iter() .map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone())) - .chain(syntax_errors.map(|syntax_error| { - Message::from_syntax_error(syntax_error, file.deref().clone(), target_version) - })) + .chain( + syntax_errors + .iter() + .map(|syntax_error| Message::from_syntax_error(syntax_error, file.deref().clone())), + ) .chain(diagnostics.into_iter().map(|diagnostic| { let noqa_offset = directives.noqa_line_for.resolve(diagnostic.start()); Message::from_diagnostic(diagnostic, file.deref().clone(), noqa_offset) @@ -581,11 +580,10 @@ pub fn lint_fix<'a>( messages: diagnostics_to_messages( diagnostics, parsed.errors(), - parsed.syntax_errors(settings.target_version), + parsed.syntax_errors(), path, &locator, &directives, - settings.target_version, ), has_syntax_error: !is_valid_syntax, }, diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index f9a54fb2cb7d0..36d35782f236a 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -3,7 +3,6 @@ use std::collections::BTreeMap; use std::io::Write; use std::ops::Deref; -use ruff_python_ast::PythonVersion; use rustc_hash::FxHashMap; pub use azure::AzureEmitter; @@ -123,14 +122,10 @@ impl Message { } /// Create a [`Message`] from the given [`SyntaxError`]. - pub fn from_syntax_error( - syntax_error: &SyntaxError, - file: SourceFile, - target_version: PythonVersion, - ) -> Message { + pub fn from_syntax_error(syntax_error: &SyntaxError, file: SourceFile) -> Message { match syntax_error.kind { SyntaxErrorKind::MatchBeforePy310 => Message::SyntaxError(SyntaxErrorMessage { - message: format!("SyntaxError: {}", syntax_error.message(target_version)), + message: format!("SyntaxError: {}", syntax_error.message()), range: syntax_error.range, file, }), diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index d5d5c0c3fb94c..75637747849d2 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -431,15 +431,16 @@ impl std::fmt::Display for LexicalErrorType { pub struct SyntaxError { pub kind: SyntaxErrorKind, pub range: TextRange, + pub target_version: PythonVersion, } impl SyntaxError { - pub fn message(&self, target_version: PythonVersion) -> String { + pub fn message(&self) -> String { match self.kind { SyntaxErrorKind::MatchBeforePy310 => format!( "Cannot use `match` statement on Python {major}.{minor} (syntax was new in Python 3.10)", - major = target_version.major, - minor = target_version.minor, + major = self.target_version.major, + minor = self.target_version.minor, ), } } diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index 02b0e6c3c9b7f..1cdaf0d5ab842 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -76,8 +76,7 @@ pub use crate::token::{Token, TokenKind}; use crate::parser::Parser; use ruff_python_ast::{ - Expr, Mod, ModExpression, ModModule, PySourceType, PythonVersion, StringFlags, StringLiteral, - Suite, + Expr, Mod, ModExpression, ModModule, PySourceType, StringFlags, StringLiteral, Suite, }; use ruff_python_trivia::CommentRanges; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -327,14 +326,9 @@ impl Parsed { &self.errors } - /// Returns the syntax errors for `target_version`. - pub fn syntax_errors( - &self, - target_version: PythonVersion, - ) -> impl Iterator { - self.syntax_errors - .iter() - .filter(move |error| target_version < error.version()) + /// Returns a list of version-related syntax errors found during parsing. + pub fn syntax_errors(&self) -> &[SyntaxError] { + &self.syntax_errors } /// Consumes the [`Parsed`] output and returns the contained syntax node. diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 582668a75f15e..d4e3baeae6e24 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -5,7 +5,8 @@ use rustc_hash::{FxBuildHasher, FxHashSet}; use ruff_python_ast::name::Name; use ruff_python_ast::{ - self as ast, ExceptHandler, Expr, ExprContext, IpyEscapeKind, Operator, Stmt, WithItem, + self as ast, ExceptHandler, Expr, ExprContext, IpyEscapeKind, Operator, PythonVersion, Stmt, + WithItem, }; use ruff_text_size::{Ranged, TextSize}; @@ -2264,10 +2265,13 @@ impl<'src> Parser<'src> { let range = self.node_range(start); - self.syntax_errors.push(SyntaxError { - kind: SyntaxErrorKind::MatchBeforePy310, - range, - }); + if self.options.target_version < PythonVersion::PY310 { + self.syntax_errors.push(SyntaxError { + kind: SyntaxErrorKind::MatchBeforePy310, + range, + target_version: self.options.target_version, + }); + } ast::StmtMatch { subject: Box::new(subject), From a0efadd6bc7bf3c9330ad02512340045ca272823 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 19 Feb 2025 15:46:16 -0500 Subject: [PATCH 19/36] make ParseOptions Clone could be Copy as well, currently because Mode and PythonVersion are --- crates/ruff_python_parser/src/parser/options.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_python_parser/src/parser/options.rs b/crates/ruff_python_parser/src/parser/options.rs index 3da27847bb385..6258216d8860a 100644 --- a/crates/ruff_python_parser/src/parser/options.rs +++ b/crates/ruff_python_parser/src/parser/options.rs @@ -20,7 +20,7 @@ use crate::{AsMode, Mode}; /// /// let options = ParseOptions::from(PySourceType::Python); /// ``` -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct ParseOptions { /// Specify the mode in which the code will be parsed. pub(crate) mode: Mode, From 4dba857836ddda131fd885d776fbd3a49d8f9669 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 19 Feb 2025 15:26:07 -0500 Subject: [PATCH 20/36] pass ParseOptions::target_version in the linter --- crates/ruff_linter/src/linter.rs | 39 +++++++++++++++----- crates/ruff_linter/src/rules/pyflakes/mod.rs | 7 +++- crates/ruff_linter/src/test.rs | 11 ++++-- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index f4bfaf4db2df4..8e0add9a6b410 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -10,10 +10,10 @@ use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; use ruff_notebook::Notebook; -use ruff_python_ast::{ModModule, PySourceType}; +use ruff_python_ast::{ModModule, PySourceType, PythonVersion}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; -use ruff_python_parser::{ParseError, Parsed, SyntaxError}; +use ruff_python_parser::{ParseError, ParseOptions, Parsed, SyntaxError}; use ruff_source_file::SourceFileBuilder; use ruff_text_size::Ranged; @@ -330,7 +330,8 @@ pub fn add_noqa_to_path( settings: &LinterSettings, ) -> Result { // Parse once. - let parsed = ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type); + // TODO(brent) resolve_target_version(path) here + let parsed = parse_unchecked_source(source_kind, source_type, settings.target_version); // Map row and column locations to byte slices (lazily). let locator = Locator::new(source_kind.source_code()); @@ -388,7 +389,8 @@ pub fn lint_only( source_type: PySourceType, source: ParseSource, ) -> LinterResult { - let parsed = source.into_parsed(source_kind, source_type); + // TODO(brent) resolve_target_version(path) here + let parsed = source.into_parsed(source_kind, source_type, settings.target_version); // Map row and column locations to byte slices (lazily). let locator = Locator::new(source_kind.source_code()); @@ -496,8 +498,8 @@ pub fn lint_fix<'a>( // Continuously fix until the source code stabilizes. loop { // Parse once. - let parsed = - ruff_python_parser::parse_unchecked_source(transformed.source_code(), source_type); + // TODO(brent) resolve_target_version(path) here + let parsed = parse_unchecked_source(&transformed, source_type, settings.target_version); // Map row and column locations to byte slices (lazily). let locator = Locator::new(transformed.source_code()); @@ -683,16 +685,33 @@ pub enum ParseSource { impl ParseSource { /// Consumes the [`ParseSource`] and returns the parsed [`Parsed`], parsing the source code if /// necessary. - fn into_parsed(self, source_kind: &SourceKind, source_type: PySourceType) -> Parsed { + fn into_parsed( + self, + source_kind: &SourceKind, + source_type: PySourceType, + target_version: PythonVersion, + ) -> Parsed { match self { - ParseSource::None => { - ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type) - } + ParseSource::None => parse_unchecked_source(source_kind, source_type, target_version), ParseSource::Precomputed(parsed) => parsed, } } } +fn parse_unchecked_source( + source_kind: &SourceKind, + source_type: PySourceType, + target_version: PythonVersion, +) -> Parsed { + let options = ParseOptions::from(source_type).with_target_version(target_version); + // SAFETY: Safe because `PySourceType` always parses to a `ModModule`. See + // `ruff_python_parser::parse_unchecked_source`. We use `parse_unchecked` (and thus + // have to unwrap) in order to pass the `PythonVersion` via `ParseOptions`. + ruff_python_parser::parse_unchecked(source_kind.source_code(), options) + .try_into_module() + .expect("PySourceType always parses into a module") +} + #[cfg(test)] mod tests { use std::path::Path; diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 0108e640ed0e7..9047a1f721ede 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -11,6 +11,7 @@ mod tests { use anyhow::Result; use regex::Regex; + use ruff_python_parser::ParseOptions; use rustc_hash::FxHashMap; use test_case::test_case; @@ -744,8 +745,10 @@ mod tests { let source_type = PySourceType::default(); let source_kind = SourceKind::Python(contents.to_string()); let settings = LinterSettings::for_rules(Linter::Pyflakes.rules()); - let parsed = - ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type); + let options = ParseOptions::from(source_type).with_target_version(settings.target_version); + let parsed = ruff_python_parser::parse_unchecked(source_kind.source_code(), options) + .try_into_module() + .expect("PySourceType always parses into a module"); let locator = Locator::new(&contents); let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents()); diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 6ded54565eee5..ae981a127c22e 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -16,7 +16,7 @@ use ruff_notebook::NotebookError; use ruff_python_ast::PySourceType; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; -use ruff_python_parser::ParseError; +use ruff_python_parser::{ParseError, ParseOptions}; use ruff_python_trivia::textwrap::dedent; use ruff_source_file::SourceFileBuilder; use ruff_text_size::Ranged; @@ -110,7 +110,10 @@ pub(crate) fn test_contents<'a>( settings: &LinterSettings, ) -> (Vec, Cow<'a, SourceKind>) { let source_type = PySourceType::from(path); - let parsed = ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type); + let options = ParseOptions::from(source_type).with_target_version(settings.target_version); + let parsed = ruff_python_parser::parse_unchecked(source_kind.source_code(), options.clone()) + .try_into_module() + .expect("PySourceType always parses into a module"); let locator = Locator::new(source_kind.source_code()); let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents()); @@ -174,7 +177,9 @@ pub(crate) fn test_contents<'a>( transformed = Cow::Owned(transformed.updated(fixed_contents, &source_map)); let parsed = - ruff_python_parser::parse_unchecked_source(transformed.source_code(), source_type); + ruff_python_parser::parse_unchecked(transformed.source_code(), options.clone()) + .try_into_module() + .expect("PySourceType always parses into a module"); let locator = Locator::new(transformed.source_code()); let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents()); From a3f8ea92cfc345fb3916e5e4f084c55fb4368891 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 19 Feb 2025 15:55:23 -0500 Subject: [PATCH 21/36] pass ParseOptions::target_version in red-knot --- Cargo.lock | 1 + crates/red_knot_project/src/lib.rs | 4 ++-- crates/red_knot_project/tests/check.rs | 4 ++-- crates/red_knot_python_semantic/src/lib.rs | 9 +++++++ .../src/semantic_index.rs | 17 +++++++++---- .../src/semantic_model.rs | 7 +++--- .../src/suppression.rs | 3 ++- crates/red_knot_python_semantic/src/types.rs | 4 +++- .../src/types/infer.rs | 13 +++++++--- crates/red_knot_test/src/assertion.rs | 3 ++- crates/red_knot_test/src/lib.rs | 6 +++-- crates/red_knot_wasm/Cargo.toml | 1 + crates/red_knot_wasm/src/lib.rs | 7 ++++-- crates/ruff_db/src/parsed.rs | 24 ++++++++++++------- 14 files changed, 72 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 26895f7982a11..7a175487bff39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2566,6 +2566,7 @@ dependencies = [ "js-sys", "log", "red_knot_project", + "red_knot_python_semantic", "ruff_db", "ruff_notebook", "ruff_python_ast", diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index 3eb3faeacc0af..c6a788616a1b3 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -6,9 +6,9 @@ use files::{Index, Indexed, IndexedFiles}; use metadata::settings::Settings; pub use metadata::{ProjectDiscoveryError, ProjectMetadata}; use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection}; -use red_knot_python_semantic::register_lints; use red_knot_python_semantic::syntax::SyntaxDiagnostic; use red_knot_python_semantic::types::check_types; +use red_knot_python_semantic::{python_version, register_lints}; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity, Span}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; @@ -334,7 +334,7 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec> { return diagnostics; } - let parsed = parsed_module(db.upcast(), file); + let parsed = parsed_module(db.upcast(), file, python_version(db.upcast())); diagnostics.extend(parsed.errors().iter().map(|error| { let diagnostic: Box = Box::new(ParseDiagnostic::new(file, error.clone())); diagnostic diff --git a/crates/red_knot_project/tests/check.rs b/crates/red_knot_project/tests/check.rs index 711cec75f2d23..9d0be4b99e84d 100644 --- a/crates/red_knot_project/tests/check.rs +++ b/crates/red_knot_project/tests/check.rs @@ -1,6 +1,6 @@ use anyhow::{anyhow, Context}; use red_knot_project::{ProjectDatabase, ProjectMetadata}; -use red_knot_python_semantic::{HasType, SemanticModel}; +use red_knot_python_semantic::{python_version, HasType, SemanticModel}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; use ruff_db::system::{SystemPath, SystemPathBuf, TestSystem}; @@ -165,7 +165,7 @@ fn run_corpus_tests(pattern: &str) -> anyhow::Result<()> { fn pull_types(db: &ProjectDatabase, file: File) { let mut visitor = PullTypesVisitor::new(db, file); - let ast = parsed_module(db, file); + let ast = parsed_module(db, file, python_version(db)); visitor.visit_body(ast.suite()); } diff --git a/crates/red_knot_python_semantic/src/lib.rs b/crates/red_knot_python_semantic/src/lib.rs index 0625fdd741bd6..1dbe0b38fe7fb 100644 --- a/crates/red_knot_python_semantic/src/lib.rs +++ b/crates/red_knot_python_semantic/src/lib.rs @@ -50,3 +50,12 @@ pub fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&UNKNOWN_RULE); registry.register_lint(&INVALID_IGNORE_COMMENT); } + +// TODO(brent) remove this. It should just be `Program::get(db).python_version(db)`, but for some +// reason `tests::check_file_skips_type_checking_when_file_cant_be_read` fails when I use `get`, so +// I factored this out instead of inlining everywhere +pub fn python_version(db: &dyn Db) -> ruff_python_ast::PythonVersion { + Program::try_get(db) + .map(|program| program.python_version(db)) + .unwrap_or_default() +} diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/crates/red_knot_python_semantic/src/semantic_index.rs index c3c72cfe2812e..69e2d110e130f 100644 --- a/crates/red_knot_python_semantic/src/semantic_index.rs +++ b/crates/red_knot_python_semantic/src/semantic_index.rs @@ -5,6 +5,7 @@ use ruff_db::files::File; use ruff_db::parsed::parsed_module; use ruff_index::{IndexSlice, IndexVec}; +use ruff_python_ast::PythonVersion; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use salsa::plumbing::AsId; use salsa::Update; @@ -45,7 +46,13 @@ type SymbolMap = hashbrown::HashMap; pub(crate) fn semantic_index(db: &dyn Db, file: File) -> SemanticIndex<'_> { let _span = tracing::trace_span!("semantic_index", file = %file.path(db)).entered(); - let parsed = parsed_module(db.upcast(), file); + // TODO(brent) need to pass the real PythonVersion here, but tests fail when I change it from + // PythonVersion::default() + // + // I've tried my hacky `python_version` helper function and also + // `Program::get(db).python_version(db)`, and many tests fail in both cases (18 with + // `python_version`, 48 with `Program::get`) + let parsed = parsed_module(db.upcast(), file, PythonVersion::default()); SemanticIndexBuilder::new(db, file, parsed).build() } @@ -409,7 +416,7 @@ mod tests { use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; use ruff_db::system::DbWithTestSystem; - use ruff_python_ast as ast; + use ruff_python_ast::{self as ast, PythonVersion}; use ruff_text_size::{Ranged, TextRange}; use crate::db::tests::TestDb; @@ -830,7 +837,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs): let use_def = index.use_def_map(comprehension_scope_id); - let module = parsed_module(&db, file).syntax(); + let module = parsed_module(&db, file, PythonVersion::default()).syntax(); let element = module.body[0] .as_expr_stmt() .unwrap() @@ -1079,7 +1086,7 @@ class C[T]: #[test] fn reachability_trivial() { let TestCase { db, file } = test_case("x = 1; x"); - let parsed = parsed_module(&db, file); + let parsed = parsed_module(&db, file, PythonVersion::default()); let scope = global_scope(&db, file); let ast = parsed.syntax(); let ast::Stmt::Expr(ast::StmtExpr { @@ -1112,7 +1119,7 @@ class C[T]: let TestCase { db, file } = test_case("x = 1;\ndef test():\n y = 4"); let index = semantic_index(&db, file); - let parsed = parsed_module(&db, file); + let parsed = parsed_module(&db, file, PythonVersion::default()); let ast = parsed.syntax(); let x_stmt = ast.body[0].as_assign_stmt().unwrap(); diff --git a/crates/red_knot_python_semantic/src/semantic_model.rs b/crates/red_knot_python_semantic/src/semantic_model.rs index dd6c455ba57a1..c6f3444d2d188 100644 --- a/crates/red_knot_python_semantic/src/semantic_model.rs +++ b/crates/red_knot_python_semantic/src/semantic_model.rs @@ -166,6 +166,7 @@ impl_binding_has_ty!(ast::ParameterWithDefault); mod tests { use ruff_db::files::system_path_to_file; use ruff_db::parsed::parsed_module; + use ruff_python_ast::PythonVersion; use crate::db::tests::TestDbBuilder; use crate::{HasType, SemanticModel}; @@ -178,7 +179,7 @@ mod tests { let foo = system_path_to_file(&db, "/src/foo.py").unwrap(); - let ast = parsed_module(&db, foo); + let ast = parsed_module(&db, foo, PythonVersion::default()); let function = ast.suite()[0].as_function_def_stmt().unwrap(); let model = SemanticModel::new(&db, foo); @@ -197,7 +198,7 @@ mod tests { let foo = system_path_to_file(&db, "/src/foo.py").unwrap(); - let ast = parsed_module(&db, foo); + let ast = parsed_module(&db, foo, PythonVersion::default()); let class = ast.suite()[0].as_class_def_stmt().unwrap(); let model = SemanticModel::new(&db, foo); @@ -217,7 +218,7 @@ mod tests { let bar = system_path_to_file(&db, "/src/bar.py").unwrap(); - let ast = parsed_module(&db, bar); + let ast = parsed_module(&db, bar, PythonVersion::default()); let import = ast.suite()[0].as_import_from_stmt().unwrap(); let alias = &import.names[0]; diff --git a/crates/red_knot_python_semantic/src/suppression.rs b/crates/red_knot_python_semantic/src/suppression.rs index 576e66cb48e4e..256de1b068538 100644 --- a/crates/red_knot_python_semantic/src/suppression.rs +++ b/crates/red_knot_python_semantic/src/suppression.rs @@ -1,4 +1,5 @@ use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus}; +use crate::python_version; use crate::types::{TypeCheckDiagnostic, TypeCheckDiagnostics}; use crate::{declare_lint, lint::LintId, Db}; use ruff_db::diagnostic::DiagnosticId; @@ -88,7 +89,7 @@ declare_lint! { #[salsa::tracked(return_ref)] pub(crate) fn suppressions(db: &dyn Db, file: File) -> Suppressions { - let parsed = parsed_module(db.upcast(), file); + let parsed = parsed_module(db.upcast(), file, python_version(db)); let source = source_text(db.upcast(), file); let mut builder = SuppressionsBuilder::new(&source, db.lint_registry()); diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 6a885af83f72b..f494e47190dda 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -4617,7 +4617,9 @@ pub(crate) mod tests { ); let events = db.take_salsa_events(); - let call = &*parsed_module(&db, bar).syntax().body[1] + let call = &*parsed_module(&db, bar, PythonVersion::default()) + .syntax() + .body[1] .as_assign_stmt() .unwrap() .value; diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index b405b4c5cbe23..457df300f9358 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -32,7 +32,7 @@ use itertools::{Either, Itertools}; use ruff_db::diagnostic::{DiagnosticId, Severity}; use ruff_db::files::File; use ruff_db::parsed::parsed_module; -use ruff_python_ast::{self as ast, AnyNodeRef, ExprContext}; +use ruff_python_ast::{self as ast, AnyNodeRef, ExprContext, PythonVersion}; use ruff_text_size::Ranged; use rustc_hash::{FxHashMap, FxHashSet}; use salsa; @@ -510,7 +510,14 @@ impl<'db> TypeInferenceBuilder<'db> { let node = scope.node(self.db()); match node { NodeWithScopeKind::Module => { - let parsed = parsed_module(self.db().upcast(), self.file()); + // TODO(brent) need to pass the real PythonVersion here, but tests fail when I + // change it from PythonVersion::default() + // + // I've tried my hacky `python_version` helper function and also + // `Program::get(db).python_version(db)`, and many tests fail in both cases (18 with + // `python_version`, 19 with `Program::get`) + let parsed = + parsed_module(self.db().upcast(), self.file(), PythonVersion::default()); self.infer_module(parsed.syntax()); } NodeWithScopeKind::Function(function) => self.infer_function_body(function.node()), @@ -6547,7 +6554,7 @@ mod tests { fn dependency_implicit_instance_attribute() -> anyhow::Result<()> { fn x_rhs_expression(db: &TestDb) -> Expression<'_> { let file_main = system_path_to_file(db, "/src/main.py").unwrap(); - let ast = parsed_module(db, file_main); + let ast = parsed_module(db, file_main, PythonVersion::default()); // Get the second statement in `main.py` (x = …) and extract the expression // node on the right-hand side: let x_rhs_node = &ast.syntax().body[1].as_assign_stmt().unwrap().value; diff --git a/crates/red_knot_test/src/assertion.rs b/crates/red_knot_test/src/assertion.rs index b03a0b8547167..b44ecaef35315 100644 --- a/crates/red_knot_test/src/assertion.rs +++ b/crates/red_knot_test/src/assertion.rs @@ -35,6 +35,7 @@ //! ``` use crate::db::Db; +use red_knot_python_semantic::python_version; use regex::Regex; use ruff_db::files::File; use ruff_db::parsed::parsed_module; @@ -58,7 +59,7 @@ impl InlineFileAssertions { pub(crate) fn from_file(db: &Db, file: File) -> Self { let source = source_text(db, file); let lines = line_index(db, file); - let parsed = parsed_module(db, file); + let parsed = parsed_module(db, file, python_version(db)); let comment_ranges = CommentRanges::from(parsed.tokens()); Self { comment_ranges, diff --git a/crates/red_knot_test/src/lib.rs b/crates/red_knot_test/src/lib.rs index cefb50fb78813..88d028142b552 100644 --- a/crates/red_knot_test/src/lib.rs +++ b/crates/red_knot_test/src/lib.rs @@ -4,7 +4,9 @@ use camino::Utf8Path; use colored::Colorize; use parser as test_parser; use red_knot_python_semantic::types::check_types; -use red_knot_python_semantic::{Program, ProgramSettings, SearchPathSettings, SitePackages}; +use red_knot_python_semantic::{ + python_version, Program, ProgramSettings, SearchPathSettings, SitePackages, +}; use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, ParseDiagnostic}; use ruff_db::files::{system_path_to_file, File, Files}; use ruff_db::panic::catch_unwind; @@ -193,7 +195,7 @@ fn run_test( let failures: Failures = test_files .into_iter() .filter_map(|test_file| { - let parsed = parsed_module(db, test_file.file); + let parsed = parsed_module(db, test_file.file, python_version(db)); let mut diagnostics: Vec> = parsed .errors() diff --git a/crates/red_knot_wasm/Cargo.toml b/crates/red_knot_wasm/Cargo.toml index 413dc2021b9ca..a5c4b997a97ba 100644 --- a/crates/red_knot_wasm/Cargo.toml +++ b/crates/red_knot_wasm/Cargo.toml @@ -20,6 +20,7 @@ default = ["console_error_panic_hook"] [dependencies] red_knot_project = { workspace = true, default-features = false, features = ["deflate"] } +red_knot_python_semantic = { workspace = true } ruff_db = { workspace = true, default-features = false, features = [] } ruff_python_ast = { workspace = true } diff --git a/crates/red_knot_wasm/src/lib.rs b/crates/red_knot_wasm/src/lib.rs index c93c22da5c2ca..486bca43ebcfb 100644 --- a/crates/red_knot_wasm/src/lib.rs +++ b/crates/red_knot_wasm/src/lib.rs @@ -1,6 +1,7 @@ use std::any::Any; use js_sys::Error; +use red_knot_python_semantic::python_version; use wasm_bindgen::prelude::*; use red_knot_project::metadata::options::{EnvironmentOptions, Options}; @@ -134,14 +135,16 @@ impl Workspace { /// Returns the parsed AST for `path` pub fn parsed(&self, file_id: &FileHandle) -> Result { - let parsed = ruff_db::parsed::parsed_module(&self.db, file_id.file); + let parsed = + ruff_db::parsed::parsed_module(&self.db, file_id.file, python_version(&self.db)); Ok(format!("{:#?}", parsed.syntax())) } /// Returns the token stream for `path` serialized as a string. pub fn tokens(&self, file_id: &FileHandle) -> Result { - let parsed = ruff_db::parsed::parsed_module(&self.db, file_id.file); + let parsed = + ruff_db::parsed::parsed_module(&self.db, file_id.file, python_version(&self.db)); Ok(format!("{:#?}", parsed.tokens())) } diff --git a/crates/ruff_db/src/parsed.rs b/crates/ruff_db/src/parsed.rs index fcc10b5844008..bea630b1acf60 100644 --- a/crates/ruff_db/src/parsed.rs +++ b/crates/ruff_db/src/parsed.rs @@ -2,8 +2,8 @@ use std::fmt::Formatter; use std::ops::Deref; use std::sync::Arc; -use ruff_python_ast::{ModModule, PySourceType}; -use ruff_python_parser::{parse_unchecked_source, Parsed}; +use ruff_python_ast::{ModModule, PySourceType, PythonVersion}; +use ruff_python_parser::{parse_unchecked, ParseOptions, Parsed}; use crate::files::{File, FilePath}; use crate::source::source_text; @@ -21,7 +21,7 @@ use crate::Db; /// The other reason is that Ruff's AST doesn't implement `Eq` which Sala requires /// for determining if a query result is unchanged. #[salsa::tracked(return_ref, no_eq)] -pub fn parsed_module(db: &dyn Db, file: File) -> ParsedModule { +pub fn parsed_module(db: &dyn Db, file: File, target_version: PythonVersion) -> ParsedModule { let _span = tracing::trace_span!("parsed_module", file = %file.path(db)).entered(); let source = source_text(db, file); @@ -37,7 +37,12 @@ pub fn parsed_module(db: &dyn Db, file: File) -> ParsedModule { .map_or(PySourceType::Python, PySourceType::from_extension), }; - ParsedModule::new(parse_unchecked_source(&source, ty)) + let options = ParseOptions::from(ty).with_target_version(target_version); + let parsed = parse_unchecked(&source, options) + .try_into_module() + .expect("PySourceType always parses into a module"); + + ParsedModule::new(parsed) } /// Cheap cloneable wrapper around the parsed module. @@ -89,6 +94,7 @@ mod tests { use crate::tests::TestDb; use crate::vendored::{VendoredFileSystemBuilder, VendoredPath}; use crate::Db; + use ruff_python_ast::PythonVersion; use zip::CompressionMethod; #[test] @@ -100,7 +106,7 @@ mod tests { let file = system_path_to_file(&db, path).unwrap(); - let parsed = parsed_module(&db, file); + let parsed = parsed_module(&db, file, PythonVersion::default()); assert!(parsed.is_valid()); @@ -116,7 +122,7 @@ mod tests { let file = system_path_to_file(&db, path).unwrap(); - let parsed = parsed_module(&db, file); + let parsed = parsed_module(&db, file, PythonVersion::default()); assert!(parsed.is_valid()); @@ -132,7 +138,7 @@ mod tests { let virtual_file = db.files().virtual_file(&db, path); - let parsed = parsed_module(&db, virtual_file.file()); + let parsed = parsed_module(&db, virtual_file.file(), PythonVersion::default()); assert!(parsed.is_valid()); @@ -148,7 +154,7 @@ mod tests { let virtual_file = db.files().virtual_file(&db, path); - let parsed = parsed_module(&db, virtual_file.file()); + let parsed = parsed_module(&db, virtual_file.file(), PythonVersion::default()); assert!(parsed.is_valid()); @@ -179,7 +185,7 @@ else: let file = vendored_path_to_file(&db, VendoredPath::new("path.pyi")).unwrap(); - let parsed = parsed_module(&db, file); + let parsed = parsed_module(&db, file, PythonVersion::default()); assert!(parsed.is_valid()); } From 3f43c82a08e4b206c457a28364a887dda51c314d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 19 Feb 2025 18:57:37 -0500 Subject: [PATCH 22/36] ignore version-related syntax errors when fuzzing --- crates/ruff_linter/src/linter.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 8e0add9a6b410..558f027dd4798 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -457,6 +457,10 @@ fn diagnostics_to_messages( builder.finish() }); + // ignore version-related syntax errors when fuzzing + #[cfg(fuzzing)] + let syntax_errors = &syntax_errors[..0]; + parse_errors .iter() .map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone())) From 3a5de09076131fe4ef68885839c281d5f0156d88 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 21 Feb 2025 17:40:57 +0000 Subject: [PATCH 23/36] Red-knot no longer panics! --- crates/red_knot_project/src/lib.rs | 4 ++-- crates/red_knot_project/tests/check.rs | 4 ++-- crates/red_knot_python_semantic/src/lib.rs | 9 ------- .../src/semantic_index.rs | 24 ++++++++----------- .../src/suppression.rs | 4 ++-- crates/red_knot_python_semantic/src/types.rs | 2 +- .../src/types/infer.rs | 23 +++++++++--------- crates/red_knot_test/src/assertion.rs | 4 ++-- crates/red_knot_test/src/lib.rs | 6 ++--- crates/red_knot_wasm/src/lib.rs | 16 +++++++++---- 10 files changed, 43 insertions(+), 53 deletions(-) diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index c6a788616a1b3..d71fb12a44b73 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -8,7 +8,7 @@ pub use metadata::{ProjectDiscoveryError, ProjectMetadata}; use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection}; use red_knot_python_semantic::syntax::SyntaxDiagnostic; use red_knot_python_semantic::types::check_types; -use red_knot_python_semantic::{python_version, register_lints}; +use red_knot_python_semantic::{Program, register_lints}; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity, Span}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; @@ -334,7 +334,7 @@ fn check_file_impl(db: &dyn Db, file: File) -> Vec> { return diagnostics; } - let parsed = parsed_module(db.upcast(), file, python_version(db.upcast())); + let parsed = parsed_module(db.upcast(), file, Program::get(db).python_version(db)); diagnostics.extend(parsed.errors().iter().map(|error| { let diagnostic: Box = Box::new(ParseDiagnostic::new(file, error.clone())); diagnostic diff --git a/crates/red_knot_project/tests/check.rs b/crates/red_knot_project/tests/check.rs index 9d0be4b99e84d..415580ca29040 100644 --- a/crates/red_knot_project/tests/check.rs +++ b/crates/red_knot_project/tests/check.rs @@ -1,6 +1,6 @@ use anyhow::{anyhow, Context}; use red_knot_project::{ProjectDatabase, ProjectMetadata}; -use red_knot_python_semantic::{python_version, HasType, SemanticModel}; +use red_knot_python_semantic::{Program, HasType, SemanticModel}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; use ruff_db::system::{SystemPath, SystemPathBuf, TestSystem}; @@ -165,7 +165,7 @@ fn run_corpus_tests(pattern: &str) -> anyhow::Result<()> { fn pull_types(db: &ProjectDatabase, file: File) { let mut visitor = PullTypesVisitor::new(db, file); - let ast = parsed_module(db, file, python_version(db)); + let ast = parsed_module(db, file, Program::get(db).python_version(db)); visitor.visit_body(ast.suite()); } diff --git a/crates/red_knot_python_semantic/src/lib.rs b/crates/red_knot_python_semantic/src/lib.rs index 1dbe0b38fe7fb..0625fdd741bd6 100644 --- a/crates/red_knot_python_semantic/src/lib.rs +++ b/crates/red_knot_python_semantic/src/lib.rs @@ -50,12 +50,3 @@ pub fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&UNKNOWN_RULE); registry.register_lint(&INVALID_IGNORE_COMMENT); } - -// TODO(brent) remove this. It should just be `Program::get(db).python_version(db)`, but for some -// reason `tests::check_file_skips_type_checking_when_file_cant_be_read` fails when I use `get`, so -// I factored this out instead of inlining everywhere -pub fn python_version(db: &dyn Db) -> ruff_python_ast::PythonVersion { - Program::try_get(db) - .map(|program| program.python_version(db)) - .unwrap_or_default() -} diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/crates/red_knot_python_semantic/src/semantic_index.rs index 69e2d110e130f..513f1c6bedcf9 100644 --- a/crates/red_knot_python_semantic/src/semantic_index.rs +++ b/crates/red_knot_python_semantic/src/semantic_index.rs @@ -5,7 +5,6 @@ use ruff_db::files::File; use ruff_db::parsed::parsed_module; use ruff_index::{IndexSlice, IndexVec}; -use ruff_python_ast::PythonVersion; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use salsa::plumbing::AsId; use salsa::Update; @@ -21,7 +20,7 @@ use crate::semantic_index::symbol::{ FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopedSymbolId, SymbolTable, }; use crate::semantic_index::use_def::{EagerBindingsKey, ScopedEagerBindingsId, UseDefMap}; -use crate::Db; +use crate::{Db, Program}; pub mod ast_ids; pub mod attribute_assignment; @@ -46,13 +45,7 @@ type SymbolMap = hashbrown::HashMap; pub(crate) fn semantic_index(db: &dyn Db, file: File) -> SemanticIndex<'_> { let _span = tracing::trace_span!("semantic_index", file = %file.path(db)).entered(); - // TODO(brent) need to pass the real PythonVersion here, but tests fail when I change it from - // PythonVersion::default() - // - // I've tried my hacky `python_version` helper function and also - // `Program::get(db).python_version(db)`, and many tests fail in both cases (18 with - // `python_version`, 48 with `Program::get`) - let parsed = parsed_module(db.upcast(), file, PythonVersion::default()); + let parsed = parsed_module(db.upcast(), file, Program::get(db).python_version(db)); SemanticIndexBuilder::new(db, file, parsed).build() } @@ -415,11 +408,10 @@ impl FusedIterator for ChildrenIter<'_> {} mod tests { use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; - use ruff_db::system::DbWithTestSystem; use ruff_python_ast::{self as ast, PythonVersion}; use ruff_text_size::{Ranged, TextRange}; - use crate::db::tests::TestDb; + use crate::db::tests::{TestDb, TestDbBuilder}; use crate::semantic_index::ast_ids::{HasScopedUseId, ScopedUseId}; use crate::semantic_index::definition::{Definition, DefinitionKind}; use crate::semantic_index::symbol::{ @@ -447,10 +439,14 @@ mod tests { } fn test_case(content: impl ToString) -> TestCase { - let mut db = TestDb::new(); - db.write_file("test.py", content).unwrap(); + const FILENAME: &str = "test.py"; - let file = system_path_to_file(&db, "test.py").unwrap(); + let db = TestDbBuilder::new() + .with_file(FILENAME, &content.to_string()) + .build() + .unwrap(); + + let file = system_path_to_file(&db, FILENAME).unwrap(); TestCase { db, file } } diff --git a/crates/red_knot_python_semantic/src/suppression.rs b/crates/red_knot_python_semantic/src/suppression.rs index 256de1b068538..3ce52e7b074c4 100644 --- a/crates/red_knot_python_semantic/src/suppression.rs +++ b/crates/red_knot_python_semantic/src/suppression.rs @@ -1,6 +1,6 @@ use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus}; -use crate::python_version; use crate::types::{TypeCheckDiagnostic, TypeCheckDiagnostics}; +use crate::Program; use crate::{declare_lint, lint::LintId, Db}; use ruff_db::diagnostic::DiagnosticId; use ruff_db::{files::File, parsed::parsed_module, source::source_text}; @@ -89,7 +89,7 @@ declare_lint! { #[salsa::tracked(return_ref)] pub(crate) fn suppressions(db: &dyn Db, file: File) -> Suppressions { - let parsed = parsed_module(db.upcast(), file, python_version(db)); + let parsed = parsed_module(db.upcast(), file, Program::get(db).python_version(db)); let source = source_text(db.upcast(), file); let mut builder = SuppressionsBuilder::new(&source, db.lint_registry()); diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index f494e47190dda..a30fc79ee1613 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -4617,7 +4617,7 @@ pub(crate) mod tests { ); let events = db.take_salsa_events(); - let call = &*parsed_module(&db, bar, PythonVersion::default()) + let call = &*parsed_module(&db, bar, Program::get(&db).python_version(&db)) .syntax() .body[1] .as_assign_stmt() diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 457df300f9358..47dbf3fd8b1fb 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -32,7 +32,7 @@ use itertools::{Either, Itertools}; use ruff_db::diagnostic::{DiagnosticId, Severity}; use ruff_db::files::File; use ruff_db::parsed::parsed_module; -use ruff_python_ast::{self as ast, AnyNodeRef, ExprContext, PythonVersion}; +use ruff_python_ast::{self as ast, AnyNodeRef, ExprContext}; use ruff_text_size::Ranged; use rustc_hash::{FxHashMap, FxHashSet}; use salsa; @@ -76,7 +76,7 @@ use crate::types::{ }; use crate::unpack::Unpack; use crate::util::subscript::{PyIndex, PySlice}; -use crate::Db; +use crate::{Db, Program}; use super::call::CallError; use super::context::{InNoTypeCheck, InferContext, WithDiagnostics}; @@ -507,17 +507,15 @@ impl<'db> TypeInferenceBuilder<'db> { } fn infer_region_scope(&mut self, scope: ScopeId<'db>) { - let node = scope.node(self.db()); + let db = self.db(); + let node = scope.node(db); match node { NodeWithScopeKind::Module => { - // TODO(brent) need to pass the real PythonVersion here, but tests fail when I - // change it from PythonVersion::default() - // - // I've tried my hacky `python_version` helper function and also - // `Program::get(db).python_version(db)`, and many tests fail in both cases (18 with - // `python_version`, 19 with `Program::get`) - let parsed = - parsed_module(self.db().upcast(), self.file(), PythonVersion::default()); + let parsed = parsed_module( + db.upcast(), + self.file(), + Program::get(db).python_version(db), + ); self.infer_module(parsed.syntax()); } NodeWithScopeKind::Function(function) => self.infer_function_body(function.node()), @@ -552,7 +550,7 @@ impl<'db> TypeInferenceBuilder<'db> { // Infer the deferred types for the definitions here to consider the end-of-scope // semantics. for definition in std::mem::take(&mut self.types.deferred) { - self.extend(infer_deferred_types(self.db(), definition)); + self.extend(infer_deferred_types(db, definition)); } assert!( self.types.deferred.is_empty(), @@ -6223,6 +6221,7 @@ mod tests { use ruff_db::files::{system_path_to_file, File}; use ruff_db::system::DbWithTestSystem; use ruff_db::testing::{assert_function_query_was_not_run, assert_function_query_was_run}; + use ruff_python_ast::PythonVersion; use super::*; diff --git a/crates/red_knot_test/src/assertion.rs b/crates/red_knot_test/src/assertion.rs index b44ecaef35315..04db9fc667009 100644 --- a/crates/red_knot_test/src/assertion.rs +++ b/crates/red_knot_test/src/assertion.rs @@ -35,7 +35,7 @@ //! ``` use crate::db::Db; -use red_knot_python_semantic::python_version; +use red_knot_python_semantic::Program; use regex::Regex; use ruff_db::files::File; use ruff_db::parsed::parsed_module; @@ -59,7 +59,7 @@ impl InlineFileAssertions { pub(crate) fn from_file(db: &Db, file: File) -> Self { let source = source_text(db, file); let lines = line_index(db, file); - let parsed = parsed_module(db, file, python_version(db)); + let parsed = parsed_module(db, file, Program::get(db).python_version(db)); let comment_ranges = CommentRanges::from(parsed.tokens()); Self { comment_ranges, diff --git a/crates/red_knot_test/src/lib.rs b/crates/red_knot_test/src/lib.rs index 88d028142b552..5e4632cb43d39 100644 --- a/crates/red_knot_test/src/lib.rs +++ b/crates/red_knot_test/src/lib.rs @@ -4,9 +4,7 @@ use camino::Utf8Path; use colored::Colorize; use parser as test_parser; use red_knot_python_semantic::types::check_types; -use red_knot_python_semantic::{ - python_version, Program, ProgramSettings, SearchPathSettings, SitePackages, -}; +use red_knot_python_semantic::{Program, ProgramSettings, SearchPathSettings, SitePackages}; use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, ParseDiagnostic}; use ruff_db::files::{system_path_to_file, File, Files}; use ruff_db::panic::catch_unwind; @@ -195,7 +193,7 @@ fn run_test( let failures: Failures = test_files .into_iter() .filter_map(|test_file| { - let parsed = parsed_module(db, test_file.file, python_version(db)); + let parsed = parsed_module(db, test_file.file, Program::get(db).python_version(db)); let mut diagnostics: Vec> = parsed .errors() diff --git a/crates/red_knot_wasm/src/lib.rs b/crates/red_knot_wasm/src/lib.rs index 486bca43ebcfb..f06266fc56fb6 100644 --- a/crates/red_knot_wasm/src/lib.rs +++ b/crates/red_knot_wasm/src/lib.rs @@ -1,13 +1,13 @@ use std::any::Any; use js_sys::Error; -use red_knot_python_semantic::python_version; use wasm_bindgen::prelude::*; use red_knot_project::metadata::options::{EnvironmentOptions, Options}; use red_knot_project::metadata::value::RangedValue; use red_knot_project::ProjectMetadata; use red_knot_project::{Db, ProjectDatabase}; +use red_knot_python_semantic::Program; use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::system::walk_directory::WalkDirectoryBuilder; @@ -135,16 +135,22 @@ impl Workspace { /// Returns the parsed AST for `path` pub fn parsed(&self, file_id: &FileHandle) -> Result { - let parsed = - ruff_db::parsed::parsed_module(&self.db, file_id.file, python_version(&self.db)); + let parsed = ruff_db::parsed::parsed_module( + &self.db, + file_id.file, + Program::get(&self.db).python_version(&self.db), + ); Ok(format!("{:#?}", parsed.syntax())) } /// Returns the token stream for `path` serialized as a string. pub fn tokens(&self, file_id: &FileHandle) -> Result { - let parsed = - ruff_db::parsed::parsed_module(&self.db, file_id.file, python_version(&self.db)); + let parsed = ruff_db::parsed::parsed_module( + &self.db, + file_id.file, + Program::get(&self.db).python_version(&self.db), + ); Ok(format!("{:#?}", parsed.tokens())) } From e47316eb6bdc1da821573d4403ca21151f489df5 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 21 Feb 2025 17:51:44 +0000 Subject: [PATCH 24/36] Fix fuzzing script and fix Rust formatting --- crates/red_knot_project/src/lib.rs | 2 +- crates/red_knot_project/tests/check.rs | 2 +- python/py-fuzzer/fuzz.py | 11 ++++++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index d71fb12a44b73..684df76c58fc3 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -8,7 +8,7 @@ pub use metadata::{ProjectDiscoveryError, ProjectMetadata}; use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection}; use red_knot_python_semantic::syntax::SyntaxDiagnostic; use red_knot_python_semantic::types::check_types; -use red_knot_python_semantic::{Program, register_lints}; +use red_knot_python_semantic::{register_lints, Program}; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity, Span}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; diff --git a/crates/red_knot_project/tests/check.rs b/crates/red_knot_project/tests/check.rs index 415580ca29040..60e84e40cab9f 100644 --- a/crates/red_knot_project/tests/check.rs +++ b/crates/red_knot_project/tests/check.rs @@ -1,6 +1,6 @@ use anyhow::{anyhow, Context}; use red_knot_project::{ProjectDatabase, ProjectMetadata}; -use red_knot_python_semantic::{Program, HasType, SemanticModel}; +use red_knot_python_semantic::{HasType, Program, SemanticModel}; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; use ruff_db::system::{SystemPath, SystemPathBuf, TestSystem}; diff --git a/python/py-fuzzer/fuzz.py b/python/py-fuzzer/fuzz.py index c4713f4028c40..181b69770ba72 100644 --- a/python/py-fuzzer/fuzz.py +++ b/python/py-fuzzer/fuzz.py @@ -58,7 +58,16 @@ def redknot_contains_bug(code: str, *, red_knot_executable: Path) -> bool: def ruff_contains_bug(code: str, *, ruff_executable: Path) -> bool: """Return `True` if the code triggers a parser error.""" completed_process = subprocess.run( - [ruff_executable, "check", "--config", "lint.select=[]", "--no-cache", "-"], + [ + ruff_executable, + "check", + "--config", + "lint.select=[]", + "--no-cache", + "--target-version", + "py313", + "-", + ], capture_output=True, text=True, input=code, From a50fa2a9969d40f26b71094effad792c798d4517 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 13:39:38 -0500 Subject: [PATCH 25/36] Revert "ignore version-related syntax errors when fuzzing" This reverts commit 3f43c82a08e4b206c457a28364a887dda51c314d. This didn't work because the fuzzing only runs with cfg(test) not cfg(fuzzing) --- crates/ruff_linter/src/linter.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 558f027dd4798..8e0add9a6b410 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -457,10 +457,6 @@ fn diagnostics_to_messages( builder.finish() }); - // ignore version-related syntax errors when fuzzing - #[cfg(fuzzing)] - let syntax_errors = &syntax_errors[..0]; - parse_errors .iter() .map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone())) From a15678163122108074bf20825c1039d461e9cf1d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 14:07:14 -0500 Subject: [PATCH 26/36] pass check_file_skips_type_checking_when by initializing settings --- crates/red_knot_project/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/red_knot_project/src/lib.rs b/crates/red_knot_project/src/lib.rs index 684df76c58fc3..524b4ba3dfb1f 100644 --- a/crates/red_knot_project/src/lib.rs +++ b/crates/red_knot_project/src/lib.rs @@ -486,12 +486,14 @@ mod tests { use crate::db::tests::TestDb; use crate::{check_file_impl, ProjectMetadata}; use red_knot_python_semantic::types::check_types; + use red_knot_python_semantic::{Program, ProgramSettings, PythonPlatform, SearchPathSettings}; use ruff_db::diagnostic::Diagnostic; use ruff_db::files::system_path_to_file; use ruff_db::source::source_text; use ruff_db::system::{DbWithTestSystem, SystemPath, SystemPathBuf}; use ruff_db::testing::assert_function_query_was_not_run; use ruff_python_ast::name::Name; + use ruff_python_ast::PythonVersion; #[test] fn check_file_skips_type_checking_when_file_cant_be_read() -> ruff_db::system::Result<()> { @@ -499,6 +501,16 @@ mod tests { let mut db = TestDb::new(project); let path = SystemPath::new("test.py"); + Program::from_settings( + &db, + ProgramSettings { + python_version: PythonVersion::default(), + python_platform: PythonPlatform::default(), + search_paths: SearchPathSettings::new(vec![SystemPathBuf::from(".")]), + }, + ) + .expect("Failed to configure program settings"); + db.write_file(path, "x = 10")?; let file = system_path_to_file(&db, path).unwrap(); From 114c207574253f1b867f062b1292be51f4cce2ec Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 14:08:37 -0500 Subject: [PATCH 27/36] fix clippy lint about &impl ToString --- crates/red_knot_python_semantic/src/semantic_index.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/crates/red_knot_python_semantic/src/semantic_index.rs index 513f1c6bedcf9..c9373d5e1792d 100644 --- a/crates/red_knot_python_semantic/src/semantic_index.rs +++ b/crates/red_knot_python_semantic/src/semantic_index.rs @@ -438,11 +438,11 @@ mod tests { file: File, } - fn test_case(content: impl ToString) -> TestCase { + fn test_case(content: &str) -> TestCase { const FILENAME: &str = "test.py"; let db = TestDbBuilder::new() - .with_file(FILENAME, &content.to_string()) + .with_file(FILENAME, content) .build() .unwrap(); From b06e1b1a6d359b91857a1aab76b0e1fde149ca84 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 14:24:46 -0500 Subject: [PATCH 28/36] pass version to new parsed_module call --- crates/red_knot_python_semantic/src/types/infer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 8c8cf8531f76c..34ccf4f3fa980 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -6814,7 +6814,7 @@ mod tests { fn dependency_own_instance_member() -> anyhow::Result<()> { fn x_rhs_expression(db: &TestDb) -> Expression<'_> { let file_main = system_path_to_file(db, "/src/main.py").unwrap(); - let ast = parsed_module(db, file_main); + let ast = parsed_module(db, file_main, PythonVersion::default()); // Get the second statement in `main.py` (x = …) and extract the expression // node on the right-hand side: let x_rhs_node = &ast.syntax().body[1].as_assign_stmt().unwrap().value; From 2eecea8d01c1573c80caba8666c23fff2512b941 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 21 Feb 2025 18:32:25 +0000 Subject: [PATCH 29/36] WIP --- crates/ruff/tests/lint.rs | 40 +++++++++++++++++++ crates/ruff_linter/src/message/mod.rs | 14 +++---- crates/ruff_python_parser/src/error.rs | 12 ++++-- .../src/parser/expression.rs | 16 ++++++-- .../src/parser/statement.rs | 11 ++++- 5 files changed, 78 insertions(+), 15 deletions(-) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 6515102f56bdd..a9e7aa5672dbb 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2568,6 +2568,46 @@ fn a005_module_shadowing_strict_default() -> Result<()> { Ok(()) } +#[test] +fn walrus_before_py38() { + // ok + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py38") + .arg("-") + .pass_stdin(r#"if x := 1: pass"#), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:1:10: E701 Multiple statements on one line (colon) + Found 1 error. + + ----- stderr ----- + " + ); + + // not ok on 3.7 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py37") + .arg("-") + .pass_stdin(r#"if x := 1: pass"#), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:1:4: SyntaxError: Cannot use named assignment expression (`:=`) on Python 3.7 (syntax was new in Python 3.8) + test.py:1:10: E701 Multiple statements on one line (colon) + Found 2 errors. + + ----- stderr ----- + " + ); +} + #[test] fn match_before_py310() { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 36d35782f236a..e958069b97531 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -16,7 +16,7 @@ pub use pylint::PylintEmitter; pub use rdjson::RdjsonEmitter; use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix}; use ruff_notebook::NotebookIndex; -use ruff_python_parser::{ParseError, SyntaxError, SyntaxErrorKind}; +use ruff_python_parser::{ParseError, SyntaxError}; use ruff_source_file::{SourceFile, SourceLocation}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; pub use sarif::SarifEmitter; @@ -123,13 +123,11 @@ impl Message { /// Create a [`Message`] from the given [`SyntaxError`]. pub fn from_syntax_error(syntax_error: &SyntaxError, file: SourceFile) -> Message { - match syntax_error.kind { - SyntaxErrorKind::MatchBeforePy310 => Message::SyntaxError(SyntaxErrorMessage { - message: format!("SyntaxError: {}", syntax_error.message()), - range: syntax_error.range, - file, - }), - } + Message::SyntaxError(SyntaxErrorMessage { + message: format!("SyntaxError: {}", syntax_error.message()), + range: syntax_error.range, + file, + }) } pub const fn as_diagnostic_message(&self) -> Option<&DiagnosticMessage> { diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 75637747849d2..e81afa04da5f6 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -437,10 +437,13 @@ pub struct SyntaxError { impl SyntaxError { pub fn message(&self) -> String { match self.kind { + SyntaxErrorKind::WalrusBeforePy38 => format!( + "Cannot use named assignment expression (`:=`) on Python {} (syntax was new in Python 3.8)", + self.target_version + ), SyntaxErrorKind::MatchBeforePy310 => format!( - "Cannot use `match` statement on Python {major}.{minor} (syntax was new in Python 3.10)", - major = self.target_version.major, - minor = self.target_version.minor, + "Cannot use `match` statement on Python {} (syntax was new in Python 3.10)", + self.target_version, ), } } @@ -448,6 +451,7 @@ impl SyntaxError { /// The earliest allowed version for the syntax associated with this error. pub const fn version(&self) -> PythonVersion { match self.kind { + SyntaxErrorKind::WalrusBeforePy38 => PythonVersion::PY38, SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, } } @@ -455,12 +459,14 @@ impl SyntaxError { #[derive(Debug, PartialEq, Clone, Copy)] pub enum SyntaxErrorKind { + WalrusBeforePy38, MatchBeforePy310, } impl SyntaxErrorKind { pub const fn as_str(self) -> &'static str { match self { + SyntaxErrorKind::WalrusBeforePy38 => "walrus-before-python-38", SyntaxErrorKind::MatchBeforePy310 => "match-before-python-310", } } diff --git a/crates/ruff_python_parser/src/parser/expression.rs b/crates/ruff_python_parser/src/parser/expression.rs index 31d8b363d4170..b837e66bcc340 100644 --- a/crates/ruff_python_parser/src/parser/expression.rs +++ b/crates/ruff_python_parser/src/parser/expression.rs @@ -7,7 +7,7 @@ use rustc_hash::{FxBuildHasher, FxHashSet}; use ruff_python_ast::name::Name; use ruff_python_ast::{ self as ast, BoolOp, CmpOp, ConversionFlag, Expr, ExprContext, FStringElement, FStringElements, - IpyEscapeKind, Number, Operator, StringFlags, UnaryOp, + IpyEscapeKind, Number, Operator, PythonVersion, StringFlags, UnaryOp, }; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -16,7 +16,7 @@ use crate::parser::{helpers, FunctionKind, Parser}; use crate::string::{parse_fstring_literal_element, parse_string_literal, StringType}; use crate::token::{TokenKind, TokenValue}; use crate::token_set::TokenSet; -use crate::{FStringErrorType, Mode, ParseErrorType}; +use crate::{FStringErrorType, Mode, ParseErrorType, SyntaxError, SyntaxErrorKind}; use super::{FStringElementsKind, Parenthesized, RecoveryContextKind}; @@ -2161,10 +2161,20 @@ impl<'src> Parser<'src> { let value = self.parse_conditional_expression_or_higher(); + let range = self.node_range(start); + + if self.options.target_version < PythonVersion::PY38 { + self.syntax_errors.push(SyntaxError { + kind: SyntaxErrorKind::WalrusBeforePy38, + range, + target_version: self.options.target_version, + }); + } + ast::ExprNamed { target: Box::new(target), value: Box::new(value.expr), - range: self.node_range(start), + range, } } diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index d4e3baeae6e24..ed708123eedba 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -1458,13 +1458,22 @@ impl<'src> Parser<'src> { ); } + let range = self.node_range(try_start); + if is_star && self.options.target_version < PythonVersion::PY311 { + self.syntax_errors.push(SyntaxError { + kind: SyntaxErrorKind::ExceptStarBeforePy311, + range, + target_version: self.options.target_version, + }); + } + ast::StmtTry { body: try_body, handlers, orelse, finalbody, is_star, - range: self.node_range(try_start), + range, } } From 53ed4acc9473666f9390027e1d3a6f1c8b7ef439 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 14:21:57 -0500 Subject: [PATCH 30/36] add ExceptStarBeforePy311 variant, refactor SyntaxError::message --- crates/ruff_python_parser/src/error.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index e81afa04da5f6..d126c30a13130 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -436,23 +436,25 @@ pub struct SyntaxError { impl SyntaxError { pub fn message(&self) -> String { - match self.kind { - SyntaxErrorKind::WalrusBeforePy38 => format!( - "Cannot use named assignment expression (`:=`) on Python {} (syntax was new in Python 3.8)", - self.target_version - ), - SyntaxErrorKind::MatchBeforePy310 => format!( - "Cannot use `match` statement on Python {} (syntax was new in Python 3.10)", - self.target_version, - ), - } + let kind = match self.kind { + SyntaxErrorKind::WalrusBeforePy38 => "named assignment expression (`:=`)", + SyntaxErrorKind::MatchBeforePy310 => "`match` statement", + SyntaxErrorKind::ExceptStarBeforePy311 => "`except*`", + }; + + format!( + "Cannot use {kind} on Python {} (syntax was new in Python {})", + self.target_version, + self.minimum_version(), + ) } /// The earliest allowed version for the syntax associated with this error. - pub const fn version(&self) -> PythonVersion { + pub const fn minimum_version(&self) -> PythonVersion { match self.kind { SyntaxErrorKind::WalrusBeforePy38 => PythonVersion::PY38, SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, + SyntaxErrorKind::ExceptStarBeforePy311 => PythonVersion::PY311, } } } @@ -461,6 +463,7 @@ impl SyntaxError { pub enum SyntaxErrorKind { WalrusBeforePy38, MatchBeforePy310, + ExceptStarBeforePy311, } impl SyntaxErrorKind { @@ -468,6 +471,7 @@ impl SyntaxErrorKind { match self { SyntaxErrorKind::WalrusBeforePy38 => "walrus-before-python-38", SyntaxErrorKind::MatchBeforePy310 => "match-before-python-310", + SyntaxErrorKind::ExceptStarBeforePy311 => "except-star-before-python-311", } } } From c832d1da368ea8bc01989d73e791f96859d82cc6 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 14:40:04 -0500 Subject: [PATCH 31/36] use latest version `ruff_linter_fixtures` to avoid syntax errors --- crates/ruff/src/cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 2059d308ac657..a8862f91c7ad5 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -613,7 +613,7 @@ mod tests { let settings = Settings { cache_dir, linter: LinterSettings { - target_version: PythonVersion::PY310, + target_version: PythonVersion::latest(), ..Default::default() }, ..Settings::default() From f5a607c6676b226277c48057f27dfd0911f20d6e Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 14:49:57 -0500 Subject: [PATCH 32/36] test except* --- crates/ruff/tests/lint.rs | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index a9e7aa5672dbb..08b450bbdf28d 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2633,3 +2633,52 @@ match 2: " ); } + +#[test] +fn except_star_before_py311() { + let stdin = r#" +try: + raise Exception +except* TypeError as e: + pass +"#; + + // ok on 3.11 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py311") + .arg("-") + .pass_stdin(stdin), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:4:22: F841 [*] Local variable `e` is assigned to but never used + Found 1 error. + [*] 1 fixable with the `--fix` option. + + ----- stderr ----- + " + ); + + // error on 3.10 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py310") + .arg("-") + .pass_stdin(stdin), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:2:1: SyntaxError: Cannot use `except*` on Python 3.10 (syntax was new in Python 3.11) + test.py:4:22: F841 [*] Local variable `e` is assigned to but never used + Found 2 errors. + [*] 1 fixable with the `--fix` option. + + ----- stderr ----- + " + ); +} From d6c2508726f5292a9c84308c91cb2e2d2beb5858 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 15:15:37 -0500 Subject: [PATCH 33/36] factor out syntax_errors!, add `add_syntax_error`, and typebefore312 --- crates/ruff_python_parser/src/error.rs | 71 ++++++++++--------- .../src/parser/expression.rs | 8 +-- crates/ruff_python_parser/src/parser/mod.rs | 11 ++- .../src/parser/statement.rs | 22 +++--- 4 files changed, 59 insertions(+), 53 deletions(-) diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index d126c30a13130..826bd7b72b87e 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -434,46 +434,49 @@ pub struct SyntaxError { pub target_version: PythonVersion, } -impl SyntaxError { - pub fn message(&self) -> String { - let kind = match self.kind { - SyntaxErrorKind::WalrusBeforePy38 => "named assignment expression (`:=`)", - SyntaxErrorKind::MatchBeforePy310 => "`match` statement", - SyntaxErrorKind::ExceptStarBeforePy311 => "`except*`", - }; +macro_rules! syntax_errors { + ($(($variant:ident, $version:ident, $error_msg:expr, $error_str:expr)$(,)?)*) => { + #[derive(Debug, PartialEq, Clone, Copy)] + pub enum SyntaxErrorKind { + $($variant,)* + } - format!( - "Cannot use {kind} on Python {} (syntax was new in Python {})", - self.target_version, - self.minimum_version(), - ) - } + impl SyntaxError { + pub fn message(&self) -> String { + let kind = match self.kind { + $(SyntaxErrorKind::$variant => $error_msg,)* + }; - /// The earliest allowed version for the syntax associated with this error. - pub const fn minimum_version(&self) -> PythonVersion { - match self.kind { - SyntaxErrorKind::WalrusBeforePy38 => PythonVersion::PY38, - SyntaxErrorKind::MatchBeforePy310 => PythonVersion::PY310, - SyntaxErrorKind::ExceptStarBeforePy311 => PythonVersion::PY311, + format!( + "Cannot use {kind} on Python {} (syntax was new in Python {})", + self.target_version, + self.minimum_version(), + ) + } + + /// The earliest allowed version for the syntax associated with this error. + pub const fn minimum_version(&self) -> PythonVersion { + match self.kind { + $(SyntaxErrorKind::$variant => PythonVersion::$version,)* + } + } } - } -} -#[derive(Debug, PartialEq, Clone, Copy)] -pub enum SyntaxErrorKind { - WalrusBeforePy38, - MatchBeforePy310, - ExceptStarBeforePy311, + impl SyntaxErrorKind { + pub const fn as_str(self) -> &'static str { + match self { + $(SyntaxErrorKind::$variant => $error_str,)* + } + } + } + }; } -impl SyntaxErrorKind { - pub const fn as_str(self) -> &'static str { - match self { - SyntaxErrorKind::WalrusBeforePy38 => "walrus-before-python-38", - SyntaxErrorKind::MatchBeforePy310 => "match-before-python-310", - SyntaxErrorKind::ExceptStarBeforePy311 => "except-star-before-python-311", - } - } +syntax_errors! { + (WalrusBeforePy38, PY38, "named assignment expression (`:=`)", "walrus-before-python-38"), + (MatchBeforePy310, PY310, "`match` statement", "match-before-python-310"), + (ExceptStarBeforePy311, PY311, "`except*`", "except-star-before-python-311"), + (TypeStmtBeforePy312, PY312, "`type` statement", "type-stmt-before-python-312"), } #[cfg(target_pointer_width = "64")] diff --git a/crates/ruff_python_parser/src/parser/expression.rs b/crates/ruff_python_parser/src/parser/expression.rs index b837e66bcc340..4b6f0e08b35fb 100644 --- a/crates/ruff_python_parser/src/parser/expression.rs +++ b/crates/ruff_python_parser/src/parser/expression.rs @@ -16,7 +16,7 @@ use crate::parser::{helpers, FunctionKind, Parser}; use crate::string::{parse_fstring_literal_element, parse_string_literal, StringType}; use crate::token::{TokenKind, TokenValue}; use crate::token_set::TokenSet; -use crate::{FStringErrorType, Mode, ParseErrorType, SyntaxError, SyntaxErrorKind}; +use crate::{FStringErrorType, Mode, ParseErrorType, SyntaxErrorKind}; use super::{FStringElementsKind, Parenthesized, RecoveryContextKind}; @@ -2164,11 +2164,7 @@ impl<'src> Parser<'src> { let range = self.node_range(start); if self.options.target_version < PythonVersion::PY38 { - self.syntax_errors.push(SyntaxError { - kind: SyntaxErrorKind::WalrusBeforePy38, - range, - target_version: self.options.target_version, - }); + self.add_syntax_error(SyntaxErrorKind::WalrusBeforePy38, range); } ast::ExprNamed { diff --git a/crates/ruff_python_parser/src/parser/mod.rs b/crates/ruff_python_parser/src/parser/mod.rs index 4840ae2ad2f6e..84314c3d21bfd 100644 --- a/crates/ruff_python_parser/src/parser/mod.rs +++ b/crates/ruff_python_parser/src/parser/mod.rs @@ -11,7 +11,7 @@ use crate::parser::progress::{ParserProgress, TokenId}; use crate::token::TokenValue; use crate::token_set::TokenSet; use crate::token_source::{TokenSource, TokenSourceCheckpoint}; -use crate::{Mode, ParseError, ParseErrorType, TokenKind}; +use crate::{Mode, ParseError, ParseErrorType, SyntaxErrorKind, TokenKind}; use crate::{Parsed, Tokens}; pub use crate::parser::options::ParseOptions; @@ -438,6 +438,15 @@ impl<'src> Parser<'src> { inner(&mut self.errors, error, ranged.range()); } + /// Add a [`SyntaxError`] with the given [`SyntaxErrorKind`] and [`TextRange`]. + fn add_syntax_error(&mut self, kind: SyntaxErrorKind, range: TextRange) { + self.syntax_errors.push(SyntaxError { + kind, + range, + target_version: self.options.target_version, + }); + } + /// Returns `true` if the current token is of the given kind. fn at(&self, kind: TokenKind) -> bool { self.current_token_kind() == kind diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index ed708123eedba..838daf12de3d4 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -17,7 +17,7 @@ use crate::parser::{ }; use crate::token::{TokenKind, TokenValue}; use crate::token_set::TokenSet; -use crate::{Mode, ParseErrorType, SyntaxError, SyntaxErrorKind}; +use crate::{Mode, ParseErrorType, SyntaxErrorKind}; use super::expression::ExpressionContext; use super::Parenthesized; @@ -910,11 +910,17 @@ impl<'src> Parser<'src> { // type x = x := 1 let value = self.parse_conditional_expression_or_higher(); + let range = self.node_range(start); + + if self.options.target_version < PythonVersion::PY312 { + self.add_syntax_error(SyntaxErrorKind::TypeStmtBeforePy312, range); + } + ast::StmtTypeAlias { name: Box::new(name), type_params, value: Box::new(value.expr), - range: self.node_range(start), + range, } } @@ -1460,11 +1466,7 @@ impl<'src> Parser<'src> { let range = self.node_range(try_start); if is_star && self.options.target_version < PythonVersion::PY311 { - self.syntax_errors.push(SyntaxError { - kind: SyntaxErrorKind::ExceptStarBeforePy311, - range, - target_version: self.options.target_version, - }); + self.add_syntax_error(SyntaxErrorKind::ExceptStarBeforePy311, range); } ast::StmtTry { @@ -2275,11 +2277,7 @@ impl<'src> Parser<'src> { let range = self.node_range(start); if self.options.target_version < PythonVersion::PY310 { - self.syntax_errors.push(SyntaxError { - kind: SyntaxErrorKind::MatchBeforePy310, - range, - target_version: self.options.target_version, - }); + self.add_syntax_error(SyntaxErrorKind::MatchBeforePy310, range); } ast::StmtMatch { From 8763beedc630b84434e44be940e0a49f8b667ab2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 15:23:38 -0500 Subject: [PATCH 34/36] test type statement --- crates/ruff/tests/lint.rs | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 08b450bbdf28d..10bd956c80ac0 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2682,3 +2682,43 @@ except* TypeError as e: " ); } + +#[test] +fn type_stmt_before_py312() { + let stdin = "type Point = tuple[float, float]"; + + // ok on 3.12 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py312") + .arg("-") + .pass_stdin(stdin), + @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + " + ); + + // error on 3.11 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py311") + .arg("-") + .pass_stdin(stdin), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:1:1: SyntaxError: Cannot use `type` statement on Python 3.11 (syntax was new in Python 3.12) + Found 1 error. + + ----- stderr ----- + " + ); +} From 832274b4104ad2a5986a23ae9bcea7d983707d19 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 15:32:47 -0500 Subject: [PATCH 35/36] detect type parameter lists --- crates/ruff/tests/lint.rs | 61 +++++++++++++++++++ crates/ruff_python_parser/src/error.rs | 1 + .../src/parser/statement.rs | 9 ++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 10bd956c80ac0..53b927f6cbb05 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2722,3 +2722,64 @@ fn type_stmt_before_py312() { " ); } + +#[test] +fn type_parameter_lists_before_py312() { + let stdin = r#" +from typing import Iterator + +def max[T](args: list[T]) -> T: + ... + +async def amax[T](args: list[T]) -> T: + ... + +class Bag[T]: + def __iter__(self) -> Iterator[T]: + ... + + def add(self, arg: T) -> None: + ... + +type ListOrSet[T] = list[T] | set[T] +"#; + + // ok on 3.12 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py312") + .arg("-") + .pass_stdin(stdin), + @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + " + ); + + // 5 errors on 3.11 (4 from parameter lists, 1 from the `type` statement) + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py311") + .arg("-") + .pass_stdin(stdin), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:4:8: SyntaxError: Cannot use type parameter list on Python 3.11 (syntax was new in Python 3.12) + test.py:7:15: SyntaxError: Cannot use type parameter list on Python 3.11 (syntax was new in Python 3.12) + test.py:10:10: SyntaxError: Cannot use type parameter list on Python 3.11 (syntax was new in Python 3.12) + test.py:17:1: SyntaxError: Cannot use `type` statement on Python 3.11 (syntax was new in Python 3.12) + test.py:17:15: SyntaxError: Cannot use type parameter list on Python 3.11 (syntax was new in Python 3.12) + Found 5 errors. + + ----- stderr ----- + " + ); +} diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 826bd7b72b87e..321ecfd226fa1 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -477,6 +477,7 @@ syntax_errors! { (MatchBeforePy310, PY310, "`match` statement", "match-before-python-310"), (ExceptStarBeforePy311, PY311, "`except*`", "except-star-before-python-311"), (TypeStmtBeforePy312, PY312, "`type` statement", "type-stmt-before-python-312"), + (TypeParamsBeforePy312, PY312, "type parameter list", "type-params-before-python-312"), } #[cfg(target_pointer_width = "64")] diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 838daf12de3d4..c78b870cf4656 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -3093,10 +3093,13 @@ impl<'src> Parser<'src> { self.expect(TokenKind::Rsqb); - ast::TypeParams { - range: self.node_range(start), - type_params, + let range = self.node_range(start); + + if self.options.target_version < PythonVersion::PY312 { + self.add_syntax_error(SyntaxErrorKind::TypeParamsBeforePy312, range); } + + ast::TypeParams { range, type_params } } /// Parses a type parameter. From 35c7cd09c7f3fe66fb0147a026d17b5e77586868 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 21 Feb 2025 15:57:12 -0500 Subject: [PATCH 36/36] detect type parameter defaults --- crates/ruff/tests/lint.rs | 50 +++++++++++++++++++ crates/ruff_python_parser/src/error.rs | 1 + .../src/parser/statement.rs | 8 ++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 53b927f6cbb05..70a59cde06dd5 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2783,3 +2783,53 @@ type ListOrSet[T] = list[T] | set[T] " ); } + +#[test] +fn type_parameter_defaults_before_py313() { + let stdin = r#" +def max[T = int](args: list[T]) -> T: + ... + +class Bag[T = int]: + ... + +type ListOrSet[T = int] = list[T] | set[T] +"#; + + // ok on 3.13 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py313") + .arg("-") + .pass_stdin(stdin), + @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + " + ); + + // 3 errors on 3.12 + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--stdin-filename", "test.py"]) + .arg("--target-version=py312") + .arg("-") + .pass_stdin(stdin), + @r" + success: false + exit_code: 1 + ----- stdout ----- + test.py:2:9: SyntaxError: Cannot use type parameter default on Python 3.12 (syntax was new in Python 3.13) + test.py:5:11: SyntaxError: Cannot use type parameter default on Python 3.12 (syntax was new in Python 3.13) + test.py:8:16: SyntaxError: Cannot use type parameter default on Python 3.12 (syntax was new in Python 3.13) + Found 3 errors. + + ----- stderr ----- + " + ); +} diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 321ecfd226fa1..4d93570ca9eeb 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -478,6 +478,7 @@ syntax_errors! { (ExceptStarBeforePy311, PY311, "`except*`", "except-star-before-python-311"), (TypeStmtBeforePy312, PY312, "`type` statement", "type-stmt-before-python-312"), (TypeParamsBeforePy312, PY312, "type parameter list", "type-params-before-python-312"), + (TypeParamDefaultBeforePy313, PY313, "type parameter default", "type-param-default-before-python-313"), } #[cfg(target_pointer_width = "64")] diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index c78b870cf4656..cbc759dc9119b 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -3257,8 +3257,14 @@ impl<'src> Parser<'src> { None }; + let range = self.node_range(start); + + if default.is_some() && self.options.target_version < PythonVersion::PY313 { + self.add_syntax_error(SyntaxErrorKind::TypeParamDefaultBeforePy313, range); + } + ast::TypeParam::TypeVar(ast::TypeParamTypeVar { - range: self.node_range(start), + range, name, bound, default,