Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mutable default argument not detected when it is a constant (by B006 and others) #11030

Open
omgMath opened this issue Apr 19, 2024 · 4 comments · May be fixed by #11122
Open

Mutable default argument not detected when it is a constant (by B006 and others) #11030

omgMath opened this issue Apr 19, 2024 · 4 comments · May be fixed by #11122
Assignees
@charliermarsh
Labels
rule Implementing or modifying a lint rule

Comments

@omgMath
Copy link

omgMath commented Apr 19, 2024

Hi
I found a case where my expectations does not meet the results reported by ruff. But first, let's start with the issue basics:

  • Keywords searched: "B006", "mutable default"
  • Ruff version: ruff 0.4.0
  • Command invoked: ruff check case.py --select ALL --ignore D,FA --isolated

Where my case.py contains the following lines:

DEFAULT_ENTRIES = [1, 2, 3]

def append_one(entries: list[int] = DEFAULT_ENTRIES) -> None:
    entries.append(1)

I would have hoped that this would be detected by rule B006 (or any other rule), but sadly it is not.

However, the rule detects the issue if instead of DEFAULT_ENTRIES we use [].
Thanks for checking!
@omgMath omgMath changed the title B006 Mutable default argument not detected when it is a constant (by B006 and others) Apr 19, 2024
@dhruvmanila
Copy link
Member

Thanks for the report. I believe this is the current limitation of many rules where they don't try to resolve the type of a variable. It will only check if it's a literal expression like [], {}, etc.

@dhruvmanila dhruvmanila added the rule Implementing or modifying a lint rule label Apr 19, 2024
@charliermarsh
Copy link
Member

I feel like we could catch this via the type annotation though?

@dhruvmanila
Copy link
Member

Yeah, it should be doable

@charliermarsh charliermarsh linked a pull request Apr 24, 2024 that will close this issue
Draft
@charliermarsh charliermarsh self-assigned this Apr 24, 2024
@gerrymanoim
Copy link

gerrymanoim commented Sep 26, 2024 •
edited
Loading

Just confirming that as of ruff 0.6.6 it still doesn't catch the constant via type annotations:

TEST = [1, 2, 3]

def test(l: list[int] = [1, 2, 3]):
    pass

def test2(l: list[int] = TEST):
    pass

Does #11122 need more work? Anything that we could help with?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@charliermarsh charliermarsh

Labels
rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Flag mutable defaults based on mutable annotations astral-sh/ruff
4 participants
@gerrymanoim @charliermarsh @omgMath @dhruvmanila