Don't suggest replacing builtin.open() with Path.open() if the latter doesn't support all options (#7637)

**Summary** Check that `closefd` and `opener` aren't being used with
`builtin.open()` before suggesting `Path.open()` because pathlib doesn't
support these arguments.

Closes #7620

**Test Plan** New cases in the fixture.
This commit is contained in:
konsti 2023-09-26 11:07:35 +02:00 committed by GitHub
parent 93b5d8a0fb
commit 26f9b4a8e6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 175 additions and 99 deletions

View file

@ -33,3 +33,14 @@ with open(p) as fp:
fp.read() fp.read()
open(p).close() open(p).close()
os.getcwdb(p) os.getcwdb(p)
# https://github.com/astral-sh/ruff/issues/7620
def opener(path, flags):
return os.open(path, flags, dir_fd=os.open('somedir', os.O_RDONLY))
open(p, closefd=False)
open(p, opener=opener)
open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
open(p, 'r', - 1, None, None, None, True, None)
open(p, 'r', - 1, None, None, None, False, opener)

View file

@ -863,7 +863,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Rule::OsPathGetctime, Rule::OsPathGetctime,
Rule::Glob, Rule::Glob,
]) { ]) {
flake8_use_pathlib::rules::replaceable_by_pathlib(checker, func); flake8_use_pathlib::rules::replaceable_by_pathlib(checker, call);
} }
if checker.enabled(Rule::PathConstructorCurrentDirectory) { if checker.enabled(Rule::PathConstructorCurrentDirectory) {
flake8_use_pathlib::rules::path_constructor_current_directory(checker, expr, func); flake8_use_pathlib::rules::path_constructor_current_directory(checker, expr, func);

View file

@ -1,5 +1,5 @@
use ruff_diagnostics::{Diagnostic, DiagnosticKind}; use ruff_diagnostics::{Diagnostic, DiagnosticKind};
use ruff_python_ast::Expr; use ruff_python_ast::{Constant, Expr, ExprCall, ExprConstant};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -15,11 +15,11 @@ use crate::rules::flake8_use_pathlib::violations::{
}; };
use crate::settings::types::PythonVersion; use crate::settings::types::PythonVersion;
pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, expr: &Expr) { pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, call: &ExprCall) {
if let Some(diagnostic_kind) = if let Some(diagnostic_kind) =
checker checker
.semantic() .semantic()
.resolve_call_path(expr) .resolve_call_path(&call.func)
.and_then(|call_path| match call_path.as_slice() { .and_then(|call_path| match call_path.as_slice() {
// PTH100 // PTH100
["os", "path", "abspath"] => Some(OsPathAbspath.into()), ["os", "path", "abspath"] => Some(OsPathAbspath.into()),
@ -86,7 +86,51 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, expr: &Expr) {
// PTH205 // PTH205
["os", "path", "getctime"] => Some(OsPathGetctime.into()), ["os", "path", "getctime"] => Some(OsPathGetctime.into()),
// PTH123 // PTH123
["" | "builtin", "open"] => Some(BuiltinOpen.into()), ["" | "builtin", "open"] => {
// `closefd` and `openener` are not supported by pathlib, so check if they are
// are set to non-default values.
// https://github.com/astral-sh/ruff/issues/7620
// Signature as of Python 3.11 (https://docs.python.org/3/library/functions.html#open):
// ```text
// 0 1 2 3 4 5
// open(file, mode='r', buffering=-1, encoding=None, errors=None, newline=None,
// 6 7
// closefd=True, opener=None)
// ^^^^ ^^^^
// ```
// For `pathlib` (https://docs.python.org/3/library/pathlib.html#pathlib.Path.open):
// ```text
// Path.open(mode='r', buffering=-1, encoding=None, errors=None, newline=None)
// ```
if call
.arguments
.find_argument("closefd", 6)
.is_some_and(|expr| {
!matches!(
expr,
Expr::Constant(ExprConstant {
value: Constant::Bool(true),
..
})
)
})
|| call
.arguments
.find_argument("opener", 7)
.is_some_and(|expr| {
!matches!(
expr,
Expr::Constant(ExprConstant {
value: Constant::None,
..
})
)
})
{
return None;
}
Some(BuiltinOpen.into())
}
// PTH124 // PTH124
["py", "path", "local"] => Some(PyPath.into()), ["py", "path", "local"] => Some(PyPath.into()),
// PTH207 // PTH207
@ -110,7 +154,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, expr: &Expr) {
_ => None, _ => None,
}) })
{ {
let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, expr.range()); let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, call.func.range());
if checker.enabled(diagnostic.kind.rule()) { if checker.enabled(diagnostic.kind.rule()) {
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);

View file

@ -275,6 +275,27 @@ full_name.py:35:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()`
34 | open(p).close() 34 | open(p).close()
35 | os.getcwdb(p) 35 | os.getcwdb(p)
| ^^^^^^^^^^ PTH109 | ^^^^^^^^^^ PTH109
36 |
37 | # https://github.com/astral-sh/ruff/issues/7620
|
full_name.py:44:1: PTH123 `open()` should be replaced by `Path.open()`
|
42 | open(p, closefd=False)
43 | open(p, opener=opener)
44 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| ^^^^ PTH123
45 | open(p, 'r', - 1, None, None, None, True, None)
46 | open(p, 'r', - 1, None, None, None, False, opener)
|
full_name.py:45:1: PTH123 `open()` should be replaced by `Path.open()`
|
43 | open(p, opener=opener)
44 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
45 | open(p, 'r', - 1, None, None, None, True, None)
| ^^^^ PTH123
46 | open(p, 'r', - 1, None, None, None, False, opener)
| |

View file

@ -740,7 +740,7 @@ UP015.py:62:1: UP015 [*] Unnecessary open mode parameters
60 | pass 60 | pass
61 | 61 |
62 | open(file="foo", mode='U', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 62 | open(file="foo", mode='U', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U') 63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U')
64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None) 64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None)
| |
@ -760,7 +760,7 @@ UP015.py:63:1: UP015 [*] Unnecessary open mode parameters
| |
62 | open(file="foo", mode='U', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 62 | open(file="foo", mode='U', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U') 63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U')
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None) 64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None)
65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| |
@ -781,7 +781,7 @@ UP015.py:64:1: UP015 [*] Unnecessary open mode parameters
62 | open(file="foo", mode='U', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 62 | open(file="foo", mode='U', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U') 63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U')
64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None) 64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| |
= help: Remove open mode parameters = help: Remove open mode parameters
@ -801,7 +801,7 @@ UP015.py:65:1: UP015 [*] Unnecessary open mode parameters
63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U') 63 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='U')
64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None) 64 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='U', newline=None, closefd=True, opener=None)
65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
66 | 66 |
67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| |
@ -822,7 +822,7 @@ UP015.py:67:1: UP015 [*] Unnecessary open mode parameters, use ""rb""
65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 65 | open(mode='U', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
66 | 66 |
67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub') 68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub')
69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None) 69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None)
| |
@ -842,7 +842,7 @@ UP015.py:68:1: UP015 [*] Unnecessary open mode parameters, use ""rb""
| |
67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub') 68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub')
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None) 69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None)
70 | open(mode='Ub', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 70 | open(mode='Ub', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| |
@ -863,7 +863,7 @@ UP015.py:69:1: UP015 [*] Unnecessary open mode parameters, use ""rb""
67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 67 | open(file="foo", mode='Ub', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub') 68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub')
69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None) 69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
70 | open(mode='Ub', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 70 | open(mode='Ub', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| |
= help: Replace with ""rb"" = help: Replace with ""rb""
@ -883,7 +883,7 @@ UP015.py:70:1: UP015 [*] Unnecessary open mode parameters, use ""rb""
68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub') 68 | open(file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None, mode='Ub')
69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None) 69 | open(file="foo", buffering=-1, encoding=None, errors=None, mode='Ub', newline=None, closefd=True, opener=None)
70 | open(mode='Ub', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) 70 | open(mode='Ub', file="foo", buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP015
71 | 71 |
72 | open = 1 72 | open = 1
| |