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.

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

False positive A003 when overriding threading.Event.set #5956

Closed
jamesbraza opened this issue Jul 21, 2023 · 4 comments · Fixed by #6074
Closed

False positive A003 when overriding threading.Event.set #5956

jamesbraza opened this issue Jul 21, 2023 · 4 comments · Fixed by #6074
Assignees
Labels
bug Something isn't working

Comments

@jamesbraza
Copy link
Contributor

from threading import Event

class MyEvent(Event):
    def set(self) -> None:
        super().set()
        print("My override")

Running ruff==0.0.279:

> ruff --isolated --select="A003" a.py
a.py:5:9: A003 Class attribute `set` is shadowing a Python builtin

This seems like a false positive, as one has to use the word set to override the Event parent class's method.

Should A003 be raised on method overrides?

@charliermarsh charliermarsh added the bug Something isn't working label Jul 23, 2023
@charliermarsh
Copy link
Member

Probably not... but somewhat low-priority for me, since we can't resolve the superclasses anyway and it's not a hugely popular rule. The most lightweight thing I could see us doing for now is ignoring methods annotated with @overrides.

@jamesbraza
Copy link
Contributor Author

Just out of curiosity, why can't ruff resolve superclasses? I believe other linters like pylint have this capability

@charliermarsh
Copy link
Member

Ruff doesn't resolve symbols across files right now, though it will in the future (I've been working on it a bit but there's no defined timeline for it).

pylint does resolve symbols across files and does more static analysis than other linters, though not all linters function that way -- Flake8 (as an alternative example) couldn't support this.

@charliermarsh
Copy link
Member

I will special-case some stuff in the standard library for now.

charliermarsh added a commit that referenced this issue Jul 25, 2023
…6074)

## Summary

If a user subclasses `threading.Event`, e.g. with:

```python
from threading import Event


class CustomEvent(Event):
    def set(self) -> None:
        ...
```

They no control over the method name (`set`). This PR allows
`threading.Event#set` and `logging.Filter#filter` overrides, and avoids
flagging A003 in such cases. Ideally, we'd avoid flagging all overridden
methods, but... that's a lot more difficult, and this is at least
_better_ than what we do now.

Closes #6057.

Closes #5956.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants