diff --git a/Cargo.lock b/Cargo.lock index 6e10fbf335..f3055c99e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1904,7 +1904,6 @@ dependencies = [ "ruff_index", "ruff_python_ast", "ruff_python_parser", - "ruff_python_trivia", "ruff_text_size", "rustc-hash 2.0.0", "salsa", @@ -2091,6 +2090,7 @@ dependencies = [ "ruff_notebook", "ruff_python_ast", "ruff_python_parser", + "ruff_python_trivia", "ruff_source_file", "ruff_text_size", "rustc-hash 2.0.0", diff --git a/crates/red_knot/src/db.rs b/crates/red_knot/src/db.rs index eb240e041f..23c1acc16a 100644 --- a/crates/red_knot/src/db.rs +++ b/crates/red_knot/src/db.rs @@ -198,3 +198,93 @@ impl salsa::ParallelDatabase for RootDatabase { }) } } + +#[cfg(test)] +pub(crate) mod tests { + use red_knot_module_resolver::{vendored_typeshed_stubs, Db as ResolverDb, Jar as ResolverJar}; + use red_knot_python_semantic::{Db as SemanticDb, Jar as SemanticJar}; + use ruff_db::files::Files; + use ruff_db::system::{DbWithTestSystem, System, TestSystem}; + use ruff_db::vendored::VendoredFileSystem; + use ruff_db::{Db as SourceDb, Jar as SourceJar, Upcast}; + + use super::{Db, Jar}; + + #[salsa::db(Jar, SemanticJar, ResolverJar, SourceJar)] + pub(crate) struct TestDb { + storage: salsa::Storage, + files: Files, + system: TestSystem, + vendored: VendoredFileSystem, + } + + impl TestDb { + pub(crate) fn new() -> Self { + Self { + storage: salsa::Storage::default(), + system: TestSystem::default(), + vendored: vendored_typeshed_stubs().snapshot(), + files: Files::default(), + } + } + } + + impl DbWithTestSystem for TestDb { + fn test_system(&self) -> &TestSystem { + &self.system + } + + fn test_system_mut(&mut self) -> &mut TestSystem { + &mut self.system + } + } + + impl SourceDb for TestDb { + fn vendored(&self) -> &VendoredFileSystem { + &self.vendored + } + + fn system(&self) -> &dyn System { + &self.system + } + + fn files(&self) -> &Files { + &self.files + } + } + + impl Upcast for TestDb { + fn upcast(&self) -> &(dyn SemanticDb + 'static) { + self + } + } + + impl Upcast for TestDb { + fn upcast(&self) -> &(dyn SourceDb + 'static) { + self + } + } + + impl Upcast for TestDb { + fn upcast(&self) -> &(dyn ResolverDb + 'static) { + self + } + } + + impl red_knot_module_resolver::Db for TestDb {} + impl red_knot_python_semantic::Db for TestDb {} + impl Db for TestDb {} + + impl salsa::Database for TestDb {} + + impl salsa::ParallelDatabase for TestDb { + fn snapshot(&self) -> salsa::Snapshot { + salsa::Snapshot::new(Self { + storage: self.storage.snapshot(), + files: self.files.snapshot(), + system: self.system.snapshot(), + vendored: self.vendored.snapshot(), + }) + } + } +} diff --git a/crates/red_knot/src/lint.rs b/crates/red_knot/src/lint.rs index e70db18d5f..32f3d8d139 100644 --- a/crates/red_knot/src/lint.rs +++ b/crates/red_knot/src/lint.rs @@ -11,7 +11,7 @@ use ruff_db::files::File; use ruff_db::parsed::{parsed_module, ParsedModule}; use ruff_db::source::{source_text, SourceText}; use ruff_python_ast as ast; -use ruff_python_ast::visitor::{walk_stmt, Visitor}; +use ruff_python_ast::visitor::{walk_expr, walk_stmt, Visitor}; use crate::db::Db; @@ -120,6 +120,25 @@ fn lint_unresolved_imports(context: &SemanticLintContext, import: AnyImportRef) } } +fn lint_maybe_undefined(context: &SemanticLintContext, name: &ast::ExprName) { + if !matches!(name.ctx, ast::ExprContext::Load) { + return; + } + let semantic = &context.semantic; + match name.ty(semantic) { + Type::Unbound => { + context.push_diagnostic(format!("Name '{}' used when not defined.", &name.id)); + } + Type::Union(union) if union.contains(semantic.db(), Type::Unbound) => { + context.push_diagnostic(format!( + "Name '{}' used when possibly not defined.", + &name.id + )); + } + _ => {} + } +} + fn lint_bad_override(context: &SemanticLintContext, class: &ast::StmtClassDef) { let semantic = &context.semantic; @@ -233,6 +252,17 @@ impl Visitor<'_> for SemanticVisitor<'_> { walk_stmt(self, stmt); } + + fn visit_expr(&mut self, expr: &ast::Expr) { + match expr { + ast::Expr::Name(name) if matches!(name.ctx, ast::ExprContext::Load) => { + lint_maybe_undefined(self.context, name); + } + _ => {} + } + + walk_expr(self, expr); + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -272,3 +302,59 @@ enum AnyImportRef<'a> { Import(&'a ast::StmtImport), ImportFrom(&'a ast::StmtImportFrom), } + +#[cfg(test)] +mod tests { + use ruff_db::files::system_path_to_file; + use ruff_db::program::{Program, SearchPathSettings, TargetVersion}; + use ruff_db::system::{DbWithTestSystem, SystemPathBuf}; + + use super::{lint_semantic, Diagnostics}; + use crate::db::tests::TestDb; + + fn setup_db() -> TestDb { + let db = TestDb::new(); + + Program::new( + &db, + TargetVersion::Py38, + SearchPathSettings { + extra_paths: Vec::new(), + workspace_root: SystemPathBuf::from("/src"), + site_packages: None, + custom_typeshed: None, + }, + ); + + db + } + + #[test] + fn undefined_variable() { + let mut db = setup_db(); + + db.write_dedented( + "/src/a.py", + " + x = int + if flag: + y = x + y + ", + ) + .unwrap(); + + let file = system_path_to_file(&db, "/src/a.py").expect("file to exist"); + let Diagnostics::List(messages) = lint_semantic(&db, file) else { + panic!("expected some diagnostics"); + }; + + assert_eq!( + *messages, + vec![ + "Name 'flag' used when not defined.", + "Name 'y' used when possibly not defined." + ] + ); + } +} diff --git a/crates/red_knot_python_semantic/Cargo.toml b/crates/red_knot_python_semantic/Cargo.toml index 7f0e13fc9a..b314905d7a 100644 --- a/crates/red_knot_python_semantic/Cargo.toml +++ b/crates/red_knot_python_semantic/Cargo.toml @@ -15,7 +15,6 @@ red_knot_module_resolver = { workspace = true } ruff_db = { workspace = true } ruff_index = { workspace = true } ruff_python_ast = { workspace = true } -ruff_python_trivia = { workspace = true } ruff_text_size = { workspace = true } bitflags = { workspace = true } diff --git a/crates/red_knot_python_semantic/src/db.rs b/crates/red_knot_python_semantic/src/db.rs index e2ca1d22cc..9704dcba19 100644 --- a/crates/red_knot_python_semantic/src/db.rs +++ b/crates/red_knot_python_semantic/src/db.rs @@ -49,7 +49,6 @@ pub(crate) mod tests { use ruff_db::system::{DbWithTestSystem, System, TestSystem}; use ruff_db::vendored::VendoredFileSystem; use ruff_db::{Db as SourceDb, Jar as SourceJar, Upcast}; - use ruff_python_trivia::textwrap; use super::{Db, Jar}; @@ -91,12 +90,6 @@ pub(crate) mod tests { pub(crate) fn clear_salsa_events(&mut self) { self.take_salsa_events(); } - - /// Write auto-dedented text to a file. - pub(crate) fn write_dedented(&mut self, path: &str, content: &str) -> anyhow::Result<()> { - self.write_file(path, textwrap::dedent(content))?; - Ok(()) - } } impl DbWithTestSystem for TestDb { diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index b78cfc3bd1..16bc2b18f5 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -239,6 +239,12 @@ pub struct UnionType<'db> { elements: FxOrderSet>, } +impl<'db> UnionType<'db> { + pub fn contains(&self, db: &'db dyn Db, ty: Type<'db>) -> bool { + self.elements(db).contains(&ty) + } +} + struct UnionTypeBuilder<'db> { elements: FxOrderSet>, db: &'db dyn Db, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 59e1c91c0c..ab3ebd106f 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -314,6 +314,7 @@ impl<'db> TypeInferenceBuilder<'db> { ast::Stmt::For(for_statement) => self.infer_for_statement(for_statement), ast::Stmt::Import(import) => self.infer_import_statement(import), ast::Stmt::ImportFrom(import) => self.infer_import_from_statement(import), + ast::Stmt::Return(ret) => self.infer_return_statement(ret), ast::Stmt::Break(_) | ast::Stmt::Continue(_) | ast::Stmt::Pass(_) => { // No-op } @@ -551,6 +552,12 @@ impl<'db> TypeInferenceBuilder<'db> { self.types.definitions.insert(definition, ty); } + fn infer_return_statement(&mut self, ret: &ast::StmtReturn) { + if let Some(value) = &ret.value { + self.infer_expression(value); + } + } + fn module_ty_from_name(&self, name: &ast::Identifier) -> Type<'db> { let module_name = ModuleName::new(&name.id); let module = diff --git a/crates/ruff_db/Cargo.toml b/crates/ruff_db/Cargo.toml index 394edaad2f..f2e3c532ac 100644 --- a/crates/ruff_db/Cargo.toml +++ b/crates/ruff_db/Cargo.toml @@ -15,6 +15,7 @@ ruff_cache = { workspace = true, optional = true } ruff_notebook = { workspace = true } ruff_python_ast = { workspace = true } ruff_python_parser = { workspace = true } +ruff_python_trivia = { workspace = true } ruff_source_file = { workspace = true } ruff_text_size = { workspace = true } diff --git a/crates/ruff_db/src/system/test.rs b/crates/ruff_db/src/system/test.rs index ca607b4ba9..e8f7383c21 100644 --- a/crates/ruff_db/src/system/test.rs +++ b/crates/ruff_db/src/system/test.rs @@ -1,4 +1,5 @@ use ruff_notebook::{Notebook, NotebookError}; +use ruff_python_trivia::textwrap; use crate::files::File; use crate::system::{DirectoryEntry, MemoryFileSystem, Metadata, Result, System, SystemPath}; @@ -150,6 +151,12 @@ pub trait DbWithTestSystem: Db + Sized { result } + /// Writes auto-dedented text to a file. + fn write_dedented(&mut self, path: &str, content: &str) -> crate::system::Result<()> { + self.write_file(path, textwrap::dedent(content))?; + Ok(()) + } + /// Writes the content of the given files and notifies the Db about the change. /// /// # Panics