mirror of https://github.com/astral-sh/ruff
[`flake8-simplify`] Apply `SIM113` when index variable is of type `int` (#21395)
## Summary Fixes #21393 Now the rule checks if the index variable is initialized as an `int` type rather than only flagging if the index variable is initialized to `0`. I used `ResolvedPythonType` to check if the index variable is an `int` type. ## Test Plan Updated snapshot test for `SIM113`. --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
This commit is contained in:
parent
43427abb61
commit
8a85a2961e
|
|
@ -46,7 +46,8 @@ def func():
|
||||||
|
|
||||||
|
|
||||||
def func():
|
def func():
|
||||||
# OK (index doesn't start at 0
|
# SIM113
|
||||||
|
# https://github.com/astral-sh/ruff/pull/21395
|
||||||
idx = 10
|
idx = 10
|
||||||
for x in range(5):
|
for x in range(5):
|
||||||
g(x, idx)
|
g(x, idx)
|
||||||
|
|
|
||||||
|
|
@ -269,3 +269,8 @@ pub(crate) const fn is_typing_extensions_str_alias_enabled(settings: &LinterSett
|
||||||
pub(crate) const fn is_extended_i18n_function_matching_enabled(settings: &LinterSettings) -> bool {
|
pub(crate) const fn is_extended_i18n_function_matching_enabled(settings: &LinterSettings) -> bool {
|
||||||
settings.preview.is_enabled()
|
settings.preview.is_enabled()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// https://github.com/astral-sh/ruff/pull/21395
|
||||||
|
pub(crate) const fn is_enumerate_for_loop_int_index_enabled(settings: &LinterSettings) -> bool {
|
||||||
|
settings.preview.is_enabled()
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -61,6 +61,7 @@ mod tests {
|
||||||
|
|
||||||
#[test_case(Rule::SplitStaticString, Path::new("SIM905.py"))]
|
#[test_case(Rule::SplitStaticString, Path::new("SIM905.py"))]
|
||||||
#[test_case(Rule::DictGetWithNoneDefault, Path::new("SIM910.py"))]
|
#[test_case(Rule::DictGetWithNoneDefault, Path::new("SIM910.py"))]
|
||||||
|
#[test_case(Rule::EnumerateForLoop, Path::new("SIM113.py"))]
|
||||||
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!(
|
let snapshot = format!(
|
||||||
"preview__{}_{}",
|
"preview__{}_{}",
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,8 @@
|
||||||
|
use crate::preview::is_enumerate_for_loop_int_index_enabled;
|
||||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||||
use ruff_python_ast::statement_visitor::{StatementVisitor, walk_stmt};
|
use ruff_python_ast::statement_visitor::{StatementVisitor, walk_stmt};
|
||||||
use ruff_python_ast::{self as ast, Expr, Int, Number, Operator, Stmt};
|
use ruff_python_ast::{self as ast, Expr, Int, Number, Operator, Stmt};
|
||||||
|
use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType};
|
||||||
use ruff_python_semantic::analyze::typing;
|
use ruff_python_semantic::analyze::typing;
|
||||||
use ruff_text_size::Ranged;
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
|
|
@ -11,6 +13,9 @@ use crate::checkers::ast::Checker;
|
||||||
/// Checks for `for` loops with explicit loop-index variables that can be replaced
|
/// Checks for `for` loops with explicit loop-index variables that can be replaced
|
||||||
/// with `enumerate()`.
|
/// with `enumerate()`.
|
||||||
///
|
///
|
||||||
|
/// In [preview], this rule checks for index variables initialized with any integer rather than only
|
||||||
|
/// a literal zero.
|
||||||
|
///
|
||||||
/// ## Why is this bad?
|
/// ## Why is this bad?
|
||||||
/// When iterating over a sequence, it's often desirable to keep track of the
|
/// When iterating over a sequence, it's often desirable to keep track of the
|
||||||
/// index of each element alongside the element itself. Prefer the `enumerate`
|
/// index of each element alongside the element itself. Prefer the `enumerate`
|
||||||
|
|
@ -35,6 +40,8 @@ use crate::checkers::ast::Checker;
|
||||||
///
|
///
|
||||||
/// ## References
|
/// ## References
|
||||||
/// - [Python documentation: `enumerate`](https://docs.python.org/3/library/functions.html#enumerate)
|
/// - [Python documentation: `enumerate`](https://docs.python.org/3/library/functions.html#enumerate)
|
||||||
|
///
|
||||||
|
/// [preview]: https://docs.astral.sh/ruff/preview/
|
||||||
#[derive(ViolationMetadata)]
|
#[derive(ViolationMetadata)]
|
||||||
#[violation_metadata(stable_since = "v0.2.0")]
|
#[violation_metadata(stable_since = "v0.2.0")]
|
||||||
pub(crate) struct EnumerateForLoop {
|
pub(crate) struct EnumerateForLoop {
|
||||||
|
|
@ -82,17 +89,21 @@ pub(crate) fn enumerate_for_loop(checker: &Checker, for_stmt: &ast::StmtFor) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ensure that the index variable was initialized to 0.
|
// Ensure that the index variable was initialized to 0 (or instance of `int` if preview is enabled).
|
||||||
let Some(value) = typing::find_binding_value(binding, checker.semantic()) else {
|
let Some(value) = typing::find_binding_value(binding, checker.semantic()) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
if !matches!(
|
if !(matches!(
|
||||||
value,
|
value,
|
||||||
Expr::NumberLiteral(ast::ExprNumberLiteral {
|
Expr::NumberLiteral(ast::ExprNumberLiteral {
|
||||||
value: Number::Int(Int::ZERO),
|
value: Number::Int(Int::ZERO),
|
||||||
..
|
..
|
||||||
})
|
})
|
||||||
) {
|
) || matches!(
|
||||||
|
ResolvedPythonType::from(value),
|
||||||
|
ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer))
|
||||||
|
) && is_enumerate_for_loop_int_index_enabled(checker.settings()))
|
||||||
|
{
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,60 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
|
||||||
|
---
|
||||||
|
SIM113 Use `enumerate()` for index variable `idx` in `for` loop
|
||||||
|
--> SIM113.py:6:9
|
||||||
|
|
|
||||||
|
4 | for x in range(5):
|
||||||
|
5 | g(x, idx)
|
||||||
|
6 | idx += 1
|
||||||
|
| ^^^^^^^^
|
||||||
|
7 | h(x)
|
||||||
|
|
|
||||||
|
|
||||||
|
SIM113 Use `enumerate()` for index variable `idx` in `for` loop
|
||||||
|
--> SIM113.py:17:9
|
||||||
|
|
|
||||||
|
15 | if g(x):
|
||||||
|
16 | break
|
||||||
|
17 | idx += 1
|
||||||
|
| ^^^^^^^^
|
||||||
|
18 | sum += h(x, idx)
|
||||||
|
|
|
||||||
|
|
||||||
|
SIM113 Use `enumerate()` for index variable `idx` in `for` loop
|
||||||
|
--> SIM113.py:27:9
|
||||||
|
|
|
||||||
|
25 | g(x)
|
||||||
|
26 | h(x, y)
|
||||||
|
27 | idx += 1
|
||||||
|
| ^^^^^^^^
|
||||||
|
|
|
||||||
|
|
||||||
|
SIM113 Use `enumerate()` for index variable `idx` in `for` loop
|
||||||
|
--> SIM113.py:36:9
|
||||||
|
|
|
||||||
|
34 | for x in range(5):
|
||||||
|
35 | sum += h(x, idx)
|
||||||
|
36 | idx += 1
|
||||||
|
| ^^^^^^^^
|
||||||
|
|
|
||||||
|
|
||||||
|
SIM113 Use `enumerate()` for index variable `idx` in `for` loop
|
||||||
|
--> SIM113.py:44:9
|
||||||
|
|
|
||||||
|
42 | for x in range(5):
|
||||||
|
43 | g(x, idx)
|
||||||
|
44 | idx += 1
|
||||||
|
| ^^^^^^^^
|
||||||
|
45 | h(x)
|
||||||
|
|
|
||||||
|
|
||||||
|
SIM113 Use `enumerate()` for index variable `idx` in `for` loop
|
||||||
|
--> SIM113.py:54:9
|
||||||
|
|
|
||||||
|
52 | for x in range(5):
|
||||||
|
53 | g(x, idx)
|
||||||
|
54 | idx += 1
|
||||||
|
| ^^^^^^^^
|
||||||
|
55 | h(x)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue