Evaluate reachability of non-definitely-bound to Ambiguous

Author: Glyphack

crates/ty_python_semantic/src/place.rs

@@ -85,6 +85,13 @@ impl<'db> Place<'db> {
         }
     }
 
+    pub(crate) fn is_definitely_bound(&self) -> bool {
+        match self {
+            Place::Type(_, Boundness::Bound) => true,
+            _ => false,
+        }
+    }
+
     #[cfg(test)]
     #[track_caller]
     pub(crate) fn expect_type(self) -> Type<'db> {

crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs

@@ -195,6 +195,7 @@
 
 use std::cmp::Ordering;
 
+use ruff_db::parsed::parsed_module;
 use ruff_index::{Idx, IndexVec};
 use rustc_hash::FxHashMap;
 
@@ -208,7 +209,8 @@ use crate::semantic_index::predicate::{
     Predicates, ScopedPredicateId,
 };
 use crate::types::{
-    IntersectionBuilder, Truthiness, Type, UnionBuilder, UnionType, infer_expression_type,
+    IntersectionBuilder, Truthiness, Type, UnionBuilder, UnionType, infer_expression,
+    infer_expression_type, infer_expression_types,
 };
 
 /// A ternary formula that defines under what conditions a binding is visible. (A ternary formula
@@ -791,7 +793,15 @@ impl ReachabilityConstraints {
     fn analyze_single(db: &dyn Db, predicate: &Predicate) -> Truthiness {
         match predicate.node {
             PredicateNode::Expression(test_expr) => {
-                let ty = infer_expression_type(db, test_expr);
+                // The cycles happen here so the check should happen before
+                let inference = infer_expression(db, test_expr);
+                if !inference.definitely_bound() {
+                    return Truthiness::Ambiguous;
+                }
+                // let ty = infer_expression_type(db, test_expr);
+                let file = test_expr.file(db);
+                let module = parsed_module(db, file).load(db);
+                let ty = inference.expression_type(test_expr.node_ref(db, &module));
                 ty.bool(db).negate_if(!predicate.is_positive)
             }
             PredicateNode::ReturnsNever(CallableAndCallExpr {

crates/ty_python_semantic/src/types.rs

@@ -24,8 +24,8 @@ pub(crate) use self::cyclic::{PairVisitor, TypeTransformer};
 pub use self::diagnostic::TypeCheckDiagnostics;
 pub(crate) use self::diagnostic::register_lints;
 pub(crate) use self::infer::{
-    infer_deferred_types, infer_definition_types, infer_expression_type, infer_expression_types,
-    infer_scope_types,
+    infer_deferred_types, infer_definition_types, infer_expression, infer_expression_type,
+    infer_expression_types, infer_scope_types,
 };
 pub(crate) use self::signatures::{CallableSignature, Signature};
 pub(crate) use self::subclass_of::{SubclassOfInner, SubclassOfType};

crates/ty_python_semantic/src/types/infer.rs

@@ -311,6 +311,28 @@ pub(crate) fn infer_expression_type<'db>(
     infer_same_file_expression_type(db, expression, &module)
 }
 
+/// TODO: Just created to verify the idea.
+#[salsa::tracked(returns(ref), cycle_fn=expression_cycle_recover, cycle_initial=expression_cycle_initial, heap_size=get_size2::GetSize::get_heap_size)]
+pub(crate) fn infer_expression<'db>(
+    db: &'db dyn Db,
+    expression: Expression<'db>,
+) -> ExpressionInference<'db> {
+    let file = expression.file(db);
+    let module = parsed_module(db, file).load(db);
+    let _span = tracing::trace_span!(
+        "infer_expression",
+        expression = ?expression.as_id(),
+        range = ?expression.node_ref(db, &module).range(),
+        ?file
+    )
+    .entered();
+
+    let index = semantic_index(db, file);
+
+    TypeInferenceBuilder::new(db, InferenceRegion::Expression(expression), index, &module)
+        .finish_expression()
+}
+
 fn single_expression_cycle_recover<'db>(
     _db: &'db dyn Db,
     _value: &Type<'db>,
@@ -640,6 +662,9 @@ struct ExpressionInferenceExtra<'db> {
     ///
     /// Falls back to `Type::Never` if an expression is missing.
     cycle_fallback: bool,
+
+    /// `true` if all expressions in this expression are definitely bound
+    all_definitely_bound: bool,
 }
 
 impl<'db> ExpressionInference<'db> {
@@ -648,6 +673,7 @@ impl<'db> ExpressionInference<'db> {
         Self {
             extra: Some(Box::new(ExpressionInferenceExtra {
                 cycle_fallback: true,
+                all_definitely_bound: true,
                 ..ExpressionInferenceExtra::default()
             })),
             expressions: FxHashMap::default(),
@@ -681,6 +707,13 @@ impl<'db> ExpressionInference<'db> {
     fn fallback_type(&self) -> Option<Type<'db>> {
         self.is_cycle_callback().then_some(Type::Never)
     }
+
+    pub(crate) fn definitely_bound(&self) -> bool {
+        match self.extra.as_ref() {
+            Some(e) => e.all_definitely_bound,
+            None => true,
+        }
+    }
 }
 
 /// Whether the intersection type is on the left or right side of the comparison.
@@ -818,6 +851,8 @@ pub(super) struct TypeInferenceBuilder<'db, 'ast> {
     ///
     /// This is used only when constructing a cycle-recovery `TypeInference`.
     cycle_fallback: bool,
+
+    all_definitely_bound: bool,
 }
 
 impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
@@ -849,6 +884,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             declarations: VecMap::default(),
             deferred: VecSet::default(),
             cycle_fallback: false,
+            all_definitely_bound: true,
         }
     }
 
@@ -6260,7 +6296,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         let db = self.db();
 
         let (resolved, constraint_keys) =
-            self.infer_place_load(PlaceExprRef::from(&expr), ast::ExprRef::Name(name_node));
+            self.record_place_load(PlaceExprRef::from(&expr), ast::ExprRef::Name(name_node));
 
         resolved
             // Not found in the module's explicitly declared global symbols?
@@ -6352,6 +6388,22 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         }
     }
 
+    /// Infer the type of a place expression from definitions, assuming a load context.
+    /// This method also returns the [`ConstraintKey`]s for each scope associated with `expr`,
+    /// which is used to narrow by condition rather than by assignment.
+    fn record_place_load(
+        &mut self,
+        place_expr: PlaceExprRef,
+        expr_ref: ast::ExprRef,
+    ) -> (PlaceAndQualifiers<'db>, Vec<(FileScopeId, ConstraintKey)>) {
+        let place = self.infer_place_load(place_expr, expr_ref);
+        if !place.0.place.is_definitely_bound() {
+            self.all_definitely_bound = false;
+        }
+
+        place
+    }
+
     /// Infer the type of a place expression from definitions, assuming a load context.
     /// This method also returns the [`ConstraintKey`]s for each scope associated with `expr`,
     /// which is used to narrow by condition rather than by assignment.
@@ -6788,7 +6840,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
 
         let mut assigned_type = None;
         if let Some(place_expr) = PlaceExpr::try_from_expr(attribute) {
-            let (resolved, keys) = self.infer_place_load(
+            let (resolved, keys) = self.record_place_load(
                 PlaceExprRef::from(&place_expr),
                 ast::ExprRef::Attribute(attribute),
             );
@@ -8338,7 +8390,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         // If `value` is a valid reference, we attempt type narrowing by assignment.
         if !value_ty.is_unknown() {
             if let Some(expr) = PlaceExpr::try_from_expr(subscript) {
-                let (place, keys) = self.infer_place_load(
+                let (place, keys) = self.record_place_load(
                     PlaceExprRef::from(&expr),
                     ast::ExprRef::Subscript(subscript),
                 );
@@ -8877,6 +8929,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             declarations,
             deferred,
             cycle_fallback,
+            all_definitely_bound: all_bound,
 
             // builder only state
             deferred_state: _,
@@ -8912,6 +8965,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                     bindings: bindings.into_boxed_slice(),
                     diagnostics,
                     cycle_fallback,
+                    all_definitely_bound: all_bound,
                 })
             });
 
@@ -8936,6 +8990,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             declarations,
             deferred,
             cycle_fallback,
+            all_definitely_bound: _,
 
             // builder only state
             deferred_state: _,
@@ -8998,6 +9053,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             deferred: _,
             bindings: _,
             declarations: _,
+            all_definitely_bound: _,
 
             // Builder only state
             deferred_state: _,
@@ -11092,6 +11148,80 @@ 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(), 651);
+    }
+
     #[test]
     fn not_literal_string() -> anyhow::Result<()> {
         let mut db = setup_db();