Skip to content

Conversation

@danparizher
Copy link
Contributor

Summary

Fixes #20422

@github-actions
Copy link
Contributor

github-actions bot commented Sep 15, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I just had a minor simplification suggestion, but this looks good to me.

Comment on lines 150 to 155
if first_arg_id == parent_name.as_str() {
let scope = checker.semantic().current_scope();
if scope.has(first_arg_id) {
return;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we combine this with the other first_arg_id == parent_name.as_str() check to avoid having to check that twice?

    if !((first_arg_id == "__class__"
        || (first_arg_id == parent_name.as_str()
            && !checker.semantic().current_scope().has(first_arg_id)))
        && second_arg_id == parent_arg.name().as_str())
    {
        return;
    }

This condition is definitely getting a bit unwieldy, but I don't think it's too much worse.

An alternative approach here would be to resolve the binding of the first argument name and compare that to the binding of the parent class, but I think this check is okay because any intervening scope should also disable the rule. The current approach is a bit simpler.

@ntBre ntBre added bug Something isn't working rule Implementing or modifying a lint rule labels Sep 17, 2025
@danparizher danparizher requested a review from ntBre September 17, 2025 22:51
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ntBre ntBre enabled auto-merge (squash) September 18, 2025 14:02
@ntBre ntBre merged commit 91995aa into astral-sh:main Sep 18, 2025
34 checks passed
@danparizher danparizher deleted the fix-20422 branch September 18, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UP008 should not apply when the class variable is shadowed

2 participants