Skip to content

Commit 85a408d

Browse files
committed
feat: implement supoort for path.open in furb103
Signed-off-by: 11happy <[email protected]>
1 parent de1a6fb commit 85a408d

File tree

3 files changed

+114
-4
lines changed

3 files changed

+114
-4
lines changed

crates/ruff_linter/resources/test/fixtures/refurb/FURB103.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,4 +152,22 @@ def bar(x):
152152
data = {"price": 100}
153153

154154
with open("test.json", "wb") as f:
155-
f.write(json.dumps(data, indent=4).encode("utf-8"))
155+
f.write(json.dumps(data, indent=4).encode("utf-8"))
156+
157+
with Path("file.txt").open("w") as f:
158+
f.write("test")
159+
160+
with Path("file.txt").open("wb") as f:
161+
f.write(b"test")
162+
163+
with Path("file.txt").open(mode="w") as f:
164+
f.write("test")
165+
166+
with Path("file.txt").open("w", encoding="utf8") as f:
167+
f.write("test")
168+
169+
with Path("file.txt").open("w", errors="ignore") as f:
170+
f.write("test")
171+
172+
with Path(foo()).open("w") as f:
173+
f.write("test")

crates/ruff_linter/src/rules/refurb/helpers.rs

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ pub(super) struct FileOpen<'a> {
127127
pub(super) keywords: Vec<&'a ast::Keyword>,
128128
/// We only check `open` operations whose file handles are used exactly once.
129129
pub(super) reference: &'a ResolvedReference,
130+
/// `Path().open()` call
131+
pub(super) path_obj: Option<&'a Expr>,
130132
}
131133

132134
impl FileOpen<'_> {
@@ -146,7 +148,10 @@ pub(super) fn find_file_opens<'a>(
146148
) -> Vec<FileOpen<'a>> {
147149
with.items
148150
.iter()
149-
.filter_map(|item| find_file_open(item, with, semantic, read_mode, python_version))
151+
.filter_map(|item| {
152+
find_file_open(item, with, semantic, read_mode, python_version)
153+
.or_else(|| find_path_open(item, with, semantic, read_mode, python_version))
154+
})
150155
.collect()
151156
}
152157

@@ -238,6 +243,79 @@ fn find_file_open<'a>(
238243
mode,
239244
keywords,
240245
reference,
246+
path_obj: None,
247+
})
248+
}
249+
250+
fn find_path_open<'a>(
251+
item: &'a ast::WithItem,
252+
with: &'a ast::StmtWith,
253+
semantic: &'a SemanticModel<'a>,
254+
read_mode: bool,
255+
python_version: PythonVersion,
256+
) -> Option<FileOpen<'a>> {
257+
let ast::ExprCall {
258+
func,
259+
arguments: ast::Arguments { args, keywords, .. },
260+
..
261+
} = item.context_expr.as_call_expr()?;
262+
let attr = func.as_attribute_expr()?;
263+
if attr.attr.as_str() != "open" {
264+
return None;
265+
}
266+
let path_call = attr.value.as_call_expr()?;
267+
let path_func = path_call.func.as_name_expr()?;
268+
if path_func.id.as_str() != "Path" {
269+
return None;
270+
}
271+
let filename = path_call.arguments.args.first()?;
272+
let mode = if args.is_empty() {
273+
OpenMode::ReadText
274+
} else {
275+
match_open_mode(args.first()?)?
276+
};
277+
let (keywords, kw_mode) = match_open_keywords(keywords, read_mode, python_version)?;
278+
let mode = kw_mode.unwrap_or(mode);
279+
match mode {
280+
OpenMode::ReadText | OpenMode::ReadBytes => {
281+
if !read_mode {
282+
return None;
283+
}
284+
}
285+
OpenMode::WriteText | OpenMode::WriteBytes => {
286+
if read_mode {
287+
return None;
288+
}
289+
}
290+
}
291+
if matches!(mode, OpenMode::ReadBytes | OpenMode::WriteBytes) && !keywords.is_empty() {
292+
return None;
293+
}
294+
let var = item.optional_vars.as_deref()?.as_name_expr()?;
295+
let scope = semantic.current_scope();
296+
let bindings: Vec<_> = scope.get_all(var.id.as_str()).collect();
297+
let binding = bindings
298+
.iter()
299+
.map(|id| semantic.binding(*id))
300+
.find(|binding| binding.range() == var.range())?;
301+
302+
let references: Vec<&ResolvedReference> = binding
303+
.references
304+
.iter()
305+
.map(|id| semantic.reference(*id))
306+
.filter(|reference| with.range().contains_range(reference.range()))
307+
.collect();
308+
309+
let [reference] = references.as_slice() else {
310+
return None;
311+
};
312+
Some(FileOpen {
313+
item,
314+
filename,
315+
mode,
316+
keywords,
317+
reference,
318+
path_obj: Some(attr.value.as_ref()),
241319
})
242320
}
243321

crates/ruff_linter/src/rules/refurb/rules/write_whole_file.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ use crate::{FixAvailability, Violation};
4747
pub(crate) struct WriteWholeFile {
4848
filename: SourceCodeSnippet,
4949
suggestion: SourceCodeSnippet,
50+
is_path_open: bool,
5051
}
5152

5253
impl Violation for WriteWholeFile {
@@ -56,7 +57,13 @@ impl Violation for WriteWholeFile {
5657
fn message(&self) -> String {
5758
let filename = self.filename.truncated_display();
5859
let suggestion = self.suggestion.truncated_display();
59-
format!("`open` and `write` should be replaced by `Path({filename}).{suggestion}`")
60+
if self.is_path_open {
61+
format!(
62+
"`Path.open()` followed by `write()` can be replaced by `Path({filename}).{suggestion}`"
63+
)
64+
} else {
65+
format!("`open` and `write` should be replaced by `Path({filename}).{suggestion}`")
66+
}
6067
}
6168
fn fix_title(&self) -> Option<String> {
6269
Some(format!(
@@ -137,6 +144,7 @@ impl<'a> Visitor<'a> for WriteMatcher<'a, '_> {
137144
&self.checker.generator().expr(open.filename),
138145
),
139146
suggestion: SourceCodeSnippet::from_str(&suggestion),
147+
is_path_open: open.path_obj.is_some(),
140148
},
141149
open.item.range(),
142150
);
@@ -221,7 +229,13 @@ fn generate_fix(
221229
)
222230
.ok()?;
223231

224-
let replacement = format!("{binding}({filename_code}).{suggestion}");
232+
let target = if let Some(path_obj) = open.path_obj {
233+
locator.slice(path_obj.range()).to_string()
234+
} else {
235+
format!("{binding}({filename_code})")
236+
};
237+
238+
let replacement = format!("{target}.{suggestion}");
225239

226240
let applicability = if checker.comment_ranges().intersects(with_stmt.range()) {
227241
Applicability::Unsafe

0 commit comments

Comments
 (0)