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

Code before:

    fn push_type(&mut self, ty: Type<'db>, seen_aliases: &mut Vec<Type<'db>>) {
        let bool_pair = if let Type::BooleanLiteral(b) = ty {
            Some(Type::BooleanLiteral(!b))
        } else {
            None
        };

        // If an alias gets here, it means we aren't unpacking aliases, and we also
        // shouldn't try to simplify aliases out of the union, because that will require
        // unpacking them.
        let should_simplify_full = !matches!(ty, Type::TypeAlias(_)) && !self.cycle_recovery;

        let mut ty_negated: Option<Type> = None;

        let mut i = 0;
        let mut inserted = false;

        let mut remove_element = |i: &mut usize, elements: &mut Vec<UnionElement<'db>>| {
            if inserted {
                elements.swap_remove(*i);
            } else {
                elements[*i] = UnionElement::Type(ty);
                *i += 1;
            }
            inserted = true;
        };

        while i < self.elements.len() {
            let element = &mut self.elements[i];

            let element_type = match element.try_reduce(self.db, ty) {
                ReduceResult::KeepIf(keep) => {
                    if !keep {
                        remove_element(&mut i, &mut self.elements);
                    } else {
                        i += 1;
                    }
                    continue;
                }
                ReduceResult::Type(ty) => ty,
                ReduceResult::CollapseToObject => {
                    self.collapse_to_object();
                    return;
                }
                ReduceResult::Ignore => {
                    return;
                }
            };

            if ty == element_type {

Review comment (bug):
Hmm, too bad. This is actually incorrect. But I'm surprised that there isn't a single failing test! 

The issue is that we now insert the element even in case where it later turns out that it's redundant? 

We also end up removing existing elements if `ty` turns out to be redundant (although, not sure when this would happen because it would mean another existing element has been redundant too?

Code after:
            }
            // Adding `object` to a union results in `object`.
            ty if ty.is_object() => {
                self.collapse_to_object();
            }
            _ => {
                self.push_type(ty, seen_aliases);
            }
        }
    }

    fn push_type(&mut self, ty: Type<'db>, seen_aliases: &mut Vec<Type<'db>>) {
        let bool_pair = if let Type::BooleanLiteral(b) = ty {
            Some(Type::BooleanLiteral(!b))
        } else {
            None
        };

        // If an alias gets here, it means we aren't unpacking aliases, and we also
        // shouldn't try to simplify aliases out of the union, because that will require
        // unpacking them.
        let should_simplify_full = !matches!(ty, Type::TypeAlias(_)) && !self.cycle_recovery;

        let mut ty_negated: Option<Type> = None;
        let mut to_remove = SmallVec::<[usize; 2]>::new();

        for (i, element) in self.elements.iter_mut().enumerate() {
            let element_type = match element.try_reduce(self.db, ty) {
                ReduceResult::KeepIf(keep) => {
                    if !keep {
                        to_remove.push(i);
                    }
                    continue;
                }
                ReduceResult::Type(ty) => ty,
                ReduceResult::CollapseToObject => {
                    self.collapse_to_object();
                    return;
                }
                ReduceResult::Ignore => {
                    return;
                }
            };

            if ty == element_type {
                return;
            }

            if Some(element_type) == bool_pair {
                self.add_in_place_impl(KnownClass::Bool.to_instance(self.db), seen_aliases);
                return;