Skip to content

Commit

Permalink
Remove symbol from type-matching API (#9968)
Browse files Browse the repository at this point in the history
## Summary

These should be no-op refactors to remove some redundant data from the
type analysis APIs.
  • Loading branch information
charliermarsh authored Feb 13, 2024
1 parent 8fba97f commit 609d0a9
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ fn is_open_call_from_pathlib(func: &Expr, semantic: &SemanticModel) -> bool {

let binding = semantic.binding(binding_id);

let Some(Expr::Call(call)) = analyze::typing::find_binding_value(&name.id, binding, semantic)
else {
let Some(Expr::Call(call)) = analyze::typing::find_binding_value(binding, semantic) else {
return false;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ pub(crate) fn enumerate_for_loop(checker: &mut Checker, for_stmt: &ast::StmtFor)
}

// Ensure that the index variable was initialized to 0.
let Some(value) = typing::find_binding_value(&index.id, binding, checker.semantic())
else {
let Some(value) = typing::find_binding_value(binding, checker.semantic()) else {
continue;
};
if !matches!(
Expand Down
44 changes: 15 additions & 29 deletions crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ pub fn resolve_assignment<'a>(
pub fn find_assigned_value<'a>(symbol: &str, semantic: &'a SemanticModel<'a>) -> Option<&'a Expr> {
let binding_id = semantic.lookup_symbol(symbol)?;
let binding = semantic.binding(binding_id);
find_binding_value(symbol, binding, semantic)
find_binding_value(binding, semantic)
}

/// Find the assigned [`Expr`] for a given [`Binding`], if any.
Expand All @@ -667,11 +667,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`.
pub fn find_binding_value<'a>(
symbol: &str,
binding: &Binding,
semantic: &'a SemanticModel,
) -> Option<&'a Expr> {
pub fn find_binding_value<'a>(binding: &Binding, semantic: &'a SemanticModel) -> Option<&'a Expr> {
match binding.kind {
// Ex) `x := 1`
BindingKind::NamedExprAssignment => {
Expand All @@ -680,7 +676,7 @@ pub fn find_binding_value<'a>(
.expressions(parent_id)
.find_map(|expr| expr.as_named_expr_expr());
if let Some(ast::ExprNamedExpr { target, value, .. }) = parent {
return match_value(symbol, target.as_ref(), value.as_ref());
return match_value(binding, target.as_ref(), value.as_ref());
}
}
// Ex) `x = 1`
Expand All @@ -689,16 +685,16 @@ pub fn find_binding_value<'a>(
let parent = semantic.statement(parent_id);
match parent {
Stmt::Assign(ast::StmtAssign { value, targets, .. }) => {
if let Some(target) = targets.iter().find(|target| defines(symbol, target)) {
return match_value(symbol, target, value.as_ref());
}
return targets
.iter()
.find_map(|target| match_value(binding, target, value.as_ref()))
}
Stmt::AnnAssign(ast::StmtAnnAssign {
value: Some(value),
target,
..
}) => {
return match_value(symbol, target, value.as_ref());
return match_value(binding, target, value.as_ref());
}
_ => {}
}
Expand All @@ -709,9 +705,9 @@ pub fn find_binding_value<'a>(
}

/// Given a target and value, find the value that's assigned to the given symbol.
fn match_value<'a>(symbol: &str, target: &Expr, value: &'a Expr) -> Option<&'a Expr> {
fn match_value<'a>(binding: &Binding, target: &Expr, value: &'a Expr) -> Option<&'a Expr> {
match target {
Expr::Name(ast::ExprName { id, .. }) if id.as_str() == symbol => Some(value),
Expr::Name(name) if name.range() == binding.range() => Some(value),
Expr::Tuple(ast::ExprTuple { elts, .. }) | Expr::List(ast::ExprList { elts, .. }) => {
match value {
Expr::Tuple(ast::ExprTuple {
Expand All @@ -722,26 +718,16 @@ fn match_value<'a>(symbol: &str, target: &Expr, value: &'a Expr) -> Option<&'a E
})
| Expr::Set(ast::ExprSet {
elts: value_elts, ..
}) => get_value_by_id(symbol, elts, value_elts),
}) => match_target(binding, elts, value_elts),
_ => None,
}
}
_ => None,
}
}

/// Returns `true` if the [`Expr`] defines the symbol.
fn defines(symbol: &str, expr: &Expr) -> bool {
match expr {
Expr::Name(ast::ExprName { id, .. }) => id == symbol,
Expr::Tuple(ast::ExprTuple { elts, .. })
| Expr::List(ast::ExprList { elts, .. })
| Expr::Set(ast::ExprSet { elts, .. }) => elts.iter().any(|elt| defines(symbol, elt)),
_ => false,
}
}

fn get_value_by_id<'a>(target_id: &str, targets: &[Expr], values: &'a [Expr]) -> Option<&'a Expr> {
/// Given a target and value, find the value that's assigned to the given symbol.
fn match_target<'a>(binding: &Binding, targets: &[Expr], values: &'a [Expr]) -> Option<&'a Expr> {
for (target, value) in targets.iter().zip(values.iter()) {
match target {
Expr::Tuple(ast::ExprTuple {
Expand All @@ -764,15 +750,15 @@ fn get_value_by_id<'a>(target_id: &str, targets: &[Expr], values: &'a [Expr]) ->
| Expr::Set(ast::ExprSet {
elts: value_elts, ..
}) => {
if let Some(result) = get_value_by_id(target_id, target_elts, value_elts) {
if let Some(result) = match_target(binding, target_elts, value_elts) {
return Some(result);
}
}
_ => (),
};
}
Expr::Name(ast::ExprName { id, .. }) => {
if *id == target_id {
Expr::Name(name) => {
if name.range() == binding.range() {
return Some(value);
}
}
Expand Down

0 comments on commit 609d0a9

Please sign in to comment.