From a51a0f16e491e358f63cd587e98fa1bdcee4a29d Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Wed, 22 Oct 2025 05:58:48 +0900 Subject: [PATCH] [`flake8-simplify`] Skip `SIM911` when unknown arguments are present (#20697) ## Summary Fixes #18778 Prevent SIM911 from triggering when zip() is called on .keys()/.values() that take any positional or keyword arguments, so Ruff never suggests the lossy rewrite. ## Test Plan Added a test case to SIM911.py. --- .../test/fixtures/flake8_simplify/SIM911.py | 4 ++++ .../rules/zip_dict_keys_and_values.rs | 20 +++++++++++++------ ...ke8_simplify__tests__SIM911_SIM911.py.snap | 5 +++++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM911.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM911.py index 3dd52b4139..ee5bed6854 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM911.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM911.py @@ -34,3 +34,7 @@ def foo(): # https://github.com/astral-sh/ruff/issues/18776 flag_stars = {} for country, stars in(zip)(flag_stars.keys(), flag_stars.values()):... + +# Regression test for https://github.com/astral-sh/ruff/issues/18778 +d = {} +for country, stars in zip(d.keys(*x), d.values("hello")):... diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/zip_dict_keys_and_values.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/zip_dict_keys_and_values.rs index 58f3a01a8a..28d123fab3 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/zip_dict_keys_and_values.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/zip_dict_keys_and_values.rs @@ -78,13 +78,18 @@ pub(crate) fn zip_dict_keys_and_values(checker: &Checker, expr: &ast::ExprCall) let [arg1, arg2] = &args[..] else { return; }; - let Some((var1, attr1)) = get_var_attr(arg1) else { + let Some((var1, attr1, args1)) = get_var_attr_args(arg1) else { return; }; - let Some((var2, attr2)) = get_var_attr(arg2) else { + let Some((var2, attr2, args2)) = get_var_attr_args(arg2) else { return; }; - if var1.id != var2.id || attr1 != "keys" || attr2 != "values" { + if var1.id != var2.id + || attr1 != "keys" + || attr2 != "values" + || !args1.is_empty() + || !args2.is_empty() + { return; } if !checker.semantic().match_builtin_expr(func, "zip") { @@ -122,8 +127,11 @@ pub(crate) fn zip_dict_keys_and_values(checker: &Checker, expr: &ast::ExprCall) ))); } -fn get_var_attr(expr: &Expr) -> Option<(&ExprName, &Identifier)> { - let Expr::Call(ast::ExprCall { func, .. }) = expr else { +fn get_var_attr_args(expr: &Expr) -> Option<(&ExprName, &Identifier, &Arguments)> { + let Expr::Call(ast::ExprCall { + func, arguments, .. + }) = expr + else { return None; }; let Expr::Attribute(ExprAttribute { value, attr, .. }) = func.as_ref() else { @@ -132,5 +140,5 @@ fn get_var_attr(expr: &Expr) -> Option<(&ExprName, &Identifier)> { let Expr::Name(var_name) = value.as_ref() else { return None; }; - Some((var_name, attr)) + Some((var_name, attr, arguments)) } diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM911_SIM911.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM911_SIM911.py.snap index ffecf5ebeb..3b2f5bdf35 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM911_SIM911.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM911_SIM911.py.snap @@ -81,6 +81,8 @@ SIM911 [*] Use ` flag_stars.items()` instead of `(zip)(flag_stars.keys(), flag_s 35 | flag_stars = {} 36 | for country, stars in(zip)(flag_stars.keys(), flag_stars.values()):... | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +37 | +38 | # Regression test for https://github.com/astral-sh/ruff/issues/18778 | help: Replace `(zip)(flag_stars.keys(), flag_stars.values())` with ` flag_stars.items()` 33 | @@ -88,3 +90,6 @@ help: Replace `(zip)(flag_stars.keys(), flag_stars.values())` with ` flag_stars. 35 | flag_stars = {} - for country, stars in(zip)(flag_stars.keys(), flag_stars.values()):... 36 + for country, stars in flag_stars.items():... +37 | +38 | # Regression test for https://github.com/astral-sh/ruff/issues/18778 +39 | d = {}