Use record_place_load

Author: Glyphack

crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs

@@ -195,7 +195,9 @@
 
 use std::cmp::Ordering;
 
+use ruff_db::parsed::parsed_module;
 use ruff_index::{Idx, IndexVec};
+use ruff_python_ast::HasNodeIndex;
 use rustc_hash::FxHashMap;
 
 use crate::Db;
@@ -792,8 +794,14 @@ impl ReachabilityConstraints {
     fn analyze_single(db: &dyn Db, predicate: &Predicate) -> Truthiness {
         match predicate.node {
             PredicateNode::Expression(test_expr) => {
+                let expression = &test_expr;
+                let file = expression.file(db);
+                let module = parsed_module(db, file).load(db);
+                let node = expression.node_ref(db, &module);
                 let ty = infer_expression_if_definitely_bound(db, test_expr);
-                ty.bool(db).negate_if(!predicate.is_positive)
+                let t = ty.bool(db).negate_if(!predicate.is_positive);
+
+                t
             }
             PredicateNode::ReturnsNever(CallableAndCallExpr {
                 callable,

crates/ty_python_semantic/src/types/infer.rs

@@ -344,14 +344,13 @@ pub(crate) fn infer_expression_if_definitely_bound<'db>(
     db: &'db dyn Db,
     expression: Expression<'db>,
 ) -> Type<'db> {
+    let inference = infer_expression_types(db, expression);
     let file = expression.file(db);
     let module = parsed_module(db, file).load(db);
-    eprintln!("inferring types");
-    let inference = infer_expression_types(db, expression);
-    eprintln!("type inferred");
     let node = expression.node_ref(db, &module);
     let b = inference.definitely_bound();
-    eprintln!("{node:?} is definitely bound: {b}");
+
+    eprintln!("node {:?} is fully bound: {}", node, b);
 
     if b {
         inference.expression_type(node)
@@ -684,7 +683,7 @@ impl<'db> ExpressionInference<'db> {
         Self {
             extra: Some(Box::new(ExpressionInferenceExtra {
                 cycle_fallback: true,
-                all_definitely_bound: false,
+                all_definitely_bound: true,
                 ..ExpressionInferenceExtra::default()
             })),
             expressions: FxHashMap::default(),
@@ -720,7 +719,6 @@ impl<'db> ExpressionInference<'db> {
     }
 
     pub(crate) fn definitely_bound(&self) -> bool {
-        // If there was a cycle fallback or there is no extra field then returns false.
         match self.extra.as_ref() {
             Some(e) => e.all_definitely_bound,
             None => true,
@@ -6400,6 +6398,31 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         }
     }
 
+    // TODO: Not needed
+    fn is_in_method(&self) -> bool {
+        let current_scope_id = self.scope().file_scope_id(self.db());
+        let current_scope = self.index.scope(current_scope_id);
+        let module = &parsed_module(self.db(), self.scope().file(self.db())).load(self.db());
+        let Some(method) = current_scope.node().as_function(module) else {
+            return false;
+        };
+
+        let Some(parent_scope_id) = current_scope.parent() else {
+            return false;
+        };
+        let parent_scope = self.index.scope(parent_scope_id);
+        let Some(_) = parent_scope.node().as_class(module) else {
+            return false;
+        };
+
+        let definition = self.index.expect_single_definition(method);
+        if let DefinitionKind::Function(_) = definition.kind(self.db()) {
+            return true;
+        } else {
+            return false;
+        }
+    }
+
     /// NOTE: first way, if any of places are not definitely bound then set the value
     /// Calls infer_place_load and records if the expression is definitely bound.
     fn record_place_load(
@@ -6410,6 +6433,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         let place = self.infer_place_load(place_expr, expr_ref);
         if !place.0.place.is_definitely_bound() {
             self.all_definitely_bound = false;
+            eprintln!("expr_ref: {:?} was not bound", expr_ref);
         }
 
         place
@@ -8932,20 +8956,20 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
     pub(super) fn finish_expression(mut self) -> ExpressionInference<'db> {
         self.infer_region();
 
-        let all_definitely_bound = match self.region {
-            InferenceRegion::Expression(expression) => {
-                let mut visitor = BoundSymbolsVisitor {
-                    builder: &self,
-                    all_definitely_bound: false,
-                };
-                let node = expression.node_ref(self.db(), self.module());
-                visitor.visit_expr(node);
-                let b = visitor.all_definitely_bound;
-                eprintln!("{node:?}\n is bound: {b}");
-                b
-            }
-            _ => false,
-        };
+        // TODO: Remove because record_place_load is used instead
+        // let all_definitely_bound = match self.region {
+        //     InferenceRegion::Expression(expression) => {
+        //         let mut visitor = BoundSymbolsVisitor {
+        //             builder: &self,
+        //             all_definitely_bound: true,
+        //         };
+        //         let node = expression.node_ref(self.db(), self.module());
+        //         visitor.visit_expr(node);
+        //         let b = visitor.all_definitely_bound;
+        //         b
+        //     }
+        //     _ => false,
+        // };
 
         let Self {
             context,
@@ -8955,9 +8979,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             declarations,
             deferred,
             cycle_fallback,
-
-            // ignored
-            all_definitely_bound: _,
+            all_definitely_bound,
 
             // builder only state
             deferred_state: _,
@@ -8980,7 +9002,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         );
 
         let extra =
-            (cycle_fallback || !bindings.is_empty() || !diagnostics.is_empty()).then(|| {
+            (cycle_fallback || !bindings.is_empty() || !diagnostics.is_empty() || !all_definitely_bound).then(|| {
                 if bindings.len() > 20 {
                     tracing::debug!(
                         "Inferred expression region `{:?}` contains {} bindings. Lookups by linear scan might be slow.",
@@ -8999,6 +9021,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
 
         expressions.shrink_to_fit();
 
+        // eprintln!(
+        //     "finished inference id {:?} and definitely bound {:?}",
+        //     scope, all_definitely_bound
+        // );
+
         ExpressionInference {
             expressions,
             extra,
@@ -10797,18 +10824,18 @@ impl<'a, 'db, 'ast> Visitor<'ast> for BoundSymbolsVisitor<'a, 'db, 'ast> {
         }
 
         match expr {
-            ast::Expr::Attribute(attr) if attr.ctx == ast::ExprContext::Load => {}
+            ast::Expr::Attribute(attr) if attr.ctx == ast::ExprContext::Load => {
+                if attr.attr.id().as_str() == "version_info" {
+                    return walk_expr(self, expr);
+                }
+            }
             ast::Expr::Name(name) if name.ctx == ast::ExprContext::Load => {}
             _ => return walk_expr(self, expr),
         };
 
         let place_expr = match PlaceExpr::try_from_expr(expr) {
             Some(place_expr) => place_expr,
             None => {
-                debug_assert!(
-                    false,
-                    "ast::Expr::Name, ast::Expr::Attribute should be a valid PlaceExpr"
-                );
                 return walk_expr(self, expr);
             }
         };
@@ -10820,6 +10847,8 @@ impl<'a, 'db, 'ast> Visitor<'ast> for BoundSymbolsVisitor<'a, 'db, 'ast> {
             .place;
 
         if !matches!(place, Place::Type(_, Boundness::Bound)) {
+            dbg!(place);
+            eprintln!("{expr:?} was not bound");
             self.all_definitely_bound = false;
             return;
         }
@@ -11222,80 +11251,6 @@ mod tests {
         assert_diagnostic_messages(&diagnostics, expected);
     }
 
-    #[test]
-    fn analyze_cycles_gridout() {
-        let mut db = setup_db();
-        let filename = "src/gridout.py";
-        db.write_dedented(
-            filename,
-            r#"
-            EMPTY = b""
-
-
-            class GridOut:
-                def __init__(self: "GridOut") -> None:
-                    self._buffer_pos = 0
-                    self._buffer = b""
-
-                def readchunk(self: "GridOut") -> bytes:
-                    if not len(self._buffer) - self._buffer_pos:
-                        raise Exception("truncated chunk")
-                    self._buffer_pos = 0
-                    return EMPTY
-
-                def _read_size_or_line(self: "GridOut", size: int = -1) -> bytes:
-                    if size > self._position:
-                        size = self._position
-                    if size == 0:
-                        return bytes()
-
-                    received = 0
-                    needed = size - received
-                    while received < size:
-                        if self._buffer:
-                            buf = self._buffer
-                            chunk_start = self._buffer_pos
-                            chunk_data = buf[self._buffer_pos :]
-                            self._buffer = EMPTY
-                        else:
-                            buf = self.readchunk()
-                            chunk_start = 0
-                            chunk_data = buf
-
-                        needed = buf.find(EMPTY, chunk_start, chunk_start + needed)
-                        if len(chunk_data) > needed:
-                            self._buffer = buf
-                            self._buffer_pos = chunk_start + needed
-                            self._position -= len(self._buffer) - self._buffer_pos
-
-                    return b""
-            "#,
-        )
-        .unwrap();
-
-        db.clear_salsa_events();
-        assert_file_diagnostics(&db, filename, &[]);
-        let events = db.take_salsa_events();
-        let cycles = salsa::attach(&db, || {
-            events
-                .iter()
-                .filter_map(|event| {
-                    if let salsa::EventKind::WillIterateCycle {
-                        database_key,
-                        iteration_count,
-                        fell_back: _,
-                    } = event.kind
-                    {
-                        Some(format!("{database_key:?}, {iteration_count:?}"))
-                    } else {
-                        None
-                    }
-                })
-                .collect::<Vec<_>>()
-        });
-        assert_eq!(cycles.len(), 2414);
-    }
-
     #[test]
     fn cyclic_dependant_attributes() {
         let mut db = setup_db();
@@ -11333,7 +11288,7 @@ mod tests {
                 })
                 .collect::<Vec<_>>()
         });
-        assert_eq!(cycles.len(), 0);
+        assert_eq!(cycles.len(), 1);
     }
 
     #[test]