From 64165bee43e8f667c053fb4a280101e15c526b05 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 4 Jun 2024 08:04:48 +0200 Subject: [PATCH] red-knot: Use `parse_unchecked` to get all parse errors (#11725) --- Cargo.lock | 24 ---------- crates/red_knot/Cargo.toml | 1 - crates/red_knot/src/lint.rs | 13 ++--- crates/red_knot/src/parse.rs | 72 ++++------------------------ crates/red_knot/src/source.rs | 10 ++++ crates/red_knot/src/symbols.rs | 43 +++++++++-------- crates/red_knot/src/types/infer.rs | 11 ++--- crates/ruff_python_parser/src/lib.rs | 9 ++-- 8 files changed, 58 insertions(+), 125 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f6c6f3ed1d..0654eab5f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1789,7 +1789,6 @@ dependencies = [ "rustc-hash", "smol_str", "tempfile", - "textwrap", "tracing", "tracing-subscriber", "tracing-tree", @@ -2690,12 +2689,6 @@ version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" -[[package]] -name = "smawk" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" - [[package]] name = "smol_str" version = "0.2.2" @@ -2845,17 +2838,6 @@ dependencies = [ "test-case-core", ] -[[package]] -name = "textwrap" -version = "0.16.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23d434d3f8967a09480fb04132ebe0a3e088c173e6d0ee7897abbdf4eab0f8b9" -dependencies = [ - "smawk", - "unicode-linebreak", - "unicode-width", -] - [[package]] name = "thiserror" version = "1.0.61" @@ -3111,12 +3093,6 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" -[[package]] -name = "unicode-linebreak" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" - [[package]] name = "unicode-normalization" version = "0.1.23" diff --git a/crates/red_knot/Cargo.toml b/crates/red_knot/Cargo.toml index 6b13660774..3f379a8925 100644 --- a/crates/red_knot/Cargo.toml +++ b/crates/red_knot/Cargo.toml @@ -35,7 +35,6 @@ tracing-subscriber = { workspace = true } tracing-tree = { workspace = true } [dev-dependencies] -textwrap = { version = "0.16.1" } tempfile = { workspace = true } [lints] diff --git a/crates/red_knot/src/lint.rs b/crates/red_knot/src/lint.rs index 0801809f52..cad38f814d 100644 --- a/crates/red_knot/src/lint.rs +++ b/crates/red_knot/src/lint.rs @@ -5,12 +5,13 @@ use std::time::Duration; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::{ModModule, StringLiteral}; +use ruff_python_parser::Parsed; use crate::cache::KeyValueCache; use crate::db::{LintDb, LintJar, QueryResult}; use crate::files::FileId; use crate::module::ModuleName; -use crate::parse::{parse, Parsed}; +use crate::parse::parse; use crate::source::{source_text, Source}; use crate::symbols::{ resolve_global_symbol, symbol_table, Definition, GlobalSymbolId, SymbolId, SymbolTable, @@ -40,7 +41,7 @@ pub(crate) fn lint_syntax(db: &dyn LintDb, file_id: FileId) -> QueryResult QueryResult QueryResult<()> { pub struct SemanticLintContext<'a> { file_id: FileId, source: Source, - parsed: Parsed, + parsed: &'a Parsed, symbols: Arc, db: &'a dyn LintDb, diagnostics: RefCell>, @@ -209,8 +210,8 @@ impl<'a> SemanticLintContext<'a> { self.file_id } - pub fn ast(&self) -> &ModModule { - self.parsed.ast() + pub fn ast(&self) -> &'a ModModule { + self.parsed.syntax() } pub fn symbols(&self) -> &SymbolTable { diff --git a/crates/red_knot/src/parse.rs b/crates/red_knot/src/parse.rs index 4e3cd4d422..393625b3ae 100644 --- a/crates/red_knot/src/parse.rs +++ b/crates/red_knot/src/parse.rs @@ -1,87 +1,33 @@ use std::ops::{Deref, DerefMut}; use std::sync::Arc; -use ruff_python_ast as ast; -use ruff_python_parser::{Mode, ParseError}; -use ruff_text_size::{Ranged, TextRange}; +use ruff_python_ast::ModModule; +use ruff_python_parser::Parsed; use crate::cache::KeyValueCache; use crate::db::{QueryResult, SourceDb}; use crate::files::FileId; use crate::source::source_text; -#[derive(Debug, Clone, PartialEq)] -pub struct Parsed { - inner: Arc, -} - -#[derive(Debug, PartialEq)] -struct ParsedInner { - ast: ast::ModModule, - errors: Vec, -} - -impl Parsed { - fn new(ast: ast::ModModule, errors: Vec) -> Self { - Self { - inner: Arc::new(ParsedInner { ast, errors }), - } - } - - pub(crate) fn from_text(text: &str) -> Self { - let result = ruff_python_parser::parse(text, Mode::Module); - - let (module, errors) = match result { - Ok(parsed) => match parsed.into_syntax() { - ast::Mod::Module(module) => (module, vec![]), - ast::Mod::Expression(expression) => ( - ast::ModModule { - range: expression.range(), - body: vec![ast::Stmt::Expr(ast::StmtExpr { - range: expression.range(), - value: expression.body, - })], - }, - vec![], - ), - }, - Err(errors) => ( - ast::ModModule { - range: TextRange::default(), - body: Vec::new(), - }, - vec![errors], - ), - }; - - Parsed::new(module, errors) - } - - pub fn ast(&self) -> &ast::ModModule { - &self.inner.ast - } - - pub fn errors(&self) -> &[ParseError] { - &self.inner.errors - } -} - #[tracing::instrument(level = "debug", skip(db))] -pub(crate) fn parse(db: &dyn SourceDb, file_id: FileId) -> QueryResult { +pub(crate) fn parse(db: &dyn SourceDb, file_id: FileId) -> QueryResult>> { let jar = db.jar()?; jar.parsed.get(&file_id, |file_id| { let source = source_text(db, *file_id)?; - Ok(Parsed::from_text(source.text())) + Ok(Arc::new(ruff_python_parser::parse_unchecked_source( + source.text(), + source.kind().into(), + ))) }) } #[derive(Debug, Default)] -pub struct ParsedStorage(KeyValueCache); +pub struct ParsedStorage(KeyValueCache>>); impl Deref for ParsedStorage { - type Target = KeyValueCache; + type Target = KeyValueCache>>; fn deref(&self) -> &Self::Target { &self.0 diff --git a/crates/red_knot/src/source.rs b/crates/red_knot/src/source.rs index 6746b90f81..f82e5de6e0 100644 --- a/crates/red_knot/src/source.rs +++ b/crates/red_knot/src/source.rs @@ -53,6 +53,16 @@ pub enum SourceKind { IpyNotebook(Arc), } +impl<'a> From<&'a SourceKind> for PySourceType { + fn from(value: &'a SourceKind) -> Self { + match value { + SourceKind::Python(_) => PySourceType::Python, + SourceKind::Stub(_) => PySourceType::Stub, + SourceKind::IpyNotebook(_) => PySourceType::Ipynb, + } + } +} + #[derive(Debug, Clone, PartialEq)] pub struct Source { kind: SourceKind, diff --git a/crates/red_knot/src/symbols.rs b/crates/red_knot/src/symbols.rs index 3d9757c911..4ac5c67694 100644 --- a/crates/red_knot/src/symbols.rs +++ b/crates/red_knot/src/symbols.rs @@ -28,7 +28,7 @@ pub fn symbol_table(db: &dyn SemanticDb, file_id: FileId) -> QueryResult Parsed { - Parsed::from_text(&dedent(code)) + use crate::symbols::{Definition, ScopeKind, SymbolId, SymbolIterator, SymbolTable}; + + fn parse(code: &str) -> Parsed { + ruff_python_parser::parse_unchecked(code, Mode::Module) + .try_into_module() + .unwrap() } fn names(it: SymbolIterator) -> Vec<&str> @@ -924,14 +927,14 @@ mod tests { #[test] fn empty() { let parsed = parse(""); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()).len(), 0); } #[test] fn simple() { let parsed = parse("x"); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["x"]); assert_eq!( table @@ -944,7 +947,7 @@ mod tests { #[test] fn annotation_only() { let parsed = parse("x: int"); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["int", "x"]); // TODO record definition } @@ -952,7 +955,7 @@ mod tests { #[test] fn import() { let parsed = parse("import foo"); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["foo"]); assert_eq!( table @@ -965,21 +968,21 @@ mod tests { #[test] fn import_sub() { let parsed = parse("import foo.bar"); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["foo"]); } #[test] fn import_as() { let parsed = parse("import foo.bar as baz"); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["baz"]); } #[test] fn import_from() { let parsed = parse("from bar import foo"); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["foo"]); assert_eq!( table @@ -999,7 +1002,7 @@ mod tests { #[test] fn assign() { let parsed = parse("x = foo"); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["foo", "x"]); assert_eq!( table @@ -1025,7 +1028,7 @@ mod tests { y = 2 ", ); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["C", "y"]); let scopes = table.root_child_scope_ids(); assert_eq!(scopes.len(), 1); @@ -1050,7 +1053,7 @@ mod tests { y = 2 ", ); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["func", "y"]); let scopes = table.root_child_scope_ids(); assert_eq!(scopes.len(), 1); @@ -1076,7 +1079,7 @@ mod tests { y = 2 ", ); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["func"]); let scopes = table.root_child_scope_ids(); assert_eq!(scopes.len(), 2); @@ -1104,7 +1107,7 @@ mod tests { x = 1 ", ); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["func"]); let scopes = table.root_child_scope_ids(); assert_eq!(scopes.len(), 1); @@ -1130,7 +1133,7 @@ mod tests { x = 1 ", ); - let table = SymbolTable::from_ast(parsed.ast()); + let table = SymbolTable::from_ast(parsed.syntax()); assert_eq!(names(table.root_symbols()), vec!["C"]); let scopes = table.root_child_scope_ids(); assert_eq!(scopes.len(), 1); @@ -1157,7 +1160,7 @@ mod tests { #[test] fn reachability_trivial() { let parsed = parse("x = 1; x"); - let ast = parsed.ast(); + let ast = parsed.syntax(); let table = SymbolTable::from_ast(ast); let x_sym = table .root_symbol_id_by_name("x") diff --git a/crates/red_knot/src/types/infer.rs b/crates/red_knot/src/types/infer.rs index 59cc771a6c..b5697ef14c 100644 --- a/crates/red_knot/src/types/infer.rs +++ b/crates/red_knot/src/types/infer.rs @@ -107,7 +107,7 @@ pub fn infer_definition_type( Ok(ty) } else { let parsed = parse(db.upcast(), file_id)?; - let ast = parsed.ast(); + let ast = parsed.syntax(); let table = symbol_table(db, file_id)?; let node = node_key.resolve_unwrap(ast.as_any_node_ref()); @@ -127,7 +127,7 @@ pub fn infer_definition_type( Ok(ty) } else { let parsed = parse(db.upcast(), file_id)?; - let ast = parsed.ast(); + let ast = parsed.syntax(); let table = symbol_table(db, file_id)?; let node = node_key .resolve(ast.as_any_node_ref()) @@ -154,14 +154,14 @@ pub fn infer_definition_type( } Definition::Assignment(node_key) => { let parsed = parse(db.upcast(), file_id)?; - let ast = parsed.ast(); + let ast = parsed.syntax(); let node = node_key.resolve_unwrap(ast.as_any_node_ref()); // TODO handle unpacking assignment correctly (here and for AnnotatedAssignment case, below) infer_expr_type(db, file_id, &node.value) } Definition::AnnotatedAssignment(node_key) => { let parsed = parse(db.upcast(), file_id)?; - let ast = parsed.ast(); + let ast = parsed.syntax(); let node = node_key.resolve_unwrap(ast.as_any_node_ref()); // TODO actually look at the annotation let Some(value) = &node.value else { @@ -213,7 +213,6 @@ fn infer_expr_type(db: &dyn SemanticDb, file_id: FileId, expr: &ast::Expr) -> Qu #[cfg(test)] mod tests { - use textwrap::dedent; use crate::db::tests::TestDb; use crate::db::{HasJar, SemanticJar}; @@ -251,7 +250,7 @@ mod tests { fn write_to_path(case: &TestCase, relpath: &str, contents: &str) -> anyhow::Result<()> { let path = case.src.path().join(relpath); - std::fs::write(path, dedent(contents))?; + std::fs::write(path, contents)?; Ok(()) } diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index a0c480cef6..d3ab7be2e9 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -64,7 +64,6 @@ //! [parsing]: https://en.wikipedia.org/wiki/Parsing //! [lexer]: crate::lexer -use std::cell::OnceCell; use std::ops::Deref; pub use crate::error::{FStringErrorType, ParseError, ParseErrorType}; @@ -308,7 +307,7 @@ impl Parsed { /// returns [`None`]. /// /// [`Some(Parsed)`]: Some - fn try_into_module(self) -> Option> { + pub fn try_into_module(self) -> Option> { match self.syntax { Mod::Module(module) => Some(Parsed { syntax: module, @@ -327,7 +326,7 @@ impl Parsed { /// Otherwise, it returns [`None`]. /// /// [`Some(Parsed)`]: Some - fn try_into_expression(self) -> Option> { + pub fn try_into_expression(self) -> Option> { match self.syntax { Mod::Module(_) => None, Mod::Expression(expression) => Some(Parsed { @@ -370,14 +369,14 @@ pub struct Tokens { raw: Vec, /// Index of the first [`TokenKind::Unknown`] token or the length of the token vector. - first_unknown_or_len: OnceCell, + first_unknown_or_len: std::sync::OnceLock, } impl Tokens { pub(crate) fn new(tokens: Vec) -> Tokens { Tokens { raw: tokens, - first_unknown_or_len: OnceCell::new(), + first_unknown_or_len: std::sync::OnceLock::new(), } }