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

FURB118 static method "false positive" #11045

Closed
Rhinopitheque opened this issue Apr 19, 2024 · 2 comments · Fixed by #11270
Closed

FURB118 static method "false positive" #11045

Rhinopitheque opened this issue Apr 19, 2024 · 2 comments · Fixed by #11270
Assignees
Labels
needs-info More information is needed from the issue author

Comments

@Rhinopitheque
Copy link

The rule FURB118 detects "false positives" on classes methods.

Keywords: "FURB118", "method"

@staticmethod 
    def get_postal_code(area_code: str) -> str:
        return area_code[:5]

Ruff v0.4.0 used with pre-commit hook:

  - repo: https://github.com/charliermarsh/ruff-pre-commit
    rev: v0.4.0
    hooks:
      - id: ruff
@charliermarsh
Copy link
Member

Can you share more on why FURB118 doesn't apply here?

@MichaReiser MichaReiser added the needs-info More information is needed from the issue author label Apr 19, 2024
@Rhinopitheque
Copy link
Author

Let's say we have this class:

class Foo:
    ...
    @staticmethod
    def get_postal_code(area_code: str) -> str:
        return area_code[:5]

If I understand the rule correctly, the suggested change would be this I suppose?

-Foo.get_postal_code("AAAAABB")
+operator.itemgetter(slice(5))("AAAAABB")

So this is not really a false positive but more than removing the staticmethod altogether makes us lose a lot of context.
Usually when people define staticmethod like that it's because the behavior is somehow linked to the class.
I feel like this rule should maybe not trigger on static method ?

I'm not so sure, feel free to close.

(Also I feel like operator.itemgetter(slice(...)) is not really readable, I'dl rather define a lambda for these cases)

Anyway, thanks for the tool and the quick response!

PS: This is a bit related to #10898

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-info More information is needed from the issue author
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants