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

[pyupgrade] Generate diagnostic for all valid f-string conversions regardless of line-length (UP032) #10238

Merged

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Mar 5, 2024

Summary

Fixes #10235

This PR changes UP032 to flag all "".format calls that can technically be rewritten to an f-string, even if rewritting it to an fstring, at least automatically, exceeds the line length (or increases the amount by which it goes over the line length).

I looked at the Git history to understand whether the check prevents some false positives (reported by an issue), but i haven't found a compelling reason to limit the rule to only flag format calls that stay in the line length limit:

I did take a look at pyupgrade and couldn't find a similar check, at least not in the rule code (maybe it's checked somewhere else?) https://github.com/asottile/pyupgrade/blob/main/pyupgrade/_plugins/fstrings.py

Breaking Change?

This could be seen as a breaking change according to ruff's versioning policy:

The behavior of a stable rule is changed

  • The scope of a stable rule is significantly increased
  • The intent of the rule changes
  • Does not include bug fixes that follow the original intent of the rule

It does increase the scope of the rule, but it is in the original intent of the rule (so it's not).

Test Plan

See changed test output

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Mar 5, 2024
Copy link
Contributor

github-actions bot commented Mar 5, 2024

ruff-ecosystem results

Linter (stable)

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

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

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

+ tests/conftest.py:492:13: UP032 Use f-string instead of `format` call

demisto/content (+67 -0 violations, +0 -0 fixes)

+ Packs/Active_Directory_Query/Integrations/Active_Directory_Query/Active_Directory_Query.py:865:17: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:562:13: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:737:22: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:809:13: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:887:26: UP032 Use f-string instead of `format` call
+ Packs/AttivoBotsink/Integrations/AttivoBotsink/AttivoBotsink.py:752:25: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/DBotPreprocessTextData/DBotPreprocessTextData.py:429:26: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:112:38: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:119:9: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:121:9: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:124:9: UP032 Use f-string instead of `format` call
+ Packs/CommonScripts/Scripts/ParseEmailFiles/ParseEmailFiles.py:3883:30: UP032 Use f-string instead of `format` call
+ Packs/CortexXDR/Scripts/XDRSyncScript/XDRSyncScript.py:221:26: UP032 Use f-string instead of `format` call
+ Packs/Cymulate/Integrations/Cymulate/Cymulate.py:329:19: UP032 Use f-string instead of `format` call
... 53 additional changes omitted for project

docker/docker-py (+3 -0 violations, +0 -0 fixes)

+ tests/unit/api_exec_test.py:14:51: UP032 Use f-string instead of `format` call
+ tests/unit/errors_test.py:128:15: UP032 Use f-string instead of `format` call
+ tests/unit/errors_test.py:142:15: UP032 Use f-string instead of `format` call

freedomofpress/securedrop (+26 -0 violations, +0 -0 fixes)

+ install_files/ansible-base/roles/restore/files/compare_torrc.py:56:9: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_rqrequeue.py:20:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_rqrequeue.py:24:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_rqworker.py:22:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_shredder_configuration.py:19:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_shredder_configuration.py:23:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py:18:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py:22:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app/test_appenv.py:77:11: UP032 Use f-string instead of `format` call
+ molecule/testinfra/common/test_fpf_apt_repo.py:31:18: UP032 Use f-string instead of `format` call
... 16 additional changes omitted for project

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

+ ibis/backends/bigquery/udf/core.py:390:16: UP032 Use f-string instead of `format` call
+ ibis/backends/bigquery/udf/core.py:440:16: UP032 Use f-string instead of `format` call
+ ibis/backends/dask/__init__.py:83:17: UP032 Use f-string instead of `format` call
+ ibis/backends/impala/ddl.py:470:16: UP032 Use f-string instead of `format` call
+ ibis/backends/pandas/__init__.py:320:17: UP032 Use f-string instead of `format` call
+ ibis/backends/snowflake/__init__.py:168:13: UP032 Use f-string instead of `format` call
+ ibis/legacy/udf/validate.py:56:17: UP032 Use f-string instead of `format` call

milvus-io/pymilvus (+3 -0 violations, +0 -0 fixes)

+ examples/example_bulkinsert_json.py:256:11: UP032 Use f-string instead of `format` call
+ examples/example_bulkinsert_numpy.py:258:11: UP032 Use f-string instead of `format` call
+ pymilvus/bulk_writer/buffer.py:112:21: UP032 Use f-string instead of `format` call

mlflow/mlflow (+37 -0 violations, +0 -0 fixes)

+ examples/multistep_workflow/main.py:43:17: UP032 Use f-string instead of `format` call
+ examples/pytorch/mnist_tensorboard_artifact.py:153:17: UP032 Use f-string instead of `format` call
+ examples/pytorch/mnist_tensorboard_artifact.py:184:9: UP032 Use f-string instead of `format` call
+ examples/pytorch/torchscript/MNIST/mnist_torchscript.py:50:17: UP032 Use f-string instead of `format` call
+ examples/pytorch/torchscript/MNIST/mnist_torchscript.py:77:9: UP032 Use f-string instead of `format` call
+ examples/remote_store/remote_server.py:31:15: UP032 Use f-string instead of `format` call
+ mlflow/langchain/utils.py:303:17: UP032 Use f-string instead of `format` call
+ mlflow/models/evaluation/evaluator_registry.py:28:21: UP032 Use f-string instead of `format` call
+ mlflow/models/utils.py:989:17: UP032 Use f-string instead of `format` call
+ mlflow/projects/_project_spec.py:175:13: UP032 Use f-string instead of `format` call
... 27 additional changes omitted for project

pypa/pip (+17 -0 violations, +0 -0 fixes)

+ src/pip/__pip-runner__.py:21:9: UP032 Use f-string instead of `format` call
+ src/pip/_internal/commands/search.py:78:23: UP032 Use f-string instead of `format` call
+ src/pip/_internal/models/candidate.py:23:16: UP032 Use f-string instead of `format` call
+ src/pip/_internal/operations/install/wheel.py:507:27: UP032 Use f-string instead of `format` call
+ src/pip/_internal/operations/install/wheel.py:517:27: UP032 Use f-string instead of `format` call
+ src/pip/_internal/req/constructors.py:135:13: UP032 Use f-string instead of `format` call
+ src/pip/_internal/req/req_install.py:225:16: UP032 Use f-string instead of `format` call
+ src/pip/_internal/req/req_uninstall.py:513:17: UP032 Use f-string instead of `format` call
+ src/pip/_internal/resolution/legacy/resolver.py:107:9: UP032 Use f-string instead of `format` call
+ src/pip/_internal/resolution/legacy/resolver.py:266:17: UP032 Use f-string instead of `format` call
... 7 additional changes omitted for project

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

+ rotkehlchen/chain/ethereum/abi.py:77:36: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/binance.py:398:25: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/bitcoinde.py:236:17: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/bitmex.py:207:17: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/iconomi.py:187:17: UP032 Use f-string instead of `format` call
+ rotkehlchen/externalapis/cryptocompare.py:184:17: UP032 Use f-string instead of `format` call

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

+ tests/test_issue342_cmake_osx_args_in_setup.py:193:9: UP032 Use f-string instead of `format` call

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

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

+ corporate/lib/stripe.py:2205:28: UP032 Use f-string instead of `format` call
+ corporate/lib/stripe.py:2207:28: UP032 Use f-string instead of `format` call
+ scripts/lib/zulip_tools.py:565:15: UP032 Use f-string instead of `format` call
+ tools/lib/provision.py:48:9: UP032 Use f-string instead of `format` call
+ zerver/actions/create_realm.py:323:19: UP032 Use f-string instead of `format` call
+ zerver/lib/bot_storage.py:48:13: UP032 Use f-string instead of `format` call
+ zerver/lib/email_mirror.py:208:25: UP032 Use f-string instead of `format` call
+ zerver/management/commands/deactivate_user.py:40:13: UP032 Use f-string instead of `format` call
+ zerver/management/commands/delete_user.py:60:17: UP032 Use f-string instead of `format` call
+ zerver/tests/test_markdown.py:2802:13: UP032 Use f-string instead of `format` call
... 9 additional changes omitted for project

indico/indico (+39 -0 violations, +0 -0 fixes)

+ indico/cli/setup.py:614:48: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/custom/greatest.py:24:12: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/custom/least.py:24:12: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/principals.py:539:46: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/principals.py:545:46: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/principals.py:596:49: UP032 Use f-string instead of `format` call
+ indico/core/plugins/__init__.py:124:33: UP032 Use f-string instead of `format` call
+ indico/modules/attachments/models/legacy_mapping.py:137:16: UP032 Use f-string instead of `format` call
+ indico/modules/categories/controllers/util.py:51:20: UP032 Use f-string instead of `format` call
+ indico/modules/categories/views.py:51:30: UP032 Use f-string instead of `format` call
... 29 additional changes omitted for project

... Truncated remaining completed project reports due to GitHub comment length restrictions

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
UP032 226 226 0 0 0

Linter (preview)

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

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

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

+ tests/conftest.py:492:13: UP032 Use f-string instead of `format` call

demisto/content (+67 -0 violations, +0 -0 fixes)

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

+ Packs/Active_Directory_Query/Integrations/Active_Directory_Query/Active_Directory_Query.py:865:17: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:562:13: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:737:22: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:809:13: UP032 Use f-string instead of `format` call
+ Packs/ArcSightESM/Integrations/ArcSightESMv2/ArcSightESMv2.py:887:26: UP032 Use f-string instead of `format` call
+ Packs/AttivoBotsink/Integrations/AttivoBotsink/AttivoBotsink.py:752:25: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/DBotPreprocessTextData/DBotPreprocessTextData.py:429:26: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:112:38: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:119:9: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:121:9: UP032 Use f-string instead of `format` call
+ Packs/Base/Scripts/GetMLModelEvaluation/GetMLModelEvaluation.py:124:9: UP032 Use f-string instead of `format` call
+ Packs/CommonScripts/Scripts/ParseEmailFiles/ParseEmailFiles.py:3883:30: UP032 Use f-string instead of `format` call
+ Packs/CortexXDR/Scripts/XDRSyncScript/XDRSyncScript.py:221:26: UP032 Use f-string instead of `format` call
+ Packs/Cymulate/Integrations/Cymulate/Cymulate.py:329:19: UP032 Use f-string instead of `format` call
... 53 additional changes omitted for project

docker/docker-py (+3 -0 violations, +0 -0 fixes)

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

+ tests/unit/api_exec_test.py:14:51: UP032 Use f-string instead of `format` call
+ tests/unit/errors_test.py:128:15: UP032 Use f-string instead of `format` call
+ tests/unit/errors_test.py:142:15: UP032 Use f-string instead of `format` call

freedomofpress/securedrop (+26 -0 violations, +0 -0 fixes)

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

+ install_files/ansible-base/roles/restore/files/compare_torrc.py:56:9: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_rqrequeue.py:20:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_rqrequeue.py:24:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_rqworker.py:22:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_shredder_configuration.py:19:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_shredder_configuration.py:23:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py:18:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py:22:13: UP032 Use f-string instead of `format` call
+ molecule/testinfra/app/test_appenv.py:77:11: UP032 Use f-string instead of `format` call
+ molecule/testinfra/common/test_fpf_apt_repo.py:31:18: UP032 Use f-string instead of `format` call
... 16 additional changes omitted for project

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

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

+ ibis/backends/bigquery/udf/core.py:390:16: UP032 Use f-string instead of `format` call
+ ibis/backends/bigquery/udf/core.py:440:16: UP032 Use f-string instead of `format` call
+ ibis/backends/dask/__init__.py:83:17: UP032 Use f-string instead of `format` call
+ ibis/backends/impala/ddl.py:470:16: UP032 Use f-string instead of `format` call
+ ibis/backends/pandas/__init__.py:320:17: UP032 Use f-string instead of `format` call
+ ibis/backends/snowflake/__init__.py:168:13: UP032 Use f-string instead of `format` call
+ ibis/legacy/udf/validate.py:56:17: UP032 Use f-string instead of `format` call

milvus-io/pymilvus (+3 -0 violations, +0 -0 fixes)

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

+ examples/example_bulkinsert_json.py:256:11: UP032 Use f-string instead of `format` call
+ examples/example_bulkinsert_numpy.py:258:11: UP032 Use f-string instead of `format` call
+ pymilvus/bulk_writer/buffer.py:112:21: UP032 Use f-string instead of `format` call

mlflow/mlflow (+37 -0 violations, +0 -0 fixes)

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

+ examples/multistep_workflow/main.py:43:17: UP032 Use f-string instead of `format` call
+ examples/pytorch/mnist_tensorboard_artifact.py:153:17: UP032 Use f-string instead of `format` call
+ examples/pytorch/mnist_tensorboard_artifact.py:184:9: UP032 Use f-string instead of `format` call
+ examples/pytorch/torchscript/MNIST/mnist_torchscript.py:50:17: UP032 Use f-string instead of `format` call
+ examples/pytorch/torchscript/MNIST/mnist_torchscript.py:77:9: UP032 Use f-string instead of `format` call
+ examples/remote_store/remote_server.py:31:15: UP032 Use f-string instead of `format` call
+ mlflow/langchain/utils.py:303:17: UP032 Use f-string instead of `format` call
+ mlflow/models/evaluation/evaluator_registry.py:28:21: UP032 Use f-string instead of `format` call
+ mlflow/models/utils.py:989:17: UP032 Use f-string instead of `format` call
+ mlflow/projects/_project_spec.py:175:13: UP032 Use f-string instead of `format` call
... 27 additional changes omitted for project

pypa/pip (+17 -0 violations, +0 -0 fixes)

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

+ src/pip/__pip-runner__.py:21:9: UP032 Use f-string instead of `format` call
+ src/pip/_internal/commands/search.py:78:23: UP032 Use f-string instead of `format` call
+ src/pip/_internal/models/candidate.py:23:16: UP032 Use f-string instead of `format` call
+ src/pip/_internal/operations/install/wheel.py:507:27: UP032 Use f-string instead of `format` call
+ src/pip/_internal/operations/install/wheel.py:517:27: UP032 Use f-string instead of `format` call
+ src/pip/_internal/req/constructors.py:135:13: UP032 Use f-string instead of `format` call
+ src/pip/_internal/req/req_install.py:225:16: UP032 Use f-string instead of `format` call
+ src/pip/_internal/req/req_uninstall.py:513:17: UP032 Use f-string instead of `format` call
+ src/pip/_internal/resolution/legacy/resolver.py:107:9: UP032 Use f-string instead of `format` call
+ src/pip/_internal/resolution/legacy/resolver.py:266:17: UP032 Use f-string instead of `format` call
... 7 additional changes omitted for project

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

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

+ rotkehlchen/chain/ethereum/abi.py:77:36: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/binance.py:398:25: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/bitcoinde.py:236:17: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/bitmex.py:207:17: UP032 Use f-string instead of `format` call
+ rotkehlchen/exchanges/iconomi.py:187:17: UP032 Use f-string instead of `format` call
+ rotkehlchen/externalapis/cryptocompare.py:184:17: UP032 Use f-string instead of `format` call

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

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

+ tests/test_issue342_cmake_osx_args_in_setup.py:193:9: UP032 Use f-string instead of `format` call

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

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

+ corporate/lib/stripe.py:2205:28: UP032 Use f-string instead of `format` call
+ corporate/lib/stripe.py:2207:28: UP032 Use f-string instead of `format` call
+ scripts/lib/zulip_tools.py:565:15: UP032 Use f-string instead of `format` call
+ tools/lib/provision.py:48:9: UP032 Use f-string instead of `format` call
+ zerver/actions/create_realm.py:323:19: UP032 Use f-string instead of `format` call
+ zerver/lib/bot_storage.py:48:13: UP032 Use f-string instead of `format` call
+ zerver/lib/email_mirror.py:208:25: UP032 Use f-string instead of `format` call
+ zerver/management/commands/deactivate_user.py:40:13: UP032 Use f-string instead of `format` call
+ zerver/management/commands/delete_user.py:60:17: UP032 Use f-string instead of `format` call
+ zerver/tests/test_markdown.py:2802:13: UP032 Use f-string instead of `format` call
... 9 additional changes omitted for project

indico/indico (+39 -0 violations, +0 -0 fixes)

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

+ indico/cli/setup.py:614:48: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/custom/greatest.py:24:12: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/custom/least.py:24:12: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/principals.py:539:46: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/principals.py:545:46: UP032 Use f-string instead of `format` call
+ indico/core/db/sqlalchemy/principals.py:596:49: UP032 Use f-string instead of `format` call
+ indico/core/plugins/__init__.py:124:33: UP032 Use f-string instead of `format` call
+ indico/modules/attachments/models/legacy_mapping.py:137:16: UP032 Use f-string instead of `format` call
+ indico/modules/categories/controllers/util.py:51:20: UP032 Use f-string instead of `format` call
+ indico/modules/categories/views.py:51:30: UP032 Use f-string instead of `format` call
... 29 additional changes omitted for project

... Truncated remaining completed project reports due to GitHub comment length restrictions

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
UP032 226 226 0 0 0

checker.locator(),
checker.settings.pycodestyle.max_line_length,
checker.settings.tab_size,
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I'm tempted to remove this too. I think it will be confusing to users if we don't offer a fix here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, but there seems to be an issue with #7810

@MichaReiser MichaReiser merged commit 6159a8e into main Mar 6, 2024
17 checks passed
@MichaReiser MichaReiser deleted the fstring-generate-diagnostic-for-all-possible-conversions branch March 6, 2024 08:58
@alex
Copy link
Contributor

alex commented Mar 7, 2024

Is it expected that ruff will not offer to automatically fix these?

@charliermarsh
Copy link
Member

It was intentional here, but in my opinion we should.

@alex
Copy link
Contributor

alex commented Mar 7, 2024

Yes, I think it'd be very valuable. I thought UP032 did offer to fix for other instances?

@charliermarsh
Copy link
Member

Yeah, it does. We used to avoid flagging UP032 if the fixed code would exceed the line length. We removed that gating here. I think the fix was left off because we were wary of a "safe" autofix that would lead to line length violations, but in my opinion we should be offering the fix as long as we're raising the diagnostic.

@alex
Copy link
Contributor

alex commented Mar 7, 2024

Yes, from the user's perspective I'd rather have a fix that may lead to line length issues than have to fix them myself :-) Cleaning up line lengths is easier than changing from "".format() to f""

@charliermarsh
Copy link
Member

Agreed. I shall PR it now and @MichaReiser and I can discuss!

@alex
Copy link
Contributor

alex commented Mar 7, 2024 via email

charliermarsh added a commit that referenced this pull request Mar 7, 2024
… (`UP032`) (#10263)

## Summary

This is a follow-up to #10238 to
offer fixes for the f-string rule regardless of the line length of the
resulting fix. To quote Alex in the linked PR:

> Yes, from the user's perspective I'd rather have a fix that may lead
to line length issues than have to fix them myself :-) Cleaning up line
lengths is easier than changing from `"".format()` to `f""`

I agree with this position, which is that if we're going to offer a
diagnostic, we should really be offering the user the ability to fix it
-- otherwise, we're just inconveniencing them.
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
…regardless of line-length (`UP032`) (astral-sh#10238)

## Summary

Fixes astral-sh#10235

This PR changes `UP032` to flag all `"".format` calls that can
technically be rewritten to an f-string, even if rewritting it to an
fstring, at least automatically, exceeds the line length (or increases
the amount by which it goes over the line length).

I looked at the Git history to understand whether the check prevents
some false positives (reported by an issue), but i haven't found a
compelling reason to limit the rule to only flag format calls that stay
in the line length limit:

* astral-sh#7818 Changed the heuristic to
determine if the fix fits to address
astral-sh#7810
* astral-sh#1905 first version of the rule 


I did take a look at pyupgrade and couldn't find a similar check, at
least not in the rule code (maybe it's checked somewhere else?)
https://github.com/asottile/pyupgrade/blob/main/pyupgrade/_plugins/fstrings.py


## Breaking Change?

This could be seen as a breaking change according to ruff's [versioning
policy](https://docs.astral.sh/ruff/versioning/):

> The behavior of a stable rule is changed
  
  * The scope of a stable rule is significantly increased
  * The intent of the rule changes
* Does not include bug fixes that follow the original intent of the rule

It does increase the scope of the rule, but it is in the original intent
of the rule (so it's not).

## Test Plan

See changed test output
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
… (`UP032`) (astral-sh#10263)

## Summary

This is a follow-up to astral-sh#10238 to
offer fixes for the f-string rule regardless of the line length of the
resulting fix. To quote Alex in the linked PR:

> Yes, from the user's perspective I'd rather have a fix that may lead
to line length issues than have to fix them myself :-) Cleaning up line
lengths is easier than changing from `"".format()` to `f""`

I agree with this position, which is that if we're going to offer a
diagnostic, we should really be offering the user the ability to fix it
-- otherwise, we're just inconveniencing them.
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.

UP032 Failing to Detect format based on Length
3 participants