[ruff] Check for shadowed map before suggesting fix (RUF058) (#15790)

## Summary

Fixes #15786 by not suggesting a fix if `map` doesn't have its builtin
binding.

## Test Plan

New test taken from the report in #15786.
This commit is contained in:
Brent Westbrook 2025-01-28 14:15:37 -05:00 committed by GitHub
parent ca53eefa6f
commit 786099a872
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 64 additions and 17 deletions

View file

@ -0,0 +1,11 @@
"""Regression test for https://github.com/astral-sh/ruff/issues/15786. This
should be separate from other tests because it shadows the `map` builtin.
This should still get a diagnostic but not a fix that would lead to an error.
"""
from itertools import starmap
map = {}
for _ in starmap(print, zip("A", "12")):
pass

View file

@ -426,7 +426,8 @@ mod tests {
#[test_case(Rule::PytestRaisesAmbiguousPattern, Path::new("RUF043.py"))]
#[test_case(Rule::UnnecessaryRound, Path::new("RUF057.py"))]
#[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))]
#[test_case(Rule::StarmapZip, Path::new("RUF058.py"))]
#[test_case(Rule::StarmapZip, Path::new("RUF058_0.py"))]
#[test_case(Rule::StarmapZip, Path::new("RUF058_1.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",

View file

@ -1,5 +1,5 @@
use crate::checkers::ast::Checker;
use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix};
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{parenthesize::parenthesized_range, Expr, ExprCall};
use ruff_python_parser::TokenKind;
@ -28,17 +28,29 @@ use ruff_text_size::{Ranged, TextRange};
/// map(func, a, b)
/// map(func, a, b, strict=True) # 3.14+
/// ```
///
/// ## Fix safety
///
/// This rule's fix is marked as unsafe if the `starmap` or `zip` expressions contain comments that
/// would be deleted by applying the fix. Otherwise, the fix can be applied safely.
///
/// ## Fix availability
///
/// This rule will emit a diagnostic but not suggest a fix if `map` has been shadowed from its
/// builtin binding.
#[derive(ViolationMetadata)]
pub(crate) struct StarmapZip;
impl AlwaysFixableViolation for StarmapZip {
impl Violation for StarmapZip {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
"`itertools.starmap` called on `zip` iterable".to_string()
}
fn fix_title(&self) -> String {
"Use `map` instead".to_string()
fn fix_title(&self) -> Option<String> {
Some("Use `map` instead".to_string())
}
}
@ -77,13 +89,21 @@ pub(crate) fn starmap_zip(checker: &mut Checker, call: &ExprCall) {
return;
}
let fix = replace_with_map(call, iterable_call, checker);
let diagnostic = Diagnostic::new(StarmapZip, call.range);
let mut diagnostic = Diagnostic::new(StarmapZip, call.range);
checker.diagnostics.push(diagnostic.with_fix(fix));
if let Some(fix) = replace_with_map(call, iterable_call, checker) {
diagnostic.set_fix(fix);
}
checker.diagnostics.push(diagnostic);
}
fn replace_with_map(starmap: &ExprCall, zip: &ExprCall, checker: &Checker) -> Fix {
/// Replace the `starmap` call with a call to the `map` builtin, if `map` has not been shadowed.
fn replace_with_map(starmap: &ExprCall, zip: &ExprCall, checker: &Checker) -> Option<Fix> {
if !checker.semantic().has_builtin_binding("map") {
return None;
}
let change_func_to_map = Edit::range_replacement("map".to_string(), starmap.func.range());
let mut remove_zip = vec![];
@ -149,5 +169,9 @@ fn replace_with_map(starmap: &ExprCall, zip: &ExprCall, checker: &Checker) -> Fi
Applicability::Safe
};
Fix::applicable_edits(change_func_to_map, remove_zip, applicability)
Some(Fix::applicable_edits(
change_func_to_map,
remove_zip,
applicability,
))
}

View file

@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF058.py:7:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
RUF058_0.py:7:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
|
5 | # Errors
6 |
@ -21,7 +21,7 @@ RUF058.py:7:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
9 9 |
10 10 |
RUF058.py:8:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
RUF058_0.py:8:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
|
7 | starmap(func, zip())
8 | starmap(func, zip([]))
@ -39,7 +39,7 @@ RUF058.py:8:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
10 10 |
11 11 | starmap(func, zip(a, b, c,),)
RUF058.py:11:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
RUF058_0.py:11:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
|
11 | starmap(func, zip(a, b, c,),)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF058
@ -56,7 +56,7 @@ RUF058.py:11:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
13 13 |
14 14 | starmap(
RUF058.py:14:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
RUF058_0.py:14:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
|
14 | / starmap(
15 | | func, # Foo
@ -88,7 +88,7 @@ RUF058.py:14:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
21 21 |
22 22 | ( # Foo
RUF058.py:22:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
RUF058_0.py:22:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
|
20 | )
21 |
@ -124,7 +124,7 @@ RUF058.py:22:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
30 28 |
31 29 | ( # Foobar
RUF058.py:31:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
RUF058_0.py:31:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
|
29 | )
30 |
@ -182,7 +182,7 @@ RUF058.py:31:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
50 42 |
51 43 | starmap(
RUF058.py:51:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
RUF058_0.py:51:1: RUF058 [*] `itertools.starmap` called on `zip` iterable
|
49 | )
50 |

View file

@ -0,0 +1,11 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF058_1.py:10:10: RUF058 `itertools.starmap` called on `zip` iterable
|
9 | map = {}
10 | for _ in starmap(print, zip("A", "12")):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF058
11 | pass
|
= help: Use `map` instead