-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[pylint] Demote PLW1510 fix to display-only
#22318
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
Conversation
Summary -- Closes #17091. `PLW1510` checks for `subprocess.run` calls without a `check` keyword argument and previously had a safe fix to add `check=False`. That's the default value, so technically it preserved the code's behavior, but as discussed in #17091 and #17087, Ruff can't actually know what the author intended. I don't think it hurts to keep this as a display-only fix instead of removing it entirely, but it definitely shouldn't be safe at the very least. Test Plan -- Existing tests
| // The fix is always set on the diagnostic, but display-only fixes aren't | ||
| // considered "fixable" in the tests. | ||
| const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This felt worth a comment here, but I think this is pretty reasonable in general. These fixes aren't usually shown to users. We could of course update the test code instead:
ruff/crates/ruff_linter/src/test.rs
Lines 354 to 359 in 8cd04df
| let fixable = diagnostic.fix().is_some_and(|fix| { | |
| matches!( | |
| fix.applicability(), | |
| Applicability::Safe | Applicability::Unsafe | |
| ) | |
| }); |
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PLW1510 | 46 | 0 | 0 | 0 | 46 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+0 -0 violations, +0 -46 fixes in 3 projects; 52 projects unchanged)
apache/superset (+0 -0 violations, +0 -12 fixes)
ruff check --no-cache --exit-zero --no-fix --output-format concise --preview --select ALL
- RELEASING/verify_release.py:31:14: PLW1510 [*] `subprocess.run` without explicit `check` argument + RELEASING/verify_release.py:31:14: PLW1510 `subprocess.run` without explicit `check` argument - RELEASING/verify_release.py:62:14: PLW1510 [*] `subprocess.run` without explicit `check` argument + RELEASING/verify_release.py:62:14: PLW1510 `subprocess.run` without explicit `check` argument - superset-extensions-cli/src/superset_extensions_cli/cli.py:108:15: PLW1510 [*] `subprocess.run` without explicit `check` argument + superset-extensions-cli/src/superset_extensions_cli/cli.py:108:15: PLW1510 `subprocess.run` without explicit `check` argument - superset-extensions-cli/src/superset_extensions_cli/cli.py:174:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + superset-extensions-cli/src/superset_extensions_cli/cli.py:174:12: PLW1510 `subprocess.run` without explicit `check` argument - superset-extensions-cli/src/superset_extensions_cli/cli.py:58:18: PLW1510 [*] `subprocess.run` without explicit `check` argument + superset-extensions-cli/src/superset_extensions_cli/cli.py:58:18: PLW1510 `subprocess.run` without explicit `check` argument - tests/unit_tests/fixtures/bash_mock.py:25:18: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/unit_tests/fixtures/bash_mock.py:25:18: PLW1510 `subprocess.run` without explicit `check` argument
bokeh/bokeh (+0 -0 violations, +0 -30 fixes)
ruff check --no-cache --exit-zero --no-fix --output-format concise --preview --select ALL
- release/system.py:43:18: PLW1510 [*] `subprocess.run` without explicit `check` argument + release/system.py:43:18: PLW1510 `subprocess.run` without explicit `check` argument - scripts/hooks/install.py:5:5: PLW1510 [*] `subprocess.run` without explicit `check` argument + scripts/hooks/install.py:5:5: PLW1510 `subprocess.run` without explicit `check` argument - scripts/hooks/protect_branches.py:10:22: PLW1510 [*] `subprocess.run` without explicit `check` argument + scripts/hooks/protect_branches.py:10:22: PLW1510 `subprocess.run` without explicit `check` argument - scripts/hooks/uninstall.py:5:5: PLW1510 [*] `subprocess.run` without explicit `check` argument + scripts/hooks/uninstall.py:5:5: PLW1510 `subprocess.run` without explicit `check` argument - setup.py:52:16: PLW1510 [*] `subprocess.run` without explicit `check` argument + setup.py:52:16: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_eslint.py:37:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_eslint.py:37:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_isort.py:58:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_isort.py:58:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_js_license_set.py:50:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_js_license_set.py:50:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_license.py:40:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_license.py:40:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_no_client_server_common.py:48:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_no_client_server_common.py:48:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_no_client_server_common.py:57:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_no_client_server_common.py:57:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_no_tornado_common.py:55:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_no_tornado_common.py:55:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_ruff.py:33:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_ruff.py:33:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/codebase/test_vermin.py:39:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/codebase/test_vermin.py:39:12: PLW1510 `subprocess.run` without explicit `check` argument - tests/support/util/project.py:46:12: PLW1510 [*] `subprocess.run` without explicit `check` argument + tests/support/util/project.py:46:12: PLW1510 `subprocess.run` without explicit `check` argument
latchbio/latch (+0 -0 violations, +0 -4 fixes)
ruff check --no-cache --exit-zero --no-fix --output-format concise --preview
- src/latch_cli/centromere/ctx.py:337:43: PLW1510 [*] `subprocess.run` without explicit `check` argument + src/latch_cli/centromere/ctx.py:337:43: PLW1510 `subprocess.run` without explicit `check` argument - src/latch_cli/centromere/ctx.py:342:43: PLW1510 [*] `subprocess.run` without explicit `check` argument + src/latch_cli/centromere/ctx.py:342:43: PLW1510 `subprocess.run` without explicit `check` argument
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PLW1510 | 46 | 0 | 0 | 0 | 46 |
Summary
Closes #17091.
PLW1510checks forsubprocess.runcalls without acheckkeyword argument and previously had a safe fix to add
check=False. That's thedefault value, so technically it preserved the code's behavior, but as discussed
in #17091 and #17087, Ruff can't actually know what the author intended.
I don't think it hurts to keep this as a display-only fix instead of removing it
entirely, but it definitely shouldn't be safe at the very least.
Test Plan
Existing tests