Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Respect tuple assignments in typing analyzer #9969

Merged
merged 1 commit into from
Feb 13, 2024
+92 −47
Merged

Respect tuple assignments in typing analyzer #9969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 14 additions & 6 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,46 @@
pass


def good1():
def func():
f = Path("FURB129.py").open()
for _line in f.readlines():
pass
f.close()


def good2(f: io.BytesIO):
def func(f: io.BytesIO):
for _line in f.readlines():
pass


def func():
with (open("FURB129.py") as f, foo as bar):
for _line in f.readlines():
pass
for _line in bar.readlines():
pass


# False positives
def bad(f):
def func(f):
for _line in f.readlines():
pass


def worse(f: codecs.StreamReader):
def func(f: codecs.StreamReader):
for _line in f.readlines():
pass


def foo():
def func():
class A:
def readlines(self) -> list[str]:
return ["a", "b", "c"]

return A()


for _line in foo().readlines():
for _line in func().readlines():
pass

# OK
Expand Down
29 changes: 25 additions & 4 deletions ...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ FURB129.py:20:14: FURB129 [*] Instead of calling `readlines()`, iterate over fil

FURB129.py:26:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
|
24 | def good1():
24 | def func():
25 | f = Path("FURB129.py").open()
26 | for _line in f.readlines():
| ^^^^^^^^^^^^^ FURB129
Expand All @@ -156,7 +156,7 @@ FURB129.py:26:18: FURB129 [*] Instead of calling `readlines()`, iterate over fil

ℹ Unsafe fix
23 23 |
24 24 | def good1():
24 24 | def func():
25 25 | f = Path("FURB129.py").open()
26 |- for _line in f.readlines():
26 |+ for _line in f:
Expand All @@ -166,7 +166,7 @@ FURB129.py:26:18: FURB129 [*] Instead of calling `readlines()`, iterate over fil

FURB129.py:32:18: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
|
31 | def good2(f: io.BytesIO):
31 | def func(f: io.BytesIO):
32 | for _line in f.readlines():
| ^^^^^^^^^^^^^ FURB129
33 | pass
Expand All @@ -176,11 +176,32 @@ FURB129.py:32:18: FURB129 [*] Instead of calling `readlines()`, iterate over fil
ℹ Unsafe fix
29 29 |
30 30 |
31 31 | def good2(f: io.BytesIO):
31 31 | def func(f: io.BytesIO):
32 |- for _line in f.readlines():
32 |+ for _line in f:
33 33 | pass
34 34 |
35 35 |

FURB129.py:38:22: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly
|
36 | def func():
37 | with (open("FURB129.py") as f, foo as bar):
38 | for _line in f.readlines():
| ^^^^^^^^^^^^^ FURB129
39 | pass
40 | for _line in bar.readlines():
|
= help: Remove `readlines()`

ℹ Unsafe fix
35 35 |
36 36 | def func():
37 37 | with (open("FURB129.py") as f, foo as bar):
38 |- for _line in f.readlines():
38 |+ for _line in f:
39 39 | pass
40 40 | for _line in bar.readlines():
41 41 | pass


90 changes: 53 additions & 37 deletions crates/ruff_python_semantic/src/analyze/typing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,17 @@ pub trait TypeChecker {
fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bool {
match binding.kind {
BindingKind::Assignment => match binding.statement(semantic) {
// Given:
//
// ```python
// x = init_expr
// ```
//
// The type checker might know how to infer the type based on `init_expr`.
Some(Stmt::Assign(ast::StmtAssign { value, .. })) => {
T::match_initializer(value.as_ref(), semantic)
}
Some(Stmt::Assign(ast::StmtAssign { targets, value, .. })) => targets
.iter()
.find_map(|target| match_value(binding, target, value.as_ref()))
.is_some_and(|value| T::match_initializer(value, semantic)),

// ```python
// x: annotation = some_expr
Expand All @@ -438,24 +441,40 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => {
T::match_annotation(annotation.as_ref(), semantic)
}

_ => false,
},

BindingKind::NamedExprAssignment => {
// ```python
// if (x := some_expr) is not None:
// ...
// ```
binding.source.is_some_and(|source| {
semantic
.expressions(source)
.find_map(|expr| expr.as_named_expr_expr())
.and_then(|ast::ExprNamedExpr { target, value, .. }| {
match_value(binding, target.as_ref(), value.as_ref())
})
.is_some_and(|value| T::match_initializer(value, semantic))
})
}

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)
}
Some(Stmt::With(ast::StmtWith { items, .. })) => items
.iter()
.find_map(|item| {
let target = item.optional_vars.as_ref()?;
let value = &item.context_expr;
match_value(binding, target, value)
})
.is_some_and(|value| T::match_initializer(value, semantic)),

_ => false,
},

Expand All @@ -475,6 +494,7 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
};
T::match_annotation(annotation.as_ref(), semantic)
}

_ => false,
},

Expand Down Expand Up @@ -775,6 +795,7 @@ pub fn find_assigned_value<'a>(symbol: &str, semantic: &'a SemanticModel<'a>) ->
///
/// This function will return a `NumberLiteral` with value `Int(42)` when called with `foo` and a
/// `StringLiteral` with value `"str"` when called with `bla`.
#[allow(clippy::single_match)]
pub fn find_binding_value<'a>(binding: &Binding, semantic: &'a SemanticModel) -> Option<&'a Expr> {
match binding.kind {
// Ex) `x := 1`
Expand All @@ -788,37 +809,32 @@ pub fn find_binding_value<'a>(binding: &Binding, semantic: &'a SemanticModel) ->
}
}
// Ex) `x = 1`
BindingKind::Assignment => {
let parent_id = binding.source?;
let parent = semantic.statement(parent_id);
match parent {
Stmt::Assign(ast::StmtAssign { value, targets, .. }) => {
return targets
.iter()
.find_map(|target| match_value(binding, target, value.as_ref()))
}
Stmt::AnnAssign(ast::StmtAnnAssign {
value: Some(value),
target,
..
}) => {
return match_value(binding, target, value.as_ref());
}
_ => {}
BindingKind::Assignment => match binding.statement(semantic) {
Some(Stmt::Assign(ast::StmtAssign { value, targets, .. })) => {
return targets
.iter()
.find_map(|target| match_value(binding, target, value.as_ref()))
}
}
Some(Stmt::AnnAssign(ast::StmtAnnAssign {
value: Some(value),
target,
..
})) => {
return match_value(binding, target, value.as_ref());
}
_ => {}
},
// 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 {
BindingKind::WithItemVar => match binding.statement(semantic) {
Some(Stmt::With(ast::StmtWith { items, .. })) => {
return items.iter().find_map(|item| {
let target = item.optional_vars.as_ref()?;
let value = &item.context_expr;
match_value(binding, target, value)
});
}
}
_ => {}
},
_ => {}
}
None
Expand Down