[ty] IDE: only provide declarations and bindings as completions (#18456)

## Summary

Previously, all symbols where provided as possible completions. In an
example like the following, both `foo` and `f` were suggested as
completions, because `f` itself is a symbol.
```py
foo = 1

f<CURSOR>
```
Similarly, in the following example, `hidden_symbol` was suggested, even
though it is not statically visible:
```py
if 1 + 2 != 3:
    hidden_symbol = 1

hidden_<CURSOR>
```

With the change suggested here, we only use statically visible
declarations and bindings as a source for completions.


## Test Plan

- Updated snapshot tests
- New test for statically hidden definitions
- Added test for star import

crates/ty_ide/src/completion.rs

@@ -127,10 +127,7 @@ f<CURSOR>
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        f
-        foo
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -143,10 +140,7 @@ g<CURSOR>
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        foo
-        g
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -175,10 +169,7 @@ f<CURSOR>
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        f
-        foo
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -208,7 +199,6 @@ def foo():
         );
 
         assert_snapshot!(test.completions(), @r"
-        f
         foo
         foofoo
         ");
@@ -259,7 +249,6 @@ def foo():
         );
 
         assert_snapshot!(test.completions(), @r"
-        f
         foo
         foofoo
         ");
@@ -276,7 +265,6 @@ def foo():
         );
 
         assert_snapshot!(test.completions(), @r"
-        f
         foo
         foofoo
         ");
@@ -295,7 +283,6 @@ def frob(): ...
         );
 
         assert_snapshot!(test.completions(), @r"
-        f
         foo
         foofoo
         frob
@@ -315,7 +302,6 @@ def frob(): ...
         );
 
         assert_snapshot!(test.completions(), @r"
-        f
         foo
         frob
         ");
@@ -334,7 +320,6 @@ def frob(): ...
         );
 
         assert_snapshot!(test.completions(), @r"
-        f
         foo
         foofoo
         foofoofoo
@@ -451,15 +436,10 @@ def frob(): ...
 ",
         );
 
-        // It's not totally clear why `for` shows up in the
-        // symbol tables of the detected scopes here. My guess
-        // is that there's perhaps some sub-optimal behavior
-        // here because the list comprehension as written is not
-        // valid.
-        assert_snapshot!(test.completions(), @r"
-        bar
-        for
-        ");
+        // TODO: it would be good if `bar` was included here, but
+        // the list comprehension is not yet valid and so we do not
+        // detect this as a definition of `bar`.
+        assert_snapshot!(test.completions(), @"<No completions found>");
     }
 
     #[test]
@@ -470,10 +450,7 @@ def frob(): ...
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        f
-        foo
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -484,10 +461,7 @@ def frob(): ...
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        f
-        foo
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -498,10 +472,7 @@ def frob(): ...
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        f
-        foo
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -512,10 +483,7 @@ def frob(): ...
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        f
-        foo
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -526,10 +494,7 @@ def frob(): ...
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        f
-        foo
-        ");
+        assert_snapshot!(test.completions(), @"foo");
     }
 
     #[test]
@@ -602,7 +567,6 @@ class Foo:
 
         assert_snapshot!(test.completions(), @r"
         Foo
-        b
         bar
         frob
         quux
@@ -621,7 +585,6 @@ class Foo:
 
         assert_snapshot!(test.completions(), @r"
         Foo
-        b
         bar
         quux
         ");
@@ -750,7 +713,6 @@ bar(o<CURSOR>
         assert_snapshot!(test.completions(), @r"
         bar
         foo
-        o
         ");
     }
 
@@ -788,7 +750,6 @@ class C:
         assert_snapshot!(test.completions(), @r"
         C
         bar
-        f
         foo
         self
         ");
@@ -825,7 +786,6 @@ class C:
         assert_snapshot!(test.completions(), @r"
         C
         bar
-        f
         foo
         self
         ");
@@ -854,11 +814,55 @@ print(f\"{some<CURSOR>
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        print
-        some
-        some_symbol
-        ");
+        assert_snapshot!(test.completions(), @"some_symbol");
+    }
+
+    #[test]
+    fn statically_invisible_symbols() {
+        let test = cursor_test(
+            "\
+if 1 + 2 != 3:
+    hidden_symbol = 1
+
+hidden_<CURSOR>
+",
+        );
+
+        assert_snapshot!(test.completions(), @"<No completions found>");
+    }
+
+    #[test]
+    fn completions_inside_unreachable_sections() {
+        let test = cursor_test(
+            "\
+import sys
+
+if sys.platform == \"not-my-current-platform\":
+    only_available_in_this_branch = 1
+
+    on<CURSOR>
+",
+        );
+
+        // TODO: ideally, `only_available_in_this_branch` should be available here, but we
+        // currently make no effort to provide a good IDE experience within sections that
+        // are unreachable
+        assert_snapshot!(test.completions(), @"sys");
+    }
+
+    #[test]
+    fn star_import() {
+        let test = cursor_test(
+            "\
+from typing import *
+
+Re<CURSOR>
+",
+        );
+
+        test.assert_completions_include("Reversible");
+        // `ReadableBuffer` is a symbol in `typing`, but it is not re-exported
+        test.assert_completions_do_not_include("ReadableBuffer");
     }
 
     // Ref: https://github.com/astral-sh/ty/issues/572
@@ -921,10 +925,7 @@ Fo<CURSOR> = float
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        Fo
-        float
-        ");
+        assert_snapshot!(test.completions(), @"Fo");
     }
 
     // Ref: https://github.com/astral-sh/ty/issues/572
@@ -999,9 +1000,7 @@ except Type<CURSOR>:
 ",
         );
 
-        assert_snapshot!(test.completions(), @r"
-        Type
-        ");
+        assert_snapshot!(test.completions(), @"<No completions found>");
     }
 
     // Ref: https://github.com/astral-sh/ty/issues/572
@@ -1029,5 +1028,29 @@ def _():
                 .collect::<Vec<String>>()
                 .join("\n")
         }
+
+        #[track_caller]
+        fn assert_completions_include(&self, expected: &str) {
+            let completions = completion(&self.db, self.file, self.cursor_offset);
+
+            assert!(
+                completions
+                    .iter()
+                    .any(|completion| completion.label == expected),
+                "Expected completions to include `{expected}`"
+            );
+        }
+
+        #[track_caller]
+        fn assert_completions_do_not_include(&self, unexpected: &str) {
+            let completions = completion(&self.db, self.file, self.cursor_offset);
+
+            assert!(
+                completions
+                    .iter()
+                    .all(|completion| completion.label != unexpected),
+                "Expected completions to not include `{unexpected}`",
+            );
+        }
     }
 }

crates/ty_python_semantic/src/semantic_model.rs

@@ -10,6 +10,7 @@ use crate::module_resolver::{Module, resolve_module};
 use crate::semantic_index::ast_ids::HasScopedExpressionId;
 use crate::semantic_index::semantic_index;
 use crate::semantic_index::symbol::FileScopeId;
+use crate::types::ide_support::all_declarations_and_bindings;
 use crate::types::{Type, binding_type, infer_scope_types};
 
 pub struct SemanticModel<'db> {
@@ -66,9 +67,10 @@ impl<'db> SemanticModel<'db> {
         };
         let mut symbols = vec![];
         for (file_scope, _) in index.ancestor_scopes(file_scope) {
-            for symbol in index.symbol_table(file_scope).symbols() {
-                symbols.push(symbol.name().clone());
-            }
+            symbols.extend(all_declarations_and_bindings(
+                self.db,
+                file_scope.to_scope_id(self.db, self.file),
+            ));
         }
         symbols
     }

crates/ty_python_semantic/src/types.rs

@@ -65,7 +65,7 @@ mod diagnostic;
 mod display;
 mod function;
 mod generics;
-mod ide_support;
+pub(crate) mod ide_support;
 mod infer;
 mod instance;
 mod mro;

crates/ty_python_semantic/src/types/ide_support.rs

@@ -8,6 +8,37 @@ use crate::types::{ClassBase, ClassLiteral, KnownClass, Type};
 use ruff_python_ast::name::Name;
 use rustc_hash::FxHashSet;
 
+pub(crate) fn all_declarations_and_bindings<'db>(
+    db: &'db dyn Db,
+    scope_id: ScopeId<'db>,
+) -> impl Iterator<Item = Name> + 'db {
+    let use_def_map = use_def_map(db, scope_id);
+    let symbol_table = symbol_table(db, scope_id);
+
+    use_def_map
+        .all_public_declarations()
+        .filter_map(move |(symbol_id, declarations)| {
+            if symbol_from_declarations(db, declarations)
+                .is_ok_and(|result| !result.symbol.is_unbound())
+            {
+                Some(symbol_table.symbol(symbol_id).name().clone())
+            } else {
+                None
+            }
+        })
+        .chain(
+            use_def_map
+                .all_public_bindings()
+                .filter_map(move |(symbol_id, bindings)| {
+                    if symbol_from_bindings(db, bindings).is_unbound() {
+                        None
+                    } else {
+                        Some(symbol_table.symbol(symbol_id).name().clone())
+                    }
+                }),
+        )
+}
+
 struct AllMembers {
     members: FxHashSet<Name>,
 }
@@ -118,24 +149,8 @@ impl AllMembers {
     }
 
     fn extend_with_declarations_and_bindings(&mut self, db: &dyn Db, scope_id: ScopeId) {
-        let use_def_map = use_def_map(db, scope_id);
-        let symbol_table = symbol_table(db, scope_id);
-
-        for (symbol_id, declarations) in use_def_map.all_public_declarations() {
-            if symbol_from_declarations(db, declarations)
-                .is_ok_and(|result| !result.symbol.is_unbound())
-            {
         self.members
-                    .insert(symbol_table.symbol(symbol_id).name().clone());
-            }
-        }
-
-        for (symbol_id, bindings) in use_def_map.all_public_bindings() {
-            if !symbol_from_bindings(db, bindings).is_unbound() {
-                self.members
-                    .insert(symbol_table.symbol(symbol_id).name().clone());
-            }
-        }
+            .extend(all_declarations_and_bindings(db, scope_id));
     }
 
     fn extend_with_class_members<'db>(