[ty] Include definition site for "members of" API

In the course of writing the "add an import" implementation,
I realized that we needed to know which symbols were in scope
and how they were defined. This was necessary to be able to
determine how to add a new import in a way that (minimally)
does not conflict with existing symbols.

I'm not sure that this is fully correct (especially for
symbol bindings) and it's unclear to me in which cases a
definition site will be missing. But this seems to work for
some of the basic cases that I tried.

crates/ty_python_semantic/src/semantic_model.rs

@@ -236,9 +236,9 @@ impl<'db> SemanticModel<'db> {
         for (file_scope, _) in index.ancestor_scopes(file_scope) {
             completions.extend(
                 all_declarations_and_bindings(self.db, file_scope.to_scope_id(self.db, self.file))
-                    .map(|member| Completion {
+                    .map(|memberdef| Completion {
-                        name: member.name,
+                        name: memberdef.member.name,
-                        ty: Some(member.ty),
+                        ty: Some(memberdef.member.ty),
                         kind: None,
                         builtin: false,
                     }),

crates/ty_python_semantic/src/types/ide_support.rs

@@ -30,35 +30,55 @@ use resolve_definition::{find_symbol_in_scope, resolve_definition};
 pub(crate) fn all_declarations_and_bindings<'db>(
     db: &'db dyn Db,
     scope_id: ScopeId<'db>,
-) -> impl Iterator<Item = Member<'db>> + 'db {
+) -> impl Iterator<Item = MemberWithDefinition<'db>> + 'db {
     let use_def_map = use_def_map(db, scope_id);
     let table = place_table(db, scope_id);
 
     use_def_map
         .all_end_of_scope_symbol_declarations()
         .filter_map(move |(symbol_id, declarations)| {
-            place_from_declarations(db, declarations)
+            let place_result = place_from_declarations(db, declarations);
+            let definition = place_result.single_declaration;
+            place_result
                 .ignore_conflicting_declarations()
                 .place
                 .ignore_possibly_unbound()
                 .map(|ty| {
                     let symbol = table.symbol(symbol_id);
-                    Member {
+                    let member = Member {
                         name: symbol.name().clone(),
                         ty,
-                    }
+                    };
+                    MemberWithDefinition { member, definition }
                 })
         })
         .chain(use_def_map.all_end_of_scope_symbol_bindings().filter_map(
             move |(symbol_id, bindings)| {
+                // It's not clear to AG how to using a bindings
+                // iterator here to get the correct definition for
+                // this binding. Below, we look through all bindings
+                // with a definition and only take one if there is
+                // exactly one. I don't think this can be wrong, but
+                // it's probably omitting definitions in some cases.
+                let mut definition = None;
+                for binding in bindings.clone() {
+                    if let Some(def) = binding.binding.definition() {
+                        if definition.is_some() {
+                            definition = None;
+                            break;
+                        }
+                        definition = Some(def);
+                    }
+                }
                 place_from_bindings(db, bindings)
                     .ignore_possibly_unbound()
                     .map(|ty| {
                         let symbol = table.symbol(symbol_id);
-                        Member {
+                        let member = Member {
                             name: symbol.name().clone(),
                             ty,
-                        }
+                        };
+                        MemberWithDefinition { member, definition }
                     })
             },
         ))
@@ -305,12 +325,15 @@ impl<'db> AllMembers<'db> {
             .map(|class| class.class_literal(db).0)
         {
             let parent_scope = parent.body_scope(db);
-            for Member { name, .. } in all_declarations_and_bindings(db, parent_scope) {
+            for memberdef in all_declarations_and_bindings(db, parent_scope) {
-                let result = ty.member(db, name.as_str());
+                let result = ty.member(db, memberdef.member.name.as_str());
                 let Some(ty) = result.place.ignore_possibly_unbound() else {
                     continue;
                 };
-                self.members.insert(Member { name, ty });
+                self.members.insert(Member {
+                    name: memberdef.member.name,
+                    ty,
+                });
             }
         }
     }
@@ -350,17 +373,27 @@ impl<'db> AllMembers<'db> {
             // class member. This gets us the right type for each
             // member, e.g., `SomeClass.__delattr__` is not a bound
             // method, but `instance_of_SomeClass.__delattr__` is.
-            for Member { name, .. } in all_declarations_and_bindings(db, class_body_scope) {
+            for memberdef in all_declarations_and_bindings(db, class_body_scope) {
-                let result = ty.member(db, name.as_str());
+                let result = ty.member(db, memberdef.member.name.as_str());
                 let Some(ty) = result.place.ignore_possibly_unbound() else {
                     continue;
                 };
-                self.members.insert(Member { name, ty });
+                self.members.insert(Member {
+                    name: memberdef.member.name,
+                    ty,
+                });
             }
         }
     }
 }
 
+/// A member of a type with an optional definition.
+#[derive(Clone, Debug)]
+pub struct MemberWithDefinition<'db> {
+    pub member: Member<'db>,
+    pub definition: Option<Definition<'db>>,
+}
+
 /// A member of a type.
 ///
 /// This represents a single item in (ideally) the list returned by