Make unpacked assignment a flag rather than a BindingKind (#8595)

## Summary

An assignment can be _both_ (e.g.) a loop variable _and_ assigned via
unpacking. In other words, unpacking is a quality of an assignment, not
a _kind_.
This commit is contained in:
Charlie Marsh 2023-11-09 18:41:30 -08:00 committed by GitHub
parent 4ebd0bd31e
commit 0ac124acef
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 27 additions and 51 deletions

View file

@ -1615,38 +1615,23 @@ impl<'a> Checker<'a> {
fn handle_node_store(&mut self, id: &'a str, expr: &Expr) { fn handle_node_store(&mut self, id: &'a str, expr: &Expr) {
let parent = self.semantic.current_statement(); let parent = self.semantic.current_statement();
let mut flags = BindingFlags::empty();
if helpers::is_unpacking_assignment(parent, expr) {
flags.insert(BindingFlags::UNPACKED_ASSIGNMENT);
}
// Match the left-hand side of an annotated assignment, like `x` in `x: int`. // Match the left-hand side of an annotated assignment, like `x` in `x: int`.
if matches!( if matches!(
parent, parent,
Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. }) Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. })
) && !self.semantic.in_annotation() ) && !self.semantic.in_annotation()
{ {
self.add_binding( self.add_binding(id, expr.range(), BindingKind::Annotation, flags);
id,
expr.range(),
BindingKind::Annotation,
BindingFlags::empty(),
);
return; return;
} }
if parent.is_for_stmt() { if parent.is_for_stmt() {
self.add_binding( self.add_binding(id, expr.range(), BindingKind::LoopVar, flags);
id,
expr.range(),
BindingKind::LoopVar,
BindingFlags::empty(),
);
return;
}
if helpers::is_unpacking_assignment(parent, expr) {
self.add_binding(
id,
expr.range(),
BindingKind::UnpackedAssignment,
BindingFlags::empty(),
);
return; return;
} }
@ -1681,7 +1666,6 @@ impl<'a> Checker<'a> {
let (all_names, all_flags) = let (all_names, all_flags) =
extract_all_names(parent, |name| self.semantic.is_builtin(name)); extract_all_names(parent, |name| self.semantic.is_builtin(name));
let mut flags = BindingFlags::empty();
if all_flags.intersects(DunderAllFlags::INVALID_OBJECT) { if all_flags.intersects(DunderAllFlags::INVALID_OBJECT) {
flags |= BindingFlags::INVALID_ALL_OBJECT; flags |= BindingFlags::INVALID_ALL_OBJECT;
} }
@ -1705,21 +1689,11 @@ impl<'a> Checker<'a> {
.current_expressions() .current_expressions()
.any(Expr::is_named_expr_expr) .any(Expr::is_named_expr_expr)
{ {
self.add_binding( self.add_binding(id, expr.range(), BindingKind::NamedExprAssignment, flags);
id,
expr.range(),
BindingKind::NamedExprAssignment,
BindingFlags::empty(),
);
return; return;
} }
self.add_binding( self.add_binding(id, expr.range(), BindingKind::Assignment, flags);
id,
expr.range(),
BindingKind::Assignment,
BindingFlags::empty(),
);
} }
fn handle_node_delete(&mut self, expr: &'a Expr) { fn handle_node_delete(&mut self, expr: &'a Expr) {

View file

@ -245,7 +245,6 @@ impl Renamer {
| BindingKind::Argument | BindingKind::Argument
| BindingKind::TypeParam | BindingKind::TypeParam
| BindingKind::NamedExprAssignment | BindingKind::NamedExprAssignment
| BindingKind::UnpackedAssignment
| BindingKind::Assignment | BindingKind::Assignment
| BindingKind::BoundException | BindingKind::BoundException
| BindingKind::LoopVar | BindingKind::LoopVar

View file

@ -46,7 +46,6 @@ pub(super) fn test_expression(expr: &Expr, semantic: &SemanticModel) -> Resoluti
BindingKind::Annotation BindingKind::Annotation
| BindingKind::Assignment | BindingKind::Assignment
| BindingKind::NamedExprAssignment | BindingKind::NamedExprAssignment
| BindingKind::UnpackedAssignment
| BindingKind::LoopVar | BindingKind::LoopVar
| BindingKind::Global | BindingKind::Global
| BindingKind::Nonlocal(_) => Resolution::RelevantLocal, | BindingKind::Nonlocal(_) => Resolution::RelevantLocal,

View file

@ -322,10 +322,9 @@ pub(crate) fn unused_variable(checker: &Checker, scope: &Scope, diagnostics: &mu
.bindings() .bindings()
.map(|(name, binding_id)| (name, checker.semantic().binding(binding_id))) .map(|(name, binding_id)| (name, checker.semantic().binding(binding_id)))
.filter_map(|(name, binding)| { .filter_map(|(name, binding)| {
if (binding.kind.is_assignment() if (binding.kind.is_assignment() || binding.kind.is_named_expr_assignment())
|| binding.kind.is_named_expr_assignment() && (!binding.is_unpacked_assignment()
|| (matches!(checker.settings.preview, PreviewMode::Enabled) || matches!(checker.settings.preview, PreviewMode::Enabled))
&& binding.kind.is_unpacked_assignment()))
&& !binding.is_nonlocal() && !binding.is_nonlocal()
&& !binding.is_global() && !binding.is_global()
&& !binding.is_used() && !binding.is_used()

View file

@ -49,7 +49,6 @@ pub(crate) fn non_ascii_name(binding: &Binding, locator: &Locator) -> Option<Dia
BindingKind::Annotation => Kind::Annotation, BindingKind::Annotation => Kind::Annotation,
BindingKind::Argument => Kind::Argument, BindingKind::Argument => Kind::Argument,
BindingKind::NamedExprAssignment => Kind::NamedExprAssignment, BindingKind::NamedExprAssignment => Kind::NamedExprAssignment,
BindingKind::UnpackedAssignment => Kind::UnpackedAssignment,
BindingKind::Assignment => Kind::Assignment, BindingKind::Assignment => Kind::Assignment,
BindingKind::TypeParam => Kind::TypeParam, BindingKind::TypeParam => Kind::TypeParam,
BindingKind::LoopVar => Kind::LoopVar, BindingKind::LoopVar => Kind::LoopVar,
@ -85,7 +84,6 @@ enum Kind {
Annotation, Annotation,
Argument, Argument,
NamedExprAssignment, NamedExprAssignment,
UnpackedAssignment,
Assignment, Assignment,
TypeParam, TypeParam,
LoopVar, LoopVar,
@ -102,7 +100,6 @@ impl fmt::Display for Kind {
Kind::Annotation => f.write_str("Annotation"), Kind::Annotation => f.write_str("Annotation"),
Kind::Argument => f.write_str("Argument"), Kind::Argument => f.write_str("Argument"),
Kind::NamedExprAssignment => f.write_str("Variable"), Kind::NamedExprAssignment => f.write_str("Variable"),
Kind::UnpackedAssignment => f.write_str("Variable"),
Kind::Assignment => f.write_str("Variable"), Kind::Assignment => f.write_str("Variable"),
Kind::TypeParam => f.write_str("Type parameter"), Kind::TypeParam => f.write_str("Type parameter"),
Kind::LoopVar => f.write_str("Variable"), Kind::LoopVar => f.write_str("Variable"),

View file

@ -87,6 +87,12 @@ impl<'a> Binding<'a> {
self.flags.intersects(BindingFlags::INVALID_ALL_OBJECT) self.flags.intersects(BindingFlags::INVALID_ALL_OBJECT)
} }
/// Return `true` if this [`Binding`] represents an unpacked assignment (e.g., `x` in
/// `(x, y) = 1, 2`).
pub const fn is_unpacked_assignment(&self) -> bool {
self.flags.intersects(BindingFlags::UNPACKED_ASSIGNMENT)
}
/// Return `true` if this [`Binding`] represents an unbound variable /// Return `true` if this [`Binding`] represents an unbound variable
/// (e.g., `x` in `x = 1; del x`). /// (e.g., `x` in `x = 1; del x`).
pub const fn is_unbound(&self) -> bool { pub const fn is_unbound(&self) -> bool {
@ -212,7 +218,7 @@ impl<'a> Binding<'a> {
bitflags! { bitflags! {
/// Flags on a [`Binding`]. /// Flags on a [`Binding`].
#[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] #[derive(Debug, Default, Copy, Clone, Eq, PartialEq)]
pub struct BindingFlags: u8 { pub struct BindingFlags: u16 {
/// The binding represents an explicit re-export. /// The binding represents an explicit re-export.
/// ///
/// For example, the binding could be `FastAPI` in: /// For example, the binding could be `FastAPI` in:
@ -284,6 +290,14 @@ bitflags! {
/// _T = "This is a private variable" /// _T = "This is a private variable"
/// ``` /// ```
const PRIVATE_DECLARATION = 1 << 7; const PRIVATE_DECLARATION = 1 << 7;
/// The binding represents an unpacked assignment.
///
/// For example, the binding could be `x` in:
/// ```python
/// (x, y) = 1, 2
/// ```
const UNPACKED_ASSIGNMENT = 1 << 8;
} }
} }
@ -393,12 +407,6 @@ pub enum BindingKind<'a> {
/// ``` /// ```
NamedExprAssignment, NamedExprAssignment,
/// A binding for a unpacking-based assignment, like `x` in:
/// ```python
/// x, y = (1, 2)
/// ```
UnpackedAssignment,
/// A binding for a "standard" assignment, like `x` in: /// A binding for a "standard" assignment, like `x` in:
/// ```python /// ```python
/// x = 1 /// x = 1