Skip to content

Conversation

@maxmynter
Copy link
Contributor

@maxmynter maxmynter commented May 10, 2025

Closes: #17967

Summary

Skip S608 for expressionless f-strings.
Expressionless f-strings are not vulnerable to SQL injections and F541 catches expressionless f-strings with an autofix.

This is achieved with guarding on the S608 f-string match with an occurrence check for expressions within it; analogous to the implementation of F451.

Test Plan

Added a test fixture and updated snapshot.

they are not vulnerable to SQL injections and F541 catches
expressionless f-strings with an autofix.
@github-actions
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

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

apache/superset (+1 -0 violations, +0 -0 fixes)

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

+ tests/integration_tests/sqllab_tests.py:191:58: RUF100 [*] Unused `noqa` directive (unused: `S608`)

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF100 1 1 0 0 0

Linter (preview)

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

apache/superset (+1 -0 violations, +0 -0 fixes)

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

+ tests/integration_tests/sqllab_tests.py:191:58: RUF100 [*] Unused `noqa` directive (unused: `S608`)

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF100 1 1 0 0 0

@maxmynter
Copy link
Contributor Author

maxmynter commented May 10, 2025

Ecosystem change is this one:

names_count = engine.execute(
    f"SELECT COUNT(*) FROM birth_names"  # noqa: F541, S608
).first()

https://github.com/apache/superset/blob/73701b729563d796d4aae628eff24f7163e7671c/tests/integration_tests/sqllab_tests.py#L190-L192

It's the intended effect of not raising S608 when there is no expression in an f-string. The newly raised RUF100 is marking the the now unused noqa of S608.

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label May 10, 2025
@MichaReiser MichaReiser merged commit b765dc4 into astral-sh:main May 10, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Autofix S608 when fstring has no placeholders

2 participants