[red-knot] add maybe-undefined lint rule (#12414)

Add a lint rule to detect if a name is definitely or possibly undefined
at a given usage.

If I create the file `undef/main.py` with contents:

```python
x = int
def foo():
    z
    return x
if flag:
    y = x
y
```

And then run `cargo run --bin red_knot -- --current-directory
../ruff-examples/undef`, I get the output:

```
Name 'z' used when not defined.
Name 'flag' used when not defined.
Name 'y' used when possibly not defined.
```

If I modify the file to add `y = 0` at the top, red-knot re-checks it
and I get the new output:

```
Name 'z' used when not defined.
Name 'flag' used when not defined.
```

Note that `int` is not flagged, since it's a builtin, and `return x` in
the function scope is not flagged, since it refers to the global `x`.

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",

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<Self>,
+        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<dyn SemanticDb> for TestDb {
+        fn upcast(&self) -> &(dyn SemanticDb + 'static) {
+            self
+        }
+    }
+
+    impl Upcast<dyn SourceDb> for TestDb {
+        fn upcast(&self) -> &(dyn SourceDb + 'static) {
+            self
+        }
+    }
+
+    impl Upcast<dyn ResolverDb> 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<Self> {
+            salsa::Snapshot::new(Self {
+                storage: self.storage.snapshot(),
+                files: self.files.snapshot(),
+                system: self.system.snapshot(),
+                vendored: self.vendored.snapshot(),
+            })
+        }
+    }
+}

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."
+            ]
+        );
+    }
+}

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 }

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 {

crates/red_knot_python_semantic/src/types.rs

@@ -239,6 +239,12 @@ pub struct UnionType<'db> {
     elements: FxOrderSet<Type<'db>>,
 }
 
+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<Type<'db>>,
     db: &'db dyn Db,

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 =

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 }
 

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