Add Autofix for ISC003 #18256

maxmynter · 2025-05-22T16:23:47Z

Summary

Add the following fix for ICS003 (explicit string concatenation): Remove the + operator to implicitly concatenate {r,f,b}strings.

Test Plan

Extended and adapted the ICS.py test fixture.

Added tests for b and rb strings
Updated the snapshot for existing tests to reflect the suggested fix

github-actions · 2025-05-22T16:30:34Z

`ruff-ecosystem` results

Linter (stable)

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

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

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

- superset/db_engine_specs/bigquery.py:81:5: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/bigquery.py:81:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/clickhouse.py:292:17: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/clickhouse.py:292:17: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:37:27: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:37:27: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:39:27: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:39:27: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:41:33: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:41:33: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:43:32: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:43:32: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:45:36: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:45:36: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:47:35: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:47:35: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:52:26: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:52:26: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:54:28: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:54:28: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:68:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:68:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/viz.py:1395:29: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/viz.py:1395:29: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/viz.py:1435:25: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/viz.py:1435:25: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:31:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:31:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:49:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:49:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:66:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:66:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:77:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:77:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/utils_tests.py:352:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/utils_tests.py:352:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated

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

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

- examples/styling/mathtext/latex_schrodinger.py:26:5: ISC003 Explicitly concatenated string should be implicitly concatenated
+ examples/styling/mathtext/latex_schrodinger.py:26:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/bokeh/command/subcommands/serve.py:899:17: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/bokeh/command/subcommands/serve.py:899:17: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/bokeh/server/session.py:90:40: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/bokeh/server/session.py:90:40: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/bokeh/util/compiler.py:411:24: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/bokeh/util/compiler.py:411:24: ISC003 [*] Explicitly concatenated string should be implicitly concatenated

latchbio/latch (+0 -0 violations, +6 -0 fixes)

- src/latch_cli/centromere/ctx.py:282:29: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/latch_cli/centromere/ctx.py:282:29: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/latch_cli/utils/path.py:181:9: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/latch_cli/utils/path.py:181:9: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/latch_cli/utils/path.py:189:9: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/latch_cli/utils/path.py:189:9: ISC003 [*] Explicitly concatenated string should be implicitly concatenated

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
ISC003	50	0	0	50	0

Linter (preview)

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

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

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

- superset/db_engine_specs/bigquery.py:81:5: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/bigquery.py:81:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/clickhouse.py:292:17: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/clickhouse.py:292:17: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:37:27: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:37:27: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:39:27: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:39:27: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:41:33: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:41:33: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:43:32: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:43:32: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:45:36: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:45:36: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:47:35: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:47:35: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:52:26: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:52:26: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:54:28: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:54:28: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/db_engine_specs/pinot.py:68:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/db_engine_specs/pinot.py:68:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/viz.py:1395:29: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/viz.py:1395:29: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- superset/viz.py:1435:25: ISC003 Explicitly concatenated string should be implicitly concatenated
+ superset/viz.py:1435:25: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:31:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:31:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:49:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:49:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:66:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:66:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/db_engine_specs/pinot_tests.py:77:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/db_engine_specs/pinot_tests.py:77:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- tests/integration_tests/utils_tests.py:352:13: ISC003 Explicitly concatenated string should be implicitly concatenated
+ tests/integration_tests/utils_tests.py:352:13: ISC003 [*] Explicitly concatenated string should be implicitly concatenated

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

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

- examples/styling/mathtext/latex_schrodinger.py:26:5: ISC003 Explicitly concatenated string should be implicitly concatenated
+ examples/styling/mathtext/latex_schrodinger.py:26:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/bokeh/command/subcommands/serve.py:899:17: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/bokeh/command/subcommands/serve.py:899:17: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/bokeh/server/session.py:90:40: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/bokeh/server/session.py:90:40: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/bokeh/util/compiler.py:411:24: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/bokeh/util/compiler.py:411:24: ISC003 [*] Explicitly concatenated string should be implicitly concatenated

latchbio/latch (+0 -0 violations, +6 -0 fixes)

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

- src/latch_cli/centromere/ctx.py:282:29: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/latch_cli/centromere/ctx.py:282:29: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/latch_cli/utils/path.py:181:9: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/latch_cli/utils/path.py:181:9: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
- src/latch_cli/utils/path.py:189:9: ISC003 Explicitly concatenated string should be implicitly concatenated
+ src/latch_cli/utils/path.py:189:9: ISC003 [*] Explicitly concatenated string should be implicitly concatenated

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
ISC003	50	0	0	50	0

maxmynter · 2025-05-22T21:26:34Z

The ecosystem changes all seem to be the change from a violation to a fixable violation.

Do we need a preview gate in this case?

cc: @MichaReiser (because we were talking preview gates on an other PR and you've categorised the issue).

MichaReiser

I think we need to change the fix to keep each part on its own line. We should also change the rule (separate PR) to only flag concatenations that can be converted to implicit string concatenations

crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/explicit.rs

crates/ruff_linter/resources/test/fixtures/flake8_implicit_str_concat/ISC.py

...r_concat/snapshots/ruff_linter__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap

maxmynter · 2025-05-26T12:47:52Z

Thanks for the review.

Changing ISC003 to only fire when fixable and preserve formatting greatly reduces pattern match boilerplate in the fix.
This also removes the Always / Sometimes fixable inconsistency.

I've implemented the AlwaysFixableViolation and dropped the Option wrapping and try_set_fix handling.

I added mulitline test cases. The Diagnostic only warns for the first violation. But testing with the binary, fixes are applied recursively so the following is fully fixed when using ruff:

cat > test.py << 'EOF'
_ = ("a"
    + "b"
    + "c"
    + "d"
    + "e")
EOF

target/debug/ruff check --diff --select ISC003 --no-cache test.py
--- test.py
+++ test.py
@@ -1,5 +1,5 @@
 _ = ("a"
-    + "b"
-    + "c"
-    + "d"
-    + "e")
+     "b"
+     "c"
+     "d"
+     "e")

Would fix 4 errors.

...r_concat/snapshots/ruff_linter__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap

crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/explicit.rs

maxmynter · 2025-05-27T14:02:53Z

Note: Currently a case like this

_ = (
    "start" +
    variable 
		# leading comment
    + "end"
    + "end"
)

Would not be reformatted. Since the first + is necessary and thus the rule is not recursively applied.

Likewise

_ = (
    "start" +
    "middle" + 
    variable 
    + "end"
    + "end"
)

will only fix the concatenation between "start" and "middle" not between "end" and "end".

Is there another way to fix this other than to make the fix recurse through the whole string rather than rely on the recursive application of the rule?

I feel it would make this implementation a lot more involved. Which I'm happy to try, but wanted to double check. If so, are there other rules you'd recommend to look at as reference?

MichaReiser · 2025-05-28T07:24:30Z

Thank you

MichaReiser · 2025-05-28T07:28:07Z

Note: Currently a case like this
_ = (
    "start" +
    variable 
		# leading comment
    + "end"
    + "end"
)
Would not be reformatted. Since the first + is necessary and thus the rule is not recursively applied.

Likewise
_ = (
    "start" +
    "middle" + 
    variable 
    + "end"
    + "end"
)
will only fix the concatenation between "start" and "middle" not between "end" and "end".

Is there another way to fix this other than to make the fix recurse through the whole string rather than rely on the recursive application of the rule?

I feel it would make this implementation a lot more involved. Which I'm happy to try, but wanted to double-check. If so, are there other rules you'd recommend to look at as reference?

Given that this is a pre-existing limitation, I'd prefer to handle this in a separate PR.

I think what we would need to do here is to "flatten-out" the binary expression for operators with the same precedence. We do this in the formatter but it can get a bit involved. But it requires you to recursively visit binary expressions and track what the last left side was (and if the operators have the same precedence and stop traversing if they don't)

* main: [ty] Support ephemeral uv virtual environments (#18335) Add a `ViolationMetadata::rule` method (#18234) Return `DiagnosticGuard` from `Checker::report_diagnostic` (#18232) [flake8_use_pathlib]: Replace os.symlink with Path.symlink_to (PTH211) (#18337) [ty] Support cancellation and retry in the server (#18273) [ty] Synthetic function-like callables (#18242) [ty] Support publishing diagnostics in the server (#18309) Add Autofix for ISC003 (#18256) [`pyupgrade`]: new rule UP050 (`useless-class-metaclass-type`) (#18334) [pycodestyle] Make `E712` suggestion not assume a context (#18328)

* main: (246 commits) [ty] Simplify signature types, use them in `CallableType` (astral-sh#18344) [ty] Support ephemeral uv virtual environments (astral-sh#18335) Add a `ViolationMetadata::rule` method (astral-sh#18234) Return `DiagnosticGuard` from `Checker::report_diagnostic` (astral-sh#18232) [flake8_use_pathlib]: Replace os.symlink with Path.symlink_to (PTH211) (astral-sh#18337) [ty] Support cancellation and retry in the server (astral-sh#18273) [ty] Synthetic function-like callables (astral-sh#18242) [ty] Support publishing diagnostics in the server (astral-sh#18309) Add Autofix for ISC003 (astral-sh#18256) [`pyupgrade`]: new rule UP050 (`useless-class-metaclass-type`) (astral-sh#18334) [pycodestyle] Make `E712` suggestion not assume a context (astral-sh#18328) put similar dunder-call tests next to each other (astral-sh#18343) [ty] Derive `PartialOrd, Ord` for `KnownInstanceType` (astral-sh#18340) [ty] Simplify `Type::try_bool()` (astral-sh#18342) [ty] Simplify `Type::normalized` slightly (astral-sh#18339) [ty] Move arviz off the list of selected primer projects (astral-sh#18336) [ty] Add --config-file CLI arg (astral-sh#18083) [ty] Tell the user why we inferred a certain Python version when reporting version-specific syntax errors (astral-sh#18295) [ty] Implement implicit inheritance from `Generic[]` for PEP-695 generic classes (astral-sh#18283) [ty] Add hint if async context manager is used in non-async with statement (astral-sh#18299) ...

maxmynter added 3 commits May 22, 2025 17:54

Add fix to ISC003

858516e

(tests) Update Snapshot for ISC003 Fix

c20d05c

(tests) Extend ICS003 Coverage

5e8ef05

MichaReiser requested changes May 25, 2025

View reviewed changes

MichaReiser added the fixes Related to suggested fixes for violations label May 25, 2025

maxmynter added 2 commits May 26, 2025 14:31

(tests) Multiline concatenations

a7d8b43

Only fixable violations fire ISC003

ca7f3e0

maxmynter requested a review from MichaReiser May 26, 2025 12:49

MichaReiser reviewed May 26, 2025

View reviewed changes

...r_concat/snapshots/ruff_linter__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap Outdated Show resolved Hide resolved

maxmynter added 3 commits May 27, 2025 00:08

Trim whitespace from fix

9dbe6d9

Clean up test fixture formatting

6937913

(fixup) Update Snapshot for cleaned test formatting

cc29cdb

dscorbett reviewed May 26, 2025

View reviewed changes

crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/explicit.rs Outdated Show resolved Hide resolved

maxmynter added 2 commits May 27, 2025 00:30

(fixup) Handle \r newlines

13e9629

Update comment to reflect \r newlines

25222a1

maxmynter requested review from MichaReiser and dscorbett May 26, 2025 22:32

MichaReiser reviewed May 27, 2025

View reviewed changes

crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/explicit.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/explicit.rs Outdated Show resolved Hide resolved

maxmynter added 4 commits May 27, 2025 13:52

Renaming

0917202

Extend Tests

ac7ab3c

Updated Trimming logic

4c62fc6

Add test cases with interleaved comment

97eda2f

maxmynter requested a review from MichaReiser May 27, 2025 14:03

Nits

ab7af9e

MichaReiser approved these changes May 28, 2025

View reviewed changes

MichaReiser merged commit 6d210dd into astral-sh:main May 28, 2025
34 checks passed

BrewTestBot mentioned this pull request May 29, 2025

ruff 0.11.12 Homebrew/homebrew-core#225103

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Autofix for ISC003 #18256

Add Autofix for ISC003 #18256

Uh oh!

maxmynter commented May 22, 2025

Uh oh!

github-actions bot commented May 22, 2025 •

edited

Loading

Uh oh!

maxmynter commented May 22, 2025 •

edited

Loading

Uh oh!

MichaReiser left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxmynter commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxmynter commented May 27, 2025 •

edited

Loading

Uh oh!

MichaReiser commented May 28, 2025

Uh oh!

MichaReiser commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!

Add Autofix for ISC003 #18256

Add Autofix for ISC003 #18256

Uh oh!

Conversation

maxmynter commented May 22, 2025

Summary

Test Plan

Uh oh!

github-actions bot commented May 22, 2025 • 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!

maxmynter commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

maxmynter commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxmynter commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaReiser commented May 28, 2025

Uh oh!

MichaReiser commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 22, 2025 •

edited

Loading

`ruff-ecosystem` results

maxmynter commented May 22, 2025 •

edited

Loading

maxmynter commented May 27, 2025 •

edited

Loading