diff --git a/crates/ruff/src/ast/operations.rs b/crates/ruff/src/ast/operations.rs index 213c026470..ec33973669 100644 --- a/crates/ruff/src/ast/operations.rs +++ b/crates/ruff/src/ast/operations.rs @@ -304,13 +304,14 @@ pub fn locate_cmpops(contents: &str) -> Vec { ops.push(LocatedCmpop::new(start, end, Cmpop::In)); } Tok::Is => { - if let Some((_, _, end)) = + let op = if let Some((_, _, end)) = tok_iter.next_if(|(_, tok, _)| matches!(tok, Tok::Not)) { - ops.push(LocatedCmpop::new(start, end, Cmpop::IsNot)); + LocatedCmpop::new(start, end, Cmpop::IsNot) } else { - ops.push(LocatedCmpop::new(start, end, Cmpop::Is)); - } + LocatedCmpop::new(start, end, Cmpop::Is) + }; + ops.push(op); } Tok::NotEqual => { ops.push(LocatedCmpop::new(start, end, Cmpop::NotEq)); diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 0305ad1e49..8fc7d416aa 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -239,51 +239,51 @@ impl<'a> Checker<'a> { 'b: 'a, { let call_path = collect_call_path(value); - if let Some(head) = call_path.first() { - if let Some(binding) = self.find_binding(head) { - match &binding.kind { - BindingKind::Importation(.., name) - | BindingKind::SubmoduleImportation(name, ..) => { - return if name.starts_with('.') { - if let Some(module) = &self.module_path { - let mut source_path = from_relative_import(module, name); - source_path.extend(call_path.into_iter().skip(1)); - Some(source_path) - } else { - None - } - } else { - let mut source_path: CallPath = name.split('.').collect(); - source_path.extend(call_path.into_iter().skip(1)); - Some(source_path) - }; + let Some(head) = call_path.first() else { + return None; + }; + let Some(binding) = self.find_binding(head) else { + return None; + }; + match &binding.kind { + BindingKind::Importation(.., name) | BindingKind::SubmoduleImportation(name, ..) => { + if name.starts_with('.') { + if let Some(module) = &self.module_path { + let mut source_path = from_relative_import(module, name); + source_path.extend(call_path.into_iter().skip(1)); + Some(source_path) + } else { + None } - BindingKind::FromImportation(.., name) => { - return if name.starts_with('.') { - if let Some(module) = &self.module_path { - let mut source_path = from_relative_import(module, name); - source_path.extend(call_path.into_iter().skip(1)); - Some(source_path) - } else { - None - } - } else { - let mut source_path: CallPath = name.split('.').collect(); - source_path.extend(call_path.into_iter().skip(1)); - Some(source_path) - }; - } - BindingKind::Builtin => { - let mut source_path: CallPath = smallvec![]; - source_path.push(""); - source_path.extend(call_path); - return Some(source_path); - } - _ => {} + } else { + let mut source_path: CallPath = name.split('.').collect(); + source_path.extend(call_path.into_iter().skip(1)); + Some(source_path) } } + BindingKind::FromImportation(.., name) => { + if name.starts_with('.') { + if let Some(module) = &self.module_path { + let mut source_path = from_relative_import(module, name); + source_path.extend(call_path.into_iter().skip(1)); + Some(source_path) + } else { + None + } + } else { + let mut source_path: CallPath = name.split('.').collect(); + source_path.extend(call_path.into_iter().skip(1)); + Some(source_path) + } + } + BindingKind::Builtin => { + let mut source_path: CallPath = smallvec![]; + source_path.push(""); + source_path.extend(call_path); + Some(source_path) + } + _ => None, } - None } /// Return `true` if a `Rule` is disabled by a `noqa` directive. @@ -4169,146 +4169,147 @@ impl<'a> Checker<'a> { } fn handle_node_load(&mut self, expr: &Expr) { - if let ExprKind::Name { id, .. } = &expr.node { - let scope_id = self.current_scope().id; + let ExprKind::Name { id, .. } = &expr.node else { + return; + }; + let scope_id = self.current_scope().id; - let mut first_iter = true; - let mut in_generator = false; - let mut import_starred = false; + let mut first_iter = true; + let mut in_generator = false; + let mut import_starred = false; - for scope_index in self.scope_stack.iter().rev() { - let scope = &self.scopes[*scope_index]; - - if matches!(scope.kind, ScopeKind::Class(_)) { - if id == "__class__" { - return; - } else if !first_iter && !in_generator { - continue; - } - } - - if let Some(index) = scope.bindings.get(&id.as_str()) { - // Mark the binding as used. - let context = self.execution_context(); - self.bindings[*index].mark_used(scope_id, Range::from_located(expr), context); - - if matches!(self.bindings[*index].kind, BindingKind::Annotation) - && !self.in_deferred_string_type_definition - && !self.in_deferred_type_definition - { - continue; - } - - // If the name of the sub-importation is the same as an alias of another - // importation and the alias is used, that sub-importation should be - // marked as used too. - // - // This handles code like: - // import pyarrow as pa - // import pyarrow.csv - // print(pa.csv.read_csv("test.csv")) - match &self.bindings[*index].kind { - BindingKind::Importation(name, full_name) - | BindingKind::SubmoduleImportation(name, full_name) => { - let has_alias = full_name - .split('.') - .last() - .map(|segment| &segment != name) - .unwrap_or_default(); - if has_alias { - // Mark the sub-importation as used. - if let Some(index) = scope.bindings.get(full_name) { - self.bindings[*index].mark_used( - scope_id, - Range::from_located(expr), - context, - ); - } - } - } - BindingKind::FromImportation(name, full_name) => { - let has_alias = full_name - .split('.') - .last() - .map(|segment| &segment != name) - .unwrap_or_default(); - if has_alias { - // Mark the sub-importation as used. - if let Some(index) = scope.bindings.get(full_name.as_str()) { - self.bindings[*index].mark_used( - scope_id, - Range::from_located(expr), - context, - ); - } - } - } - _ => {} - } + for scope_index in self.scope_stack.iter().rev() { + let scope = &self.scopes[*scope_index]; + if matches!(scope.kind, ScopeKind::Class(_)) { + if id == "__class__" { return; + } else if !first_iter && !in_generator { + continue; } - - first_iter = false; - in_generator = matches!(scope.kind, ScopeKind::Generator); - import_starred = import_starred || scope.import_starred; } - if import_starred { - if self.settings.rules.enabled(&Rule::ImportStarUsage) { - let mut from_list = vec![]; - for scope_index in self.scope_stack.iter().rev() { - let scope = &self.scopes[*scope_index]; - for binding in scope.bindings.values().map(|index| &self.bindings[*index]) { - if let BindingKind::StarImportation(level, module) = &binding.kind { - from_list.push(helpers::format_import_from( - level.as_ref(), - module.as_deref(), - )); + if let Some(index) = scope.bindings.get(&id.as_str()) { + // Mark the binding as used. + let context = self.execution_context(); + self.bindings[*index].mark_used(scope_id, Range::from_located(expr), context); + + if matches!(self.bindings[*index].kind, BindingKind::Annotation) + && !self.in_deferred_string_type_definition + && !self.in_deferred_type_definition + { + continue; + } + + // If the name of the sub-importation is the same as an alias of another + // importation and the alias is used, that sub-importation should be + // marked as used too. + // + // This handles code like: + // import pyarrow as pa + // import pyarrow.csv + // print(pa.csv.read_csv("test.csv")) + match &self.bindings[*index].kind { + BindingKind::Importation(name, full_name) + | BindingKind::SubmoduleImportation(name, full_name) => { + let has_alias = full_name + .split('.') + .last() + .map(|segment| &segment != name) + .unwrap_or_default(); + if has_alias { + // Mark the sub-importation as used. + if let Some(index) = scope.bindings.get(full_name) { + self.bindings[*index].mark_used( + scope_id, + Range::from_located(expr), + context, + ); } } } - from_list.sort(); - - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::ImportStarUsage { - name: id.to_string(), - sources: from_list, - }, - Range::from_located(expr), - )); + BindingKind::FromImportation(name, full_name) => { + let has_alias = full_name + .split('.') + .last() + .map(|segment| &segment != name) + .unwrap_or_default(); + if has_alias { + // Mark the sub-importation as used. + if let Some(index) = scope.bindings.get(full_name.as_str()) { + self.bindings[*index].mark_used( + scope_id, + Range::from_located(expr), + context, + ); + } + } + } + _ => {} } + return; } - if self.settings.rules.enabled(&Rule::UndefinedName) { - // Allow __path__. - if self.path.ends_with("__init__.py") && id == "__path__" { - return; - } + first_iter = false; + in_generator = matches!(scope.kind, ScopeKind::Generator); + import_starred = import_starred || scope.import_starred; + } - // Allow "__module__" and "__qualname__" in class scopes. - if (id == "__module__" || id == "__qualname__") - && matches!(self.current_scope().kind, ScopeKind::Class(..)) - { - return; - } - - // Avoid flagging if NameError is handled. - if let Some(handler_names) = self.except_handlers.last() { - if handler_names - .iter() - .any(|call_path| call_path.as_slice() == ["NameError"]) - { - return; + if import_starred { + if self.settings.rules.enabled(&Rule::ImportStarUsage) { + let mut from_list = vec![]; + for scope_index in self.scope_stack.iter().rev() { + let scope = &self.scopes[*scope_index]; + for binding in scope.bindings.values().map(|index| &self.bindings[*index]) { + if let BindingKind::StarImportation(level, module) = &binding.kind { + from_list.push(helpers::format_import_from( + level.as_ref(), + module.as_deref(), + )); + } } } + from_list.sort(); self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedName { name: id.clone() }, + pyflakes::rules::ImportStarUsage { + name: id.to_string(), + sources: from_list, + }, Range::from_located(expr), )); } + return; + } + + if self.settings.rules.enabled(&Rule::UndefinedName) { + // Allow __path__. + if self.path.ends_with("__init__.py") && id == "__path__" { + return; + } + + // Allow "__module__" and "__qualname__" in class scopes. + if (id == "__module__" || id == "__qualname__") + && matches!(self.current_scope().kind, ScopeKind::Class(..)) + { + return; + } + + // Avoid flagging if NameError is handled. + if let Some(handler_names) = self.except_handlers.last() { + if handler_names + .iter() + .any(|call_path| call_path.as_slice() == ["NameError"]) + { + return; + } + } + + self.diagnostics.push(Diagnostic::new( + pyflakes::rules::UndefinedName { name: id.clone() }, + Range::from_located(expr), + )); } } @@ -4506,26 +4507,29 @@ impl<'a> Checker<'a> { where 'b: 'a, { - if let ExprKind::Name { id, .. } = &expr.node { - if operations::on_conditional_branch( - &mut self.parents.iter().rev().map(std::convert::Into::into), - ) { - return; - } - - let scope = - &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))]; - if scope.bindings.remove(&id.as_str()).is_none() - && self.settings.rules.enabled(&Rule::UndefinedName) - { - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedName { - name: id.to_string(), - }, - Range::from_located(expr), - )); - } + let ExprKind::Name { id, .. } = &expr.node else { + return; + }; + if operations::on_conditional_branch( + &mut self.parents.iter().rev().map(std::convert::Into::into), + ) { + return; } + + let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))]; + if scope.bindings.remove(&id.as_str()).is_some() { + return; + } + if !self.settings.rules.enabled(&Rule::UndefinedName) { + return; + } + + self.diagnostics.push(Diagnostic::new( + pyflakes::rules::UndefinedName { + name: id.to_string(), + }, + Range::from_located(expr), + )); } fn visit_docstring<'b>(&mut self, python_ast: &'b Suite) -> bool diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 39a579128f..58edc56f0b 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -322,7 +322,7 @@ fn push_codes(str: &mut String, codes: impl Iterator) { if !first { str.push_str(", "); } - let _ = write!(str, "{}", code); + let _ = write!(str, "{code}"); first = false; } } diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs b/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs index a968ae31c2..efaa29b419 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs @@ -18,8 +18,6 @@ //! method() //! ``` -use std::iter; - use ruff_macros::{define_violation, derive_message_formats}; use rustc_hash::FxHashMap; use rustpython_parser::ast::{Expr, ExprKind, Stmt}; @@ -174,27 +172,19 @@ pub fn unused_loop_control_variable( if matches!(certainty, Certainty::Certain) && checker.patch(diagnostic.kind.rule()) { // Find the `BindingKind::LoopVar` corresponding to the name. let scope = checker.current_scope(); - if let Some(binding) = iter::once(scope.bindings.get(name)) - .flatten() - .chain( - iter::once(scope.rebounds.get(name)) - .flatten() - .into_iter() - .flatten(), - ) + let binding = scope + .bindings + .get(name) + .into_iter() + .chain(scope.rebounds.get(name).into_iter().flatten()) .find_map(|index| { let binding = &checker.bindings[*index]; - if let Some(source) = &binding.source { - if source == &RefEquality(stmt) { - Some(binding) - } else { - None - } - } else { - None - } - }) - { + binding + .source + .as_ref() + .and_then(|source| (source == &RefEquality(stmt)).then_some(binding)) + }); + if let Some(binding) = binding { if matches!(binding.kind, BindingKind::LoopVar) { if !binding.used() { diagnostic.amend(Fix::replacement( diff --git a/crates/ruff_cli/src/args.rs b/crates/ruff_cli/src/args.rs index 037d5ff741..6fcd6e8c54 100644 --- a/crates/ruff_cli/src/args.rs +++ b/crates/ruff_cli/src/args.rs @@ -478,7 +478,7 @@ impl ConfigProcessor for &Overrides { .ignore .iter() .cloned() - .chain(self.extend_ignore.iter().cloned().into_iter()) + .chain(self.extend_ignore.iter().cloned()) .flatten() .collect(), extend_select: self.extend_select.clone().unwrap_or_default(), diff --git a/crates/ruff_cli/src/printer.rs b/crates/ruff_cli/src/printer.rs index 70098ea67e..38f7dc24d5 100644 --- a/crates/ruff_cli/src/printer.rs +++ b/crates/ruff_cli/src/printer.rs @@ -639,7 +639,7 @@ fn print_fixed(stdout: &mut T, fixed: &FxHashMap) -> ); let s = if total == 1 { "" } else { "s" }; - let label = format!("Fixed {total} error{s}:", total = total, s = s); + let label = format!("Fixed {total} error{s}:"); writeln!(stdout, "{}", label.bold().green())?; for (filename, table) in fixed diff --git a/crates/ruff_dev/src/generate_options.rs b/crates/ruff_dev/src/generate_options.rs index f4e285c8df..68f6ca6b62 100644 --- a/crates/ruff_dev/src/generate_options.rs +++ b/crates/ruff_dev/src/generate_options.rs @@ -4,7 +4,7 @@ use ruff::settings::options::Options; use ruff::settings::options_base::{ConfigurationOptions, OptionEntry, OptionField}; fn emit_field(output: &mut String, name: &str, field: &OptionField, group_name: Option<&str>) { - output.push_str(&format!("#### [`{0}`](#{0})\n", name)); + output.push_str(&format!("#### [`{name}`](#{name})\n")); output.push('\n'); output.push_str(field.doc); output.push_str("\n\n"); @@ -40,7 +40,7 @@ pub fn generate() -> String { // Generate all the sub-groups. for (group_name, entry) in &sorted_options { let OptionEntry::Group(fields) = entry else { continue; }; - output.push_str(&format!("### `{}`\n", group_name)); + output.push_str(&format!("### `{group_name}`\n")); output.push('\n'); for (name, entry) in fields.iter().sorted_by_key(|(name, _)| name) { let OptionEntry::Field(field) = entry else { continue; }; diff --git a/crates/ruff_python_formatter/src/newlines.rs b/crates/ruff_python_formatter/src/newlines.rs index 35b3fed54c..33f7ced3ba 100644 --- a/crates/ruff_python_formatter/src/newlines.rs +++ b/crates/ruff_python_formatter/src/newlines.rs @@ -78,16 +78,10 @@ impl<'a> Visitor<'a> for NewlineNormalizer { let required_newlines = match self.trailer { Trailer::FunctionDef | Trailer::ClassDef => self.depth.max_newlines(), Trailer::Docstring if matches!(self.scope, Scope::Class) => 1, - Trailer::Import => { - if matches!( - stmt.node, - StmtKind::Import { .. } | StmtKind::ImportFrom { .. } - ) { - 0 - } else { - 1 - } - } + Trailer::Import => usize::from(!matches!( + stmt.node, + StmtKind::Import { .. } | StmtKind::ImportFrom { .. } + )), _ => 0, }; let present_newlines = stmt