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

Code before:
            // If adding a string literal, look for an existing `UnionElement::StringLiterals` to
            // add it to, or an existing element that is a super-type of string literals, which
            // means we shouldn't add it. Otherwise, add a new `UnionElement::StringLiterals`
            // containing it.
            Type::StringLiteral(literal) => {
                let mut found = None;
                let mut to_remove = None;
                let mut ty_negated = None;
                for (index, element) in self.elements.iter_mut().enumerate() {
                    match element {
                        UnionElement::StringLiterals(literals) => {
                            if should_widen(literals.len(), self.recursively_defined) {
                                let replace_with = KnownClass::Str.to_instance(self.db);
                                self.add_in_place_impl(replace_with, seen_aliases);
                                return;
                            }
                            found = Some(literals);
                            continue;
                        }
                        UnionElement::Type(existing) => {
                            if ty.is_subtype_of(self.db, *existing) {
                                return;
                            }
                            if existing.is_subtype_of(self.db, ty) {
                                to_remove = Some(index);
                                continue;
                            }
                            let negated = ty_negated.get_or_insert_with(|| ty.negate(self.db));
                            if negated.is_subtype_of(self.db, *existing) {
                                // The type that includes both this new element, and its negation
                                // (or a supertype of its negation), must be simply `object`.
                                self.collapse_to_object();
                                return;
                            }
                        }
                        _ => {}
                    }
                }
                if let Some(found) = found {
                    found.insert(literal);
                } else {
                    self.elements
                        .push(UnionElement::StringLiterals(FxOrderSet::from_iter([
                            literal,
                        ])));
                }
                if let Some(index) = to_remove {
                    self.elements.swap_remove(index);
                }
            }
            // Same for bytes literals as for string literals, above.

Review comment (suggestion):
I had to play around with this code a little to figure out how we even get to this branch, and why it's correct here for `to_remove` to be `Option<usize>` rather than `Vec<usize>`. It might be helpful to add a comment here (and similar comments to the `if existing.is_subtype_of(db, ty)` calls in the `Type::IntLiteral()` and `Type::BytesLiteral` branches below):

```suggestion
                            // e.g. `existing` could be `Literal[""] & Any`,
                            // and `ty` could be `Literal[""]`
                            if existing.is_subtype_of(self.db, ty) {
```

Code after:
            // If adding a string literal, look for an existing `UnionElement::StringLiterals` to
            // add it to, or an existing element that is a super-type of string literals, which
            // means we shouldn't add it. Otherwise, add a new `UnionElement::StringLiterals`
            // containing it.
            Type::StringLiteral(literal) => {
                let mut found = None;
                let mut to_remove = None;
                let mut ty_negated = None;
                for (index, element) in self.elements.iter_mut().enumerate() {
                    match element {
                        UnionElement::StringLiterals(literals) => {
                            if should_widen(literals.len(), self.recursively_defined) {
                                let replace_with = KnownClass::Str.to_instance(self.db);
                                self.add_in_place_impl(replace_with, seen_aliases);
                                return;
                            }
                            found = Some(literals);
                            continue;
                        }
                        UnionElement::Type(existing) => {
                            // e.g. `existing` could be `Literal[""] & Any`,
                            // and `ty` could be `Literal[""]`
                            if ty.is_subtype_of(self.db, *existing) {
                                return;
                            }
                            if existing.is_subtype_of(self.db, ty) {
                                to_remove = Some(index);
                                continue;
                            }
                            let negated = ty_negated.get_or_insert_with(|| ty.negate(self.db));
                            if negated.is_subtype_of(self.db, *existing) {
                                // The type that includes both this new element, and its negation
                                // (or a supertype of its negation), must be simply `object`.
                                self.collapse_to_object();
                                return;
                            }
                        }
                        _ => {}
                    }
                }
                if let Some(found) = found {
                    found.insert(literal);
                } else {
                    self.elements
                        .push(UnionElement::StringLiterals(FxOrderSet::from_iter([
                            literal,
                        ])));
                }
                if let Some(index) = to_remove {
                    self.elements.swap_remove(index);
                }