diff --git a/src/cache.rs b/src/cache.rs index 776136a00e..d8638fd1da 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -139,20 +139,21 @@ pub fn get( return None; }; - if let Ok(encoded) = read_sync(cache_key(path, settings, autofix)) { - match bincode::deserialize::(&encoded[..]) { - Ok(CheckResult { - metadata: CacheMetadata { mtime }, - messages, - }) => { - if FileTime::from_last_modification_time(metadata).unix_seconds() == mtime { - return Some(messages); - } - } - Err(e) => error!("Failed to deserialize encoded cache entry: {e:?}"), + let encoded = read_sync(cache_key(path, settings, autofix)).ok()?; + let (mtime, messages) = match bincode::deserialize::(&encoded[..]) { + Ok(CheckResult { + metadata: CacheMetadata { mtime }, + messages, + }) => (mtime, messages), + Err(e) => { + error!("Failed to deserialize encoded cache entry: {e:?}"); + return None; } + }; + if FileTime::from_last_modification_time(metadata).unix_seconds() != mtime { + return None; } - None + Some(messages) } /// Set a value in the cache. diff --git a/src/check_ast.rs b/src/check_ast.rs index cdd1d8b8f4..551c2cf1df 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -142,15 +142,16 @@ impl<'a> Checker<'a> { // If we're in an f-string, override the location. RustPython doesn't produce // reliable locations for expressions within f-strings, so we use the // span of the f-string itself as a best-effort default. - if let Some(range) = self.in_f_string { - self.checks.push(Check { + let check = if let Some(range) = self.in_f_string { + Check { location: range.location, end_location: range.end_location, ..check - }); + } } else { - self.checks.push(check); - } + check + }; + self.checks.push(check); } /// Add multiple `Check` items to the `Checker`. diff --git a/src/check_lines.rs b/src/check_lines.rs index b8dfd81a41..3d84051c02 100644 --- a/src/check_lines.rs +++ b/src/check_lines.rs @@ -20,19 +20,18 @@ static URL_REGEX: Lazy = Lazy::new(|| Regex::new(r"^https?://\S+$").unwra /// Whether the given line is too long and should be reported. fn should_enforce_line_length(line: &str, length: usize, limit: usize) -> bool { - if length > limit { - let mut chunks = line.split_whitespace(); - if let (Some(first), Some(_)) = (chunks.next(), chunks.next()) { - // Do not enforce the line length for commented lines that end with a URL - // or contain only a single word. - !(first == "#" && chunks.last().map_or(true, |c| URL_REGEX.is_match(c))) - } else { - // Single word / no printable chars - no way to make the line shorter - false - } - } else { - false + if length <= limit { + return false; } + let mut chunks = line.split_whitespace(); + let (Some(first), Some(_)) = (chunks.next(), chunks.next()) else { + // Single word / no printable chars - no way to make the line shorter + return false; + }; + + // Do not enforce the line length for commented lines that end with a URL + // or contain only a single word. + !(first == "#" && chunks.last().map_or(true, |c| URL_REGEX.is_match(c))) } pub fn check_lines( diff --git a/src/directives.rs b/src/directives.rs index 0a3055e8ff..f11a81133d 100644 --- a/src/directives.rs +++ b/src/directives.rs @@ -93,32 +93,34 @@ pub fn extract_isort_directives(lxr: &[LexResult], locator: &SourceCodeLocator) continue; } - if matches!(tok, Tok::Comment) { - // TODO(charlie): Modify RustPython to include the comment text in the token. - let comment_text = locator.slice_source_code_range(&Range { - location: start, - end_location: end, - }); + if !matches!(tok, Tok::Comment) { + continue; + } - if comment_text == "# isort: split" { - splits.push(start.row()); - } else if comment_text == "# isort: skip_file" { - skip_file = true; - } else if off.is_some() { - if comment_text == "# isort: on" { - if let Some(start) = off { - for row in start.row() + 1..=end.row() { - exclusions.insert(row); - } + // TODO(charlie): Modify RustPython to include the comment text in the token. + let comment_text = locator.slice_source_code_range(&Range { + location: start, + end_location: end, + }); + + if comment_text == "# isort: split" { + splits.push(start.row()); + } else if comment_text == "# isort: skip_file" { + skip_file = true; + } else if off.is_some() { + if comment_text == "# isort: on" { + if let Some(start) = off { + for row in start.row() + 1..=end.row() { + exclusions.insert(row); } - off = None; - } - } else { - if comment_text.contains("isort: skip") { - exclusions.insert(start.row()); - } else if comment_text == "# isort: off" { - off = Some(start); } + off = None; + } + } else { + if comment_text.contains("isort: skip") { + exclusions.insert(start.row()); + } else if comment_text == "# isort: off" { + off = Some(start); } } } diff --git a/src/pyflakes/cformat.rs b/src/pyflakes/cformat.rs index b1784ab1a2..33178db53c 100644 --- a/src/pyflakes/cformat.rs +++ b/src/pyflakes/cformat.rs @@ -25,29 +25,30 @@ impl TryFrom<&str> for CFormatSummary { let mut keywords = FxHashSet::default(); for format_part in format_string.parts { - if let CFormatPart::Spec(CFormatSpec { + let CFormatPart::Spec(CFormatSpec { mapping_key, min_field_width, precision, .. - }) = format_part.1 + }) = format_part.1 else { - match mapping_key { - Some(k) => { - keywords.insert(k); - } - None => { - num_positional += 1; - } - }; - if min_field_width == Some(CFormatQuantity::FromValuesTuple) { - num_positional += 1; - starred = true; + continue; + }; + match mapping_key { + Some(k) => { + keywords.insert(k); } - if precision == Some(CFormatQuantity::FromValuesTuple) { + None => { num_positional += 1; - starred = true; } + }; + if min_field_width == Some(CFormatQuantity::FromValuesTuple) { + num_positional += 1; + starred = true; + } + if precision == Some(CFormatQuantity::FromValuesTuple) { + num_positional += 1; + starred = true; } } diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index 61ce01d9f7..4a5f008cbb 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -77,43 +77,39 @@ pub(crate) fn percent_format_extra_named_arguments( if summary.num_positional > 0 { return None; } - - if let ExprKind::Dict { keys, values } = &right.node { - if values.len() > keys.len() { - return None; // contains **x splat - } - - let missing: Vec<&String> = keys - .iter() - .filter_map(|k| match &k.node { - // We can only check that string literals exist - ExprKind::Constant { - value: Constant::Str(value), - .. - } => { - if summary.keywords.contains(value) { - None - } else { - Some(value) - } - } - _ => None, - }) - .collect(); - - if missing.is_empty() { - None - } else { - Some(Check::new( - CheckKind::PercentFormatExtraNamedArguments( - missing.iter().map(|&s| s.clone()).collect(), - ), - location, - )) - } - } else { - None + let ExprKind::Dict { keys, values } = &right.node else { + return None; + }; + if values.len() > keys.len() { + return None; // contains **x splat } + + let missing: Vec<&String> = keys + .iter() + .filter_map(|k| match &k.node { + // We can only check that string literals exist + ExprKind::Constant { + value: Constant::Str(value), + .. + } => { + if summary.keywords.contains(value) { + None + } else { + Some(value) + } + } + _ => None, + }) + .collect(); + + if missing.is_empty() { + return None; + } + + Some(Check::new( + CheckKind::PercentFormatExtraNamedArguments(missing.iter().map(|&s| s.clone()).collect()), + location, + )) } /// F505 diff --git a/src/pyflakes/format.rs b/src/pyflakes/format.rs index ff88dfa44b..4abae1bd63 100644 --- a/src/pyflakes/format.rs +++ b/src/pyflakes/format.rs @@ -43,30 +43,31 @@ impl TryFrom<&str> for FormatSummary { let mut keywords = FxHashSet::default(); for format_part in format_string.format_parts { - if let FormatPart::Field { + let FormatPart::Field { field_name, format_spec, .. - } = format_part - { + } = format_part else { + continue; + }; + let parsed = FieldName::parse(&field_name)?; + match parsed.field_type { + FieldType::Auto => autos.insert(autos.len()), + FieldType::Index(i) => indexes.insert(i), + FieldType::Keyword(k) => keywords.insert(k), + }; + + let nested = FormatString::from_str(&format_spec)?; + for nested_part in nested.format_parts { + let FormatPart::Field { field_name, .. } = nested_part else { + continue; + }; let parsed = FieldName::parse(&field_name)?; match parsed.field_type { FieldType::Auto => autos.insert(autos.len()), FieldType::Index(i) => indexes.insert(i), FieldType::Keyword(k) => keywords.insert(k), }; - - let nested = FormatString::from_str(&format_spec)?; - for nested_part in nested.format_parts { - if let FormatPart::Field { field_name, .. } = nested_part { - let parsed = FieldName::parse(&field_name)?; - match parsed.field_type { - FieldType::Auto => autos.insert(autos.len()), - FieldType::Index(i) => indexes.insert(i), - FieldType::Keyword(k) => keywords.insert(k), - }; - } - } } } diff --git a/src/pyflakes/plugins/invalid_print_syntax.rs b/src/pyflakes/plugins/invalid_print_syntax.rs index 8e0e3d6cec..f05968983a 100644 --- a/src/pyflakes/plugins/invalid_print_syntax.rs +++ b/src/pyflakes/plugins/invalid_print_syntax.rs @@ -6,19 +6,22 @@ use crate::checks::{Check, CheckKind}; /// F633 pub fn invalid_print_syntax(checker: &mut Checker, left: &Expr) { - if let ExprKind::Name { id, .. } = &left.node { - if id == "print" { - let scope = checker.current_scope(); - if let Some(Binding { - kind: BindingKind::Builtin, - .. - }) = scope.values.get("print") - { - checker.add_check(Check::new( - CheckKind::InvalidPrintSyntax, - Range::from_located(left), - )); - } - } + let ExprKind::Name { id, .. } = &left.node else { + return; + }; + if id != "print" { + return; } + let scope = checker.current_scope(); + let Some(Binding { + kind: BindingKind::Builtin, + .. + }) = scope.values.get("print") else + { + return; + }; + checker.add_check(Check::new( + CheckKind::InvalidPrintSyntax, + Range::from_located(left), + )); } diff --git a/src/pyflakes/plugins/raise_not_implemented.rs b/src/pyflakes/plugins/raise_not_implemented.rs index f9db1a98b3..29e7b0ea2a 100644 --- a/src/pyflakes/plugins/raise_not_implemented.rs +++ b/src/pyflakes/plugins/raise_not_implemented.rs @@ -26,15 +26,16 @@ fn match_not_implemented(expr: &Expr) -> Option<&Expr> { /// F901 pub fn raise_not_implemented(checker: &mut Checker, expr: &Expr) { - if let Some(expr) = match_not_implemented(expr) { - let mut check = Check::new(CheckKind::RaiseNotImplemented, Range::from_located(expr)); - if checker.patch(check.kind.code()) { - check.amend(Fix::replacement( - "NotImplementedError".to_string(), - expr.location, - expr.end_location.unwrap(), - )); - } - checker.add_check(check); + let Some(expr) = match_not_implemented(expr) else { + return; + }; + let mut check = Check::new(CheckKind::RaiseNotImplemented, Range::from_located(expr)); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + "NotImplementedError".to_string(), + expr.location, + expr.end_location.unwrap(), + )); } + checker.add_check(check); } diff --git a/src/pygrep_hooks/checks.rs b/src/pygrep_hooks/checks.rs index c2a32d2b07..08f116d928 100644 --- a/src/pygrep_hooks/checks.rs +++ b/src/pygrep_hooks/checks.rs @@ -5,11 +5,14 @@ use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; pub fn no_eval(checker: &mut Checker, func: &Expr) { - if let ExprKind::Name { id, .. } = &func.node { - if id == "eval" { - if checker.is_builtin("eval") { - checker.add_check(Check::new(CheckKind::NoEval, Range::from_located(func))); - } - } + let ExprKind::Name { id, .. } = &func.node else { + return; + }; + if id != "eval" { + return; } + if !checker.is_builtin("eval") { + return; + } + checker.add_check(Check::new(CheckKind::NoEval, Range::from_located(func))); } diff --git a/src/pylint/plugins/consider_merging_isinstance.rs b/src/pylint/plugins/consider_merging_isinstance.rs index f1a476938f..4f67f22e23 100644 --- a/src/pylint/plugins/consider_merging_isinstance.rs +++ b/src/pylint/plugins/consider_merging_isinstance.rs @@ -20,25 +20,28 @@ pub fn consider_merging_isinstance( let mut obj_to_types: FxHashMap)> = FxHashMap::default(); for value in values { - if let ExprKind::Call { func, args, .. } = &value.node { - if matches!(&func.node, ExprKind::Name { id, .. } if id == "isinstance") { - if let [obj, types] = &args[..] { - let (num_calls, matches) = obj_to_types - .entry(obj.to_string()) - .or_insert_with(|| (0, FxHashSet::default())); - - *num_calls += 1; - matches.extend(match &types.node { - ExprKind::Tuple { elts, .. } => { - elts.iter().map(std::string::ToString::to_string).collect() - } - _ => { - vec![types.to_string()] - } - }); - } - } + let ExprKind::Call { func, args, .. } = &value.node else { + continue; + }; + if !matches!(&func.node, ExprKind::Name { id, .. } if id == "isinstance") { + continue; } + let [obj, types] = &args[..] else { + continue; + }; + let (num_calls, matches) = obj_to_types + .entry(obj.to_string()) + .or_insert_with(|| (0, FxHashSet::default())); + + *num_calls += 1; + matches.extend(match &types.node { + ExprKind::Tuple { elts, .. } => { + elts.iter().map(std::string::ToString::to_string).collect() + } + _ => { + vec![types.to_string()] + } + }); } for (obj, (num_calls, types)) in obj_to_types { diff --git a/src/pylint/plugins/consider_using_from_import.rs b/src/pylint/plugins/consider_using_from_import.rs index ccf2af19a7..c17f475bc2 100644 --- a/src/pylint/plugins/consider_using_from_import.rs +++ b/src/pylint/plugins/consider_using_from_import.rs @@ -7,14 +7,17 @@ use crate::Check; /// PLR0402 pub fn consider_using_from_import(checker: &mut Checker, alias: &Alias) { - if let Some(asname) = &alias.node.asname { - if let Some((module, name)) = alias.node.name.rsplit_once('.') { - if name == asname { - checker.add_check(Check::new( - CheckKind::ConsiderUsingFromImport(module.to_string(), name.to_string()), - Range::from_located(alias), - )); - } - } + let Some(asname) = &alias.node.asname else { + return; + }; + let Some((module, name)) = alias.node.name.rsplit_once('.') else { + return; + }; + if name != asname { + return; } + checker.add_check(Check::new( + CheckKind::ConsiderUsingFromImport(module.to_string(), name.to_string()), + Range::from_located(alias), + )); } diff --git a/src/pylint/plugins/consider_using_sys_exit.rs b/src/pylint/plugins/consider_using_sys_exit.rs index 5ccd2d4c58..189245270e 100644 --- a/src/pylint/plugins/consider_using_sys_exit.rs +++ b/src/pylint/plugins/consider_using_sys_exit.rs @@ -61,27 +61,29 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) - /// RUF004 pub fn consider_using_sys_exit(checker: &mut Checker, func: &Expr) { - if let ExprKind::Name { id, .. } = &func.node { - for name in ["exit", "quit"] { - if id == name { - if !(name == "exit" && is_module_star_imported(checker, "sys")) - && checker.is_builtin(name) - { - let mut check = - Check::new(CheckKind::ConsiderUsingSysExit, Range::from_located(func)); - if checker.patch(check.kind.code()) { - if let Some(content) = get_member_import_name_alias(checker, "sys", "exit") - { - check.amend(Fix::replacement( - content, - func.location, - func.end_location.unwrap(), - )); - } - } - checker.add_check(check); - } + let ExprKind::Name { id, .. } = &func.node else { + return; + }; + for name in ["exit", "quit"] { + if id != name { + continue; + } + if name == "exit" && is_module_star_imported(checker, "sys") { + continue; + } + if !checker.is_builtin(name) { + continue; + } + let mut check = Check::new(CheckKind::ConsiderUsingSysExit, Range::from_located(func)); + if checker.patch(check.kind.code()) { + if let Some(content) = get_member_import_name_alias(checker, "sys", "exit") { + check.amend(Fix::replacement( + content, + func.location, + func.end_location.unwrap(), + )); } } + checker.add_check(check); } } diff --git a/src/pylint/plugins/misplaced_comparison_constant.rs b/src/pylint/plugins/misplaced_comparison_constant.rs index a985363d70..7be13b5c48 100644 --- a/src/pylint/plugins/misplaced_comparison_constant.rs +++ b/src/pylint/plugins/misplaced_comparison_constant.rs @@ -14,35 +14,43 @@ pub fn misplaced_comparison_constant( ops: &[Cmpop], comparators: &[Expr], ) { - if let ([op], [right]) = (ops, comparators) { - if matches!( - op, - Cmpop::Eq | Cmpop::NotEq | Cmpop::Lt | Cmpop::LtE | Cmpop::Gt | Cmpop::GtE, - ) && matches!(&left.node, &ExprKind::Constant { .. }) - && !matches!(&right.node, &ExprKind::Constant { .. }) - { - let reversed_op = match op { - Cmpop::Eq => "==", - Cmpop::NotEq => "!=", - Cmpop::Lt => ">", - Cmpop::LtE => ">=", - Cmpop::Gt => "<", - Cmpop::GtE => "<=", - _ => unreachable!("Expected comparison operator"), - }; - let suggestion = format!("{right} {reversed_op} {left}"); - let mut check = Check::new( - CheckKind::MisplacedComparisonConstant(suggestion.clone()), - Range::from_located(expr), - ); - if checker.patch(check.kind.code()) { - check.amend(Fix::replacement( - suggestion, - expr.location, - expr.end_location.unwrap(), - )); - } - checker.add_check(check); - } + let ([op], [right]) = (ops, comparators) else { + return; + }; + + if !matches!( + op, + Cmpop::Eq | Cmpop::NotEq | Cmpop::Lt | Cmpop::LtE | Cmpop::Gt | Cmpop::GtE, + ) { + return; } + if !matches!(&left.node, &ExprKind::Constant { .. }) { + return; + } + if matches!(&right.node, &ExprKind::Constant { .. }) { + return; + } + + let reversed_op = match op { + Cmpop::Eq => "==", + Cmpop::NotEq => "!=", + Cmpop::Lt => ">", + Cmpop::LtE => ">=", + Cmpop::Gt => "<", + Cmpop::GtE => "<=", + _ => unreachable!("Expected comparison operator"), + }; + let suggestion = format!("{right} {reversed_op} {left}"); + let mut check = Check::new( + CheckKind::MisplacedComparisonConstant(suggestion.clone()), + Range::from_located(expr), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + suggestion, + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); } diff --git a/src/pylint/plugins/property_with_parameters.rs b/src/pylint/plugins/property_with_parameters.rs index f4dc0d775f..59900577c8 100644 --- a/src/pylint/plugins/property_with_parameters.rs +++ b/src/pylint/plugins/property_with_parameters.rs @@ -12,23 +12,24 @@ pub fn property_with_parameters( decorator_list: &[Expr], args: &Arguments, ) { - if decorator_list + if !decorator_list .iter() .any(|d| matches!(&d.node, ExprKind::Name { id, .. } if id == "property")) { - if checker.is_builtin("property") - && args - .args - .iter() - .chain(args.posonlyargs.iter()) - .chain(args.kwonlyargs.iter()) - .count() - > 1 - { - checker.add_check(Check::new( - CheckKind::PropertyWithParameters, - Range::from_located(stmt), - )); - } + return; + } + if checker.is_builtin("property") + && args + .args + .iter() + .chain(args.posonlyargs.iter()) + .chain(args.kwonlyargs.iter()) + .count() + > 1 + { + checker.add_check(Check::new( + CheckKind::PropertyWithParameters, + Range::from_located(stmt), + )); } } diff --git a/src/pylint/plugins/useless_import_alias.rs b/src/pylint/plugins/useless_import_alias.rs index 6ef789f0e4..5eafc10121 100644 --- a/src/pylint/plugins/useless_import_alias.rs +++ b/src/pylint/plugins/useless_import_alias.rs @@ -8,17 +8,23 @@ use crate::Check; /// PLC0414 pub fn useless_import_alias(checker: &mut Checker, alias: &Alias) { - if let Some(asname) = &alias.node.asname { - if !alias.node.name.contains('.') && &alias.node.name == asname { - let mut check = Check::new(CheckKind::UselessImportAlias, Range::from_located(alias)); - if checker.patch(check.kind.code()) { - check.amend(Fix::replacement( - asname.to_string(), - alias.location, - alias.end_location.unwrap(), - )); - } - checker.add_check(check); - } + let Some(asname) = &alias.node.asname else { + return; + }; + if alias.node.name.contains('.') { + return; } + if &alias.node.name != asname { + return; + } + + let mut check = Check::new(CheckKind::UselessImportAlias, Range::from_located(alias)); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + asname.to_string(), + alias.location, + alias.end_location.unwrap(), + )); + } + checker.add_check(check); } diff --git a/src/pyupgrade/fixes.rs b/src/pyupgrade/fixes.rs index b892d98919..8a0ff67567 100644 --- a/src/pyupgrade/fixes.rs +++ b/src/pyupgrade/fixes.rs @@ -113,26 +113,28 @@ pub fn remove_super_arguments(locator: &SourceCodeLocator, expr: &Expr) -> Optio let mut tree = libcst_native::parse_module(&contents, None).ok()?; - if let Some(Statement::Simple(body)) = tree.body.first_mut() { - if let Some(SmallStatement::Expr(body)) = body.body.first_mut() { - if let Expression::Call(body) = &mut body.value { - body.args = vec![]; - body.whitespace_before_args = ParenthesizableWhitespace::default(); - body.whitespace_after_func = ParenthesizableWhitespace::default(); + let Statement::Simple(body) = tree.body.first_mut()? else { + return None; + }; + let SmallStatement::Expr(body) = body.body.first_mut()? else { + return None; + }; + let Expression::Call(body) = &mut body.value else { + return None; + }; - let mut state = CodegenState::default(); - tree.codegen(&mut state); + body.args = vec![]; + body.whitespace_before_args = ParenthesizableWhitespace::default(); + body.whitespace_after_func = ParenthesizableWhitespace::default(); - return Some(Fix::replacement( - state.to_string(), - range.location, - range.end_location, - )); - } - } - } + let mut state = CodegenState::default(); + tree.codegen(&mut state); - None + Some(Fix::replacement( + state.to_string(), + range.location, + range.end_location, + )) } /// UP010 diff --git a/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs b/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs index 29dec5e8d3..fa87271b68 100644 --- a/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs +++ b/src/pyupgrade/plugins/convert_named_tuple_functional_to_class.rs @@ -17,27 +17,27 @@ fn match_named_tuple_assign<'a>( targets: &'a [Expr], value: &'a Expr, ) -> Option<(&'a str, &'a [Expr], &'a [Keyword], &'a ExprKind)> { - if let Some(target) = targets.get(0) { - if let ExprKind::Name { id: typename, .. } = &target.node { - if let ExprKind::Call { - func, - args, - keywords, - } = &value.node - { - if match_module_member( - func, - "typing", - "NamedTuple", - &checker.from_imports, - &checker.import_aliases, - ) { - return Some((typename, args, keywords, &func.node)); - } - } - } + let target = targets.get(0)?; + let ExprKind::Name { id: typename, .. } = &target.node else { + return None; + }; + let ExprKind::Call { + func, + args, + keywords, + } = &value.node else { + return None; + }; + if !match_module_member( + func, + "typing", + "NamedTuple", + &checker.from_imports, + &checker.import_aliases, + ) { + return None; } - None + Some((typename, args, keywords, &func.node)) } /// Generate a `StmtKind::AnnAssign` representing the provided property @@ -78,13 +78,14 @@ fn create_property_assignment_stmt( /// Match the `defaults` keyword in a `NamedTuple(...)` call. fn match_defaults(keywords: &[Keyword]) -> Result<&[Expr]> { - match keywords.iter().find(|keyword| { + let defaults = keywords.iter().find(|keyword| { if let Some(arg) = &keyword.node.arg { arg.as_str() == "defaults" } else { false } - }) { + }); + match defaults { Some(defaults) => match &defaults.node.value.node { ExprKind::List { elts, .. } => Ok(elts), ExprKind::Tuple { elts, .. } => Ok(elts), @@ -96,53 +97,45 @@ fn match_defaults(keywords: &[Keyword]) -> Result<&[Expr]> { /// Create a list of property assignments from the `NamedTuple` arguments. fn create_properties_from_args(args: &[Expr], defaults: &[Expr]) -> Result> { - if let Some(fields) = args.get(1) { - if let ExprKind::List { elts, .. } = &fields.node { - let padded_defaults = if elts.len() >= defaults.len() { - std::iter::repeat(None) - .take(elts.len() - defaults.len()) - .chain(defaults.iter().map(Some)) - } else { - bail!("Defaults must be `None` or an iterable of at least the number of fields") - }; - elts.iter() - .zip(padded_defaults) - .map(|(field, default)| { - if let ExprKind::Tuple { elts, .. } = &field.node { - if let [field_name, annotation] = elts.as_slice() { - if let ExprKind::Constant { - value: Constant::Str(property), - .. - } = &field_name.node - { - if IDENTIFIER_REGEX.is_match(property) - && !KWLIST.contains(&property.as_str()) - { - Ok(create_property_assignment_stmt( - property, - &annotation.node, - default.map(|d| &d.node), - )) - } else { - bail!("Invalid property name: {}", property) - } - } else { - bail!("Expected `field_name` to be `Constant::Str`") - } - } else { - bail!("Expected `elts` to have exactly two elements") - } - } else { - bail!("Expected `field` to be `ExprKind::Tuple`") - } - }) - .collect() - } else { - bail!("Expected argument to be `ExprKind::List`") - } + let Some(fields) = args.get(1) else { + return Ok(vec![]); + }; + let ExprKind::List { elts, .. } = &fields.node else { + bail!("Expected argument to be `ExprKind::List`"); + }; + + let padded_defaults = if elts.len() >= defaults.len() { + std::iter::repeat(None) + .take(elts.len() - defaults.len()) + .chain(defaults.iter().map(Some)) } else { - Ok(vec![]) - } + bail!("Defaults must be `None` or an iterable of at least the number of fields") + }; + elts.iter() + .zip(padded_defaults) + .map(|(field, default)| { + let ExprKind::Tuple { elts, .. } = &field.node else { + bail!("Expected `field` to be `ExprKind::Tuple`") + }; + let [field_name, annotation] = elts.as_slice() else { + bail!("Expected `elts` to have exactly two elements") + }; + let ExprKind::Constant { + value: Constant::Str(property), + .. + } = &field_name.node else { + bail!("Expected `field_name` to be `Constant::Str`") + }; + if !IDENTIFIER_REGEX.is_match(property) || KWLIST.contains(&property.as_str()) { + bail!("Invalid property name: {}", property) + } + Ok(create_property_assignment_stmt( + property, + &annotation.node, + default.map(|d| &d.node), + )) + }) + .collect() } /// Generate a `StmtKind:ClassDef` statement based on the provided body and @@ -188,26 +181,27 @@ pub fn convert_named_tuple_functional_to_class( targets: &[Expr], value: &Expr, ) { - if let Some((typename, args, keywords, base_class)) = - match_named_tuple_assign(checker, targets, value) + let Some((typename, args, keywords, base_class)) = + match_named_tuple_assign(checker, targets, value) else { - match match_defaults(keywords) { - Ok(defaults) => { - if let Ok(properties) = create_properties_from_args(args, defaults) { - let mut check = Check::new( - CheckKind::ConvertNamedTupleFunctionalToClass(typename.to_string()), - Range::from_located(stmt), - ); - if checker.patch(check.kind.code()) { - match convert_to_class(stmt, typename, properties, base_class) { - Ok(fix) => check.amend(fix), - Err(err) => error!("Failed to convert `NamedTuple`: {err}"), - } + return; + }; + match match_defaults(keywords) { + Ok(defaults) => { + if let Ok(properties) = create_properties_from_args(args, defaults) { + let mut check = Check::new( + CheckKind::ConvertNamedTupleFunctionalToClass(typename.to_string()), + Range::from_located(stmt), + ); + if checker.patch(check.kind.code()) { + match convert_to_class(stmt, typename, properties, base_class) { + Ok(fix) => check.amend(fix), + Err(err) => error!("Failed to convert `NamedTuple`: {err}"), } - checker.add_check(check); } + checker.add_check(check); } - Err(err) => error!("Failed to parse defaults: {err}"), } + Err(err) => error!("Failed to parse defaults: {err}"), } } diff --git a/src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs b/src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs index 6465f45a8c..740f996034 100644 --- a/src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs +++ b/src/pyupgrade/plugins/convert_typed_dict_functional_to_class.rs @@ -20,27 +20,27 @@ fn match_typed_dict_assign<'a>( targets: &'a [Expr], value: &'a Expr, ) -> Option<(&'a str, &'a [Expr], &'a [Keyword], &'a ExprKind)> { - if let Some(target) = targets.get(0) { - if let ExprKind::Name { id: class_name, .. } = &target.node { - if let ExprKind::Call { - func, - args, - keywords, - } = &value.node - { - if match_module_member( - func, - "typing", - "TypedDict", - &checker.from_imports, - &checker.import_aliases, - ) { - return Some((class_name, args, keywords, &func.node)); - } - } - } + let target = targets.get(0)?; + let ExprKind::Name { id: class_name, .. } = &target.node else { + return None; + }; + let ExprKind::Call { + func, + args, + keywords, + } = &value.node else { + return None; + }; + if !match_module_member( + func, + "typing", + "TypedDict", + &checker.from_imports, + &checker.import_aliases, + ) { + return None; } - None + Some((class_name, args, keywords, &func.node)) } /// Generate a `StmtKind::AnnAssign` representing the provided property @@ -127,15 +127,13 @@ fn get_properties_from_dict_literal(keys: &[Expr], values: &[Expr]) -> Result Result> { - if let ExprKind::Name { id, .. } = &func.node { - if id == "dict" { - get_properties_from_keywords(keywords) - } else { - bail!("Expected `id` to be `\"dict\"`") - } - } else { + let ExprKind::Name { id, .. } = &func.node else { bail!("Expected `func` to be `ExprKind::Name`") + }; + if id != "dict" { + bail!("Expected `id` to be `\"dict\"`") } + get_properties_from_keywords(keywords) } // Deprecated in Python 3.11, removed in Python 3.13. @@ -158,15 +156,11 @@ fn get_properties_from_keywords(keywords: &[Keyword]) -> Result> { // The only way to have the `total` keyword is to use the args version, like: // (`TypedDict('name', {'a': int}, total=True)`) fn get_total_from_only_keyword(keywords: &[Keyword]) -> Option<&KeywordData> { - match keywords.get(0) { - Some(keyword) => match &keyword.node.arg { - Some(arg) => match arg.as_str() { - "total" => Some(&keyword.node), - _ => None, - }, - None => None, - }, - None => None, + let keyword = keywords.get(0)?; + let arg = &keyword.node.arg.as_ref()?; + match arg.as_str() { + "total" => Some(&keyword.node), + _ => None, } } @@ -225,24 +219,27 @@ pub fn convert_typed_dict_functional_to_class( targets: &[Expr], value: &Expr, ) { - if let Some((class_name, args, keywords, base_class)) = - match_typed_dict_assign(checker, targets, value) + let Some((class_name, args, keywords, base_class)) = + match_typed_dict_assign(checker, targets, value) else { - match get_properties_and_total(args, keywords) { - Err(err) => error!("Failed to parse TypedDict: {err}"), - Ok((body, total_keyword)) => { - let mut check = Check::new( - CheckKind::ConvertTypedDictFunctionalToClass(class_name.to_string()), - Range::from_located(stmt), - ); - if checker.patch(check.kind.code()) { - match convert_to_class(stmt, class_name, body, total_keyword, base_class) { - Ok(fix) => check.amend(fix), - Err(err) => error!("Failed to convert TypedDict: {err}"), - }; - } - checker.add_check(check); - } + return; + }; + let (body, total_keyword) = match get_properties_and_total(args, keywords) { + Err(err) => { + error!("Failed to parse TypedDict: {err}"); + return; } + Ok(args) => args, + }; + let mut check = Check::new( + CheckKind::ConvertTypedDictFunctionalToClass(class_name.to_string()), + Range::from_located(stmt), + ); + if checker.patch(check.kind.code()) { + match convert_to_class(stmt, class_name, body, total_keyword, base_class) { + Ok(fix) => check.amend(fix), + Err(err) => error!("Failed to convert TypedDict: {err}"), + }; } + checker.add_check(check); } diff --git a/src/pyupgrade/plugins/deprecated_unittest_alias.rs b/src/pyupgrade/plugins/deprecated_unittest_alias.rs index c127fcc57b..8a47d53e21 100644 --- a/src/pyupgrade/plugins/deprecated_unittest_alias.rs +++ b/src/pyupgrade/plugins/deprecated_unittest_alias.rs @@ -29,24 +29,28 @@ static DEPRECATED_ALIASES: Lazy> = Lazy::n /// UP005 pub fn deprecated_unittest_alias(checker: &mut Checker, expr: &Expr) { - if let ExprKind::Attribute { value, attr, .. } = &expr.node { - if let Some(&target) = DEPRECATED_ALIASES.get(attr.as_str()) { - if let ExprKind::Name { id, .. } = &value.node { - if id == "self" { - let mut check = Check::new( - CheckKind::DeprecatedUnittestAlias(attr.to_string(), target.to_string()), - Range::from_located(expr), - ); - if checker.patch(check.kind.code()) { - check.amend(Fix::replacement( - format!("self.{target}"), - expr.location, - expr.end_location.unwrap(), - )); - } - checker.add_check(check); - } - } - } + let ExprKind::Attribute { value, attr, .. } = &expr.node else { + return; + }; + let Some(&target) = DEPRECATED_ALIASES.get(attr.as_str()) else { + return; + }; + let ExprKind::Name { id, .. } = &value.node else { + return; + }; + if id != "self" { + return; } + let mut check = Check::new( + CheckKind::DeprecatedUnittestAlias(attr.to_string(), target.to_string()), + Range::from_located(expr), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + format!("self.{target}"), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); } diff --git a/src/pyupgrade/plugins/super_call_with_parameters.rs b/src/pyupgrade/plugins/super_call_with_parameters.rs index 2a9d27afc9..5fee7af156 100644 --- a/src/pyupgrade/plugins/super_call_with_parameters.rs +++ b/src/pyupgrade/plugins/super_call_with_parameters.rs @@ -9,20 +9,22 @@ use crate::pyupgrade::checks; pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { // Only bother going through the super check at all if we're in a `super` call. // (We check this in `check_super_args` too, so this is just an optimization.) - if helpers::is_super_call_with_arguments(func, args) { - let scope = checker.current_scope(); - let parents: Vec<&Stmt> = checker - .parent_stack - .iter() - .map(|index| checker.parents[*index]) - .collect(); - if let Some(mut check) = checks::super_args(scope, &parents, expr, func, args) { - if checker.patch(check.kind.code()) { - if let Some(fix) = pyupgrade::fixes::remove_super_arguments(checker.locator, expr) { - check.amend(fix); - } - } - checker.add_check(check); + if !helpers::is_super_call_with_arguments(func, args) { + return; + } + let scope = checker.current_scope(); + let parents: Vec<&Stmt> = checker + .parent_stack + .iter() + .map(|index| checker.parents[*index]) + .collect(); + let Some(mut check) = checks::super_args(scope, &parents, expr, func, args) else { + return; + }; + if checker.patch(check.kind.code()) { + if let Some(fix) = pyupgrade::fixes::remove_super_arguments(checker.locator, expr) { + check.amend(fix); } } + checker.add_check(check); } diff --git a/src/pyupgrade/plugins/type_of_primitive.rs b/src/pyupgrade/plugins/type_of_primitive.rs index b369b84a96..f467c3087c 100644 --- a/src/pyupgrade/plugins/type_of_primitive.rs +++ b/src/pyupgrade/plugins/type_of_primitive.rs @@ -8,16 +8,17 @@ use crate::pyupgrade::checks; /// UP003 pub fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { - if let Some(mut check) = checks::type_of_primitive(func, args, Range::from_located(expr)) { - if checker.patch(check.kind.code()) { - if let CheckKind::TypeOfPrimitive(primitive) = &check.kind { - check.amend(Fix::replacement( - primitive.builtin(), - expr.location, - expr.end_location.unwrap(), - )); - } + let Some(mut check) = checks::type_of_primitive(func, args, Range::from_located(expr)) else { + return; + }; + if checker.patch(check.kind.code()) { + if let CheckKind::TypeOfPrimitive(primitive) = &check.kind { + check.amend(Fix::replacement( + primitive.builtin(), + expr.location, + expr.end_location.unwrap(), + )); } - checker.add_check(check); } + checker.add_check(check); } diff --git a/src/pyupgrade/plugins/unnecessary_encode_utf8.rs b/src/pyupgrade/plugins/unnecessary_encode_utf8.rs index 42a6f6dfed..ecce2a7520 100644 --- a/src/pyupgrade/plugins/unnecessary_encode_utf8.rs +++ b/src/pyupgrade/plugins/unnecessary_encode_utf8.rs @@ -9,17 +9,17 @@ use crate::source_code_locator::SourceCodeLocator; const UTF8_LITERALS: &[&str] = &["utf-8", "utf8", "utf_8", "u8", "utf", "cp65001"]; fn match_encoded_variable(func: &Expr) -> Option<&Expr> { - if let ExprKind::Attribute { + let ExprKind::Attribute { value: variable, attr, .. - } = &func.node - { - if attr == "encode" { - return Some(variable); - } + } = &func.node else { + return None; + }; + if attr != "encode" { + return None; } - None + Some(variable) } fn is_utf8_encoding_arg(arg: &Expr) -> bool { @@ -109,39 +109,27 @@ pub fn unnecessary_encode_utf8( args: &Vec, kwargs: &Vec, ) { - if let Some(variable) = match_encoded_variable(func) { - match &variable.node { - ExprKind::Constant { - value: Constant::Str(literal), - .. - } => { - // "str".encode() - // "str".encode("utf-8") - if is_default_encode(args, kwargs) { - if literal.is_ascii() { - // "foo".encode() - checker.add_check(replace_with_bytes_literal( - expr, - variable, - checker.locator, - checker.patch(&CheckCode::UP012), - )); - } else { - // "unicode text©".encode("utf-8") - if let Some(check) = delete_default_encode_arg_or_kwarg( - expr, - args, - kwargs, - checker.patch(&CheckCode::UP012), - ) { - checker.add_check(check); - } - } - } - } - // f"foo{bar}".encode(*args, **kwargs) - ExprKind::JoinedStr { .. } => { - if is_default_encode(args, kwargs) { + let Some(variable) = match_encoded_variable(func) else { + return; + }; + match &variable.node { + ExprKind::Constant { + value: Constant::Str(literal), + .. + } => { + // "str".encode() + // "str".encode("utf-8") + if is_default_encode(args, kwargs) { + if literal.is_ascii() { + // "foo".encode() + checker.add_check(replace_with_bytes_literal( + expr, + variable, + checker.locator, + checker.patch(&CheckCode::UP012), + )); + } else { + // "unicode text©".encode("utf-8") if let Some(check) = delete_default_encode_arg_or_kwarg( expr, args, @@ -152,7 +140,20 @@ pub fn unnecessary_encode_utf8( } } } - _ => {} } + // f"foo{bar}".encode(*args, **kwargs) + ExprKind::JoinedStr { .. } => { + if is_default_encode(args, kwargs) { + if let Some(check) = delete_default_encode_arg_or_kwarg( + expr, + args, + kwargs, + checker.patch(&CheckCode::UP012), + ) { + checker.add_check(check); + } + } + } + _ => {} } } diff --git a/src/pyupgrade/plugins/unnecessary_future_import.rs b/src/pyupgrade/plugins/unnecessary_future_import.rs index 342a6de865..5a2260b90e 100644 --- a/src/pyupgrade/plugins/unnecessary_future_import.rs +++ b/src/pyupgrade/plugins/unnecessary_future_import.rs @@ -49,36 +49,35 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo } } - if !removable_index.is_empty() { - let mut check = Check::new( - CheckKind::UnnecessaryFutureImport( - removable_names.into_iter().map(String::from).collect(), - ), - Range::from_located(stmt), - ); - if checker.patch(check.kind.code()) { - let context = checker.binding_context(); - let deleted: Vec<&Stmt> = checker - .deletions - .iter() - .map(|index| checker.parents[*index]) - .collect(); - match fixes::remove_unnecessary_future_import( - checker.locator, - &removable_index, - checker.parents[context.defined_by], - context.defined_in.map(|index| checker.parents[index]), - &deleted, - ) { - Ok(fix) => { - if fix.content.is_empty() || fix.content == "pass" { - checker.deletions.insert(context.defined_by); - } - check.amend(fix); - } - Err(e) => error!("Failed to remove __future__ import: {e}"), - } - } - checker.add_check(check); + if removable_index.is_empty() { + return; } + let mut check = Check::new( + CheckKind::UnnecessaryFutureImport(removable_names.into_iter().map(String::from).collect()), + Range::from_located(stmt), + ); + if checker.patch(check.kind.code()) { + let context = checker.binding_context(); + let deleted: Vec<&Stmt> = checker + .deletions + .iter() + .map(|index| checker.parents[*index]) + .collect(); + match fixes::remove_unnecessary_future_import( + checker.locator, + &removable_index, + checker.parents[context.defined_by], + context.defined_in.map(|index| checker.parents[index]), + &deleted, + ) { + Ok(fix) => { + if fix.content.is_empty() || fix.content == "pass" { + checker.deletions.insert(context.defined_by); + } + check.amend(fix); + } + Err(e) => error!("Failed to remove __future__ import: {e}"), + } + } + checker.add_check(check); } diff --git a/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs b/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs index c4fdad632e..7273825c91 100644 --- a/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs +++ b/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs @@ -6,15 +6,16 @@ use crate::pyupgrade::checks; /// UP011 pub fn unnecessary_lru_cache_params(checker: &mut Checker, decorator_list: &[Expr]) { - if let Some(mut check) = checks::unnecessary_lru_cache_params( + let Some(mut check) = checks::unnecessary_lru_cache_params( decorator_list, checker.settings.target_version, &checker.from_imports, &checker.import_aliases, - ) { - if checker.patch(check.kind.code()) { - check.amend(Fix::deletion(check.location, check.end_location)); - } - checker.add_check(check); + ) else { + return; + }; + if checker.patch(check.kind.code()) { + check.amend(Fix::deletion(check.location, check.end_location)); } + checker.add_check(check); } diff --git a/src/pyupgrade/plugins/useless_metaclass_type.rs b/src/pyupgrade/plugins/useless_metaclass_type.rs index e7a0883740..381a539a46 100644 --- a/src/pyupgrade/plugins/useless_metaclass_type.rs +++ b/src/pyupgrade/plugins/useless_metaclass_type.rs @@ -8,31 +8,31 @@ use crate::pyupgrade::checks; /// UP001 pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, targets: &[Expr]) { - if let Some(mut check) = - checks::useless_metaclass_type(targets, value, Range::from_located(stmt)) - { - if checker.patch(check.kind.code()) { - let context = checker.binding_context(); - let deleted: Vec<&Stmt> = checker - .deletions - .iter() - .map(|index| checker.parents[*index]) - .collect(); + let Some(mut check) = + checks::useless_metaclass_type(targets, value, Range::from_located(stmt)) else { + return; + }; + if checker.patch(check.kind.code()) { + let context = checker.binding_context(); + let deleted: Vec<&Stmt> = checker + .deletions + .iter() + .map(|index| checker.parents[*index]) + .collect(); - match helpers::remove_stmt( - checker.parents[context.defined_by], - context.defined_in.map(|index| checker.parents[index]), - &deleted, - ) { - Ok(fix) => { - if fix.content.is_empty() || fix.content == "pass" { - checker.deletions.insert(context.defined_by); - } - check.amend(fix); + match helpers::remove_stmt( + checker.parents[context.defined_by], + context.defined_in.map(|index| checker.parents[index]), + &deleted, + ) { + Ok(fix) => { + if fix.content.is_empty() || fix.content == "pass" { + checker.deletions.insert(context.defined_by); } - Err(e) => error!("Failed to fix remove metaclass type: {e}"), + check.amend(fix); } + Err(e) => error!("Failed to fix remove metaclass type: {e}"), } - checker.add_check(check); } + checker.add_check(check); } diff --git a/src/pyupgrade/plugins/useless_object_inheritance.rs b/src/pyupgrade/plugins/useless_object_inheritance.rs index f6ede9d3eb..81ec20697b 100644 --- a/src/pyupgrade/plugins/useless_object_inheritance.rs +++ b/src/pyupgrade/plugins/useless_object_inheritance.rs @@ -13,18 +13,19 @@ pub fn useless_object_inheritance( keywords: &[Keyword], ) { let scope = checker.current_scope(); - if let Some(mut check) = checks::useless_object_inheritance(name, bases, scope) { - if checker.patch(check.kind.code()) { - if let Some(fix) = pyupgrade::fixes::remove_class_def_base( - checker.locator, - stmt.location, - check.location, - bases, - keywords, - ) { - check.amend(fix); - } + let Some(mut check) = checks::useless_object_inheritance(name, bases, scope) else { + return; + }; + if checker.patch(check.kind.code()) { + if let Some(fix) = pyupgrade::fixes::remove_class_def_base( + checker.locator, + stmt.location, + check.location, + bases, + keywords, + ) { + check.amend(fix); } - checker.add_check(check); } + checker.add_check(check); }