Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

introduce OpenArgument type

This commit is contained in:
Brent Westbrook 2025-11-19 16:19:39 -05:00
parent 9b38bc776e
commit fd9e0b17cd
No known key found for this signature in database
4 changed files with 74 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
crates/ruff_linter/src/rules/refurb/helpers.rs
View File

@ -120,16 +120,13 @@ impl OpenMode {
pub(super) struct FileOpen<'a> {
/// With item where the open happens, we use it for the reporting range.
pub(super) item: &'a ast::WithItem,
/// Filename expression used as the first argument in `open`, we use it in the diagnostic message.
pub(super) filename: Option<&'a Expr>,
/// The file open mode.
pub(super) mode: OpenMode,
/// The file open keywords.
pub(super) keywords: Vec<&'a ast::Keyword>,
/// We only check `open` operations whose file handles are used exactly once.
pub(super) reference: &'a ResolvedReference,
/// Pathlib Path object used to open the file, if any.
pub(super) path_obj: Option<&'a Expr>,
pub(super) argument: OpenArgument<'a>,
}
impl FileOpen<'_> {
@ -140,6 +137,45 @@ impl FileOpen<'_> {
}
}
#[derive(Debug)]
pub(super) enum OpenArgument<'a> {
/// The filename argument to `open`, e.g. "foo.txt" in:
///
/// ```py
/// f = open("foo.txt")
/// ```
Builtin { filename: &'a Expr },
/// The `Path` receiver of a `pathlib.Path.open` call, e.g. the `p` in the
/// context manager in:
///
/// ```py
/// p = Path("foo.txt")
/// with p.open() as f: ...
/// ```
///
/// or `Path("foo.txt")` in
///
/// ```py
/// with Path("foo.txt").open() as f: ...
/// ```
Pathlib { path: &'a Expr },
}
impl OpenArgument<'_> {
pub(super) fn display<'src>(&self, source: &'src str) -> &'src str {
&source[self.range()]
}
}
impl Ranged for OpenArgument<'_> {
fn range(&self) -> TextRange {
match self {
OpenArgument::Builtin { filename } => filename.range(),
OpenArgument::Pathlib { path } => path.range(),
}
}
}
/// Find and return all `open` operations in the given `with` statement.
pub(super) fn find_file_opens<'a>(
with: &'a ast::StmtWith,
@ -156,16 +192,14 @@ pub(super) fn find_file_opens<'a>(
.collect()
}
#[expect(clippy::too_many_arguments)]
fn resolve_file_open<'a>(
item: &'a ast::WithItem,
with: &'a ast::StmtWith,
semantic: &'a SemanticModel<'a>,
read_mode: bool,
filename: Option<&'a Expr>,
mode: OpenMode,
keywords: Vec<&'a ast::Keyword>,
path_obj: Option<&'a Expr>,
argument: OpenArgument<'a>,
) -> Option<FileOpen<'a>> {
match mode {
OpenMode::ReadText | OpenMode::ReadBytes => {
@ -203,11 +237,10 @@ fn resolve_file_open<'a>(
Some(FileOpen {
item,
filename,
mode,
keywords,
reference,
path_obj,
argument,
})
}
@ -251,10 +284,9 @@ fn find_file_open<'a>(
with,
semantic,
read_mode,
Some(filename),
mode,
keywords,
None,
OpenArgument::Builtin { filename },
)
}
@ -279,15 +311,6 @@ fn find_path_open<'a>(
return None;
}
let attr = func.as_attribute_expr()?;
let filename = if let Expr::Call(path_call) = attr.value.as_ref() {
if path_call.arguments.args.len() == 1 {
path_call.arguments.args.first()
} else {
None
}
} else {
None
};
let mode = if args.is_empty() {
OpenMode::ReadText
} else {
@ -301,10 +324,11 @@ fn find_path_open<'a>(
with,
semantic,
read_mode,
filename,
mode,
keywords,
Some(attr.value.as_ref()),
OpenArgument::Pathlib {
path: attr.value.as_ref(),
},
)
}

16
crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs
View File

@ -10,7 +10,7 @@ use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;
use crate::importer::ImportRequest;
use crate::rules::refurb::helpers::{FileOpen, find_file_opens};
use crate::rules::refurb::helpers::{FileOpen, OpenArgument, find_file_opens};
use crate::{FixAvailability, Violation};
/// ## What it does
@ -114,17 +114,11 @@ impl<'a> Visitor<'a> for ReadMatcher<'a, '_> {
.position(|open| open.is_ref(read_from))
{
let open = self.candidates.remove(open);
let filename_display = if let Some(filename) = open.filename {
self.checker.generator().expr(filename)
} else if let Some(path_obj) = open.path_obj {
self.checker.locator().slice(path_obj.range()).to_string()
} else {
return;
};
let filename_display = open.argument.display(self.checker.source());
let suggestion = make_suggestion(&open, self.checker.generator());
let mut diagnostic = self.checker.report_diagnostic(
ReadWholeFile {
filename: SourceCodeSnippet::from_str(&filename_display),
filename: SourceCodeSnippet::from_str(filename_display),
suggestion: SourceCodeSnippet::from_str(&suggestion),
},
open.item.range(),
@ -190,7 +184,9 @@ fn generate_fix(
if with_stmt.items.len() != 1 {
return None;
}
let filename = open.filename?;
let OpenArgument::Builtin { filename } = open.argument else {
return None;
};
let locator = checker.locator();
let filename_code = locator.slice(filename.range());

41
crates/ruff_linter/src/rules/refurb/rules/write_whole_file.rs
View File

@ -9,7 +9,7 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;
use crate::importer::ImportRequest;
use crate::rules::refurb::helpers::{FileOpen, find_file_opens};
use crate::rules::refurb::helpers::{FileOpen, OpenArgument, find_file_opens};
use crate::{FixAvailability, Locator, Violation};
/// ## What it does
@ -46,7 +46,6 @@ pub(crate) struct WriteWholeFile {
filename: SourceCodeSnippet,
suggestion: SourceCodeSnippet,
is_path_open: bool,
has_filename: bool,
}
impl Violation for WriteWholeFile {
@ -57,29 +56,23 @@ impl Violation for WriteWholeFile {
let filename = self.filename.truncated_display();
let suggestion = self.suggestion.truncated_display();
if self.is_path_open {
if self.has_filename {
format!(
"`Path().open()` followed by `write()` can be replaced by `Path({filename}).{suggestion}`"
)
} else {
format!(
"`Path.open()` followed by `write()` can be replaced by `{filename}.{suggestion}`"
)
}
} else {
format!("`open` and `write` should be replaced by `Path({filename}).{suggestion}`")
}
}
fn fix_title(&self) -> Option<String> {
let formatted = if self.has_filename {
let formatted = if self.is_path_open {
format!(
"Replace with `Path({}).{}`",
"Replace with `{}.{}`",
self.filename.truncated_display(),
self.suggestion.truncated_display(),
)
} else {
format!(
"Replace with `{}.{}`",
"Replace with `Path({}).{}`",
self.filename.truncated_display(),
self.suggestion.truncated_display(),
)
@ -149,27 +142,15 @@ impl<'a> Visitor<'a> for WriteMatcher<'a, '_> {
.position(|open| open.is_ref(write_to))
{
let open = self.candidates.remove(open);
let has_filename: bool;
let filename_display: String;
if self.loop_counter == 0 {
if let Some(filename) = open.filename {
filename_display = self.checker.generator().expr(filename);
has_filename = true;
} else if let Some(path_obj) = open.path_obj {
filename_display =
self.checker.locator().slice(path_obj.range()).to_string();
has_filename = false;
} else {
return;
}
let filename_display = open.argument.display(self.checker.source());
let suggestion = make_suggestion(&open, content, self.checker.locator());
let mut diagnostic = self.checker.report_diagnostic(
WriteWholeFile {
filename: SourceCodeSnippet::from_str(&filename_display),
filename: SourceCodeSnippet::from_str(filename_display),
suggestion: SourceCodeSnippet::from_str(&suggestion),
is_path_open: open.path_obj.is_some(),
has_filename,
is_path_open: matches!(open.argument, OpenArgument::Pathlib { .. }),
},
open.item.range(),
);
@ -243,12 +224,12 @@ fn generate_fix(
)
.ok()?;
let target = if let Some(path_obj) = open.path_obj {
locator.slice(path_obj.range()).to_string()
} else {
let filename = open.filename?;
let target = match open.argument {
OpenArgument::Builtin { filename } => {
let filename_code = locator.slice(filename.range());
format!("{binding}({filename_code})")
}
OpenArgument::Pathlib { path } => locator.slice(path.range()).to_string(),
};
let replacement = format!("{target}.{suggestion}");

12
crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB103_FURB103_1.py.snap
View File

@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB103 [*] `Path().open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test")`
FURB103 [*] `Path.open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test")`
--> FURB103_1.py:3:6
|
1 | from pathlib import Path
@ -20,7 +20,7 @@ help: Replace with `Path("file.txt").write_text("test")`
5 | with Path("file.txt").open("wb") as f:
6 | f.write(b"test")
FURB103 [*] `Path().open()` followed by `write()` can be replaced by `Path("file.txt").write_bytes(b"test")`
FURB103 [*] `Path.open()` followed by `write()` can be replaced by `Path("file.txt").write_bytes(b"test")`
--> FURB103_1.py:6:6
|
4 | f.write("test")
@ -40,7 +40,7 @@ help: Replace with `Path("file.txt").write_bytes(b"test")`
8 | with Path("file.txt").open(mode="w") as f:
9 | f.write("test")
FURB103 [*] `Path().open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test")`
FURB103 [*] `Path.open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test")`
--> FURB103_1.py:9:6
|
7 | f.write(b"test")
@ -60,7 +60,7 @@ help: Replace with `Path("file.txt").write_text("test")`
11 | with Path("file.txt").open("w", encoding="utf8") as f:
12 | f.write("test")
FURB103 [*] `Path().open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test", encoding="utf8")`
FURB103 [*] `Path.open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test", encoding="utf8")`
--> FURB103_1.py:12:6
|
10 | f.write("test")
@ -80,7 +80,7 @@ help: Replace with `Path("file.txt").write_text("test", encoding="utf8")`
14 | with Path("file.txt").open("w", errors="ignore") as f:
15 | f.write("test")
FURB103 [*] `Path().open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test", errors="ignore")`
FURB103 [*] `Path.open()` followed by `write()` can be replaced by `Path("file.txt").write_text("test", errors="ignore")`
--> FURB103_1.py:15:6
|
13 | f.write("test")
@ -100,7 +100,7 @@ help: Replace with `Path("file.txt").write_text("test", errors="ignore")`
17 | with Path(foo()).open("w") as f:
18 | f.write("test")
FURB103 [*] `Path().open()` followed by `write()` can be replaced by `Path(foo()).write_text("test")`
FURB103 [*] `Path.open()` followed by `write()` can be replaced by `Path(foo()).write_text("test")`
--> FURB103_1.py:18:6
|
16 | f.write("test")