Repository: astral-sh/ruff
File: crates/ty_python_semantic/src/semantic_index/use_def.rs (Rust)

Code before:

    pub(crate) fn all_end_of_scope_symbol_bindings<'map>(
        &'map self,
    ) -> impl Iterator<Item = (ScopedSymbolId, BindingWithConstraintsIterator<'map, 'db>)> + 'map
    {
        self.end_of_scope_symbols
            .indices()
            .map(|symbol_id| (symbol_id, self.end_of_scope_symbol_bindings(symbol_id)))
    }

    pub(crate) fn all_reachable_symbols_with_definitions<'map>(
        &'map self,
        db: &'db dyn crate::Db,
    ) -> impl Iterator<Item = (ScopedSymbolId, Type<'db>, Definition<'db>)> {
        self.reachable_definitions_by_symbol
            .iter_enumerated()
            .flat_map(|(symbol_id, reachable_definition)| {
                let declarations = self.declarations_iterator(
                    &reachable_definition.declarations,
                    BoundnessAnalysis::AssumeBound,
                );
                let declaration_place_result = place_from_declarations(db, declarations);
                let declaration = declaration_place_result.first_declaration.and_then(
                    |first_reachable_definition| {
                        let ty = declaration_place_result
                            .ignore_conflicting_declarations()
                            .place
                            .ignore_possibly_undefined()?;
                        Some((symbol_id, ty, first_reachable_definition))
                    },
                );

                let bindings = self.bindings_iterator(
                    &reachable_definition.bindings,
                    BoundnessAnalysis::AssumeBound,
                );
                let place_with_definition = place_from_bindings(db, bindings);
                let binding =
                    place_with_definition
                        .first_definition
                        .and_then(|first_reachable_definition| {
                            let ty = place_with_definition.place.ignore_possibly_undefined()?;
                            Some((symbol_id, ty, first_reachable_definition))
                        });

                [declaration, binding]
            })
            .flatten()
    }

    /// This function is intended to be called only once inside `TypeInferenceBuilder::infer_function_body`.

Review comment (suggestion):
A lot of stuff here (privileging the first definition, ignoring conflicting declarations, etc) seem like very use-case-specific policy decisions, which feels a bit out-of-place for the use-def map. Also in terms of layering, using `place_from_declarations` and `place_from_bindings` here is a bit off -- those are type-inference-level functions, should be layered above the use-def map, which is just responsible for definitions.

So while I think adding this API is good, I feel like it's doing too much. I was originally thinking (when we were chatting in Discord) that it could just return an iterator of all bindings and declarations for each symbol, but I see that that is complicated because they are already separated in `ReachableDefinitions`. It now looks to me like a return type of `(symbol_id, bindings_iterator, declarations_iterator)` might be best? That feels like it fulfills the responsibility of the use-def map -- the rest is up to the caller to decide what to do with those iterators of definitions. (It does I guess mean that chaining in some form will still be needed on the caller side.)

Code after:

    pub(crate) fn all_end_of_scope_symbol_bindings<'map>(
        &'map self,
    ) -> impl Iterator<Item = (ScopedSymbolId, BindingWithConstraintsIterator<'map, 'db>)> + 'map
    {
        self.end_of_scope_symbols
            .indices()
            .map(|symbol_id| (symbol_id, self.end_of_scope_symbol_bindings(symbol_id)))
    }

    pub(crate) fn all_reachable_symbols<'map>(
        &'map self,
    ) -> impl Iterator<
        Item = (
            ScopedSymbolId,
            DeclarationsIterator<'map, 'db>,
            BindingWithConstraintsIterator<'map, 'db>,
        ),
    > + 'map {
        self.reachable_definitions_by_symbol.iter_enumerated().map(
            |(symbol_id, reachable_definitions)| {
                let declarations = self.declarations_iterator(
                    &reachable_definitions.declarations,
                    BoundnessAnalysis::AssumeBound,
                );
                let bindings = self.bindings_iterator(
                    &reachable_definitions.bindings,
                    BoundnessAnalysis::AssumeBound,
                );
                (symbol_id, declarations, bindings)
            },
        )
    }

    /// This function is intended to be called only once inside `TypeInferenceBuilder::infer_function_body`.
    pub(crate) fn can_implicitly_return_none(&self, db: &dyn crate::Db) -> bool {
        !self
            .reachability_constraints
            .evaluate(db, &self.predicates, self.end_of_scope_reachability)
            .is_always_false()
    }

    pub(crate) fn binding_reachability(
        &self,
        db: &dyn crate::Db,
        binding: &BindingWithConstraints<'_, 'db>,
    ) -> Truthiness {
        self.reachability_constraints.evaluate(
            db,
            &self.predicates,
            binding.reachability_constraint,