From c1491a7a72a2aec276401da2d50871162d84adc3 Mon Sep 17 00:00:00 2001 From: chiri Date: Tue, 20 Jan 2026 18:15:46 +0300 Subject: [PATCH] [`flake8-simplify`] Make fix unsafe if it deletes comments (`SIM911`) (#22661) ## Summary ## Test Plan --- .../test/fixtures/flake8_simplify/SIM911.py | 10 +++++++ .../rules/zip_dict_keys_and_values.rs | 19 ++++++++++--- ...ke8_simplify__tests__SIM911_SIM911.py.snap | 27 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 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 ee5bed6854..f5627cd6cc 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM911.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM911.py @@ -38,3 +38,13 @@ 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")):... + + +flag_stars = {"USA": 50, "Slovenia": 3, "Panama": 2, "Australia": 6} + +for country, stars in zip( + flag_stars.keys(), + # text + flag_stars.values(), +): + print(f"{country}'s flag has {stars} stars.") 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 19169b72c7..f84c7eb2a6 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 @@ -1,4 +1,5 @@ use ast::{ExprAttribute, ExprName, Identifier}; +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{self as ast, Arguments, Expr}; use ruff_python_semantic::analyze::typing::is_dict; @@ -30,6 +31,9 @@ use crate::{checkers::ast::Checker, fix::snippet::SourceCodeSnippet}; /// print(f"{country}'s flag has {stars} stars.") /// ``` /// +/// ## Fix safety +/// This rule's fix is marked as safe, unless the expression contains comments. +/// /// ## References /// - [Python documentation: `dict.items`](https://docs.python.org/3/library/stdtypes.html#dict.items) #[derive(ViolationMetadata)] @@ -122,10 +126,17 @@ pub(crate) fn zip_dict_keys_and_values(checker: &Checker, expr: &ast::ExprCall) }, expr.range(), ); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - expected, - expr.range(), - ))); + + let applicability = if checker.comment_ranges().intersects(expr.range()) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement(expected, expr.range()), + applicability, + )); } fn get_var_attr_args(expr: &Expr) -> Option<(&ExprName, &Identifier, &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 3b2f5bdf35..c342594320 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 @@ -93,3 +93,30 @@ help: Replace `(zip)(flag_stars.keys(), flag_stars.values())` with ` flag_stars. 37 | 38 | # Regression test for https://github.com/astral-sh/ruff/issues/18778 39 | d = {} + +SIM911 [*] Use `dict.items()` instead of `zip(dict.keys(), dict.values())` + --> SIM911.py:45:23 + | +43 | flag_stars = {"USA": 50, "Slovenia": 3, "Panama": 2, "Australia": 6} +44 | +45 | for country, stars in zip( + | _______________________^ +46 | | flag_stars.keys(), +47 | | # text +48 | | flag_stars.values(), +49 | | ): + | |_^ +50 | print(f"{country}'s flag has {stars} stars.") + | +help: Replace `zip(dict.keys(), dict.values())` with `dict.items()` +42 | +43 | flag_stars = {"USA": 50, "Slovenia": 3, "Panama": 2, "Australia": 6} +44 | + - for country, stars in zip( + - flag_stars.keys(), + - # text + - flag_stars.values(), + - ): +45 + for country, stars in flag_stars.items(): +46 | print(f"{country}'s flag has {stars} stars.") +note: This is an unsafe fix and may change runtime behavior