Skip to content

Commit

Permalink
Add type inference
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Feb 13, 2024
1 parent d9e6d77 commit 0bd61b1
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 88 deletions.
51 changes: 35 additions & 16 deletions crates/ruff_linter/src/rules/refurb/rules/readlines_in_for.rs
View file
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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);
}
75 changes: 9 additions & 66 deletions ...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap
View file
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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
Expand All @@ -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()]
Expand All @@ -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()}
Expand All @@ -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():
Expand All @@ -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 |
Expand All @@ -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 |
Expand All @@ -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()
Expand All @@ -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():
Expand All @@ -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


Loading

0 comments on commit 0bd61b1

Please sign in to comment.