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

[RUF027] missing-f-string-syntax has false negatives if calling a function inside brackets or accessing attribute of interpolated variable #15227

Open
sjdemartini opened this issue Jan 2, 2025 · 2 comments · May be fixed by #15247
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule

Comments

@sjdemartini
Copy link
Contributor

sjdemartini commented Jan 2, 2025

With RUF027 enabled, I would expect it to flag both of these, which it currently does not:

  1. function calls within the bracketed expression, like print("Hello {foo()}")
  2. bracketed expressions that access an attribute of an in-scope variable, like print("Hello {foo.bar}")

Neither of these situations seems to be explicitly part of the listed exceptions to the rule, and don't seem like they should be.

Example:

def fibonacci(n):
    """Compute the nth number in the Fibonacci sequence."""
    if n == 0:
        return 0
    elif n == 1:
        return 1
    else:
        return fibonacci(n - 1) + fibonacci(n - 2)

class Foo:
    BAR = 5

name = "Sarah"
day_of_week = "Tuesday"

print("Hello {name}! It is {day_of_week} today!")  # Correctly flagged by RUF027
print("Hello {fibonacci}") # Correctly flagged by RUF027

print("Hello {fibonacci(1)}") # Should be flagged by RUF027 but is not
print("Hello {Foo.BAR}") # Should be flagged by RUF027 but is not

Playground: https://play.ruff.rs/ff81d90f-a1d6-4842-8b1b-b31ede9edbaf

@dhruvmanila
Copy link
Member

Yeah, this seems like something that we should catch. It could be that we're deliberately ignoring expressions that are not pure variables although I'll have to check the source code to confirm. Thanks for the report.

@dhruvmanila dhruvmanila added rule Implementing or modifying a lint rule preview Related to preview mode features labels Jan 3, 2025
@MichaReiser
Copy link
Member

The rule only flags f-strings referencing a local variable to reduce false positives. This can be found here:

if let ast::Expr::Name(ast::ExprName { id, .. }) = element.expression.as_ref() {
if arg_names.contains(id) {
return false;
}
if semantic
// the parsed expression nodes have incorrect ranges
// so we need to use the range of the literal for the
// lookup in order to get reasonable results.
.simulate_runtime_load_at_location_in_scope(
id,
literal.range(),
semantic.scope_id,
)
.map_or(true, |id| semantic.binding(id).kind.is_builtin())
{
return false;
}
has_name = true;
}

We could extend the rule to support more expressions, e.g. attribute access or call expressions. The ideal solution is probably to extract the left-most name and try to lookup that name

@InSyncWithFoo InSyncWithFoo linked a pull request Jan 4, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants