From ef4809a2ba9abeb2d9335797076b5e7b07f28326 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Sat, 1 Feb 2025 01:07:08 +0000 Subject: [PATCH 1/6] [`ruff`] IO operations performed on closed IO objects (`RUF061`) --- .../resources/test/fixtures/ruff/RUF061.py | 98 ++++++++ .../src/checkers/ast/analyze/bindings.rs | 6 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + .../ruff/rules/operation_on_closed_io.rs | 171 +++++++++++++ ...uff__tests__preview__RUF061_RUF061.py.snap | 229 ++++++++++++++++++ ruff.schema.json | 2 + 8 files changed, 510 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF061.py create mode 100644 crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF061.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF061.py new file mode 100644 index 00000000000000..468f2a383a29f2 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF061.py @@ -0,0 +1,98 @@ +### Errors + +def exhaustiveness(): + with open() as f: + ... + + f.__iter__() + f.__next__() + f.detach() + f.fileno() + f.flush() + f.isatty() + f.read() + f.readline() + f.readlines() + f.reconfigure() + f.seek() + f.seekable() + f.tell() + f.truncate() + f.writable() + f.write() + f.writelines() + +def contains(): + with open() as f: + ... + + _ = '' in f + _ = '' not in f + _ = '' in f is {} + _ = '' not in f == {} + + +def for_loop(): + with open() as f: + ... + + for _ in f: ... + + +def mode_is_unimportant(): + with open("", "r") as f: + ... + + f.write() + + +def _(): + with open() as f: + ... + + _ = f.name + f.readlines() + + +### No errors + +def non_operations(): + with open() as f: + ... + + _ = f.name + _ = f.line_buffering() + + +def compare_but_not_contains(): + with open() as f: + ... + + _ = a != f + _ = '' is not f not in {} + + +def for_loop_wrapped(): + with open() as f: + ... + + for _ in foo(f): ... + + +def aliasing(): + with open() as f: + ... + + g = f + g.readlines() + + +def multiple(): + with open() as f: + f.read() + + with open() as f: + f.write() + + with open() as f: + f.seek() diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs index 3815ea21e30c26..e70b10237e71e0 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs @@ -24,6 +24,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::PytestUnittestRaisesAssertion, Rule::ForLoopWrites, Rule::CustomTypeVarForSelf, + Rule::OperationOnClosedIO, ]) { return; } @@ -123,5 +124,10 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::OperationOnClosedIO) { + if let Some(mut diagnostics) = ruff::rules::operation_on_closed_io(checker, binding) { + checker.diagnostics.append(&mut diagnostics); + } + } } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index bac27d3205e812..a4fe968475635b 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1008,6 +1008,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "056") => (RuleGroup::Preview, rules::ruff::rules::FalsyDictGetFallback), (Ruff, "057") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRound), (Ruff, "058") => (RuleGroup::Preview, rules::ruff::rules::StarmapZip), + (Ruff, "061") => (RuleGroup::Preview, rules::ruff::rules::OperationOnClosedIO), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 5ed899c945f254..1c0be5c965d21c 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -429,6 +429,7 @@ mod tests { #[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))] #[test_case(Rule::StarmapZip, Path::new("RUF058_0.py"))] #[test_case(Rule::StarmapZip, Path::new("RUF058_1.py"))] + #[test_case(Rule::OperationOnClosedIO, Path::new("RUF061.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 834709136b75bb..e6d6d3e015ecf8 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -24,6 +24,7 @@ pub(crate) use mutable_fromkeys_value::*; pub(crate) use needless_else::*; pub(crate) use never_union::*; pub(crate) use none_not_at_end_of_union::*; +pub(crate) use operation_on_closed_io::*; pub(crate) use parenthesize_chained_operators::*; pub(crate) use post_init_default::*; pub(crate) use pytest_raises_ambiguous_pattern::*; @@ -79,6 +80,7 @@ mod mutable_fromkeys_value; mod needless_else; mod never_union; mod none_not_at_end_of_union; +mod operation_on_closed_io; mod parenthesize_chained_operators; mod post_init_default; mod pytest_raises_ambiguous_pattern; diff --git a/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs new file mode 100644 index 00000000000000..8457d8a46b2e27 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs @@ -0,0 +1,171 @@ +use crate::checkers::ast::Checker; +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::AnyNodeRef; +use ruff_python_semantic::{Binding, BindingKind, NodeId, SemanticModel}; +use ruff_text_size::{Ranged, TextRange}; + +/// ## What it does +/// Checks for usages of IO operation methods of context variables +/// outside of the original `with` statement. +/// +/// ## Why is this bad? +/// Such operations will raise `ValueError: I/O operation on closed file` at runtime. +/// +/// ## Example +/// +/// ```python +/// with open(".txt") as f: +/// f.read() +/// +/// with open(".md", "w"): +/// f.write("") +/// ``` +/// +/// Use instead: +/// +/// ```python +/// with open(".txt") as f: +/// f.read() +/// +/// with open(".md", "w") as f: +/// f.write("") +/// ``` +#[derive(ViolationMetadata)] +pub(crate) struct OperationOnClosedIO; + +impl Violation for OperationOnClosedIO { + #[derive_message_formats] + fn message(&self) -> String { + "IO operation performed on closed IO object".to_string() + } +} + +/// RUF061 +pub(crate) fn operation_on_closed_io( + checker: &Checker, + binding: &Binding, +) -> Option> { + if !matches!(&binding.kind, BindingKind::WithItemVar) { + return None; + } + + let semantic = checker.semantic(); + let with = binding.statement(semantic)?.as_with_stmt()?; + + let mut diagnostics = vec![]; + + for reference_id in binding.references() { + let reference = semantic.reference(reference_id); + + if reference.end() <= with.end() { + continue; + } + + let Some(expression_id) = reference.expression_id() else { + continue; + }; + + let Some(range) = method_reference(expression_id, semantic) + .or_else(|| contains_check(expression_id, semantic)) + .or_else(|| for_loop(expression_id, semantic)) + else { + continue; + }; + + let diagnostic = Diagnostic::new(OperationOnClosedIO, range); + + diagnostics.push(diagnostic); + } + + Some(diagnostics) +} + +/// `f.write(...)` +fn method_reference(expression_id: NodeId, semantic: &SemanticModel) -> Option { + let mut ancestors = semantic.expressions(expression_id); + + let _io_object_ref = ancestors.next()?; + let attribute = ancestors.next()?.as_attribute_expr()?; + + if !is_io_operation_method(&attribute.attr.id) { + return None; + } + + Some(attribute.range) +} + +fn is_io_operation_method(name: &str) -> bool { + matches!( + name, + "__iter__" + | "__next__" + | "detach" + | "fileno" + | "flush" + | "isatty" + | "read" + | "readline" + | "readlines" + | "reconfigure" + | "seek" + | "seekable" + | "tell" + | "truncate" + | "writable" + | "write" + | "writelines" + ) +} + +/// `_ in f` +fn contains_check(expression_id: NodeId, semantic: &SemanticModel) -> Option { + let mut ancestors = semantic.expressions(expression_id); + + let io_object_ref = AnyNodeRef::from(ancestors.next()?); + let compare = ancestors.next()?.as_compare_expr()?; + + compare + .comparators + .iter() + .enumerate() + .find_map(|(index, comparator)| { + if !io_object_ref.ptr_eq(comparator.into()) { + return None; + } + + let op = compare.ops[index]; + + if !op.is_in() && !op.is_not_in() { + return None; + } + + let start = if index == 0 { + compare.left.start() + } else { + compare.comparators[index - 1].start() + }; + let end = comparator.end(); + + Some(TextRange::new(start, end)) + }) +} + +/// `for _ in f: ...` +fn for_loop(expression_id: NodeId, semantic: &SemanticModel) -> Option { + let mut ancestor_statements = semantic.statements(expression_id); + + let io_object_ref = AnyNodeRef::from(semantic.expression(expression_id)?); + + let for_loop = ancestor_statements.next()?.as_for_stmt()?; + let iter = for_loop.iter.as_ref(); + + if !io_object_ref.ptr_eq(iter.into()) { + return None; + } + + let start = for_loop.target.start(); + let end = iter.end(); + + Some(TextRange::new(start, end)) +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap new file mode 100644 index 00000000000000..27119bc85c6910 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap @@ -0,0 +1,229 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF061.py:7:5: RUF061 IO operation performed on closed IO object + | +5 | ... +6 | +7 | f.__iter__() + | ^^^^^^^^^^ RUF061 +8 | f.__next__() +9 | f.detach() + | + +RUF061.py:8:5: RUF061 IO operation performed on closed IO object + | + 7 | f.__iter__() + 8 | f.__next__() + | ^^^^^^^^^^ RUF061 + 9 | f.detach() +10 | f.fileno() + | + +RUF061.py:9:5: RUF061 IO operation performed on closed IO object + | + 7 | f.__iter__() + 8 | f.__next__() + 9 | f.detach() + | ^^^^^^^^ RUF061 +10 | f.fileno() +11 | f.flush() + | + +RUF061.py:10:5: RUF061 IO operation performed on closed IO object + | + 8 | f.__next__() + 9 | f.detach() +10 | f.fileno() + | ^^^^^^^^ RUF061 +11 | f.flush() +12 | f.isatty() + | + +RUF061.py:11:5: RUF061 IO operation performed on closed IO object + | + 9 | f.detach() +10 | f.fileno() +11 | f.flush() + | ^^^^^^^ RUF061 +12 | f.isatty() +13 | f.read() + | + +RUF061.py:12:5: RUF061 IO operation performed on closed IO object + | +10 | f.fileno() +11 | f.flush() +12 | f.isatty() + | ^^^^^^^^ RUF061 +13 | f.read() +14 | f.readline() + | + +RUF061.py:13:5: RUF061 IO operation performed on closed IO object + | +11 | f.flush() +12 | f.isatty() +13 | f.read() + | ^^^^^^ RUF061 +14 | f.readline() +15 | f.readlines() + | + +RUF061.py:14:5: RUF061 IO operation performed on closed IO object + | +12 | f.isatty() +13 | f.read() +14 | f.readline() + | ^^^^^^^^^^ RUF061 +15 | f.readlines() +16 | f.reconfigure() + | + +RUF061.py:15:5: RUF061 IO operation performed on closed IO object + | +13 | f.read() +14 | f.readline() +15 | f.readlines() + | ^^^^^^^^^^^ RUF061 +16 | f.reconfigure() +17 | f.seek() + | + +RUF061.py:16:5: RUF061 IO operation performed on closed IO object + | +14 | f.readline() +15 | f.readlines() +16 | f.reconfigure() + | ^^^^^^^^^^^^^ RUF061 +17 | f.seek() +18 | f.seekable() + | + +RUF061.py:17:5: RUF061 IO operation performed on closed IO object + | +15 | f.readlines() +16 | f.reconfigure() +17 | f.seek() + | ^^^^^^ RUF061 +18 | f.seekable() +19 | f.tell() + | + +RUF061.py:18:5: RUF061 IO operation performed on closed IO object + | +16 | f.reconfigure() +17 | f.seek() +18 | f.seekable() + | ^^^^^^^^^^ RUF061 +19 | f.tell() +20 | f.truncate() + | + +RUF061.py:19:5: RUF061 IO operation performed on closed IO object + | +17 | f.seek() +18 | f.seekable() +19 | f.tell() + | ^^^^^^ RUF061 +20 | f.truncate() +21 | f.writable() + | + +RUF061.py:20:5: RUF061 IO operation performed on closed IO object + | +18 | f.seekable() +19 | f.tell() +20 | f.truncate() + | ^^^^^^^^^^ RUF061 +21 | f.writable() +22 | f.write() + | + +RUF061.py:21:5: RUF061 IO operation performed on closed IO object + | +19 | f.tell() +20 | f.truncate() +21 | f.writable() + | ^^^^^^^^^^ RUF061 +22 | f.write() +23 | f.writelines() + | + +RUF061.py:22:5: RUF061 IO operation performed on closed IO object + | +20 | f.truncate() +21 | f.writable() +22 | f.write() + | ^^^^^^^ RUF061 +23 | f.writelines() + | + +RUF061.py:23:5: RUF061 IO operation performed on closed IO object + | +21 | f.writable() +22 | f.write() +23 | f.writelines() + | ^^^^^^^^^^^^ RUF061 +24 | +25 | def contains(): + | + +RUF061.py:29:9: RUF061 IO operation performed on closed IO object + | +27 | ... +28 | +29 | _ = '' in f + | ^^^^^^^ RUF061 +30 | _ = '' not in f +31 | _ = '' in f is {} + | + +RUF061.py:30:9: RUF061 IO operation performed on closed IO object + | +29 | _ = '' in f +30 | _ = '' not in f + | ^^^^^^^^^^^ RUF061 +31 | _ = '' in f is {} +32 | _ = '' not in f == {} + | + +RUF061.py:31:9: RUF061 IO operation performed on closed IO object + | +29 | _ = '' in f +30 | _ = '' not in f +31 | _ = '' in f is {} + | ^^^^^^^ RUF061 +32 | _ = '' not in f == {} + | + +RUF061.py:32:9: RUF061 IO operation performed on closed IO object + | +30 | _ = '' not in f +31 | _ = '' in f is {} +32 | _ = '' not in f == {} + | ^^^^^^^^^^^ RUF061 + | + +RUF061.py:39:9: RUF061 IO operation performed on closed IO object + | +37 | ... +38 | +39 | for _ in f: ... + | ^^^^^^ RUF061 + | + +RUF061.py:46:5: RUF061 IO operation performed on closed IO object + | +44 | ... +45 | +46 | f.write() + | ^^^^^^^ RUF061 + | + +RUF061.py:54:5: RUF061 IO operation performed on closed IO object + | +53 | _ = f.name +54 | f.readlines() + | ^^^^^^^^^^^ RUF061 + | diff --git a/ruff.schema.json b/ruff.schema.json index 50af727c32d745..093fbcb4f73701 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3957,6 +3957,8 @@ "RUF056", "RUF057", "RUF058", + "RUF06", + "RUF061", "RUF1", "RUF10", "RUF100", From c22bde821cf0212ad9feff9ca7c5208f165994ee Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Mon, 3 Feb 2025 14:49:54 +0000 Subject: [PATCH 2/6] Per review --- .../fixtures/ruff/{RUF061.py => RUF050.py} | 8 ++ crates/ruff_linter/src/codes.rs | 2 +- crates/ruff_linter/src/rules/ruff/mod.rs | 2 +- .../ruff/rules/operation_on_closed_io.rs | 28 ++++-- ...ff__tests__preview__RUF050_RUF050.py.snap} | 96 +++++++++---------- ruff.schema.json | 5 +- 6 files changed, 78 insertions(+), 63 deletions(-) rename crates/ruff_linter/resources/test/fixtures/ruff/{RUF061.py => RUF050.py} (91%) rename crates/ruff_linter/src/rules/ruff/snapshots/{ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap => ruff_linter__rules__ruff__tests__preview__RUF050_RUF050.py.snap} (54%) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF061.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF050.py similarity index 91% rename from crates/ruff_linter/resources/test/fixtures/ruff/RUF061.py rename to crates/ruff_linter/resources/test/fixtures/ruff/RUF050.py index 468f2a383a29f2..960f6ec65c5fcb 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF061.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF050.py @@ -56,6 +56,14 @@ def _(): ### No errors +def non_io(): + from somewhere import Lorem + + with Lorem() as l: + l.write("") + + print(l.read()) + def non_operations(): with open() as f: ... diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index a4fe968475635b..764d859d174ee0 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1002,13 +1002,13 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "047") => (RuleGroup::Preview, rules::ruff::rules::NeedlessElse), (Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing), (Ruff, "049") => (RuleGroup::Preview, rules::ruff::rules::DataclassEnum), + (Ruff, "050") => (RuleGroup::Preview, rules::ruff::rules::OperationOnClosedIO), (Ruff, "051") => (RuleGroup::Preview, rules::ruff::rules::IfKeyInDictDel), (Ruff, "052") => (RuleGroup::Preview, rules::ruff::rules::UsedDummyVariable), (Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression), (Ruff, "056") => (RuleGroup::Preview, rules::ruff::rules::FalsyDictGetFallback), (Ruff, "057") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRound), (Ruff, "058") => (RuleGroup::Preview, rules::ruff::rules::StarmapZip), - (Ruff, "061") => (RuleGroup::Preview, rules::ruff::rules::OperationOnClosedIO), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 1c0be5c965d21c..7ebbab43f7cced 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -88,6 +88,7 @@ mod tests { #[test_case(Rule::FalsyDictGetFallback, Path::new("RUF056.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))] + #[test_case(Rule::OperationOnClosedIO, Path::new("RUF050.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( @@ -429,7 +430,6 @@ mod tests { #[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))] #[test_case(Rule::StarmapZip, Path::new("RUF058_0.py"))] #[test_case(Rule::StarmapZip, Path::new("RUF058_1.py"))] - #[test_case(Rule::OperationOnClosedIO, Path::new("RUF061.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs index 8457d8a46b2e27..17c648aca71a15 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs @@ -2,6 +2,7 @@ use crate::checkers::ast::Checker; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::AnyNodeRef; +use ruff_python_semantic::analyze::typing; use ruff_python_semantic::{Binding, BindingKind, NodeId, SemanticModel}; use ruff_text_size::{Ranged, TextRange}; @@ -41,7 +42,7 @@ impl Violation for OperationOnClosedIO { } } -/// RUF061 +/// RUF050 pub(crate) fn operation_on_closed_io( checker: &Checker, binding: &Binding, @@ -53,6 +54,10 @@ pub(crate) fn operation_on_closed_io( let semantic = checker.semantic(); let with = binding.statement(semantic)?.as_with_stmt()?; + if !typing::is_io_base(binding, semantic) { + return None; + } + let mut diagnostics = vec![]; for reference_id in binding.references() { @@ -66,9 +71,9 @@ pub(crate) fn operation_on_closed_io( continue; }; - let Some(range) = method_reference(expression_id, semantic) - .or_else(|| contains_check(expression_id, semantic)) - .or_else(|| for_loop(expression_id, semantic)) + let Some(range) = method_reference_range(expression_id, semantic) + .or_else(|| contains_check_range(expression_id, semantic)) + .or_else(|| for_loop_target_in_iter_range(expression_id, semantic)) else { continue; }; @@ -81,8 +86,8 @@ pub(crate) fn operation_on_closed_io( Some(diagnostics) } -/// `f.write(...)` -fn method_reference(expression_id: NodeId, semantic: &SemanticModel) -> Option { +// `f.write(...)` +fn method_reference_range(expression_id: NodeId, semantic: &SemanticModel) -> Option { let mut ancestors = semantic.expressions(expression_id); let _io_object_ref = ancestors.next()?; @@ -118,8 +123,8 @@ fn is_io_operation_method(name: &str) -> bool { ) } -/// `_ in f` -fn contains_check(expression_id: NodeId, semantic: &SemanticModel) -> Option { +// `_ in f` +fn contains_check_range(expression_id: NodeId, semantic: &SemanticModel) -> Option { let mut ancestors = semantic.expressions(expression_id); let io_object_ref = AnyNodeRef::from(ancestors.next()?); @@ -151,8 +156,11 @@ fn contains_check(expression_id: NodeId, semantic: &SemanticModel) -> Option Option { +// `for _ in f: ...` +fn for_loop_target_in_iter_range( + expression_id: NodeId, + semantic: &SemanticModel, +) -> Option { let mut ancestor_statements = semantic.statements(expression_id); let io_object_ref = AnyNodeRef::from(semantic.expression(expression_id)?); diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF050_RUF050.py.snap similarity index 54% rename from crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap rename to crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF050_RUF050.py.snap index 27119bc85c6910..ea12794ae20650 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF061_RUF061.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF050_RUF050.py.snap @@ -1,229 +1,229 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs --- -RUF061.py:7:5: RUF061 IO operation performed on closed IO object +RUF050.py:7:5: RUF050 IO operation performed on closed IO object | 5 | ... 6 | 7 | f.__iter__() - | ^^^^^^^^^^ RUF061 + | ^^^^^^^^^^ RUF050 8 | f.__next__() 9 | f.detach() | -RUF061.py:8:5: RUF061 IO operation performed on closed IO object +RUF050.py:8:5: RUF050 IO operation performed on closed IO object | 7 | f.__iter__() 8 | f.__next__() - | ^^^^^^^^^^ RUF061 + | ^^^^^^^^^^ RUF050 9 | f.detach() 10 | f.fileno() | -RUF061.py:9:5: RUF061 IO operation performed on closed IO object +RUF050.py:9:5: RUF050 IO operation performed on closed IO object | 7 | f.__iter__() 8 | f.__next__() 9 | f.detach() - | ^^^^^^^^ RUF061 + | ^^^^^^^^ RUF050 10 | f.fileno() 11 | f.flush() | -RUF061.py:10:5: RUF061 IO operation performed on closed IO object +RUF050.py:10:5: RUF050 IO operation performed on closed IO object | 8 | f.__next__() 9 | f.detach() 10 | f.fileno() - | ^^^^^^^^ RUF061 + | ^^^^^^^^ RUF050 11 | f.flush() 12 | f.isatty() | -RUF061.py:11:5: RUF061 IO operation performed on closed IO object +RUF050.py:11:5: RUF050 IO operation performed on closed IO object | 9 | f.detach() 10 | f.fileno() 11 | f.flush() - | ^^^^^^^ RUF061 + | ^^^^^^^ RUF050 12 | f.isatty() 13 | f.read() | -RUF061.py:12:5: RUF061 IO operation performed on closed IO object +RUF050.py:12:5: RUF050 IO operation performed on closed IO object | 10 | f.fileno() 11 | f.flush() 12 | f.isatty() - | ^^^^^^^^ RUF061 + | ^^^^^^^^ RUF050 13 | f.read() 14 | f.readline() | -RUF061.py:13:5: RUF061 IO operation performed on closed IO object +RUF050.py:13:5: RUF050 IO operation performed on closed IO object | 11 | f.flush() 12 | f.isatty() 13 | f.read() - | ^^^^^^ RUF061 + | ^^^^^^ RUF050 14 | f.readline() 15 | f.readlines() | -RUF061.py:14:5: RUF061 IO operation performed on closed IO object +RUF050.py:14:5: RUF050 IO operation performed on closed IO object | 12 | f.isatty() 13 | f.read() 14 | f.readline() - | ^^^^^^^^^^ RUF061 + | ^^^^^^^^^^ RUF050 15 | f.readlines() 16 | f.reconfigure() | -RUF061.py:15:5: RUF061 IO operation performed on closed IO object +RUF050.py:15:5: RUF050 IO operation performed on closed IO object | 13 | f.read() 14 | f.readline() 15 | f.readlines() - | ^^^^^^^^^^^ RUF061 + | ^^^^^^^^^^^ RUF050 16 | f.reconfigure() 17 | f.seek() | -RUF061.py:16:5: RUF061 IO operation performed on closed IO object +RUF050.py:16:5: RUF050 IO operation performed on closed IO object | 14 | f.readline() 15 | f.readlines() 16 | f.reconfigure() - | ^^^^^^^^^^^^^ RUF061 + | ^^^^^^^^^^^^^ RUF050 17 | f.seek() 18 | f.seekable() | -RUF061.py:17:5: RUF061 IO operation performed on closed IO object +RUF050.py:17:5: RUF050 IO operation performed on closed IO object | 15 | f.readlines() 16 | f.reconfigure() 17 | f.seek() - | ^^^^^^ RUF061 + | ^^^^^^ RUF050 18 | f.seekable() 19 | f.tell() | -RUF061.py:18:5: RUF061 IO operation performed on closed IO object +RUF050.py:18:5: RUF050 IO operation performed on closed IO object | 16 | f.reconfigure() 17 | f.seek() 18 | f.seekable() - | ^^^^^^^^^^ RUF061 + | ^^^^^^^^^^ RUF050 19 | f.tell() 20 | f.truncate() | -RUF061.py:19:5: RUF061 IO operation performed on closed IO object +RUF050.py:19:5: RUF050 IO operation performed on closed IO object | 17 | f.seek() 18 | f.seekable() 19 | f.tell() - | ^^^^^^ RUF061 + | ^^^^^^ RUF050 20 | f.truncate() 21 | f.writable() | -RUF061.py:20:5: RUF061 IO operation performed on closed IO object +RUF050.py:20:5: RUF050 IO operation performed on closed IO object | 18 | f.seekable() 19 | f.tell() 20 | f.truncate() - | ^^^^^^^^^^ RUF061 + | ^^^^^^^^^^ RUF050 21 | f.writable() 22 | f.write() | -RUF061.py:21:5: RUF061 IO operation performed on closed IO object +RUF050.py:21:5: RUF050 IO operation performed on closed IO object | 19 | f.tell() 20 | f.truncate() 21 | f.writable() - | ^^^^^^^^^^ RUF061 + | ^^^^^^^^^^ RUF050 22 | f.write() 23 | f.writelines() | -RUF061.py:22:5: RUF061 IO operation performed on closed IO object +RUF050.py:22:5: RUF050 IO operation performed on closed IO object | 20 | f.truncate() 21 | f.writable() 22 | f.write() - | ^^^^^^^ RUF061 + | ^^^^^^^ RUF050 23 | f.writelines() | -RUF061.py:23:5: RUF061 IO operation performed on closed IO object +RUF050.py:23:5: RUF050 IO operation performed on closed IO object | 21 | f.writable() 22 | f.write() 23 | f.writelines() - | ^^^^^^^^^^^^ RUF061 + | ^^^^^^^^^^^^ RUF050 24 | 25 | def contains(): | -RUF061.py:29:9: RUF061 IO operation performed on closed IO object +RUF050.py:29:9: RUF050 IO operation performed on closed IO object | 27 | ... 28 | 29 | _ = '' in f - | ^^^^^^^ RUF061 + | ^^^^^^^ RUF050 30 | _ = '' not in f 31 | _ = '' in f is {} | -RUF061.py:30:9: RUF061 IO operation performed on closed IO object +RUF050.py:30:9: RUF050 IO operation performed on closed IO object | 29 | _ = '' in f 30 | _ = '' not in f - | ^^^^^^^^^^^ RUF061 + | ^^^^^^^^^^^ RUF050 31 | _ = '' in f is {} 32 | _ = '' not in f == {} | -RUF061.py:31:9: RUF061 IO operation performed on closed IO object +RUF050.py:31:9: RUF050 IO operation performed on closed IO object | 29 | _ = '' in f 30 | _ = '' not in f 31 | _ = '' in f is {} - | ^^^^^^^ RUF061 + | ^^^^^^^ RUF050 32 | _ = '' not in f == {} | -RUF061.py:32:9: RUF061 IO operation performed on closed IO object +RUF050.py:32:9: RUF050 IO operation performed on closed IO object | 30 | _ = '' not in f 31 | _ = '' in f is {} 32 | _ = '' not in f == {} - | ^^^^^^^^^^^ RUF061 + | ^^^^^^^^^^^ RUF050 | -RUF061.py:39:9: RUF061 IO operation performed on closed IO object +RUF050.py:39:9: RUF050 IO operation performed on closed IO object | 37 | ... 38 | 39 | for _ in f: ... - | ^^^^^^ RUF061 + | ^^^^^^ RUF050 | -RUF061.py:46:5: RUF061 IO operation performed on closed IO object +RUF050.py:46:5: RUF050 IO operation performed on closed IO object | 44 | ... 45 | 46 | f.write() - | ^^^^^^^ RUF061 + | ^^^^^^^ RUF050 | -RUF061.py:54:5: RUF061 IO operation performed on closed IO object +RUF050.py:54:5: RUF050 IO operation performed on closed IO object | 53 | _ = f.name 54 | f.readlines() - | ^^^^^^^^^^^ RUF061 + | ^^^^^^^^^^^ RUF050 | diff --git a/ruff.schema.json b/ruff.schema.json index 093fbcb4f73701..8d167b7c7f44c8 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3951,14 +3951,13 @@ "RUF048", "RUF049", "RUF05", + "RUF050", "RUF051", "RUF052", "RUF055", "RUF056", "RUF057", "RUF058", - "RUF06", - "RUF061", "RUF1", "RUF10", "RUF100", @@ -4274,4 +4273,4 @@ ] } } -} \ No newline at end of file +} From 66416ef2b9952fc8d2489a13ea9f9846068a1f84 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Mon, 3 Feb 2025 15:24:16 +0000 Subject: [PATCH 3/6] Fix --- ...nap => ruff_linter__rules__ruff__tests__RUF050_RUF050.py.snap} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename crates/ruff_linter/src/rules/ruff/snapshots/{ruff_linter__rules__ruff__tests__preview__RUF050_RUF050.py.snap => ruff_linter__rules__ruff__tests__RUF050_RUF050.py.snap} (100%) diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF050_RUF050.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF050_RUF050.py.snap similarity index 100% rename from crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF050_RUF050.py.snap rename to crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF050_RUF050.py.snap From 80d45603de03257e563fef01d488892b6515db1b Mon Sep 17 00:00:00 2001 From: InSync Date: Mon, 3 Feb 2025 22:34:17 +0700 Subject: [PATCH 4/6] Fix --- ruff.schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruff.schema.json b/ruff.schema.json index 8d167b7c7f44c8..35e614fa2486a2 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4273,4 +4273,4 @@ ] } } -} +} \ No newline at end of file From f10c65946923e3934bdda6ed76ab015389729af2 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Mon, 3 Feb 2025 16:00:03 +0000 Subject: [PATCH 5/6] Per review --- .../rules/ruff/rules/operation_on_closed_io.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs index 17c648aca71a15..60def95f30d112 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs @@ -71,9 +71,9 @@ pub(crate) fn operation_on_closed_io( continue; }; - let Some(range) = method_reference_range(expression_id, semantic) - .or_else(|| contains_check_range(expression_id, semantic)) - .or_else(|| for_loop_target_in_iter_range(expression_id, semantic)) + let Some(range) = io_method_range(expression_id, semantic) + .or_else(|| in_io_range(expression_id, semantic)) + .or_else(|| for_loop_io_range(expression_id, semantic)) else { continue; }; @@ -87,7 +87,7 @@ pub(crate) fn operation_on_closed_io( } // `f.write(...)` -fn method_reference_range(expression_id: NodeId, semantic: &SemanticModel) -> Option { +fn io_method_range(expression_id: NodeId, semantic: &SemanticModel) -> Option { let mut ancestors = semantic.expressions(expression_id); let _io_object_ref = ancestors.next()?; @@ -124,7 +124,7 @@ fn is_io_operation_method(name: &str) -> bool { } // `_ in f` -fn contains_check_range(expression_id: NodeId, semantic: &SemanticModel) -> Option { +fn in_io_range(expression_id: NodeId, semantic: &SemanticModel) -> Option { let mut ancestors = semantic.expressions(expression_id); let io_object_ref = AnyNodeRef::from(ancestors.next()?); @@ -150,17 +150,13 @@ fn contains_check_range(expression_id: NodeId, semantic: &SemanticModel) -> Opti } else { compare.comparators[index - 1].start() }; - let end = comparator.end(); - Some(TextRange::new(start, end)) + Some(TextRange::new(start, comparator.end())) }) } // `for _ in f: ...` -fn for_loop_target_in_iter_range( - expression_id: NodeId, - semantic: &SemanticModel, -) -> Option { +fn for_loop_io_range(expression_id: NodeId, semantic: &SemanticModel) -> Option { let mut ancestor_statements = semantic.statements(expression_id); let io_object_ref = AnyNodeRef::from(semantic.expression(expression_id)?); From 087b749804ae8e497584743d32513b67d883edea Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Mon, 3 Feb 2025 16:00:55 +0000 Subject: [PATCH 6/6] This too --- .../src/rules/ruff/rules/operation_on_closed_io.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs index 60def95f30d112..b4774ffac368b7 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/operation_on_closed_io.rs @@ -168,8 +168,5 @@ fn for_loop_io_range(expression_id: NodeId, semantic: &SemanticModel) -> Option< return None; } - let start = for_loop.target.start(); - let end = iter.end(); - - Some(TextRange::new(start, end)) + Some(TextRange::new(for_loop.target.start(), iter.end())) }