From b73a7227ad88f5bafaeaf13b7818e418a5ec69b6 Mon Sep 17 00:00:00 2001 From: Aleksei Latyshev <alex_700_95@mail.ru> Date: Wed, 7 Feb 2024 21:29:24 +0100 Subject: [PATCH 1/4] [`refurb`] Implement `readlines_in_for` lint (FURB129) --- .../resources/test/fixtures/refurb/FURB129.py | 67 +++++ .../src/checkers/ast/analyze/comprehension.rs | 5 +- .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/refurb/mod.rs | 1 + .../ruff_linter/src/rules/refurb/rules/mod.rs | 2 + .../rules/refurb/rules/readlines_in_for.rs | 71 +++++ ...es__refurb__tests__FURB129_FURB129.py.snap | 242 ++++++++++++++++++ ruff.schema.json | 2 + 9 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py create mode 100644 crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs create mode 100644 crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py new file mode 100644 index 0000000000000..efe099bfee64c --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py @@ -0,0 +1,67 @@ +# Lint +import codecs +import io +from pathlib import Path + +with open('FURB129.py') as f: + for _line in f.readlines(): + pass + a = [line.lower() for line in f.readlines()] + b = {line.upper() for line in f.readlines()} + c = {line.lower(): line.upper() for line in f.readlines()} + +with Path('FURB129.py').open() as f: + for _line in f.readlines(): + pass + +for _line in open('FURB129.py').readlines(): + pass + +for _line in Path('FURB129.py').open().readlines(): + pass + + +def good1(): + f = Path('FURB129.py').open() + for _line in f.readlines(): + pass + f.close() + + +def good2(f: io.BytesIO): + for _line in f.readlines(): + pass + + +# False positives +def bad(f): + for _line in f.readlines(): + pass + + +def worse(f: codecs.StreamReader): + for _line in f.readlines(): + pass + + +def foo(): + class A: + def readlines(self) -> list[str]: + return ["a", "b", "c"] + + return A() + + +for _line in foo().readlines(): + pass + +# OK +for _line in ["a", "b", "c"]: + pass +with open('FURB129.py') as f: + for _line in f: + pass + for _line in f.readlines(10): + pass + for _not_line in f.readline(): + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/comprehension.rs b/crates/ruff_linter/src/checkers/ast/analyze/comprehension.rs index c3d368be2954a..821e62c283597 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/comprehension.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/comprehension.rs @@ -2,11 +2,14 @@ use ruff_python_ast::Comprehension; use crate::checkers::ast::Checker; use crate::codes::Rule; -use crate::rules::flake8_simplify; +use crate::rules::{flake8_simplify, refurb}; /// Run lint rules over a [`Comprehension`] syntax nodes. pub(crate) fn comprehension(comprehension: &Comprehension, checker: &mut Checker) { if checker.enabled(Rule::InDictKeys) { flake8_simplify::rules::key_in_dict_comprehension(checker, comprehension); } + if checker.enabled(Rule::ReadlinesInFor) { + refurb::rules::readlines_in_comprehension(checker, comprehension); + } } diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 3ceac945740fc..e8476a8331edb 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1312,6 +1312,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryDictIndexLookup) { pylint::rules::unnecessary_dict_index_lookup(checker, for_stmt); } + if checker.enabled(Rule::ReadlinesInFor) { + refurb::rules::readlines_in_for(checker, for_stmt); + } if !is_async { if checker.enabled(Rule::ReimplementedBuiltin) { flake8_simplify::rules::convert_for_loop_to_any_all(checker, stmt); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ac97ac50ba1f2..76ef18b48674f 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1019,6 +1019,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { #[allow(deprecated)] (Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend), (Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator), + (Refurb, "129") => (RuleGroup::Preview, rules::refurb::rules::ReadlinesInFor), #[allow(deprecated)] (Refurb, "131") => (RuleGroup::Nursery, rules::refurb::rules::DeleteFullSlice), #[allow(deprecated)] diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 26ba58041933b..de5f0bdde06dd 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -17,6 +17,7 @@ mod tests { #[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))] #[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))] #[test_case(Rule::ReimplementedOperator, Path::new("FURB118.py"))] + #[test_case(Rule::ReadlinesInFor, Path::new("FURB129.py"))] #[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))] #[test_case(Rule::CheckAndRemoveFromSet, Path::new("FURB132.py"))] #[test_case(Rule::IfExprMinMax, Path::new("FURB136.py"))] diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index 532003ee143df..a69a798cf5ec8 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -9,6 +9,7 @@ pub(crate) use math_constant::*; pub(crate) use metaclass_abcmeta::*; pub(crate) use print_empty_string::*; pub(crate) use read_whole_file::*; +pub(crate) use readlines_in_for::*; pub(crate) use redundant_log_base::*; pub(crate) use regex_flag_alias::*; pub(crate) use reimplemented_operator::*; @@ -30,6 +31,7 @@ mod math_constant; mod metaclass_abcmeta; mod print_empty_string; mod read_whole_file; +mod readlines_in_for; mod redundant_log_base; mod regex_flag_alias; mod reimplemented_operator; diff --git a/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs b/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs new file mode 100644 index 0000000000000..42981237cff1c --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs @@ -0,0 +1,71 @@ +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{Comprehension, Expr, StmtFor}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for uses of `readlines()` when iterating a file object line-by-line. +/// +/// ## Why is this bad? +/// Instead of iterating through `list[str]` which is returned from `readlines()`, use the iteration +/// through a file object which is a more convenient and performant way. +/// +/// ## Example +/// ```python +/// with open("file.txt") as f: +/// for line in f.readlines(): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// with open("file.txt") as f: +/// for line in f: +/// ... +/// ``` +/// +/// ## References +/// - [Python documentation: `io.IOBase.readlines`](https://docs.python.org/3/library/io.html#io.IOBase.readlines) +/// - [Python documentation: methods of file objects](https://docs.python.org/3/tutorial/inputoutput.html#methods-of-file-objects) +/// +#[violation] +pub(crate) struct ReadlinesInFor; + +impl AlwaysFixableViolation for ReadlinesInFor { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use of readlines() in for loop") + } + + fn fix_title(&self) -> String { + "Remove readlines()".into() + } +} + +fn readlines_in_iter(checker: &mut Checker, iter_expr: &Expr) -> Option<()> { + let expr_call = iter_expr.as_call_expr()?; + let attr_expr = expr_call.func.as_attribute_expr()?; + (attr_expr.attr.as_str() == "readlines" && expr_call.arguments.is_empty()).then(|| { + checker + .diagnostics + .push( + Diagnostic::new(ReadlinesInFor, expr_call.range()).with_fix(Fix::unsafe_edit( + Edit::range_deletion( + expr_call.range().add_start(attr_expr.value.range().len()), + ), + )), + ); + }) +} + +// FURB129 +pub(crate) fn readlines_in_for(checker: &mut Checker, for_stmt: &StmtFor) { + readlines_in_iter(checker, for_stmt.iter.as_ref()); +} + +// FURB129 +pub(crate) fn readlines_in_comprehension(checker: &mut Checker, comprehension: &Comprehension) { + readlines_in_iter(checker, &comprehension.iter); +} diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap new file mode 100644 index 0000000000000..23da81d71d7c5 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap @@ -0,0 +1,242 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB129.py:7:18: FURB129 [*] Use of readlines() in for loop + | +6 | with open('FURB129.py') as f: +7 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +8 | pass +9 | a = [line.lower() for line in f.readlines()] + | + = help: Remove readlines() + +ℹ Unsafe fix +4 4 | from pathlib import Path +5 5 | +6 6 | with open('FURB129.py') as f: +7 |- for _line in f.readlines(): + 7 |+ for _line in f: +8 8 | pass +9 9 | a = [line.lower() for line in f.readlines()] +10 10 | b = {line.upper() for line in f.readlines()} + +FURB129.py:9:35: FURB129 [*] Use of readlines() in for loop + | + 7 | for _line in f.readlines(): + 8 | pass + 9 | a = [line.lower() for line in f.readlines()] + | ^^^^^^^^^^^^^ FURB129 +10 | b = {line.upper() for line in f.readlines()} +11 | c = {line.lower(): line.upper() for line in f.readlines()} + | + = help: Remove readlines() + +ℹ Unsafe fix +6 6 | with open('FURB129.py') as f: +7 7 | for _line in f.readlines(): +8 8 | pass +9 |- a = [line.lower() for line in f.readlines()] + 9 |+ a = [line.lower() for line in f] +10 10 | b = {line.upper() for line in f.readlines()} +11 11 | c = {line.lower(): line.upper() for line in f.readlines()} +12 12 | + +FURB129.py:10:35: FURB129 [*] Use of readlines() in for loop + | + 8 | pass + 9 | a = [line.lower() for line in f.readlines()] +10 | b = {line.upper() for line in f.readlines()} + | ^^^^^^^^^^^^^ FURB129 +11 | c = {line.lower(): line.upper() for line in f.readlines()} + | + = help: Remove readlines() + +ℹ Unsafe fix +7 7 | for _line in f.readlines(): +8 8 | pass +9 9 | a = [line.lower() for line in f.readlines()] +10 |- b = {line.upper() for line in f.readlines()} + 10 |+ b = {line.upper() for line in f} +11 11 | c = {line.lower(): line.upper() for line in f.readlines()} +12 12 | +13 13 | with Path('FURB129.py').open() as f: + +FURB129.py:11:49: FURB129 [*] Use of readlines() in for loop + | + 9 | a = [line.lower() for line in f.readlines()] +10 | b = {line.upper() for line in f.readlines()} +11 | c = {line.lower(): line.upper() for line in f.readlines()} + | ^^^^^^^^^^^^^ FURB129 +12 | +13 | with Path('FURB129.py').open() as f: + | + = help: Remove readlines() + +ℹ Unsafe fix +8 8 | pass +9 9 | a = [line.lower() for line in f.readlines()] +10 10 | b = {line.upper() for line in f.readlines()} +11 |- c = {line.lower(): line.upper() for line in f.readlines()} + 11 |+ c = {line.lower(): line.upper() for line in f} +12 12 | +13 13 | with Path('FURB129.py').open() as f: +14 14 | for _line in f.readlines(): + +FURB129.py:14:18: FURB129 [*] Use of readlines() in for loop + | +13 | with Path('FURB129.py').open() as f: +14 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +15 | pass + | + = help: Remove readlines() + +ℹ Unsafe fix +11 11 | c = {line.lower(): line.upper() for line in f.readlines()} +12 12 | +13 13 | with Path('FURB129.py').open() as f: +14 |- for _line in f.readlines(): + 14 |+ for _line in f: +15 15 | pass +16 16 | +17 17 | for _line in open('FURB129.py').readlines(): + +FURB129.py:17:14: FURB129 [*] Use of readlines() in for loop + | +15 | pass +16 | +17 | for _line in open('FURB129.py').readlines(): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 +18 | pass + | + = help: Remove readlines() + +ℹ Unsafe fix +14 14 | for _line in f.readlines(): +15 15 | pass +16 16 | +17 |-for _line in open('FURB129.py').readlines(): + 17 |+for _line in open('FURB129.py'): +18 18 | pass +19 19 | +20 20 | for _line in Path('FURB129.py').open().readlines(): + +FURB129.py:20:14: FURB129 [*] Use of readlines() in for loop + | +18 | pass +19 | +20 | for _line in Path('FURB129.py').open().readlines(): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 +21 | pass + | + = help: Remove readlines() + +ℹ Unsafe fix +17 17 | for _line in open('FURB129.py').readlines(): +18 18 | pass +19 19 | +20 |-for _line in Path('FURB129.py').open().readlines(): + 20 |+for _line in Path('FURB129.py').open(): +21 21 | pass +22 22 | +23 23 | + +FURB129.py:26:18: FURB129 [*] Use of readlines() in for loop + | +24 | def good1(): +25 | f = Path('FURB129.py').open() +26 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +27 | pass +28 | f.close() + | + = help: Remove readlines() + +ℹ Unsafe fix +23 23 | +24 24 | def good1(): +25 25 | f = Path('FURB129.py').open() +26 |- for _line in f.readlines(): + 26 |+ for _line in f: +27 27 | pass +28 28 | f.close() +29 29 | + +FURB129.py:32:18: FURB129 [*] Use of readlines() in for loop + | +31 | def good2(f: io.BytesIO): +32 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +33 | pass + | + = help: Remove readlines() + +ℹ Unsafe fix +29 29 | +30 30 | +31 31 | def good2(f: io.BytesIO): +32 |- for _line in f.readlines(): + 32 |+ for _line in f: +33 33 | pass +34 34 | +35 35 | + +FURB129.py:38:18: FURB129 [*] Use of readlines() in for loop + | +36 | # False positives +37 | def bad(f): +38 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +39 | pass + | + = help: Remove readlines() + +ℹ Unsafe fix +35 35 | +36 36 | # False positives +37 37 | def bad(f): +38 |- for _line in f.readlines(): + 38 |+ for _line in f: +39 39 | pass +40 40 | +41 41 | + +FURB129.py:43:18: FURB129 [*] Use of readlines() in for loop + | +42 | def worse(f: codecs.StreamReader): +43 | for _line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +44 | pass + | + = help: Remove readlines() + +ℹ Unsafe fix +40 40 | +41 41 | +42 42 | def worse(f: codecs.StreamReader): +43 |- for _line in f.readlines(): + 43 |+ for _line in f: +44 44 | pass +45 45 | +46 46 | + +FURB129.py:55:14: FURB129 [*] Use of readlines() in for loop + | +55 | for _line in foo().readlines(): + | ^^^^^^^^^^^^^^^^^ FURB129 +56 | pass + | + = help: Remove readlines() + +ℹ Unsafe fix +52 52 | return A() +53 53 | +54 54 | +55 |-for _line in foo().readlines(): + 55 |+for _line in foo(): +56 56 | pass +57 57 | +58 58 | # OK + + diff --git a/ruff.schema.json b/ruff.schema.json index c5c9a126a985b..67b451f1a5511 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2980,6 +2980,8 @@ "FURB11", "FURB113", "FURB118", + "FURB12", + "FURB129", "FURB13", "FURB131", "FURB132", From a3d8ee66140b2c50e68d09c2b667995c318d8871 Mon Sep 17 00:00:00 2001 From: Charlie Marsh <charlie.r.marsh@gmail.com> Date: Mon, 12 Feb 2024 21:16:32 -0500 Subject: [PATCH 2/4] Format fixture --- .../resources/test/fixtures/refurb/FURB129.py | 14 +++--- ...es__refurb__tests__FURB129_FURB129.py.snap | 43 ++++++++++--------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py index efe099bfee64c..1f42b1a10bfd0 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py @@ -1,28 +1,28 @@ -# Lint import codecs import io from pathlib import Path -with open('FURB129.py') as f: +# Errors +with open("FURB129.py") as f: for _line in f.readlines(): pass a = [line.lower() for line in f.readlines()] b = {line.upper() for line in f.readlines()} c = {line.lower(): line.upper() for line in f.readlines()} -with Path('FURB129.py').open() as f: +with Path("FURB129.py").open() as f: for _line in f.readlines(): pass -for _line in open('FURB129.py').readlines(): +for _line in open("FURB129.py").readlines(): pass -for _line in Path('FURB129.py').open().readlines(): +for _line in Path("FURB129.py").open().readlines(): pass def good1(): - f = Path('FURB129.py').open() + f = Path("FURB129.py").open() for _line in f.readlines(): pass f.close() @@ -58,7 +58,7 @@ def readlines(self) -> list[str]: # OK for _line in ["a", "b", "c"]: pass -with open('FURB129.py') as f: +with open("FURB129.py") as f: for _line in f: pass for _line in f.readlines(10): diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap index 23da81d71d7c5..b0879739607b9 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap @@ -3,7 +3,8 @@ source: crates/ruff_linter/src/rules/refurb/mod.rs --- FURB129.py:7:18: FURB129 [*] Use of readlines() in for loop | -6 | with open('FURB129.py') as f: +5 | # Errors +6 | with open("FURB129.py") as f: 7 | for _line in f.readlines(): | ^^^^^^^^^^^^^ FURB129 8 | pass @@ -12,9 +13,9 @@ FURB129.py:7:18: FURB129 [*] Use of readlines() in for loop = help: Remove readlines() ℹ Unsafe fix -4 4 | from pathlib import Path -5 5 | -6 6 | with open('FURB129.py') as f: +4 4 | +5 5 | # Errors +6 6 | with open("FURB129.py") as f: 7 |- for _line in f.readlines(): 7 |+ for _line in f: 8 8 | pass @@ -33,7 +34,7 @@ FURB129.py:9:35: FURB129 [*] Use of readlines() in for loop = help: Remove readlines() ℹ Unsafe fix -6 6 | with open('FURB129.py') as f: +6 6 | with open("FURB129.py") as f: 7 7 | for _line in f.readlines(): 8 8 | pass 9 |- a = [line.lower() for line in f.readlines()] @@ -60,7 +61,7 @@ FURB129.py:10:35: FURB129 [*] Use of readlines() in for loop 10 |+ b = {line.upper() for line in f} 11 11 | c = {line.lower(): line.upper() for line in f.readlines()} 12 12 | -13 13 | with Path('FURB129.py').open() as f: +13 13 | with Path("FURB129.py").open() as f: FURB129.py:11:49: FURB129 [*] Use of readlines() in for loop | @@ -69,7 +70,7 @@ FURB129.py:11:49: FURB129 [*] Use of readlines() in for loop 11 | c = {line.lower(): line.upper() for line in f.readlines()} | ^^^^^^^^^^^^^ FURB129 12 | -13 | with Path('FURB129.py').open() as f: +13 | with Path("FURB129.py").open() as f: | = help: Remove readlines() @@ -80,12 +81,12 @@ FURB129.py:11:49: FURB129 [*] Use of readlines() in for loop 11 |- c = {line.lower(): line.upper() for line in f.readlines()} 11 |+ c = {line.lower(): line.upper() for line in f} 12 12 | -13 13 | with Path('FURB129.py').open() as f: +13 13 | with Path("FURB129.py").open() as f: 14 14 | for _line in f.readlines(): FURB129.py:14:18: FURB129 [*] Use of readlines() in for loop | -13 | with Path('FURB129.py').open() as f: +13 | with Path("FURB129.py").open() as f: 14 | for _line in f.readlines(): | ^^^^^^^^^^^^^ FURB129 15 | pass @@ -95,18 +96,18 @@ FURB129.py:14:18: FURB129 [*] Use of readlines() in for loop ℹ Unsafe fix 11 11 | c = {line.lower(): line.upper() for line in f.readlines()} 12 12 | -13 13 | with Path('FURB129.py').open() as f: +13 13 | with Path("FURB129.py").open() as f: 14 |- for _line in f.readlines(): 14 |+ for _line in f: 15 15 | pass 16 16 | -17 17 | for _line in open('FURB129.py').readlines(): +17 17 | for _line in open("FURB129.py").readlines(): FURB129.py:17:14: FURB129 [*] Use of readlines() in for loop | 15 | pass 16 | -17 | for _line in open('FURB129.py').readlines(): +17 | for _line in open("FURB129.py").readlines(): | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 18 | pass | @@ -116,28 +117,28 @@ FURB129.py:17:14: FURB129 [*] Use of readlines() in for loop 14 14 | for _line in f.readlines(): 15 15 | pass 16 16 | -17 |-for _line in open('FURB129.py').readlines(): - 17 |+for _line in open('FURB129.py'): +17 |-for _line in open("FURB129.py").readlines(): + 17 |+for _line in open("FURB129.py"): 18 18 | pass 19 19 | -20 20 | for _line in Path('FURB129.py').open().readlines(): +20 20 | for _line in Path("FURB129.py").open().readlines(): FURB129.py:20:14: FURB129 [*] Use of readlines() in for loop | 18 | pass 19 | -20 | for _line in Path('FURB129.py').open().readlines(): +20 | for _line in Path("FURB129.py").open().readlines(): | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 21 | pass | = help: Remove readlines() ℹ Unsafe fix -17 17 | for _line in open('FURB129.py').readlines(): +17 17 | for _line in open("FURB129.py").readlines(): 18 18 | pass 19 19 | -20 |-for _line in Path('FURB129.py').open().readlines(): - 20 |+for _line in Path('FURB129.py').open(): +20 |-for _line in Path("FURB129.py").open().readlines(): + 20 |+for _line in Path("FURB129.py").open(): 21 21 | pass 22 22 | 23 23 | @@ -145,7 +146,7 @@ FURB129.py:20:14: FURB129 [*] Use of readlines() in for loop FURB129.py:26:18: FURB129 [*] Use of readlines() in for loop | 24 | def good1(): -25 | f = Path('FURB129.py').open() +25 | f = Path("FURB129.py").open() 26 | for _line in f.readlines(): | ^^^^^^^^^^^^^ FURB129 27 | pass @@ -156,7 +157,7 @@ FURB129.py:26:18: FURB129 [*] Use of readlines() in for loop ℹ Unsafe fix 23 23 | 24 24 | def good1(): -25 25 | f = Path('FURB129.py').open() +25 25 | f = Path("FURB129.py").open() 26 |- for _line in f.readlines(): 26 |+ for _line in f: 27 27 | pass From d9e6d77d255b370185f3adf4b47973cc04970f2c Mon Sep 17 00:00:00 2001 From: Charlie Marsh <charlie.r.marsh@gmail.com> Date: Mon, 12 Feb 2024 21:20:32 -0500 Subject: [PATCH 3/4] Quote function --- .../rules/refurb/rules/readlines_in_for.rs | 42 ++++++++-------- ...es__refurb__tests__FURB129_FURB129.py.snap | 48 +++++++++---------- 2 files changed, 46 insertions(+), 44 deletions(-) diff --git a/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs b/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs index 42981237cff1c..9911e9a921078 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs @@ -36,36 +36,38 @@ pub(crate) struct ReadlinesInFor; impl AlwaysFixableViolation for ReadlinesInFor { #[derive_message_formats] fn message(&self) -> String { - format!("Use of readlines() in for loop") + format!("Use of `readlines()` in loop") } fn fix_title(&self) -> String { - "Remove readlines()".into() + "Remove `readlines()`".into() } } -fn readlines_in_iter(checker: &mut Checker, iter_expr: &Expr) -> Option<()> { - let expr_call = iter_expr.as_call_expr()?; - let attr_expr = expr_call.func.as_attribute_expr()?; - (attr_expr.attr.as_str() == "readlines" && expr_call.arguments.is_empty()).then(|| { - checker - .diagnostics - .push( - Diagnostic::new(ReadlinesInFor, expr_call.range()).with_fix(Fix::unsafe_edit( - Edit::range_deletion( - expr_call.range().add_start(attr_expr.value.range().len()), - ), - )), - ); - }) -} - -// FURB129 +/// FURB129 pub(crate) fn readlines_in_for(checker: &mut Checker, for_stmt: &StmtFor) { readlines_in_iter(checker, for_stmt.iter.as_ref()); } -// FURB129 +/// FURB129 pub(crate) fn readlines_in_comprehension(checker: &mut Checker, comprehension: &Comprehension) { readlines_in_iter(checker, &comprehension.iter); } + +fn readlines_in_iter(checker: &mut Checker, iter_expr: &Expr) { + let Expr::Call(expr_call) = iter_expr else { + return; + }; + + let Expr::Attribute(expr_attr) = expr_call.func.as_ref() else { + return; + }; + + if expr_attr.attr.as_str() == "readlines" && expr_call.arguments.is_empty() { + let mut diagnostic = Diagnostic::new(ReadlinesInFor, expr_call.range()); + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_deletion( + expr_call.range().add_start(expr_attr.value.range().len()), + ))); + checker.diagnostics.push(diagnostic); + } +} diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap index b0879739607b9..22f0a60abcf73 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/refurb/mod.rs --- -FURB129.py:7:18: FURB129 [*] Use of readlines() in for loop +FURB129.py:7:18: FURB129 [*] Use of `readlines()` in loop | 5 | # Errors 6 | with open("FURB129.py") as f: @@ -10,7 +10,7 @@ FURB129.py:7:18: FURB129 [*] Use of readlines() in for loop 8 | pass 9 | a = [line.lower() for line in f.readlines()] | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 4 4 | @@ -22,7 +22,7 @@ FURB129.py:7:18: FURB129 [*] Use of readlines() in for loop 9 9 | a = [line.lower() for line in f.readlines()] 10 10 | b = {line.upper() for line in f.readlines()} -FURB129.py:9:35: FURB129 [*] Use of readlines() in for loop +FURB129.py:9:35: FURB129 [*] Use of `readlines()` in loop | 7 | for _line in f.readlines(): 8 | pass @@ -31,7 +31,7 @@ FURB129.py:9:35: FURB129 [*] Use of readlines() in for loop 10 | b = {line.upper() for line in f.readlines()} 11 | c = {line.lower(): line.upper() for line in f.readlines()} | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 6 6 | with open("FURB129.py") as f: @@ -43,7 +43,7 @@ FURB129.py:9:35: FURB129 [*] Use of readlines() in for loop 11 11 | c = {line.lower(): line.upper() for line in f.readlines()} 12 12 | -FURB129.py:10:35: FURB129 [*] Use of readlines() in for loop +FURB129.py:10:35: FURB129 [*] Use of `readlines()` in loop | 8 | pass 9 | a = [line.lower() for line in f.readlines()] @@ -51,7 +51,7 @@ FURB129.py:10:35: FURB129 [*] Use of readlines() in for loop | ^^^^^^^^^^^^^ FURB129 11 | c = {line.lower(): line.upper() for line in f.readlines()} | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 7 7 | for _line in f.readlines(): @@ -63,7 +63,7 @@ FURB129.py:10:35: FURB129 [*] Use of readlines() in for loop 12 12 | 13 13 | with Path("FURB129.py").open() as f: -FURB129.py:11:49: FURB129 [*] Use of readlines() in for loop +FURB129.py:11:49: FURB129 [*] Use of `readlines()` in loop | 9 | a = [line.lower() for line in f.readlines()] 10 | b = {line.upper() for line in f.readlines()} @@ -72,7 +72,7 @@ FURB129.py:11:49: FURB129 [*] Use of readlines() in for loop 12 | 13 | with Path("FURB129.py").open() as f: | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 8 8 | pass @@ -84,14 +84,14 @@ FURB129.py:11:49: FURB129 [*] Use of readlines() in for loop 13 13 | with Path("FURB129.py").open() as f: 14 14 | for _line in f.readlines(): -FURB129.py:14:18: FURB129 [*] Use of readlines() in for loop +FURB129.py:14:18: FURB129 [*] Use of `readlines()` in loop | 13 | with Path("FURB129.py").open() as f: 14 | for _line in f.readlines(): | ^^^^^^^^^^^^^ FURB129 15 | pass | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 11 11 | c = {line.lower(): line.upper() for line in f.readlines()} @@ -103,7 +103,7 @@ FURB129.py:14:18: FURB129 [*] Use of readlines() in for loop 16 16 | 17 17 | for _line in open("FURB129.py").readlines(): -FURB129.py:17:14: FURB129 [*] Use of readlines() in for loop +FURB129.py:17:14: FURB129 [*] Use of `readlines()` in loop | 15 | pass 16 | @@ -111,7 +111,7 @@ FURB129.py:17:14: FURB129 [*] Use of readlines() in for loop | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 18 | pass | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 14 14 | for _line in f.readlines(): @@ -123,7 +123,7 @@ FURB129.py:17:14: FURB129 [*] Use of readlines() in for loop 19 19 | 20 20 | for _line in Path("FURB129.py").open().readlines(): -FURB129.py:20:14: FURB129 [*] Use of readlines() in for loop +FURB129.py:20:14: FURB129 [*] Use of `readlines()` in loop | 18 | pass 19 | @@ -131,7 +131,7 @@ FURB129.py:20:14: FURB129 [*] Use of readlines() in for loop | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB129 21 | pass | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 17 17 | for _line in open("FURB129.py").readlines(): @@ -143,7 +143,7 @@ FURB129.py:20:14: FURB129 [*] Use of readlines() in for loop 22 22 | 23 23 | -FURB129.py:26:18: FURB129 [*] Use of readlines() in for loop +FURB129.py:26:18: FURB129 [*] Use of `readlines()` in loop | 24 | def good1(): 25 | f = Path("FURB129.py").open() @@ -152,7 +152,7 @@ FURB129.py:26:18: FURB129 [*] Use of readlines() in for loop 27 | pass 28 | f.close() | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 23 23 | @@ -164,14 +164,14 @@ FURB129.py:26:18: FURB129 [*] Use of readlines() in for loop 28 28 | f.close() 29 29 | -FURB129.py:32:18: FURB129 [*] Use of readlines() in for loop +FURB129.py:32:18: FURB129 [*] Use of `readlines()` in loop | 31 | def good2(f: io.BytesIO): 32 | for _line in f.readlines(): | ^^^^^^^^^^^^^ FURB129 33 | pass | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 29 29 | @@ -183,7 +183,7 @@ FURB129.py:32:18: FURB129 [*] Use of readlines() in for loop 34 34 | 35 35 | -FURB129.py:38:18: FURB129 [*] Use of readlines() in for loop +FURB129.py:38:18: FURB129 [*] Use of `readlines()` in loop | 36 | # False positives 37 | def bad(f): @@ -191,7 +191,7 @@ FURB129.py:38:18: FURB129 [*] Use of readlines() in for loop | ^^^^^^^^^^^^^ FURB129 39 | pass | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 35 35 | @@ -203,14 +203,14 @@ FURB129.py:38:18: FURB129 [*] Use of readlines() in for loop 40 40 | 41 41 | -FURB129.py:43:18: FURB129 [*] Use of readlines() in for loop +FURB129.py:43:18: FURB129 [*] Use of `readlines()` in loop | 42 | def worse(f: codecs.StreamReader): 43 | for _line in f.readlines(): | ^^^^^^^^^^^^^ FURB129 44 | pass | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 40 40 | @@ -222,13 +222,13 @@ FURB129.py:43:18: FURB129 [*] Use of readlines() in for loop 45 45 | 46 46 | -FURB129.py:55:14: FURB129 [*] Use of readlines() in for loop +FURB129.py:55:14: FURB129 [*] Use of `readlines()` in loop | 55 | for _line in foo().readlines(): | ^^^^^^^^^^^^^^^^^ FURB129 56 | pass | - = help: Remove readlines() + = help: Remove `readlines()` ℹ Unsafe fix 52 52 | return A() From 0bd61b14db2459eee179341ee69b03c11f4a0deb Mon Sep 17 00:00:00 2001 From: Charlie Marsh <charlie.r.marsh@gmail.com> Date: Mon, 12 Feb 2024 22:08:42 -0500 Subject: [PATCH 4/4] Add type inference --- .../rules/refurb/rules/readlines_in_for.rs | 51 ++++--- ...es__refurb__tests__FURB129_FURB129.py.snap | 75 ++-------- .../src/analyze/typing.rs | 132 +++++++++++++++++- 3 files changed, 170 insertions(+), 88 deletions(-) diff --git a/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs b/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs index 9911e9a921078..e548b499844df 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs @@ -1,42 +1,43 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{Comprehension, Expr, StmtFor}; +use ruff_python_semantic::analyze::typing; +use ruff_python_semantic::analyze::typing::is_io_base_expr; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for uses of `readlines()` when iterating a file object line-by-line. +/// Checks for uses of `readlines()` when iterating over a file line-by-line. /// /// ## Why is this bad? -/// Instead of iterating through `list[str]` which is returned from `readlines()`, use the iteration -/// through a file object which is a more convenient and performant way. +/// Rather than iterating over all lines in a file by calling `readlines()`, +/// it's more convenient and performant to iterate over the file object +/// directly. /// /// ## Example /// ```python -/// with open("file.txt") as f: -/// for line in f.readlines(): +/// with open("file.txt") as fp: +/// for line in fp.readlines(): /// ... /// ``` /// /// Use instead: /// ```python -/// with open("file.txt") as f: -/// for line in f: +/// with open("file.txt") as fp: +/// for line in fp: /// ... /// ``` /// /// ## References /// - [Python documentation: `io.IOBase.readlines`](https://docs.python.org/3/library/io.html#io.IOBase.readlines) -/// - [Python documentation: methods of file objects](https://docs.python.org/3/tutorial/inputoutput.html#methods-of-file-objects) -/// #[violation] pub(crate) struct ReadlinesInFor; impl AlwaysFixableViolation for ReadlinesInFor { #[derive_message_formats] fn message(&self) -> String { - format!("Use of `readlines()` in loop") + format!("Instead of calling `readlines()`, iterate over file object directly") } fn fix_title(&self) -> String { @@ -63,11 +64,29 @@ fn readlines_in_iter(checker: &mut Checker, iter_expr: &Expr) { return; }; - if expr_attr.attr.as_str() == "readlines" && expr_call.arguments.is_empty() { - let mut diagnostic = Diagnostic::new(ReadlinesInFor, expr_call.range()); - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_deletion( - expr_call.range().add_start(expr_attr.value.range().len()), - ))); - checker.diagnostics.push(diagnostic); + if expr_attr.attr.as_str() != "readlines" || !expr_call.arguments.is_empty() { + return; } + + // Determine whether `fp` in `fp.readlines()` was bound to a file object. + if let Expr::Name(name) = expr_attr.value.as_ref() { + if !checker + .semantic() + .resolve_name(name) + .map(|id| checker.semantic().binding(id)) + .is_some_and(|binding| typing::is_io_base(binding, checker.semantic())) + { + return; + } + } else { + if !is_io_base_expr(expr_attr.value.as_ref(), checker.semantic()) { + return; + } + } + + let mut diagnostic = Diagnostic::new(ReadlinesInFor, expr_call.range()); + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_deletion( + expr_call.range().add_start(expr_attr.value.range().len()), + ))); + checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap index 22f0a60abcf73..2d3c00155b559 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/refurb/mod.rs --- -FURB129.py:7:18: FURB129 [*] Use of `readlines()` in loop +FURB129.py:7:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 5 | # Errors 6 | with open("FURB129.py") as f: @@ -22,7 +22,7 @@ FURB129.py:7:18: FURB129 [*] Use of `readlines()` in loop 9 9 | a = [line.lower() for line in f.readlines()] 10 10 | b = {line.upper() for line in f.readlines()} -FURB129.py:9:35: FURB129 [*] Use of `readlines()` in loop +FURB129.py:9:35: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 7 | for _line in f.readlines(): 8 | pass @@ -43,7 +43,7 @@ FURB129.py:9:35: FURB129 [*] Use of `readlines()` in loop 11 11 | c = {line.lower(): line.upper() for line in f.readlines()} 12 12 | -FURB129.py:10:35: FURB129 [*] Use of `readlines()` in loop +FURB129.py:10:35: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 8 | pass 9 | a = [line.lower() for line in f.readlines()] @@ -63,7 +63,7 @@ FURB129.py:10:35: FURB129 [*] Use of `readlines()` in loop 12 12 | 13 13 | with Path("FURB129.py").open() as f: -FURB129.py:11:49: FURB129 [*] Use of `readlines()` in loop +FURB129.py:11:49: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 9 | a = [line.lower() for line in f.readlines()] 10 | b = {line.upper() for line in f.readlines()} @@ -84,7 +84,7 @@ FURB129.py:11:49: FURB129 [*] Use of `readlines()` in loop 13 13 | with Path("FURB129.py").open() as f: 14 14 | for _line in f.readlines(): -FURB129.py:14:18: FURB129 [*] Use of `readlines()` in loop +FURB129.py:14:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 13 | with Path("FURB129.py").open() as f: 14 | for _line in f.readlines(): @@ -103,7 +103,7 @@ FURB129.py:14:18: FURB129 [*] Use of `readlines()` in loop 16 16 | 17 17 | for _line in open("FURB129.py").readlines(): -FURB129.py:17:14: FURB129 [*] Use of `readlines()` in loop +FURB129.py:17:14: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 15 | pass 16 | @@ -123,7 +123,7 @@ FURB129.py:17:14: FURB129 [*] Use of `readlines()` in loop 19 19 | 20 20 | for _line in Path("FURB129.py").open().readlines(): -FURB129.py:20:14: FURB129 [*] Use of `readlines()` in loop +FURB129.py:20:14: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 18 | pass 19 | @@ -143,7 +143,7 @@ FURB129.py:20:14: FURB129 [*] Use of `readlines()` in loop 22 22 | 23 23 | -FURB129.py:26:18: FURB129 [*] Use of `readlines()` in loop +FURB129.py:26:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 24 | def good1(): 25 | f = Path("FURB129.py").open() @@ -164,7 +164,7 @@ FURB129.py:26:18: FURB129 [*] Use of `readlines()` in loop 28 28 | f.close() 29 29 | -FURB129.py:32:18: FURB129 [*] Use of `readlines()` in loop +FURB129.py:32:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly | 31 | def good2(f: io.BytesIO): 32 | for _line in f.readlines(): @@ -183,61 +183,4 @@ FURB129.py:32:18: FURB129 [*] Use of `readlines()` in loop 34 34 | 35 35 | -FURB129.py:38:18: FURB129 [*] Use of `readlines()` in loop - | -36 | # False positives -37 | def bad(f): -38 | for _line in f.readlines(): - | ^^^^^^^^^^^^^ FURB129 -39 | pass - | - = help: Remove `readlines()` - -ℹ Unsafe fix -35 35 | -36 36 | # False positives -37 37 | def bad(f): -38 |- for _line in f.readlines(): - 38 |+ for _line in f: -39 39 | pass -40 40 | -41 41 | - -FURB129.py:43:18: FURB129 [*] Use of `readlines()` in loop - | -42 | def worse(f: codecs.StreamReader): -43 | for _line in f.readlines(): - | ^^^^^^^^^^^^^ FURB129 -44 | pass - | - = help: Remove `readlines()` - -ℹ Unsafe fix -40 40 | -41 41 | -42 42 | def worse(f: codecs.StreamReader): -43 |- for _line in f.readlines(): - 43 |+ for _line in f: -44 44 | pass -45 45 | -46 46 | - -FURB129.py:55:14: FURB129 [*] Use of `readlines()` in loop - | -55 | for _line in foo().readlines(): - | ^^^^^^^^^^^^^^^^^ FURB129 -56 | pass - | - = help: Remove `readlines()` - -ℹ Unsafe fix -52 52 | return A() -53 53 | -54 54 | -55 |-for _line in foo().readlines(): - 55 |+for _line in foo(): -56 56 | pass -57 57 | -58 58 | # OK - diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 5c60b21f44e9b..48d3b597f176e 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -406,7 +406,7 @@ where } /// Abstraction for a type checker, conservatively checks for the intended type(s). -trait TypeChecker { +pub trait TypeChecker { /// Check annotation expression to match the intended type(s). fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool; /// Check initializer expression to match the intended type(s). @@ -441,6 +441,24 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo _ => false, }, + BindingKind::WithItemVar => match binding.statement(semantic) { + // ```python + // with open("file.txt") as x: + // ... + // ``` + Some(Stmt::With(ast::StmtWith { items, .. })) => { + let Some(item) = items.iter().find(|item| { + item.optional_vars + .as_ref() + .is_some_and(|vars| vars.range().contains_range(binding.range)) + }) else { + return false; + }; + T::match_initializer(&item.context_expr, semantic) + } + _ => false, + }, + BindingKind::Argument => match binding.statement(semantic) { // ```python // def foo(x: annotation): @@ -565,35 +583,125 @@ impl BuiltinTypeChecker for TupleChecker { const EXPR_TYPE: PythonType = PythonType::Tuple; } -/// Test whether the given binding (and the given name) can be considered a list. +pub struct IoBaseChecker; + +impl TypeChecker for IoBaseChecker { + fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool { + semantic + .resolve_call_path(annotation) + .is_some_and(|call_path| { + if semantic.match_typing_call_path(&call_path, "IO") { + return true; + } + if semantic.match_typing_call_path(&call_path, "BinaryIO") { + return true; + } + if semantic.match_typing_call_path(&call_path, "TextIO") { + return true; + } + matches!( + call_path.as_slice(), + [ + "io", + "IOBase" + | "RawIOBase" + | "BufferedIOBase" + | "TextIOBase" + | "BytesIO" + | "StringIO" + | "BufferedReader" + | "BufferedWriter" + | "BufferedRandom" + | "BufferedRWPair" + | "TextIOWrapper" + ] | ["os", "Path" | "PathLike"] + | [ + "pathlib", + "Path" | "PurePath" | "PurePosixPath" | "PureWindowsPath" + ] + ) + }) + } + + fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool { + let Expr::Call(ast::ExprCall { func, .. }) = initializer else { + return false; + }; + + // Ex) `pathlib.Path("file.txt")` + if let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() { + if attr.as_str() == "open" { + if let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() { + return semantic.resolve_call_path(func).is_some_and(|call_path| { + matches!( + call_path.as_slice(), + [ + "pathlib", + "Path" | "PurePath" | "PurePosixPath" | "PureWindowsPath" + ] + ) + }); + } + } + } + + // Ex) `open("file.txt")` + semantic + .resolve_call_path(func.as_ref()) + .is_some_and(|call_path| { + matches!( + call_path.as_slice(), + ["io", "open" | "open_code"] | ["os" | "", "open"] + ) + }) + } +} + +/// Test whether the given binding can be considered a list. +/// /// For this, we check what value might be associated with it through it's initialization and /// what annotation it has (we consider `list` and `typing.List`). pub fn is_list(binding: &Binding, semantic: &SemanticModel) -> bool { check_type::<ListChecker>(binding, semantic) } -/// Test whether the given binding (and the given name) can be considered a dictionary. +/// Test whether the given binding can be considered a dictionary. +/// /// For this, we check what value might be associated with it through it's initialization and /// what annotation it has (we consider `dict` and `typing.Dict`). pub fn is_dict(binding: &Binding, semantic: &SemanticModel) -> bool { check_type::<DictChecker>(binding, semantic) } -/// Test whether the given binding (and the given name) can be considered a set. +/// Test whether the given binding can be considered a set. +/// /// For this, we check what value might be associated with it through it's initialization and /// what annotation it has (we consider `set` and `typing.Set`). pub fn is_set(binding: &Binding, semantic: &SemanticModel) -> bool { check_type::<SetChecker>(binding, semantic) } -/// Test whether the given binding (and the given name) can be considered a -/// tuple. For this, we check what value might be associated with it through +/// Test whether the given binding can be considered a tuple. +/// +/// For this, we check what value might be associated with it through /// it's initialization and what annotation it has (we consider `tuple` and /// `typing.Tuple`). pub fn is_tuple(binding: &Binding, semantic: &SemanticModel) -> bool { check_type::<TupleChecker>(binding, semantic) } +/// Test whether the given binding can be considered a file-like object (i.e., a type that +/// implements `io.IOBase`). +pub fn is_io_base(binding: &Binding, semantic: &SemanticModel) -> bool { + check_type::<IoBaseChecker>(binding, semantic) +} + +/// Test whether the given expression can be considered a file-like object (i.e., a type that +/// implements `io.IOBase`). +pub fn is_io_base_expr(expr: &Expr, semantic: &SemanticModel) -> bool { + IoBaseChecker::match_initializer(expr, semantic) +} + /// Find the [`ParameterWithDefault`] corresponding to the given [`Binding`]. #[inline] fn find_parameter<'a>( @@ -699,6 +807,18 @@ pub fn find_binding_value<'a>(binding: &Binding, semantic: &'a SemanticModel) -> _ => {} } } + // Ex) `with open("file.txt") as f:` + BindingKind::WithItemVar => { + let parent_id = binding.source?; + let parent = semantic.statement(parent_id); + if let Stmt::With(ast::StmtWith { items, .. }) = parent { + return items.iter().find_map(|item| { + let target = item.optional_vars.as_ref()?; + let value = &item.context_expr; + match_value(binding, target, value) + }); + } + } _ => {} } None