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

Add autofixit for B004 #3545

Closed
Skylion007 opened this issue Mar 15, 2023 · 7 comments · Fixed by #5788
Closed

Add autofixit for B004 #3545

Skylion007 opened this issue Mar 15, 2023 · 7 comments · Fixed by #5788
Assignees
@glokta1
Labels
fixes Related to suggested fixes for violations good first issue Good for newcomers

Comments

@Skylion007
Copy link
Contributor

This check seems autofixable with minimal chance of being an incorrect fixit. I would recommend enabling autofixes for this check by using callable(x) when possible instead of hasattr(x, "__call__"). Should be a good first issue for someone to implement.
@Skylion007
Copy link
Contributor Author

Ah, I just found an edge case that makes this a bit more complex to autofix:

  
               func = getattr(self._f, '__call__', self._f.__init__)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B004

Is also flagged. I am not sure if this is a bug from flake8-bugprone or if it wants us to replace it with

               func = self._f if callable(f) else self._f.__init__

@JonathanPlasse
Copy link
Contributor

We could ignore the complex case.

@charliermarsh charliermarsh added good first issue Good for newcomers fixes Related to suggested fixes for violations labels Mar 15, 2023
@charliermarsh
Copy link
Member

Agreed, we should be able to fix these.

@glokta1
Copy link

glokta1 commented Mar 17, 2023

Hi, I'd like to take this up!

@charliermarsh
Copy link
Member

@glokta1 - Awesome! Please ask me here or on Discord if you have questions.

@JonathanPlasse
Copy link
Contributor

JonathanPlasse commented Mar 17, 2023 •
edited
Loading

Unrelated.
How can I join the discord?

@charliermarsh
Copy link
Member

There's a link in the README! Clicking this should work: https://discord.com/invite/c9MhzV8aU5

@density density mentioned this issue Jul 15, 2023
Merged
charliermarsh pushed a commit that referenced this issue Jul 16, 2023
@density
Add autofix for B004 (#5788)
f012ed2 
## Summary

Adds autofix for `hasattr` case of B004. I don't think it's safe (or
simple) to implement it for the `getattr` case because, inter alia,
calling `getattr` may have side effects.

Fixes #3545

## Test Plan

Existing tests were sufficient. Updated snapshots
evanrittenhouse pushed a commit to evanrittenhouse/ruff that referenced this issue Jul 19, 2023
@density @evanrittenhouse
Add autofix for B004 (astral-sh#5788)
94b8d00 
## Summary

Adds autofix for `hasattr` case of B004. I don't think it's safe (or
simple) to implement it for the `getattr` case because, inter alia,
calling `getattr` may have side effects.

Fixes astral-sh#3545

## Test Plan

Existing tests were sufficient. Updated snapshots
konstin pushed a commit that referenced this issue Jul 19, 2023
@density @konstin
Add autofix for B004 (#5788)
4b94364 
## Summary

Adds autofix for `hasattr` case of B004. I don't think it's safe (or
simple) to implement it for the `getattr` case because, inter alia,
calling `getattr` may have side effects.

Fixes #3545

## Test Plan

Existing tests were sufficient. Updated snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@glokta1 glokta1

Labels
fixes Related to suggested fixes for violations good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add autofix for B004 density/ruff
4 participants
@charliermarsh @Skylion007 @JonathanPlasse @glokta1