mirror of https://github.com/astral-sh/ruff
[`flake8-use-pathlib`] Make fixes unsafe when types change in compound statements (`PTH104`, `PTH105`, `PTH109`, `PTH115`) (#22009)
## Summary Fixes https://github.com/astral-sh/ruff/issues/21794 ## Test Plan `cargo nextest run flake8_use_pathlib`
This commit is contained in:
parent
0bd7a94c27
commit
883701ae88
|
|
@ -136,4 +136,38 @@ os.chmod("pth1_file", 0o700, None, True, 1, *[1], **{"x": 1}, foo=1)
|
|||
os.rename("pth1_file", "pth1_file1", None, None, 1, *[1], **{"x": 1}, foo=1)
|
||||
os.replace("pth1_file1", "pth1_file", None, None, 1, *[1], **{"x": 1}, foo=1)
|
||||
|
||||
os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1)
|
||||
os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1)
|
||||
|
||||
# See: https://github.com/astral-sh/ruff/issues/21794
|
||||
import sys
|
||||
|
||||
if os.rename("pth1.py", "pth1.py.bak"):
|
||||
print("rename: truthy")
|
||||
else:
|
||||
print("rename: falsey")
|
||||
|
||||
if os.replace("pth1.py.bak", "pth1.py"):
|
||||
print("replace: truthy")
|
||||
else:
|
||||
print("replace: falsey")
|
||||
|
||||
try:
|
||||
for _ in os.getcwd():
|
||||
print("getcwd: iterable")
|
||||
break
|
||||
except TypeError as e:
|
||||
print("getcwd: not iterable")
|
||||
|
||||
try:
|
||||
for _ in os.getcwdb():
|
||||
print("getcwdb: iterable")
|
||||
break
|
||||
except TypeError as e:
|
||||
print("getcwdb: not iterable")
|
||||
|
||||
try:
|
||||
for _ in os.readlink(sys.executable):
|
||||
print("readlink: iterable")
|
||||
break
|
||||
except TypeError as e:
|
||||
print("readlink: not iterable")
|
||||
|
|
|
|||
|
|
@ -210,6 +210,7 @@ pub(crate) fn is_argument_non_default(arguments: &Arguments, name: &str, positio
|
|||
|
||||
/// Returns `true` if the given call is a top-level expression in its statement.
|
||||
/// This means the call's return value is not used, so return type changes don't matter.
|
||||
pub(crate) fn is_top_level_expression_call(checker: &Checker) -> bool {
|
||||
pub(crate) fn is_top_level_expression_in_statement(checker: &Checker) -> bool {
|
||||
checker.semantic().current_expression_parent().is_none()
|
||||
&& checker.semantic().current_statement().is_expr_stmt()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use ruff_text_size::Ranged;
|
|||
use crate::checkers::ast::Checker;
|
||||
use crate::importer::ImportRequest;
|
||||
use crate::preview::is_fix_os_getcwd_enabled;
|
||||
use crate::rules::flake8_use_pathlib::helpers::is_top_level_expression_call;
|
||||
use crate::rules::flake8_use_pathlib::helpers::is_top_level_expression_in_statement;
|
||||
use crate::{FixAvailability, Violation};
|
||||
|
||||
/// ## What it does
|
||||
|
|
@ -89,7 +89,7 @@ pub(crate) fn os_getcwd(checker: &Checker, call: &ExprCall, segments: &[&str]) {
|
|||
|
||||
// Unsafe when the fix would delete comments or change a used return value
|
||||
let applicability = if checker.comment_ranges().intersects(range)
|
||||
|| !is_top_level_expression_call(checker)
|
||||
|| !is_top_level_expression_in_statement(checker)
|
||||
{
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use crate::checkers::ast::Checker;
|
|||
use crate::preview::is_fix_os_readlink_enabled;
|
||||
use crate::rules::flake8_use_pathlib::helpers::{
|
||||
check_os_pathlib_single_arg_calls, is_keyword_only_argument_non_default,
|
||||
is_top_level_expression_call,
|
||||
is_top_level_expression_in_statement,
|
||||
};
|
||||
use crate::{FixAvailability, Violation};
|
||||
|
||||
|
|
@ -86,7 +86,7 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str])
|
|||
return;
|
||||
}
|
||||
|
||||
let applicability = if !is_top_level_expression_call(checker) {
|
||||
let applicability = if !is_top_level_expression_in_statement(checker) {
|
||||
// Unsafe because the return type changes (str/bytes -> Path)
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use crate::checkers::ast::Checker;
|
|||
use crate::preview::is_fix_os_rename_enabled;
|
||||
use crate::rules::flake8_use_pathlib::helpers::{
|
||||
check_os_pathlib_two_arg_calls, has_unknown_keywords_or_starred_expr,
|
||||
is_keyword_only_argument_non_default, is_top_level_expression_call,
|
||||
is_keyword_only_argument_non_default, is_top_level_expression_in_statement,
|
||||
};
|
||||
use crate::{FixAvailability, Violation};
|
||||
|
||||
|
|
@ -92,7 +92,7 @@ pub(crate) fn os_rename(checker: &Checker, call: &ExprCall, segments: &[&str]) {
|
|||
);
|
||||
|
||||
// Unsafe when the fix would delete comments or change a used return value
|
||||
let applicability = if !is_top_level_expression_call(checker) {
|
||||
let applicability = if !is_top_level_expression_in_statement(checker) {
|
||||
// Unsafe because the return type changes (None -> Path)
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use crate::checkers::ast::Checker;
|
|||
use crate::preview::is_fix_os_replace_enabled;
|
||||
use crate::rules::flake8_use_pathlib::helpers::{
|
||||
check_os_pathlib_two_arg_calls, has_unknown_keywords_or_starred_expr,
|
||||
is_keyword_only_argument_non_default, is_top_level_expression_call,
|
||||
is_keyword_only_argument_non_default, is_top_level_expression_in_statement,
|
||||
};
|
||||
use crate::{FixAvailability, Violation};
|
||||
|
||||
|
|
@ -95,7 +95,7 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str])
|
|||
);
|
||||
|
||||
// Unsafe when the fix would delete comments or change a used return value
|
||||
let applicability = if !is_top_level_expression_call(checker) {
|
||||
let applicability = if !is_top_level_expression_in_statement(checker) {
|
||||
// Unsafe because the return type changes (None -> Path)
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -567,5 +567,64 @@ PTH121 `os.path.samefile()` should be replaced by `Path.samefile()`
|
|||
138 |
|
||||
139 | os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1)
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
140 |
|
||||
141 | # See: https://github.com/astral-sh/ruff/issues/21794
|
||||
|
|
||||
help: Replace with `Path(...).samefile()`
|
||||
|
||||
PTH104 `os.rename()` should be replaced by `Path.rename()`
|
||||
--> full_name.py:144:4
|
||||
|
|
||||
142 | import sys
|
||||
143 |
|
||||
144 | if os.rename("pth1.py", "pth1.py.bak"):
|
||||
| ^^^^^^^^^
|
||||
145 | print("rename: truthy")
|
||||
146 | else:
|
||||
|
|
||||
help: Replace with `Path(...).rename(...)`
|
||||
|
||||
PTH105 `os.replace()` should be replaced by `Path.replace()`
|
||||
--> full_name.py:149:4
|
||||
|
|
||||
147 | print("rename: falsey")
|
||||
148 |
|
||||
149 | if os.replace("pth1.py.bak", "pth1.py"):
|
||||
| ^^^^^^^^^^
|
||||
150 | print("replace: truthy")
|
||||
151 | else:
|
||||
|
|
||||
help: Replace with `Path(...).replace(...)`
|
||||
|
||||
PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
--> full_name.py:155:14
|
||||
|
|
||||
154 | try:
|
||||
155 | for _ in os.getcwd():
|
||||
| ^^^^^^^^^
|
||||
156 | print("getcwd: iterable")
|
||||
157 | break
|
||||
|
|
||||
help: Replace with `Path.cwd()`
|
||||
|
||||
PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
--> full_name.py:162:14
|
||||
|
|
||||
161 | try:
|
||||
162 | for _ in os.getcwdb():
|
||||
| ^^^^^^^^^^
|
||||
163 | print("getcwdb: iterable")
|
||||
164 | break
|
||||
|
|
||||
help: Replace with `Path.cwd()`
|
||||
|
||||
PTH115 `os.readlink()` should be replaced by `Path.readlink()`
|
||||
--> full_name.py:169:14
|
||||
|
|
||||
168 | try:
|
||||
169 | for _ in os.readlink(sys.executable):
|
||||
| ^^^^^^^^^^^
|
||||
170 | print("readlink: iterable")
|
||||
171 | break
|
||||
|
|
||||
help: Replace with `Path(...).readlink()`
|
||||
|
|
|
|||
|
|
@ -1037,5 +1037,142 @@ PTH121 `os.path.samefile()` should be replaced by `Path.samefile()`
|
|||
138 |
|
||||
139 | os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1)
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
140 |
|
||||
141 | # See: https://github.com/astral-sh/ruff/issues/21794
|
||||
|
|
||||
help: Replace with `Path(...).samefile()`
|
||||
|
||||
PTH104 [*] `os.rename()` should be replaced by `Path.rename()`
|
||||
--> full_name.py:144:4
|
||||
|
|
||||
142 | import sys
|
||||
143 |
|
||||
144 | if os.rename("pth1.py", "pth1.py.bak"):
|
||||
| ^^^^^^^^^
|
||||
145 | print("rename: truthy")
|
||||
146 | else:
|
||||
|
|
||||
help: Replace with `Path(...).rename(...)`
|
||||
140 |
|
||||
141 | # See: https://github.com/astral-sh/ruff/issues/21794
|
||||
142 | import sys
|
||||
143 + import pathlib
|
||||
144 |
|
||||
- if os.rename("pth1.py", "pth1.py.bak"):
|
||||
145 + if pathlib.Path("pth1.py").rename("pth1.py.bak"):
|
||||
146 | print("rename: truthy")
|
||||
147 | else:
|
||||
148 | print("rename: falsey")
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
PTH105 [*] `os.replace()` should be replaced by `Path.replace()`
|
||||
--> full_name.py:149:4
|
||||
|
|
||||
147 | print("rename: falsey")
|
||||
148 |
|
||||
149 | if os.replace("pth1.py.bak", "pth1.py"):
|
||||
| ^^^^^^^^^^
|
||||
150 | print("replace: truthy")
|
||||
151 | else:
|
||||
|
|
||||
help: Replace with `Path(...).replace(...)`
|
||||
140 |
|
||||
141 | # See: https://github.com/astral-sh/ruff/issues/21794
|
||||
142 | import sys
|
||||
143 + import pathlib
|
||||
144 |
|
||||
145 | if os.rename("pth1.py", "pth1.py.bak"):
|
||||
146 | print("rename: truthy")
|
||||
147 | else:
|
||||
148 | print("rename: falsey")
|
||||
149 |
|
||||
- if os.replace("pth1.py.bak", "pth1.py"):
|
||||
150 + if pathlib.Path("pth1.py.bak").replace("pth1.py"):
|
||||
151 | print("replace: truthy")
|
||||
152 | else:
|
||||
153 | print("replace: falsey")
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
PTH109 [*] `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
--> full_name.py:155:14
|
||||
|
|
||||
154 | try:
|
||||
155 | for _ in os.getcwd():
|
||||
| ^^^^^^^^^
|
||||
156 | print("getcwd: iterable")
|
||||
157 | break
|
||||
|
|
||||
help: Replace with `Path.cwd()`
|
||||
140 |
|
||||
141 | # See: https://github.com/astral-sh/ruff/issues/21794
|
||||
142 | import sys
|
||||
143 + import pathlib
|
||||
144 |
|
||||
145 | if os.rename("pth1.py", "pth1.py.bak"):
|
||||
146 | print("rename: truthy")
|
||||
--------------------------------------------------------------------------------
|
||||
153 | print("replace: falsey")
|
||||
154 |
|
||||
155 | try:
|
||||
- for _ in os.getcwd():
|
||||
156 + for _ in pathlib.Path.cwd():
|
||||
157 | print("getcwd: iterable")
|
||||
158 | break
|
||||
159 | except TypeError as e:
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
PTH109 [*] `os.getcwd()` should be replaced by `Path.cwd()`
|
||||
--> full_name.py:162:14
|
||||
|
|
||||
161 | try:
|
||||
162 | for _ in os.getcwdb():
|
||||
| ^^^^^^^^^^
|
||||
163 | print("getcwdb: iterable")
|
||||
164 | break
|
||||
|
|
||||
help: Replace with `Path.cwd()`
|
||||
140 |
|
||||
141 | # See: https://github.com/astral-sh/ruff/issues/21794
|
||||
142 | import sys
|
||||
143 + import pathlib
|
||||
144 |
|
||||
145 | if os.rename("pth1.py", "pth1.py.bak"):
|
||||
146 | print("rename: truthy")
|
||||
--------------------------------------------------------------------------------
|
||||
160 | print("getcwd: not iterable")
|
||||
161 |
|
||||
162 | try:
|
||||
- for _ in os.getcwdb():
|
||||
163 + for _ in pathlib.Path.cwd():
|
||||
164 | print("getcwdb: iterable")
|
||||
165 | break
|
||||
166 | except TypeError as e:
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()`
|
||||
--> full_name.py:169:14
|
||||
|
|
||||
168 | try:
|
||||
169 | for _ in os.readlink(sys.executable):
|
||||
| ^^^^^^^^^^^
|
||||
170 | print("readlink: iterable")
|
||||
171 | break
|
||||
|
|
||||
help: Replace with `Path(...).readlink()`
|
||||
140 |
|
||||
141 | # See: https://github.com/astral-sh/ruff/issues/21794
|
||||
142 | import sys
|
||||
143 + import pathlib
|
||||
144 |
|
||||
145 | if os.rename("pth1.py", "pth1.py.bak"):
|
||||
146 | print("rename: truthy")
|
||||
--------------------------------------------------------------------------------
|
||||
167 | print("getcwdb: not iterable")
|
||||
168 |
|
||||
169 | try:
|
||||
- for _ in os.readlink(sys.executable):
|
||||
170 + for _ in pathlib.Path(sys.executable).readlink():
|
||||
171 | print("readlink: iterable")
|
||||
172 | break
|
||||
173 | except TypeError as e:
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
|
|
|||
Loading…
Reference in New Issue