Repository: astral-sh/ruff
File: crates/ruff_linter/src/checkers/ast/mod.rs (Rust)

Code before:
                value,
                range: _,
                node_index: _,
            }) => {
                self.visit_expr(value);

                self.semantic.flags |= SemanticModelFlags::NAMED_EXPRESSION_ASSIGNMENT;
                self.visit_expr(target);
            }
            _ => visitor::walk_expr(self, expr),
        }

        // Unlike most comprehension checks (which run in `analyze::expression`, after scope pop),
        // PERF102 needs `is_unused()` which requires the generator scope to still be active.
        if self.is_rule_enabled(Rule::IncorrectDictIterator) {
            let generators = match expr {
                Expr::ListComp(comp) => comp.generators.as_slice(),
                Expr::SetComp(comp) => comp.generators.as_slice(),
                Expr::Generator(comp) => comp.generators.as_slice(),
                Expr::DictComp(comp) => comp.generators.as_slice(),
                _ => &[],
            };
            for generator in generators {
                perflint::rules::incorrect_dict_iterator_comprehension(self, generator);
            }
        }

        // Step 3: Clean-up
        match expr {
            Expr::Lambda(_)
            | Expr::Generator(_)
            | Expr::ListComp(_)
            | Expr::DictComp(_)
            | Expr::SetComp(_) => {
                self.analyze.scopes.push(self.semantic.scope_id);
                self.semantic.pop_scope(); // Lambda/Generator/Comprehension scope
            }
            _ => {}
        }

        // Step 4: Analysis
        match expr {
            Expr::StringLiteral(string_literal) => {
                analyze::string_like(string_literal.into(), self);
            }
            Expr::BytesLiteral(bytes_literal) => analyze::string_like(bytes_literal.into(), self),
            Expr::FString(f_string) => analyze::string_like(f_string.into(), self),
            Expr::TString(t_string) => analyze::string_like(t_string.into(), self),
            _ => {}
        }


Review comment (refactor):
Hmm, this might be okay, but this code feels out of place here to me. I checked how we handle this for loops, and it looks like we push deferred scopes here:

https://github.com/astral-sh/ruff/blob/f14edd8661e2803254f89265548c7487f47a09f6/crates/ruff_linter/src/checkers/ast/analyze/statement.rs#L1242

and then analyze them here:

https://github.com/astral-sh/ruff/blob/f14edd8661e2803254f89265548c7487f47a09f6/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs#L8-L11

I think it might make sense to follow the same pattern for comprehensions and to push the scopes here in `analyze::comprehension`:

https://github.com/astral-sh/ruff/blob/f14edd8661e2803254f89265548c7487f47a09f6/crates/ruff_linter/src/checkers/ast/analyze/comprehension.rs#L7-L8

and then analyze them in a new `deferred_comprehension` function. It's kind of overkill for one rule, but it will give us some reusable infrastructure for the future, which feels better to me than sticking this one check in the middle of `visit_expr`. What do you think?

Code after:
            Expr::Named(ast::ExprNamed {
                target,
                value,
                range: _,
                node_index: _,
            }) => {
                self.visit_expr(value);

                self.semantic.flags |= SemanticModelFlags::NAMED_EXPRESSION_ASSIGNMENT;
                self.visit_expr(target);
            }
            _ => visitor::walk_expr(self, expr),
        }

        // Step 3: Clean-up
        match expr {
            Expr::Lambda(_)
            | Expr::Generator(_)
            | Expr::ListComp(_)
            | Expr::DictComp(_)
            | Expr::SetComp(_) => {
                self.analyze.scopes.push(self.semantic.scope_id);
                self.semantic.pop_scope(); // Lambda/Generator/Comprehension scope
            }
            _ => {}
        }

        // Step 4: Analysis
        match expr {
            Expr::StringLiteral(string_literal) => {
                analyze::string_like(string_literal.into(), self);
            }
            Expr::BytesLiteral(bytes_literal) => analyze::string_like(bytes_literal.into(), self),
            Expr::FString(f_string) => analyze::string_like(f_string.into(), self),
            Expr::TString(t_string) => analyze::string_like(t_string.into(), self),
            _ => {}
        }

        self.semantic.flags = flags_snapshot;
        analyze::expression(expr, self);
        self.semantic.pop_node();
    }

    fn visit_except_handler(&mut self, except_handler: &'a ExceptHandler) {
        let flags_snapshot = self.semantic.flags;
        self.semantic.flags |= SemanticModelFlags::EXCEPTION_HANDLER;

        // Step 1: Binding
        let binding = match except_handler {
            ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
                type_: _,