mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-07-24 05:25:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Misc. stylistic changes from flipping through rules late at night (#5757)

Browse source 
## Summary

This is really bad PR hygiene, but a mix of: using `Locator`-based fixes
in a few places (in lieu of `Generator`-based fixes), using match syntax
to avoid `.len() == 1` checks, using common helpers in more places, etc.

## Test Plan

`cargo test`
This commit is contained in:
Charlie Marsh 2023-07-14 01:23:47 -04:00 • committed by GitHub
parent 875e04e369
commit 5a4516b812
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 127 additions and 169 deletions
Download patch file Download diff file Expand all files Collapse all files

5
crates/ruff/resources/test/fixtures/flake8_pyi/PYI016.pyi vendored
View file

@ -59,7 +59,6 @@ field18: typing.Union[
],
] # Error, newline and comment will not be emitted in message
# Should emit in cases with `typing.Union` instead of `|`
field19: typing.Union[int, int] # Error
@ -71,3 +70,7 @@ field21: typing.Union[int, int | str] # Error
# Should emit only once in cases with multiple nested `typing.Union`
field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
# Should emit in cases with newlines
field23: set[ # foo
int] | set[int]

6
crates/ruff/src/rules/airflow/rules/task_variable_name.rs
View file

@ -51,11 +51,9 @@ pub(crate) fn variable_name_task_id(
value: &Expr,
) -> Option<Diagnostic> {
// If we have more than one target, we can't do anything.
if targets.len() != 1 {
let [target] = targets else {
return None;
}
let target = &targets[0];
};
let Expr::Name(ast::ExprName { id, .. }) = target else {
return None;
};

37
crates/ruff/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs
View file

@ -5,6 +5,7 @@ use rustpython_parser::ast::{self, Expr, Operator, Ranged};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::any_over_expr;
use ruff_python_semantic::SemanticModel;
use crate::checkers::ast::Checker;
@ -52,7 +53,7 @@ fn matches_sql_statement(string: &str) -> bool {
SQL_REGEX.is_match(string)
}
fn unparse_string_format_expression(checker: &mut Checker, expr: &Expr) -> Option<String> {
fn matches_string_format_expression(expr: &Expr, model: &SemanticModel) -> bool {
match expr {
// "select * from table where val = " + "str" + ...
// "select * from table where val = %s" % ...
@ -60,45 +61,37 @@ fn unparse_string_format_expression(checker: &mut Checker, expr: &Expr) -> Optio
op: Operator::Add | Operator::Mod,
..
}) => {
let Some(parent) = checker.semantic().expr_parent() else {
if any_over_expr(expr, &has_string_literal) {
return Some(checker.generator().expr(expr));
}
return None;
};
// Only evaluate the full BinOp, not the nested components.
let Expr::BinOp(_) = parent else {
if model
.expr_parent()
.map_or(true, |parent| !parent.is_bin_op_expr())
{
if any_over_expr(expr, &has_string_literal) {
return Some(checker.generator().expr(expr));
return true;
}
return None;
};
None
}
false
}
Expr::Call(ast::ExprCall { func, .. }) => {
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() else {
return None;
return false;
};
// "select * from table where val = {}".format(...)
if attr == "format" && string_literal(value).is_some() {
return Some(checker.generator().expr(expr));
};
None
attr == "format" && string_literal(value).is_some()
}
// f"select * from table where val = {val}"
Expr::JoinedStr(_) => Some(checker.generator().expr(expr)),
_ => None,
Expr::JoinedStr(_) => true,
_ => false,
}
}
/// S608
pub(crate) fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) {
match unparse_string_format_expression(checker, expr) {
Some(string) if matches_sql_statement(&string) => {
if matches_string_format_expression(expr, checker.semantic()) {
if matches_sql_statement(&checker.generator().expr(expr)) {
checker
.diagnostics
.push(Diagnostic::new(HardcodedSQLExpression, expr.range()));
}
_ => (),
}
}

6
crates/ruff/src/rules/flake8_bugbear/rules/assignment_to_os_environ.rs
View file

@ -47,12 +47,12 @@ impl Violation for AssignmentToOsEnviron {
format!("Assigning to `os.environ` doesn't clear the environment")
}
}
/// B003
pub(crate) fn assignment_to_os_environ(checker: &mut Checker, targets: &[Expr]) {
if targets.len() != 1 {
let [target] = targets else {
return;
}
let target = &targets[0];
};
let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = target else {
return;
};

16
crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs
View file

@ -1,5 +1,4 @@
use ruff_text_size::TextRange;
use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Identifier, Ranged};
use rustpython_parser::ast::{self, Constant, Expr, Ranged};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
@ -47,15 +46,6 @@ impl AlwaysAutofixableViolation for GetAttrWithConstant {
"Replace `getattr` with attribute access".to_string()
}
}
fn attribute(value: &Expr, attr: &str) -> Expr {
ast::ExprAttribute {
value: Box::new(value.clone()),
attr: Identifier::new(attr.to_string(), TextRange::default()),
ctx: ExprContext::Load,
range: TextRange::default(),
}
.into()
}
/// B009
pub(crate) fn getattr_with_constant(
@ -83,14 +73,14 @@ pub(crate) fn getattr_with_constant(
if !is_identifier(value) {
return;
}
if is_mangled_private(value.as_str()) {
if is_mangled_private(value) {
return;
}
let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
checker.generator().expr(&attribute(obj, value)),
format!("{}.{}", checker.locator.slice(obj.range()), value),
expr.range(),
)));
}

7
crates/ruff/src/rules/flake8_bugbear/rules/strip_with_multi_characters.rs
View file

@ -55,14 +55,11 @@ pub(crate) fn strip_with_multi_characters(
if !matches!(attr.as_str(), "strip" | "lstrip" | "rstrip") {
return;
}
if args.len() != 1 {
return;
}
let Expr::Constant(ast::ExprConstant {
let [Expr::Constant(ast::ExprConstant {
value: Constant::Str(value),
..
}) = &args[0]
})] = args
else {
return;
};

8
crates/ruff/src/rules/flake8_comprehensions/rules/helpers.rs
View file

@ -14,16 +14,16 @@ pub(super) fn exactly_one_argument_with_matching_function<'a>(
args: &'a [Expr],
keywords: &[Keyword],
) -> Option<&'a Expr> {
if !keywords.is_empty() {
let [arg] = args else {
return None;
}
if args.len() != 1 {
};
if !keywords.is_empty() {
return None;
}
if expr_name(func)? != name {
return None;
}
Some(&args[0])
Some(arg)
}
pub(super) fn first_argument_with_matching_function<'a>(

10
crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs
View file

@ -82,10 +82,9 @@ pub(crate) fn unnecessary_dict_comprehension(
value: &Expr,
generators: &[Comprehension],
) {
if generators.len() != 1 {
let [generator] = generators else {
return;
}
let generator = &generators[0];
};
if !generator.ifs.is_empty() || generator.is_async {
return;
}
@ -123,10 +122,9 @@ pub(crate) fn unnecessary_list_set_comprehension(
elt: &Expr,
generators: &[Comprehension],
) {
if generators.len() != 1 {
let [generator] = generators else {
return;
}
let generator = &generators[0];
};
if !generator.ifs.is_empty() || generator.is_async {
return;
}

5
crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs
View file

@ -61,10 +61,7 @@ pub(crate) fn duplicate_class_field_definition<'a, 'b>(
// Extract the property name from the assignment statement.
let target = match stmt {
Stmt::Assign(ast::StmtAssign { targets, .. }) => {
if targets.len() != 1 {
continue;
}
if let Expr::Name(ast::ExprName { id, .. }) = &targets[0] {
if let [Expr::Name(ast::ExprName { id, .. })] = targets.as_slice() {
id
} else {
continue;

5
crates/ruff/src/rules/flake8_pyi/rules/duplicate_union_member.rs
View file

@ -52,10 +52,9 @@ pub(crate) fn duplicate_union_member<'a>(checker: &mut Checker, expr: &'a Expr)
// If the parent node is not a `BinOp` we will not perform a fix
if let Some(Expr::BinOp(ast::ExprBinOp { left, right, .. })) = parent {
// Replace the parent with its non-duplicate child.
let child = if expr == left.as_ref() { right } else { left };
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
checker
.generator()
.expr(if expr == left.as_ref() { right } else { left }),
checker.locator.slice(child.range()).to_string(),
parent.unwrap().range(),
)));
}

6
crates/ruff/src/rules/flake8_pyi/rules/prefix_type_params.rs
View file

@ -60,10 +60,10 @@ impl Violation for UnprefixedTypeParam {
/// PYI001
pub(crate) fn prefix_type_params(checker: &mut Checker, value: &Expr, targets: &[Expr]) {
if targets.len() != 1 {
let [target] = targets else {
return;
}
if let Expr::Name(ast::ExprName { id, .. }) = &targets[0] {
};
if let Expr::Name(ast::ExprName { id, .. }) = target {
if id.starts_with('_') {
return;
}

10
crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs
View file

@ -399,10 +399,9 @@ pub(crate) fn argument_simple_defaults(checker: &mut Checker, arguments: &Argume
/// PYI015
pub(crate) fn assignment_default_in_stub(checker: &mut Checker, targets: &[Expr], value: &Expr) {
if targets.len() != 1 {
let [target] = targets else {
return;
}
let target = &targets[0];
};
if !target.is_name_expr() {
return;
}
@ -471,10 +470,9 @@ pub(crate) fn unannotated_assignment_in_stub(
targets: &[Expr],
value: &Expr,
) {
if targets.len() != 1 {
let [target] = targets else {
return;
}
let target = &targets[0];
};
let Expr::Name(ast::ExprName { id, .. }) = target else {
return;
};

86
crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI016_PYI016.pyi.snap
View file

@ -395,68 +395,90 @@ PYI016.pyi:57:5: PYI016 Duplicate union member `set[int]`
|
= help: Remove duplicate union member `set[int]`
PYI016.pyi:64:28: PYI016 Duplicate union member `int`
PYI016.pyi:63:28: PYI016 Duplicate union member `int`
|
63 | # Should emit in cases with `typing.Union` instead of `|`
64 | field19: typing.Union[int, int] # Error
62 | # Should emit in cases with `typing.Union` instead of `|`
63 | field19: typing.Union[int, int] # Error
| ^^^ PYI016
65 |
66 | # Should emit in cases with nested `typing.Union`
64 |
65 | # Should emit in cases with nested `typing.Union`
|
= help: Remove duplicate union member `int`
PYI016.pyi:67:41: PYI016 Duplicate union member `int`
PYI016.pyi:66:41: PYI016 Duplicate union member `int`
|
66 | # Should emit in cases with nested `typing.Union`
67 | field20: typing.Union[int, typing.Union[int, str]] # Error
65 | # Should emit in cases with nested `typing.Union`
66 | field20: typing.Union[int, typing.Union[int, str]] # Error
| ^^^ PYI016
68 |
69 | # Should emit in cases with mixed `typing.Union` and `|`
67 |
68 | # Should emit in cases with mixed `typing.Union` and `|`
|
= help: Remove duplicate union member `int`
PYI016.pyi:70:28: PYI016 [*] Duplicate union member `int`
PYI016.pyi:69:28: PYI016 [*] Duplicate union member `int`
|
69 | # Should emit in cases with mixed `typing.Union` and `|`
70 | field21: typing.Union[int, int | str] # Error
68 | # Should emit in cases with mixed `typing.Union` and `|`
69 | field21: typing.Union[int, int | str] # Error
| ^^^ PYI016
71 |
72 | # Should emit only once in cases with multiple nested `typing.Union`
70 |
71 | # Should emit only once in cases with multiple nested `typing.Union`
|
= help: Remove duplicate union member `int`
ℹ Fix
67 67 | field20: typing.Union[int, typing.Union[int, str]] # Error
68 68 |
69 69 | # Should emit in cases with mixed `typing.Union` and `|`
70 |-field21: typing.Union[int, int | str] # Error
70 |+field21: typing.Union[int, str] # Error
71 71 |
72 72 | # Should emit only once in cases with multiple nested `typing.Union`
73 73 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
66 66 | field20: typing.Union[int, typing.Union[int, str]] # Error
67 67 |
68 68 | # Should emit in cases with mixed `typing.Union` and `|`
69 |-field21: typing.Union[int, int | str] # Error
69 |+field21: typing.Union[int, str] # Error
70 70 |
71 71 | # Should emit only once in cases with multiple nested `typing.Union`
72 72 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
PYI016.pyi:73:41: PYI016 Duplicate union member `int`
PYI016.pyi:72:41: PYI016 Duplicate union member `int`
|
72 | # Should emit only once in cases with multiple nested `typing.Union`
73 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
71 | # Should emit only once in cases with multiple nested `typing.Union`
72 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
| ^^^ PYI016
73 |
74 | # Should emit in cases with newlines
|
= help: Remove duplicate union member `int`
PYI016.pyi:73:59: PYI016 Duplicate union member `int`
PYI016.pyi:72:59: PYI016 Duplicate union member `int`
|
72 | # Should emit only once in cases with multiple nested `typing.Union`
73 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
71 | # Should emit only once in cases with multiple nested `typing.Union`
72 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
| ^^^ PYI016
73 |
74 | # Should emit in cases with newlines
|
= help: Remove duplicate union member `int`
PYI016.pyi:73:64: PYI016 Duplicate union member `int`
PYI016.pyi:72:64: PYI016 Duplicate union member `int`
|
72 | # Should emit only once in cases with multiple nested `typing.Union`
73 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
71 | # Should emit only once in cases with multiple nested `typing.Union`
72 | field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
| ^^^ PYI016
73 |
74 | # Should emit in cases with newlines
|
= help: Remove duplicate union member `int`
PYI016.pyi:76:12: PYI016 [*] Duplicate union member `set[int]`
|
74 | # Should emit in cases with newlines
75 | field23: set[ # foo
76 | int] | set[int]
| ^^^^^^^^ PYI016
|
= help: Remove duplicate union member `set[int]`
ℹ Fix
73 73 |
74 74 | # Should emit in cases with newlines
75 75 | field23: set[ # foo
76 |- int] | set[int]
76 |+ int]

15
crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs
View file

@ -420,10 +420,10 @@ impl From<bool> for Bool {
}
fn is_one_line_return_bool(stmts: &[Stmt]) -> Option<Bool> {
if stmts.len() != 1 {
let [stmt] = stmts else {
return None;
}
let Stmt::Return(ast::StmtReturn { value, range: _ }) = &stmts[0] else {
};
let Stmt::Return(ast::StmtReturn { value, range: _ }) = stmt else {
return None;
};
let Some(Expr::Constant(ast::ExprConstant { value, .. })) = value.as_deref() else {
@ -859,15 +859,12 @@ pub(crate) fn manual_dict_lookup(
let Expr::Name(ast::ExprName { id, .. }) = left.as_ref() else {
return;
};
if !(id == target && ops.len() == 1 && ops[0] == CmpOp::Eq) {
if !(id == target && matches!(ops.as_slice(), [CmpOp::Eq])) {
return;
}
if comparators.len() != 1 {
return;
}
let Expr::Constant(ast::ExprConstant {
let [Expr::Constant(ast::ExprConstant {
value: constant, ..
}) = &comparators[0]
})] = comparators.as_slice()
else {
return;
};

33
crates/ruff/src/rules/flake8_simplify/rules/ast_ifexp.rs
View file

@ -1,8 +1,9 @@
use ruff_text_size::TextRange;
use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Ranged, UnaryOp};
use rustpython_parser::ast::{self, Expr, ExprContext, Ranged, UnaryOp};
use ruff_diagnostics::{AlwaysAutofixableViolation, AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::{is_const_false, is_const_true};
use crate::checkers::ast::Checker;
use crate::registry::AsRule;
@ -141,16 +142,7 @@ pub(crate) fn explicit_true_false_in_ifexpr(
body: &Expr,
orelse: &Expr,
) {
let Expr::Constant(ast::ExprConstant { value, .. }) = &body else {
return;
};
if !matches!(value, Constant::Bool(true)) {
return;
}
let Expr::Constant(ast::ExprConstant { value, .. }) = &orelse else {
return;
};
if !matches!(value, Constant::Bool(false)) {
if !is_const_true(body) || !is_const_false(orelse) {
return;
}
@ -161,9 +153,9 @@ pub(crate) fn explicit_true_false_in_ifexpr(
expr.range(),
);
if checker.patch(diagnostic.kind.rule()) {
if matches!(test, Expr::Compare(_)) {
if test.is_compare_expr() {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
checker.generator().expr(&test.clone()),
checker.locator.slice(test.range()).to_string(),
expr.range(),
)));
} else if checker.semantic().is_builtin("bool") {
@ -195,16 +187,7 @@ pub(crate) fn explicit_false_true_in_ifexpr(
body: &Expr,
orelse: &Expr,
) {
let Expr::Constant(ast::ExprConstant { value, .. }) = &body else {
return;
};
if !matches!(value, Constant::Bool(false)) {
return;
}
let Expr::Constant(ast::ExprConstant { value, .. }) = &orelse else {
return;
};
if !matches!(value, Constant::Bool(true)) {
if !is_const_false(body) || !is_const_true(orelse) {
return;
}
@ -239,7 +222,7 @@ pub(crate) fn twisted_arms_in_ifexpr(
) {
let Expr::UnaryOp(ast::ExprUnaryOp {
op,
operand: test_operand,
operand,
range: _,
}) = &test
else {
@ -250,7 +233,7 @@ pub(crate) fn twisted_arms_in_ifexpr(
}
// Check if the test operand and else branch use the same variable.
let Expr::Name(ast::ExprName { id: test_id, .. }) = test_operand.as_ref() else {
let Expr::Name(ast::ExprName { id: test_id, .. }) = operand.as_ref() else {
return;
};
let Expr::Name(ast::ExprName { id: orelse_id, .. }) = orelse else {

10
crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs
View file

@ -136,13 +136,7 @@ fn is_exception_check(stmt: &Stmt) -> bool {
else {
return false;
};
if body.len() != 1 {
return false;
}
if matches!(body[0], Stmt::Raise(_)) {
return true;
}
false
matches!(body.as_slice(), [Stmt::Raise(_)])
}
/// SIM201
@ -287,7 +281,7 @@ pub(crate) fn double_negation(checker: &mut Checker, expr: &Expr, op: UnaryOp, o
if checker.patch(diagnostic.kind.rule()) {
if checker.semantic().in_boolean_test() {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
checker.generator().expr(operand),
checker.locator.slice(operand.range()).to_string(),
expr.range(),
)));
} else if checker.semantic().is_builtin("bool") {

6
crates/ruff/src/rules/flake8_type_checking/rules/empty_type_checking_block.rs
View file

@ -48,11 +48,9 @@ impl AlwaysAutofixableViolation for EmptyTypeCheckingBlock {
/// TCH005
pub(crate) fn empty_type_checking_block(checker: &mut Checker, stmt: &ast::StmtIf) {
if stmt.body.len() != 1 {
let [stmt] = stmt.body.as_slice() else {
return;
}
let stmt = &stmt.body[0];
};
if !stmt.is_pass_stmt() {
return;
}

5
crates/ruff/src/rules/pandas_vet/rules/assignment_to_df.rs
View file

@ -38,10 +38,9 @@ impl Violation for PandasDfVariableName {
/// PD901
pub(crate) fn assignment_to_df(targets: &[Expr]) -> Option<Diagnostic> {
if targets.len() != 1 {
let [target] = targets else {
return None;
}
let target = &targets[0];
};
let Expr::Name(ast::ExprName { id, .. }) = target else {
return None;
};

5
crates/ruff/src/rules/pylint/rules/bad_string_format_type.rs
View file

@ -128,10 +128,9 @@ fn is_valid_constant(formats: &[CFormatStrOrBytes<String>], value: &Expr) -> boo
let formats = collect_specs(formats);
// If there is more than one format, this is not valid python and we should
// return true so that no error is reported
if formats.len() != 1 {
let [format] = formats.as_slice() else {
return true;
}
let format = formats[0];
};
equivalent(format, value)
}

5
crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs
View file

@ -48,10 +48,7 @@ pub(crate) fn useless_metaclass_type(
value: &Expr,
targets: &[Expr],
) {
if targets.len() != 1 {
return;
}
let Expr::Name(ast::ExprName { id, .. }) = targets.first().unwrap() else {
let [Expr::Name(ast::ExprName { id, .. })] = targets else {
return;
};
if id != "__metaclass__" {

5
crates/ruff/src/rules/pyupgrade/rules/yield_in_for_loop.rs
View file

@ -99,11 +99,10 @@ impl<'a> StatementVisitor<'a> for YieldFromVisitor<'a> {
return;
}
// If there's any logic besides a yield, don't rewrite.
if body.len() != 1 {
let [body] = body.as_slice() else {
return;
}
};
// If the body is not a yield, don't rewrite.
let body = &body[0];
if let Stmt::Expr(ast::StmtExpr { value, range: _ }) = &body {
if let Expr::Yield(ast::ExprYield {
value: Some(value),

5
crates/ruff_python_ast/src/helpers.rs
View file

@ -549,10 +549,7 @@ pub fn is_assignment_to_a_dunder(stmt: &Stmt) -> bool {
// annotation. This is what pycodestyle (as of 2.9.1) does.
match stmt {
Stmt::Assign(ast::StmtAssign { targets, .. }) => {
if targets.len() != 1 {
return false;
}
if let Expr::Name(ast::ExprName { id, .. }) = &targets[0] {
if let [Expr::Name(ast::ExprName { id, .. })] = targets.as_slice() {
is_dunder(id)
} else {
false