Implement RUF027: `Missing F-String Syntax` lint by snowsignal · Pull Request #9728 · astral-sh/ruff

snowsignal · 2024-01-31T03:24:54Z

Summary

This PR implements a new rule, RUF027.

What it does

Checks for strings that contain f-string syntax but are not f-strings.

Why is this bad?

An f-string missing an f at the beginning won't format anything, and instead treat the interpolation syntax as literal.

Example

name = "Sarah"
dayofweek = "Tuesday"
msg = "Hello {name}! It is {dayofweek} today!"

It should instead be:

name = "Sarah"
dayofweek = "Tuesday"
msg = f"Hello {name}! It is {dayofweek} today!"

Heuristics

Since there are many possible string literals which contain syntax similar to f-strings yet are not intended to be,
this lint will disqualify any literal that satisfies any of the following conditions:

The string literal is a standalone expression. For example, a docstring.
The literal is part of a function call with keyword arguments that match at least one variable (for example: format("Message: {value}", value = "Hello World"))
The literal (or a parent expression of the literal) has a direct method call on it (for example: "{value}".format(...))
The string has no {...} expression sections, or uses invalid f-string syntax.
The string references variables that are not in scope, or it doesn't capture variables at all.
Any format specifiers in the potential f-string are invalid.

Test Plan

I created a new test file, RUF027.py, which is both an example of what the lint should catch and a way to test edge cases that may trigger false positives.

crates/ruff_linter/src/rules/ruff/rules/helpers.rs

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs

charliermarsh

Excellent.

crates/ruff_linter/src/rules/ruff/rules/helpers.rs

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs

crates/ruff_linter/resources/test/fixtures/ruff/RUF027.py

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs

MichaReiser

This is looking good. I mainly left a few nit comments and some suggestions for more tests (I fear, some of them will be painful)

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

crates/ruff_linter/src/rules/ruff/rules/helpers.rs

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs

.../ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027.py.snap

crates/ruff_linter/src/rules/ruff/rules/helpers.rs

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

crates/ruff_linter/src/rules/ruff/rules/helpers.rs

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs

github-actions · 2024-01-31T23:20:56Z

`ruff-ecosystem` results

Linter (stable)

ℹ️ ecosystem check encountered linter errors. (no lint changes; 1 project error)

sphinx-doc/sphinx (error)

ruff failed
  Cause: Selection of unstable rules without the `--preview` flag is not allowed. Enable preview or remove selection of:
	- FURB113
	- FURB131
	- FURB132

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+91 -0 violations, +0 -0 fixes in 7 projects; 36 projects unchanged)

apache/airflow (+4 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ airflow/models/dag.py:2155:30: RUF027 Possible f-string without an `f` prefix
+ airflow/providers/amazon/aws/hooks/s3.py:746:21: RUF027 Possible f-string without an `f` prefix
+ tests/providers/docker/operators/test_docker.py:566:28: RUF027 Possible f-string without an `f` prefix
+ tests/test_utils/asserts.py:94:16: RUF027 Possible f-string without an `f` prefix

bokeh/bokeh (+9 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ examples/plotting/customjs_expr.py:26:49: RUF027 Possible f-string without an `f` prefix
+ examples/styling/mathtext/latex_normal_distribution.py:47:13: RUF027 Possible f-string without an `f` prefix
+ examples/styling/mathtext/latex_normal_distribution.py:48:13: RUF027 Possible f-string without an `f` prefix
+ examples/styling/mathtext/latex_normal_distribution.py:49:13: RUF027 Possible f-string without an `f` prefix
+ examples/styling/mathtext/latex_normal_distribution.py:50:13: RUF027 Possible f-string without an `f` prefix
+ examples/styling/mathtext/latex_normal_distribution.py:51:13: RUF027 Possible f-string without an `f` prefix
+ examples/styling/mathtext/latex_normal_distribution.py:52:13: RUF027 Possible f-string without an `f` prefix
+ examples/styling/mathtext/latex_normal_distribution.py:53:13: RUF027 Possible f-string without an `f` prefix
+ src/bokeh/models/annotations/legends.py:581:28: RUF027 Possible f-string without an `f` prefix

ibis-project/ibis (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ ibis/backends/exasol/__init__.py:70:13: RUF027 Possible f-string without an `f` prefix
+ ibis/backends/snowflake/__init__.py:338:9: RUF027 Possible f-string without an `f` prefix

pandas-dev/pandas (+5 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ pandas/io/formats/format.py:1266:27: RUF027 Possible f-string without an `f` prefix
+ pandas/io/formats/format.py:1268:27: RUF027 Possible f-string without an `f` prefix
+ pandas/tests/dtypes/test_dtypes.py:1063:13: RUF027 Possible f-string without an `f` prefix
+ pandas/tests/indexes/base_class/test_formats.py:138:35: RUF027 Possible f-string without an `f` prefix
+ pandas/tests/indexes/base_class/test_formats.py:141:16: RUF027 Possible f-string without an `f` prefix

rotki/rotki (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ rotkehlchen/tests/utils/blockchain.py:410:38: RUF027 Possible f-string without an `f` prefix

scikit-build/scikit-build-core (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ src/scikit_build_core/builder/wheel_tag.py:84:27: RUF027 Possible f-string without an `f` prefix

zulip/zulip (+69 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ tools/droplets/create.py:321:21: RUF027 Possible f-string without an `f` prefix
+ zerver/actions/streams.py:1575:13: RUF027 Possible f-string without an `f` prefix
+ zerver/actions/streams.py:1576:13: RUF027 Possible f-string without an `f` prefix
+ zerver/actions/streams.py:1577:13: RUF027 Possible f-string without an `f` prefix
+ zerver/lib/redis_utils.py:44:40: RUF027 Possible f-string without an `f` prefix
+ zerver/lib/typed_endpoint.py:366:32: RUF027 Possible f-string without an `f` prefix
+ zerver/management/commands/edit_linkifiers.py:14:12: RUF027 Possible f-string without an `f` prefix
+ zerver/migrations/0423_fix_email_gateway_attachment_owner.py:93:21: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/curl_param_value_generators.py:281:9: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1031:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1046:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1055:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1096:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1133:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1157:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1307:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1398:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1409:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:1487:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:303:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:315:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:321:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:333:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:343:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:355:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:362:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:379:17: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:454:25: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:469:25: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:477:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:572:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:588:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:627:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:796:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:809:17: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:934:45: RUF027 Possible f-string without an `f` prefix
+ zerver/openapi/python_examples.py:959:45: RUF027 Possible f-string without an `f` prefix
... 32 additional changes omitted for project

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
RUF027	91	91	0	0	0

codspeed-hq · 2024-02-01T23:54:54Z

CodSpeed Performance Report

Merging #9728 will not alter performance

_{Comparing jane/linter/rule/fstring (3e5953e) with main (25d9305)}

Summary

✅ 30 untouched benchmarks

zanieb · 2024-02-02T21:13:40Z

Really weird false positive

https://github.com/pandas-dev/pandas/blob/c3f7fee653fef5598055227821b6974f1eb87968/pandas/core/window/doc.py#L96-L115

They're basically doing "{foo}".replace("{foo}", foo) which such an anti-pattern that I'm not sure I mind?

zanieb · 2024-02-02T21:15:21Z

Here's another false positive

https://github.com/DisnakeDev/disnake/blob/a24abf43297215b6fe70e0bc61b2ec1dada84b9f/disnake/client.py#L2446

They're doing data = {"foo": 1, "bar": 2}; "{foo} {bar}".format_map(data) which seems like a true false positive. Perhaps we should ignore strings with a .<call> attached

zanieb · 2024-02-02T21:16:37Z

This false positive is no good

https://github.com/apache/airflow/blob/3ec781946a7fcb5fa5bc99449d59d5981e6257ab/airflow/www/utils.py#L437-L446

e.g. "{foo}".format(foo=1) should definitely not be flagged

The vast majority of the false positives in the ecosystem are this.

I think we should also not flag " = "{foo}"; x.format(foo=1) if feasible.

zanieb · 2024-02-02T21:20:01Z

This false positive is due to some inline JavaScript written as a string

https://github.com/bokeh/bokeh/blob/829b2a75c402d0d0abd7e37ff201fbdfd949d857/examples/plotting/customjs_expr.py#L26-L38

I think that's okay...

snowsignal · 2024-02-02T21:22:14Z

e.g. "{foo}".format(foo=1) should definitely not be flagged

I think this ecosystem check might be outdated, this should be fixed.

zanieb · 2024-02-02T21:23:11Z

e.g. "{foo}".format(foo=1) should definitely not be flagged

I think this ecosystem check might be outdated, this should be fixed.

Sweet. It'll update when the build succeeds.

snowsignal · 2024-02-02T21:24:20Z

Perhaps we should ignore strings with a .<call> attached

This is probably the best approach, honestly.

…t documentation

…he literal parts of f-strings

…erals with f-string parts

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs

…n the string literal

snowsignal added the rule Implementing or modifying a lint rule label Jan 31, 2024

snowsignal requested review from charliermarsh and dhruvmanila January 31, 2024 03:24

snowsignal commented Jan 31, 2024

View reviewed changes

crates/ruff_linter/src/rules/ruff/rules/helpers.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/ruff/rules/helpers.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs Show resolved Hide resolved

snowsignal force-pushed the jane/linter/rule/fstring branch from e566318 to 5525b24 Compare January 31, 2024 03:35

charliermarsh reviewed Jan 31, 2024

View reviewed changes

MichaReiser reviewed Jan 31, 2024

View reviewed changes

dhruvmanila reviewed Jan 31, 2024

View reviewed changes

snowsignal added 9 commits February 2, 2024 13:33

Implement the MissingFStringSyntax lint

58b8e9d

Fix a few clippy issues

64153a9

Address some suggestions

e761390

Implement tests for triple-quoted strings

1522143

Implement additional test cases, include heuristic information in lin…

70ba873

…t documentation

Improve comments/documentation, MissingFStringSyntax now applies to t…

9554649

…he literal parts of f-strings

kwargs is now pulled from all parent functions

04978a5

Address remaining suggestions, correctly lint concatenated string lit…

71f362a

…erals with f-string parts

Ensure format specifiers are valid when checking for f-string syntax

664be28

snowsignal force-pushed the jane/linter/rule/fstring branch from 9d4336e to 664be28 Compare February 2, 2024 21:33

snowsignal requested review from MichaReiser, charliermarsh and zanieb February 2, 2024 21:41

charliermarsh approved these changes Feb 2, 2024

View reviewed changes

snowsignal added 4 commits February 2, 2024 14:35

Address suggestions from @charliermarsh, exclude any use of .<call> o…

69f9353

…n the string literal

Update tests to match new suggestion title

d454025

Fix issue where <literal>.<call> expressions were not being ignored

736bf6d

Elaborate and clean up lint documentation

3e5953e

snowsignal enabled auto-merge (squash) February 3, 2024 00:18

snowsignal merged commit e0a6034 into main Feb 3, 2024

snowsignal deleted the jane/linter/rule/fstring branch February 3, 2024 00:21

miccal mentioned this pull request Feb 6, 2024

ruff 0.2.1 Homebrew/homebrew-core#161912

Merged

ocaballeror mentioned this pull request Feb 6, 2024

RUF027 false negative on method calls #9857

Closed

johnslavik mentioned this pull request Mar 15, 2024

Bump ruff-pre-commit to v0.3.2 python-poetry/cleo#412

Merged

Conversation

snowsignal commented Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What it does

Why is this bad?

Example

Heuristics

Test Plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

charliermarsh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

codspeed-hq bot commented Feb 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #9728 will not alter performance

Summary

Uh oh!

zanieb commented Feb 2, 2024

Uh oh!

zanieb commented Feb 2, 2024

Uh oh!

zanieb commented Feb 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zanieb commented Feb 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowsignal commented Feb 2, 2024

Uh oh!

zanieb commented Feb 2, 2024

Uh oh!

snowsignal commented Feb 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

snowsignal commented Jan 31, 2024 •

edited

Loading

github-actions bot commented Jan 31, 2024 •

edited

Loading

`ruff-ecosystem` results

codspeed-hq bot commented Feb 1, 2024 •

edited

Loading

zanieb commented Feb 2, 2024 •

edited

Loading

zanieb commented Feb 2, 2024 •

edited

Loading