Merge remote-tracking branch 'origin/main' into dcreager/callable-return

* origin/main:
  [ty] Consistent ordering of constraint set specializations, take 2 (#21983)
  [ty] Remove invalid statement-keyword completions in for-statements (#21979)
  [ty] Avoid caching trivial is-redundant-with calls (#21989)

crates/ty_ide/src/completion.rs

@@ -490,6 +490,17 @@ pub fn completion<'db>(
             !ty.is_notimplemented(db)
         });
     }
+    if is_specifying_for_statement_iterable(&parsed, offset, typed.as_deref()) {
+        // Remove all keywords that doesn't make sense given the context,
+        // even if they are syntatically valid, e.g. `None`.
+        completions.retain(|item| {
+            let Some(kind) = item.kind else { return true };
+            if kind != CompletionKind::Keyword {
+                return true;
+            }
+            matches!(item.name.as_str(), "await" | "lambda" | "yield")
+        });
+    }
     completions.into_completions()
 }
 
@@ -1607,12 +1618,7 @@ fn is_in_definition_place(
 /// Returns true when the cursor sits on a binding statement.
 /// E.g. naming a parameter, type parameter, or `for` <name>).
 fn is_in_variable_binding(parsed: &ParsedModuleRef, offset: TextSize, typed: Option<&str>) -> bool {
-    let range = if let Some(typed) = typed {
+    let range = typed_text_range(typed, offset);
-        let start = offset.saturating_sub(typed.text_len());
-        TextRange::new(start, offset)
-    } else {
-        TextRange::empty(offset)
-    };
 
     let covering = covering_node(parsed.syntax().into(), range);
     covering.ancestors().any(|node| match node {
@@ -1667,6 +1673,36 @@ fn is_raising_exception(tokens: &[Token]) -> bool {
     false
 }
 
+/// Returns true when the cursor is after the `in` keyword in a
+/// `for x in <CURSOR>` statement.
+fn is_specifying_for_statement_iterable(
+    parsed: &ParsedModuleRef,
+    offset: TextSize,
+    typed: Option<&str>,
+) -> bool {
+    let range = typed_text_range(typed, offset);
+
+    let covering = covering_node(parsed.syntax().into(), range);
+    covering.parent().is_some_and(|node| {
+        matches!(
+           node, ast::AnyNodeRef::StmtFor(stmt_for) if stmt_for.iter.range().contains_range(range)
+        )
+    })
+}
+
+/// Returns the `TextRange` of the `typed` text.
+///
+/// `typed` should be the text immediately before the
+/// provided cursor `offset`.
+fn typed_text_range(typed: Option<&str>, offset: TextSize) -> TextRange {
+    if let Some(typed) = typed {
+        let start = offset.saturating_sub(typed.text_len());
+        TextRange::new(start, offset)
+    } else {
+        TextRange::empty(offset)
+    }
+}
+
 /// Order completions according to the following rules:
 ///
 /// 1) Names with no underscore prefix
@@ -5865,6 +5901,62 @@ def foo(param: s<CURSOR>)
         .contains("str");
     }
 
+    #[test]
+    fn no_statement_keywords_in_for_statement_simple1() {
+        completion_test_builder(
+            "\
+for x in a<CURSOR>
+",
+        )
+        .build()
+        .contains("lambda")
+        .contains("await")
+        .not_contains("raise")
+        .not_contains("False");
+    }
+
+    #[test]
+    fn no_statement_keywords_in_for_statement_simple2() {
+        completion_test_builder(
+            "\
+for x, y, _ in a<CURSOR>
+",
+        )
+        .build()
+        .contains("lambda")
+        .contains("await")
+        .not_contains("raise")
+        .not_contains("False");
+    }
+
+    #[test]
+    fn no_statement_keywords_in_for_statement_simple3() {
+        completion_test_builder(
+            "\
+for i, (x, y, z) in a<CURSOR>
+",
+        )
+        .build()
+        .contains("lambda")
+        .contains("await")
+        .not_contains("raise")
+        .not_contains("False");
+    }
+
+    #[test]
+    fn no_statement_keywords_in_for_statement_complex() {
+        completion_test_builder(
+            "\
+for i, (obj.x, (a[0], b['k']), _), *rest in a<CURSOR>
+",
+        )
+        .build()
+        .contains("lambda")
+        .contains("await")
+        .not_contains("raise")
+        .not_contains("False");
+    }
+
     #[test]
     fn favour_symbols_currently_imported() {
         let snapshot = CursorTest::builder()

crates/ty_python_semantic/src/types.rs

@@ -2028,10 +2028,23 @@ impl<'db> Type<'db> {
     /// Return `true` if it would be redundant to add `self` to a union that already contains `other`.
     ///
     /// See [`TypeRelation::Redundancy`] for more details.
-    #[salsa::tracked(cycle_initial=is_redundant_with_cycle_initial, heap_size=ruff_memory_usage::heap_size)]
     pub(crate) fn is_redundant_with(self, db: &'db dyn Db, other: Type<'db>) -> bool {
-        self.has_relation_to(db, other, InferableTypeVars::None, TypeRelation::Redundancy)
+        #[salsa::tracked(cycle_initial=is_redundant_with_cycle_initial, heap_size=ruff_memory_usage::heap_size)]
-            .is_always_satisfied(db)
+        fn is_redundant_with_impl<'db>(
+            db: &'db dyn Db,
+            self_ty: Type<'db>,
+            other: Type<'db>,
+        ) -> bool {
+            self_ty
+                .has_relation_to(db, other, InferableTypeVars::None, TypeRelation::Redundancy)
+                .is_always_satisfied(db)
+        }
+
+        if self == other {
+            return true;
+        }
+
+        is_redundant_with_impl(db, self, other)
     }
 
     fn has_relation_to(

crates/ty_python_semantic/src/types/constraints.rs

@@ -538,7 +538,7 @@ impl<'db> ConstrainedTypeVar<'db> {
         if let Type::Union(lower_union) = lower {
             let mut result = Node::AlwaysTrue;
             for lower_element in lower_union.elements(db) {
-                result = result.and(
+                result = result.and_with_offset(
                     db,
                     ConstrainedTypeVar::new_node(db, typevar, *lower_element, upper),
                 );
@@ -553,13 +553,13 @@ impl<'db> ConstrainedTypeVar<'db> {
         {
             let mut result = Node::AlwaysTrue;
             for upper_element in upper_intersection.iter_positive(db) {
-                result = result.and(
+                result = result.and_with_offset(
                     db,
                     ConstrainedTypeVar::new_node(db, typevar, lower, upper_element),
                 );
             }
             for upper_element in upper_intersection.iter_negative(db) {
-                result = result.and(
+                result = result.and_with_offset(
                     db,
                     ConstrainedTypeVar::new_node(db, typevar, lower, upper_element.negate(db)),
                 );
@@ -1148,6 +1148,11 @@ impl<'db> Node<'db> {
     fn or_with_offset(self, db: &'db dyn Db, other: Self) -> Self {
         // To ensure that `self` appears before `other` in `source_order`, we add the maximum
         // `source_order` of the lhs to all of the `source_order`s in the rhs.
+        //
+        // TODO: If we store `other_offset` as a new field on InteriorNode, we might be able to
+        // avoid all of the extra work in the calls to with_adjusted_source_order, and apply the
+        // adjustment lazily when walking a BDD tree. (ditto below in the other _with_offset
+        // methods)
         let other_offset = self.max_source_order(db);
         self.or_inner(db, other, other_offset)
     }
@@ -2074,7 +2079,13 @@ impl<'db> InteriorNode<'db> {
             //
             // We also have to check if there are any derived facts that depend on the constraint
             // we're about to remove. If so, we need to "remember" them by AND-ing them in with the
-            // corresponding branch.
+            // corresponding branch. We currently reuse the `source_order` of the constraint being
+            // removed when we add these derived facts.
+            //
+            // TODO: This might not be stable enough, if we add more than one derived fact for this
+            // constraint. If we still see inconsistent test output, we might need a more complex
+            // way of tracking source order for derived facts.
+            let self_source_order = self.source_order(db);
             let if_true = path
                 .walk_edge(
                     db,
@@ -2448,6 +2459,7 @@ impl<'db> InteriorNode<'db> {
                     // represent that intersection. We also need to add the new constraint to our
                     // seen set and (if we haven't already seen it) to the to-visit queue.
                     if seen_constraints.insert(intersection_constraint) {
+                        source_orders.insert(intersection_constraint, next_source_order);
                         to_visit.extend(
                             (seen_constraints.iter().copied())
                                 .filter(|seen| *seen != intersection_constraint)