Skip to content
New issue

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

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

Already on GitHub? # to your account

RUF027 false negative on method calls #9857

Closed
ocaballeror opened this issue Feb 6, 2024 · 3 comments · Fixed by #9865
Closed

RUF027 false negative on method calls #9857

ocaballeror opened this issue Feb 6, 2024 · 3 comments · Fixed by #9865
Assignees
Labels
bug Something isn't working

Comments

@ocaballeror
Copy link

From what I read in the original PR, the rule is meant to ignore string literals that have an immediate method call on them to avoid false positives, but from what I'm seeing, every line with a method call is completely ignored, even if the method is not invoked on the string itself. Is this intended behavior?

Sample code:

import logging


class Test:
    def method(self, arg):
        pass


name = "john"

print("hello {name}")  # correctly fails
logging.info("hello {name}")  # should fail but doesn't
Test().method("hello {name}")  # also should fail but doesn't

Invoked with:

ruff --isolated --select RUF027 --preview thing.py

Ruff version:

ruff 0.2.1
@MichaReiser MichaReiser added the question Asking for support or clarification label Feb 6, 2024
@MichaReiser
Copy link
Member

It seems intentional. This is from the documentation explaining when the rule doesn't trigger.

/// 3. The literal (or a parent expression of the literal) has a direct method call on it (for example: "{value}".format(...))

@snowsignal can you help us understand the motivation behind it?

@ocaballeror
Copy link
Author

What does a "parent expresssion of the literal" mean exactly? If it means it won't trigger on things like logging.info("{var}"), then the value of this rule decreases significantly IMO.

Shouldn't it only count if a method is called on the string itself?

@snowsignal
Copy link
Contributor

Shouldn't it only count if a method is called on the string itself?

You're right, this rule only applies to the string itself, plus any parent expressions that the string is in. The example you gave is definitely something we should catch. Luckily, I think I have a good idea of why this is happening, and I'll try to make a fix for this soon.

A parent expression of the literal would be something like func("{name}") - something like logging.info("{var}") shouldn't be excluded by this rule, because we aren't calling a method on info("{var}") - rather, it is the method that is being called.

@snowsignal snowsignal self-assigned this Feb 6, 2024
@charliermarsh charliermarsh added bug Something isn't working and removed question Asking for support or clarification labels Feb 7, 2024
snowsignal added a commit that referenced this issue Feb 8, 2024
…ethod calls (#9865)

Fixes #9857.

## Summary

Statements like `logging.info("Today it is: {day}")` will no longer be
ignored by RUF027. As before, statements like `"Today it is:
{day}".format(day="Tuesday")` will continue to be ignored.

## Test Plan

The snapshot tests were expanded to include new cases. Additionally, the
snapshot tests have been split in two to separate positive cases from
negative cases.
nkxxll pushed a commit to nkxxll/ruff that referenced this issue Mar 10, 2024
…ethod calls (astral-sh#9865)

Fixes astral-sh#9857.

## Summary

Statements like `logging.info("Today it is: {day}")` will no longer be
ignored by RUF027. As before, statements like `"Today it is:
{day}".format(day="Tuesday")` will continue to be ignored.

## Test Plan

The snapshot tests were expanded to include new cases. Additionally, the
snapshot tests have been split in two to separate positive cases from
negative cases.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants