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

[flake8-simplify] Extend open-file-with-context-handler to work with other standard-library IO modules (SIM115) #12959

Merged
merged 7 commits into from
Aug 22, 2024

Conversation

diceroll123
Copy link
Contributor

Summary

Closes #7313

Test Plan

cargo test

Copy link
Contributor

github-actions bot commented Aug 18, 2024

ruff-ecosystem results

Linter (stable)

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

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

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

+ airflow/models/dag.py:793:24: SIM115 Use a context manager for opening files
- airflow/models/dag.py:793:24: SIM115 Use context handler for opening files
+ airflow/providers/amazon/aws/hooks/s3.py:1451:20: SIM115 Use a context manager for opening files
- airflow/providers/amazon/aws/hooks/s3.py:1451:20: SIM115 Use context handler for opening files
+ airflow/providers/ftp/hooks/ftp.py:183:33: SIM115 Use a context manager for opening files
- airflow/providers/ftp/hooks/ftp.py:183:33: SIM115 Use context handler for opening files
+ airflow/providers/ftp/hooks/ftp.py:216:28: SIM115 Use a context manager for opening files
- airflow/providers/ftp/hooks/ftp.py:216:28: SIM115 Use context handler for opening files
+ airflow/utils/file.py:161:16: SIM115 Use a context manager for opening files
- airflow/utils/file.py:161:16: SIM115 Use context handler for opening files
+ dev/breeze/src/airflow_breeze/utils/console.py:86:16: SIM115 Use a context manager for opening files
- dev/breeze/src/airflow_breeze/utils/console.py:86:16: SIM115 Use context handler for opening files
+ dev/send_email.py:79:32: SIM115 Use a context manager for opening files
- dev/send_email.py:79:32: SIM115 Use context handler for opening files
+ dev/stats/explore_pr_candidates.ipynb:cell 2:1:8: SIM115 Use a context manager for opening files
- dev/stats/explore_pr_candidates.ipynb:cell 2:1:8: SIM115 Use context handler for opening files
+ docs/exts/operators_and_hooks_ref.py:181:25: SIM115 Use a context manager for opening files
- docs/exts/operators_and_hooks_ref.py:181:25: SIM115 Use context handler for opening files
+ scripts/ci/pre_commit/check_deferrable_default.py:66:25: SIM115 Use a context manager for opening files
- scripts/ci/pre_commit/check_deferrable_default.py:66:25: SIM115 Use context handler for opening files
+ scripts/ci/pre_commit/validate_operators_init.py:227:26: SIM115 Use a context manager for opening files
- scripts/ci/pre_commit/validate_operators_init.py:227:26: SIM115 Use context handler for opening files
+ tests/system/providers/amazon/aws/example_local_to_s3.py:40:12: SIM115 Use a context manager for opening files
- tests/system/providers/amazon/aws/example_local_to_s3.py:40:12: SIM115 Use context handler for opening files
+ tests/system/providers/weaviate/example_weaviate_cohere.py:57:26: SIM115 Use a context manager for opening files
- tests/system/providers/weaviate/example_weaviate_cohere.py:57:26: SIM115 Use context handler for opening files
+ tests/system/providers/weaviate/example_weaviate_cohere.py:72:26: SIM115 Use a context manager for opening files
- tests/system/providers/weaviate/example_weaviate_cohere.py:72:26: SIM115 Use context handler for opening files
+ tests/system/providers/weaviate/example_weaviate_dynamic_mapping_dag.py:56:27: SIM115 Use a context manager for opening files
- tests/system/providers/weaviate/example_weaviate_dynamic_mapping_dag.py:56:27: SIM115 Use context handler for opening files
... 18 additional changes omitted for project

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

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

+ RELEASING/generate_email.py:51:32: SIM115 Use a context manager for opening files
- RELEASING/generate_email.py:51:32: SIM115 Use context handler for opening files

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

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

+ docs/bokeh/source/conf.py:69:14: SIM115 Use a context manager for opening files
- docs/bokeh/source/conf.py:69:14: SIM115 Use context handler for opening files
+ examples/server/app/spectrogram/main.py:32:17: SIM115 Use a context manager for opening files
- examples/server/app/spectrogram/main.py:32:17: SIM115 Use context handler for opening files
+ release/build.py:157:29: SIM115 Use a context manager for opening files
- release/build.py:157:29: SIM115 Use context handler for opening files
+ release/remote.py:31:16: SIM115 Use a context manager for opening files
- release/remote.py:31:16: SIM115 Use context handler for opening files
+ release/remote.py:33:16: SIM115 Use a context manager for opening files
- release/remote.py:33:16: SIM115 Use context handler for opening files
+ scripts/milestone.py:318:11: SIM115 Use a context manager for opening files
- scripts/milestone.py:318:11: SIM115 Use context handler for opening files
+ scripts/sri.py:38:21: SIM115 Use a context manager for opening files
- scripts/sri.py:38:21: SIM115 Use context handler for opening files
+ src/bokeh/settings.py:801:47: SIM115 Use a context manager for opening files
- src/bokeh/settings.py:801:47: SIM115 Use context handler for opening files
+ src/bokeh/settings.py:820:34: SIM115 Use a context manager for opening files
... 11 additional changes omitted for project

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
SIM115 78 39 39 0 0

Linter (preview)

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

apache/airflow (+36 -24 violations, +0 -0 fixes)

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

+ airflow/models/dag.py:793:24: SIM115 Use a context manager for opening files
- airflow/models/dag.py:793:24: SIM115 Use context handler for opening files
+ airflow/providers/amazon/aws/hooks/s3.py:1451:20: SIM115 Use a context manager for opening files
- airflow/providers/amazon/aws/hooks/s3.py:1451:20: SIM115 Use context handler for opening files
+ airflow/providers/amazon/aws/hooks/s3.py:1453:20: SIM115 Use a context manager for opening files
+ airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py:247:29: SIM115 Use a context manager for opening files
+ airflow/providers/docker/operators/docker.py:487:19: SIM115 Use a context manager for opening files
+ airflow/providers/ftp/hooks/ftp.py:183:33: SIM115 Use a context manager for opening files
- airflow/providers/ftp/hooks/ftp.py:183:33: SIM115 Use context handler for opening files
+ airflow/providers/ftp/hooks/ftp.py:216:28: SIM115 Use a context manager for opening files
- airflow/providers/ftp/hooks/ftp.py:216:28: SIM115 Use context handler for opening files
+ airflow/providers/google/cloud/hooks/cloud_sql.py:934:22: SIM115 Use a context manager for opening files
+ airflow/providers/google/cloud/transfers/cassandra_to_gcs.py:195:27: SIM115 Use a context manager for opening files
+ airflow/providers/google/cloud/transfers/cassandra_to_gcs.py:212:35: SIM115 Use a context manager for opening files
+ airflow/providers/google/cloud/transfers/cassandra_to_gcs.py:228:34: SIM115 Use a context manager for opening files
+ airflow/providers/google/cloud/transfers/sql_to_gcs.py:354:27: SIM115 Use a context manager for opening files
+ airflow/providers/google/cloud/transfers/sql_to_gcs.py:465:34: SIM115 Use a context manager for opening files
+ airflow/utils/file.py:159:30: SIM115 Use a context manager for opening files
+ airflow/utils/file.py:161:16: SIM115 Use a context manager for opening files
- airflow/utils/file.py:161:16: SIM115 Use context handler for opening files
+ dev/breeze/src/airflow_breeze/utils/console.py:86:16: SIM115 Use a context manager for opening files
- dev/breeze/src/airflow_breeze/utils/console.py:86:16: SIM115 Use context handler for opening files
+ dev/breeze/src/airflow_breeze/utils/kubernetes_utils.py:185:24: SIM115 Use a context manager for opening files
+ dev/breeze/src/airflow_breeze/utils/parallel.py:62:12: SIM115 Use a context manager for opening files
+ dev/send_email.py:79:32: SIM115 Use a context manager for opening files
- dev/send_email.py:79:32: SIM115 Use context handler for opening files
+ dev/stats/explore_pr_candidates.ipynb:cell 2:1:8: SIM115 Use a context manager for opening files
- dev/stats/explore_pr_candidates.ipynb:cell 2:1:8: SIM115 Use context handler for opening files
... 32 additional changes omitted for project

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

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

+ RELEASING/generate_email.py:51:32: SIM115 Use a context manager for opening files
- RELEASING/generate_email.py:51:32: SIM115 Use context handler for opening files
+ superset/commands/dataset/importers/v1/utils.py:198:16: SIM115 Use a context manager for opening files
+ tests/integration_tests/email_tests.py:160:22: SIM115 Use a context manager for opening files
+ tests/integration_tests/email_tests.py:44:22: SIM115 Use a context manager for opening files
+ tests/integration_tests/email_tests.py:64:22: SIM115 Use a context manager for opening files
+ tests/integration_tests/email_tests.py:95:22: SIM115 Use a context manager for opening files

bokeh/bokeh (+17 -14 violations, +0 -0 fixes)

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

+ docs/bokeh/source/conf.py:69:14: SIM115 Use a context manager for opening files
- docs/bokeh/source/conf.py:69:14: SIM115 Use context handler for opening files
+ examples/server/app/spectrogram/main.py:32:17: SIM115 Use a context manager for opening files
- examples/server/app/spectrogram/main.py:32:17: SIM115 Use context handler for opening files
+ release/build.py:157:29: SIM115 Use a context manager for opening files
- release/build.py:157:29: SIM115 Use context handler for opening files
+ release/remote.py:31:16: SIM115 Use a context manager for opening files
- release/remote.py:31:16: SIM115 Use context handler for opening files
+ release/remote.py:33:16: SIM115 Use a context manager for opening files
- release/remote.py:33:16: SIM115 Use context handler for opening files
+ scripts/milestone.py:318:11: SIM115 Use a context manager for opening files
- scripts/milestone.py:318:11: SIM115 Use context handler for opening files
+ scripts/sri.py:38:21: SIM115 Use a context manager for opening files
- scripts/sri.py:38:21: SIM115 Use context handler for opening files
... 17 additional changes omitted for project

python-poetry/poetry (+1 -0 violations, +0 -0 fixes)

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

+ src/poetry/inspection/lazy_wheel.py:278:41: SIM115 Use a context manager for opening files

reflex-dev/reflex (+1 -0 violations, +0 -0 fixes)

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

+ reflex/utils/prerequisites.py:678:14: SIM115 Use a context manager for opening files

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

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

+ zerver/management/commands/backup.py:124:36: SIM115 Use a context manager for opening files

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

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

+ indico/modules/events/export.py:149:24: SIM115 Use a context manager for opening files
+ indico/modules/events/export.py:413:24: SIM115 Use a context manager for opening files
+ indico/modules/events/static/offline.py:91:21: SIM115 Use a context manager for opening files
+ indico/modules/events/util.py:369:17: SIM115 Use a context manager for opening files
+ indico/modules/events/util.py:710:21: SIM115 Use a context manager for opening files
+ indico/modules/users/export.py:79:17: SIM115 Use a context manager for opening files

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
SIM115 107 68 39 0 0

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks, this overall looks reasonable! I think it might make sense for it to be a preview-only change first, though, since it increases the scope of the rule. We also need to update the docs for the rule, which currently just says that the rule

Checks for uses of the builtin open() function without an associated context manager.

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Aug 19, 2024
@diceroll123 diceroll123 marked this pull request as draft August 21, 2024 00:34
@diceroll123 diceroll123 marked this pull request as ready for review August 22, 2024 09:27
@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 22, 2024

Looking at the ecosystem results: a lot of them seem to be regarding io.StringIO() and io.BytesIO(). Those two weren't ones that I suggested in #12959 (comment), and I'm inclined to think that they probably shouldn't be part of this rule: although they can be used as context managers, they're in-memory buffers rather than functions or classes that manipulate files on disk in some way. So it's not nearly as much of an issue if you forget to close them after you're done with them -- there's a small risk of a memory leak, but the in-memory buffer will be discarded after the function scope ends if it's only assigned to a local variable.

(I'll push some fixes to address this)

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks @diceroll123! I'll wait to check the final ecosystem report before merging, but this LGTM now.

@AlexWaygood AlexWaygood changed the title [flake8-simplify] - extend open-file-with-context-handler to work with tempfile (SIM115) [flake8-simplify] Extend open-file-with-context-handler to work with other standard-library IO modules (SIM115) Aug 22, 2024
@AlexWaygood AlexWaygood merged commit d37e2e5 into astral-sh:main Aug 22, 2024
20 checks passed
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 this pull request may close these issues.

Consider suggesting using tempfile classes as context managers
3 participants