Remove some indexing (#6728)

**Summary** A common pattern in the code used to be
```rust
if statements.len() != 1 {
    return;
}
use_single_entry(statements[0])?;
```
which can be better expressed as
```rust
let [statement] = statements else {
    return;
};
use_single_entry(statements)?;
```

Direct indexing can cause panics if you don't manually take care of
checking the length, while matching (such as if-let or let-else) can
never panic.

This isn't a complete refactor, i've just removed some of the obvious
cases. I've specifically looked for `.len() != 1` and fixed those.

**Test Plan** No functional changes
This commit is contained in:
konsti 2023-08-21 16:56:15 +02:00 committed by GitHub
parent 2405536d03
commit aafde6db28
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 50 additions and 67 deletions

View file

@ -90,8 +90,7 @@ impl AlwaysAutofixableViolation for DuplicateHandlerException {
#[derive_message_formats]
fn message(&self) -> String {
let DuplicateHandlerException { names } = self;
if names.len() == 1 {
let name = &names[0];
if let [name] = names.as_slice() {
format!("Exception handler with duplicate exception: `{name}`")
} else {
let names = names.iter().map(|name| format!("`{name}`")).join(", ");

View file

@ -50,28 +50,28 @@ impl AlwaysAutofixableViolation for UnnecessaryPass {
/// PIE790
pub(crate) fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) {
if body.len() > 1 {
// This only catches the case in which a docstring makes a `pass` statement
// redundant. Consider removing all `pass` statements instead.
if !is_docstring_stmt(&body[0]) {
return;
}
// The second statement must be a `pass` statement.
let stmt = &body[1];
if !stmt.is_pass_stmt() {
return;
}
let mut diagnostic = Diagnostic::new(UnnecessaryPass, stmt.range());
if checker.patch(diagnostic.kind.rule()) {
let edit = if let Some(index) = trailing_comment_start_offset(stmt, checker.locator()) {
Edit::range_deletion(stmt.range().add_end(index))
} else {
autofix::edits::delete_stmt(stmt, None, checker.locator(), checker.indexer())
};
diagnostic.set_fix(Fix::automatic(edit));
}
checker.diagnostics.push(diagnostic);
let [first, second, ..] = body else {
return;
};
// This only catches the case in which a docstring makes a `pass` statement
// redundant. Consider removing all `pass` statements instead.
if !is_docstring_stmt(first) {
return;
}
// The second statement must be a `pass` statement.
if !second.is_pass_stmt() {
return;
}
let mut diagnostic = Diagnostic::new(UnnecessaryPass, second.range());
if checker.patch(diagnostic.kind.rule()) {
let edit = if let Some(index) = trailing_comment_start_offset(second, checker.locator()) {
Edit::range_deletion(second.range().add_end(index))
} else {
autofix::edits::delete_stmt(second, None, checker.locator(), checker.indexer())
};
diagnostic.set_fix(Fix::automatic(edit));
}
checker.diagnostics.push(diagnostic);
}

View file

@ -22,10 +22,7 @@ impl AlwaysAutofixableViolation for NonEmptyStubBody {
/// PYI010
pub(crate) fn non_empty_stub_body(checker: &mut Checker, body: &[Stmt]) {
if body.len() != 1 {
return;
}
if let Stmt::Expr(ast::StmtExpr { value, range: _ }) = &body[0] {
if let [Stmt::Expr(ast::StmtExpr { value, range: _ })] = body {
if let Expr::Constant(ast::ExprConstant { value, .. }) = value.as_ref() {
if matches!(value, Constant::Ellipsis | Constant::Str(_)) {
return;

View file

@ -465,16 +465,15 @@ fn match_eq_target(expr: &Expr) -> Option<(&str, &Expr)> {
else {
return None;
};
if ops.len() != 1 || comparators.len() != 1 {
return None;
}
if !matches!(&ops[0], CmpOp::Eq) {
if ops != &[CmpOp::Eq] {
return None;
}
let Expr::Name(ast::ExprName { id, .. }) = left.as_ref() else {
return None;
};
let comparator = &comparators[0];
let [comparator] = comparators.as_slice() else {
return None;
};
if !matches!(&comparator, Expr::Name(_)) {
return None;
}

View file

@ -878,7 +878,7 @@ pub(crate) fn use_dict_get_with_default(checker: &mut Checker, stmt_if: &ast::St
else {
return;
};
if body_var.len() != 1 {
let [body_var] = body_var.as_slice() else {
return;
};
let Stmt::Assign(ast::StmtAssign {
@ -889,7 +889,7 @@ pub(crate) fn use_dict_get_with_default(checker: &mut Checker, stmt_if: &ast::St
else {
return;
};
if orelse_var.len() != 1 {
let [orelse_var] = orelse_var.as_slice() else {
return;
};
let Expr::Compare(ast::ExprCompare {
@ -901,27 +901,16 @@ pub(crate) fn use_dict_get_with_default(checker: &mut Checker, stmt_if: &ast::St
else {
return;
};
if test_dict.len() != 1 {
let [test_dict] = test_dict.as_slice() else {
return;
}
};
let (expected_var, expected_value, default_var, default_value) = match ops[..] {
[CmpOp::In] => (
&body_var[0],
body_value,
&orelse_var[0],
orelse_value.as_ref(),
),
[CmpOp::NotIn] => (
&orelse_var[0],
orelse_value,
&body_var[0],
body_value.as_ref(),
),
[CmpOp::In] => (body_var, body_value, orelse_var, orelse_value.as_ref()),
[CmpOp::NotIn] => (orelse_var, orelse_value, body_var, body_value.as_ref()),
_ => {
return;
}
};
let test_dict = &test_dict[0];
let Expr::Subscript(ast::ExprSubscript {
value: expected_subscript,
slice: expected_slice,

View file

@ -267,13 +267,10 @@ fn match_loop(stmt: &Stmt) -> Option<Loop> {
else {
return None;
};
if nested_body.len() != 1 {
return None;
}
if !nested_elif_else_clauses.is_empty() {
return None;
}
let Stmt::Return(ast::StmtReturn { value, range: _ }) = &nested_body[0] else {
let [Stmt::Return(ast::StmtReturn { value, range: _ })] = nested_body.as_slice() else {
return None;
};
let Some(value) = value else {

View file

@ -114,14 +114,17 @@ pub(crate) fn static_join_to_fstring(checker: &mut Checker, expr: &Expr, joiner:
return;
};
if !keywords.is_empty() || args.len() != 1 {
// If there are kwargs or more than one argument, this is some non-standard
// string join call.
// If there are kwargs or more than one argument, this is some non-standard
// string join call.
if !keywords.is_empty() {
return;
}
let [arg] = args.as_slice() else {
return;
};
// Get the elements to join; skip (e.g.) generators, sets, etc.
let joinees = match &args[0] {
let joinees = match &arg {
Expr::List(ast::ExprList { elts, .. }) if is_static_length(elts) => elts,
Expr::Tuple(ast::ExprTuple { elts, .. }) if is_static_length(elts) => elts,
_ => return,

View file

@ -54,9 +54,9 @@ impl Violation for TypeOfPrimitive {
/// UP003
pub(crate) fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) {
if args.len() != 1 {
let [arg] = args else {
return;
}
};
if !checker
.semantic()
.resolve_call_path(func)
@ -64,7 +64,7 @@ pub(crate) fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr,
{
return;
}
let Expr::Constant(ast::ExprConstant { value, .. }) = &args[0] else {
let Expr::Constant(ast::ExprConstant { value, .. }) = &arg else {
return;
};
let Some(primitive) = Primitive::from_constant(value) else {

View file

@ -90,12 +90,11 @@ fn is_stdin(files: &[PathBuf], stdin_filename: Option<&Path>) -> bool {
return true;
}
let [file] = files else {
return false;
};
// If the user provided exactly `-`, read from standard input.
if files.len() == 1 && files[0] == Path::new("-") {
return true;
}
false
file == Path::new("-")
}
pub fn run(