From f8519be360edb9704b7af723ac322bc77f653164 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:08:00 -0400 Subject: [PATCH] Update LibCST-based fixes for Python 3.15 (#28616) Summary -- This PR bumps our libcst version to 1.9 and incorporates the new support for Python 3.15 into fixes that involved imports and unpacking in comprehensions. The unpacking fixes affect three C4 rules and are in the first (non-empty) commit, while the lazy import fixes are a bit more widespread and are in the second commit. I admittedly wasn't that familiar with our usage of libcst before this, but it looks like we have kind of a weird mix between using libcst and additional manual string manipulation. My changes preserve that with a couple of `format!("lazy {fix}")` calls. libcst also itself makes this change a bit awkward by using separate node types for lazy and non-lazy imports, so I ended up returning the parts of the imports instead of `Import` types directly in a couple of cases. Test Plan -- Updated existing snapshots for the C4 rules and new mdtests for the import helpers --- Cargo.lock | 8 +- Cargo.toml | 2 +- .../resources/mdtest/fixes/imports.md | 123 ++++++++++++++++++ .../pyupgrade/deprecated-mock-import.md | 29 ++++- crates/ruff_linter/src/cst/matchers.rs | 48 ++++--- crates/ruff_linter/src/fix/codemods.rs | 38 ++++-- crates/ruff_linter/src/importer/mod.rs | 18 ++- .../ruff_linter/src/rules/airflow/helpers.rs | 10 +- .../unnecessary_comprehension_in_call.rs | 4 - .../rules/unnecessary_list_call.rs | 9 +- .../unnecessary_literal_within_dict_call.rs | 8 -- ...y-comprehension-in-call_C419_py315.py.snap | 17 ++- ...__unnecessary-list-call_C411_py315.py.snap | 8 +- ...iteral-within-dict-call_C418_py315.py.snap | 8 +- .../rules/isort/rules/add_required_imports.rs | 7 +- .../pyupgrade/rules/deprecated_mock_import.rs | 72 +++++----- 16 files changed, 297 insertions(+), 112 deletions(-) create mode 100644 crates/ruff_linter/resources/mdtest/fixes/imports.md diff --git a/Cargo.lock b/Cargo.lock index 802aea6fb9..280a760590 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1939,9 +1939,9 @@ checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "libcst" -version = "1.8.6" +version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6aea7143e4a0ed59b87a1ee71e198500889f8b005311136be15e84c97a6fcd8d" +checksum = "21b68d9c60cba9c333f89596ef5c4d3b6c9bd0b7d2e48a9810df6fa7976102a3" dependencies = [ "annotate-snippets", "libcst_derive", @@ -1954,9 +1954,9 @@ dependencies = [ [[package]] name = "libcst_derive" -version = "1.8.6" +version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0903173ea316c34a44d0497161e04d9210af44f5f5e89bf2f55d9a254c9a0e8d" +checksum = "57a1ac6fb3fbdc3408a2f138e54d11c3dd338f12c2efc3db47477cc2227d89c2" dependencies = [ "quote", "syn 2.0.119", diff --git a/Cargo.toml b/Cargo.toml index 56e6ac5949..1d04b3af02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -146,7 +146,7 @@ jiff = { version = "0.2.0", default-features = false, features = [ jod-thread = { version = "1.0.0", default-features = false } js-sys = { version = "0.3.69", default-features = false, features = ["std"] } libc = { version = "0.2.153", default-features = false, features = ["std"] } -libcst = { version = "1.8.4", default-features = false } +libcst = { version = "1.9.0", default-features = false } log = { version = "0.4.17", default-features = false } lsp-server = { version = "0.10.0", default-features = false } lsp-types = { package = "gen-lsp-types", version = "0.11.0", default-features = false, features = ["url"] } diff --git a/crates/ruff_linter/resources/mdtest/fixes/imports.md b/crates/ruff_linter/resources/mdtest/fixes/imports.md new file mode 100644 index 0000000000..7e8088d1d5 --- /dev/null +++ b/crates/ruff_linter/resources/mdtest/fixes/imports.md @@ -0,0 +1,123 @@ +# Import fixes + +The shared import-editing helpers preserve explicit `lazy` syntax when extending, moving, or replacing +imports. + +```toml +target-version = "py315" + +[lint] +preview = true +select = ["UP017", "TC004", "AIR321"] +``` + +## Extending an import + +`UP017` adds `UTC` to the existing lazy import. + +```py +lazy from datetime import timezone + +timezone.utc # snapshot: datetime-timezone-utc +``` + +```snapshot +error[UP017]: Use `datetime.UTC` alias + --> src/mdtest_snippet.py:3:1 + | +3 | timezone.utc # snapshot: datetime-timezone-utc + | ^^^^^^^^^^^^ +help: Convert to `datetime.UTC` alias + | + - lazy from datetime import timezone +1 + lazy from datetime import timezone, UTC +2 | + - timezone.utc # snapshot: datetime-timezone-utc +3 + UTC # snapshot: datetime-timezone-utc + | +``` + +## Moving part of an import + +`TC004` preserves `lazy` on both the imports moved to runtime and those remaining in the type-checking +block. + +```py +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + lazy import os, sys # snapshot: runtime-import-in-type-checking-block + lazy from pathlib import Path, PurePath # snapshot: runtime-import-in-type-checking-block + +print(os, Path) +``` + +```snapshot +error[TC004]: Move import `os` out of type-checking block. Import is used for more than type hinting. + --> src/mdtest_snippet.py:4:17 + | +4 | lazy import os, sys # snapshot: runtime-import-in-type-checking-block + | ^^ +5 | lazy from pathlib import Path, PurePath # snapshot: runtime-import-in-type-checking-block +6 | +7 | print(os, Path) + | -- Used at runtime here +help: Move out of type-checking block + | +1 | from typing import TYPE_CHECKING +2 + lazy import os +3 | +4 | if TYPE_CHECKING: + - lazy import os, sys # snapshot: runtime-import-in-type-checking-block +5 + lazy import sys # snapshot: runtime-import-in-type-checking-block +6 | lazy from pathlib import Path, PurePath # snapshot: runtime-import-in-type-checking-block + | +note: This is an unsafe fix and may change runtime behavior + + +error[TC004]: Move import `pathlib.Path` out of type-checking block. Import is used for more than type hinting. + --> src/mdtest_snippet.py:5:30 + | +5 | lazy from pathlib import Path, PurePath # snapshot: runtime-import-in-type-checking-block + | ^^^^ +6 | +7 | print(os, Path) + | ---- Used at runtime here +help: Move out of type-checking block + | +1 | from typing import TYPE_CHECKING +2 + lazy from pathlib import Path +3 | +4 | if TYPE_CHECKING: +5 | lazy import os, sys # snapshot: runtime-import-in-type-checking-block + - lazy from pathlib import Path, PurePath # snapshot: runtime-import-in-type-checking-block +6 + lazy from pathlib import PurePath # snapshot: runtime-import-in-type-checking-block +7 | + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Replacing an import + +`AIR321` preserves `lazy` when generating an import from the replacement module. + +```py +lazy from airflow.utils.timezone import convert_to_utc + +convert_to_utc # snapshot: airflow31-moved +``` + +```snapshot +error[AIR321]: `airflow.utils.timezone.convert_to_utc` is moved in Airflow 3.1 + --> src/mdtest_snippet.py:3:1 + | +3 | convert_to_utc # snapshot: airflow31-moved + | ^^^^^^^^^^^^^^ +help: `convert_to_utc` has been moved to `airflow.sdk.timezone` since Airflow 3.1 (with apache-airflow-task-sdk>=1.1.0). + | + - lazy from airflow.utils.timezone import convert_to_utc +1 + lazy from airflow.sdk.timezone import convert_to_utc +2 | + | +note: This is an unsafe fix and may change runtime behavior +``` diff --git a/crates/ruff_linter/resources/mdtest/pyupgrade/deprecated-mock-import.md b/crates/ruff_linter/resources/mdtest/pyupgrade/deprecated-mock-import.md index 213b8adf1a..4c922a3579 100644 --- a/crates/ruff_linter/resources/mdtest/pyupgrade/deprecated-mock-import.md +++ b/crates/ruff_linter/resources/mdtest/pyupgrade/deprecated-mock-import.md @@ -47,18 +47,37 @@ help: Import from `unittest.mock` instead ## Explicit lazy imports -An explicit lazy import is diagnosed, but the fixer does not support the `lazy` keyword, so no fix -is offered. +The replacement imports retain `lazy`, including when a `from` import is split. ```py -lazy from mock import patch # snapshot: deprecated-mock-import +lazy from mock import mock, patch # snapshot: deprecated-mock-import +lazy import mock # snapshot: deprecated-mock-import ``` ```snapshot error[UP026]: `mock` is deprecated, use `unittest.mock` --> src/mdtest_snippet.py:1:1 | -1 | lazy from mock import patch # snapshot: deprecated-mock-import - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +1 | lazy from mock import mock, patch # snapshot: deprecated-mock-import + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Import from `unittest.mock` instead + | + - lazy from mock import mock, patch # snapshot: deprecated-mock-import +1 + lazy from unittest.mock import patch +2 + lazy from unittest import mock # snapshot: deprecated-mock-import +3 | lazy import mock # snapshot: deprecated-mock-import + | + + +error[UP026]: `mock` is deprecated, use `unittest.mock` + --> src/mdtest_snippet.py:2:13 + | +2 | lazy import mock # snapshot: deprecated-mock-import + | ^^^^ +help: Import from `unittest.mock` instead + | +1 | lazy from mock import mock, patch # snapshot: deprecated-mock-import + - lazy import mock # snapshot: deprecated-mock-import +2 + lazy from unittest import mock # snapshot: deprecated-mock-import + | ``` diff --git a/crates/ruff_linter/src/cst/matchers.rs b/crates/ruff_linter/src/cst/matchers.rs index a07fcccf10..3461d0735f 100644 --- a/crates/ruff_linter/src/cst/matchers.rs +++ b/crates/ruff_linter/src/cst/matchers.rs @@ -3,8 +3,8 @@ use anyhow::{Result, bail}; use libcst_native::{ Arg, Attribute, Call, Comparison, CompoundStatement, Dict, Expression, FormattedString, FormattedStringContent, FormattedStringExpression, FunctionDef, GeneratorExp, If, Import, - ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda, ListComp, Module, SmallStatement, - Statement, Suite, Tuple, With, + ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda, LazyImport, LazyImportFrom, + ListComp, Module, NameOrAttribute, SmallStatement, Statement, Suite, Tuple, With, }; use ruff_python_codegen::Stylist; @@ -22,36 +22,44 @@ pub(crate) fn match_statement(statement_text: &str) -> Result> { } } -pub(crate) fn match_import<'a, 'b>(statement: &'a mut Statement<'b>) -> Result<&'a mut Import<'b>> { - if let Statement::Simple(expr) = statement { - if let Some(SmallStatement::Import(expr)) = expr.body.first_mut() { - Ok(expr) - } else { - bail!("Expected SmallStatement::Import") - } - } else { +pub(crate) fn match_import<'a, 'b>( + statement: &'a mut Statement<'b>, +) -> Result<(&'a mut Vec>, bool)> { + let Statement::Simple(statement) = statement else { bail!("Expected Statement::Simple") + }; + match statement.body.first_mut() { + Some(SmallStatement::Import(Import { names, .. })) => Ok((names, false)), + Some(SmallStatement::LazyImport(LazyImport { names, .. })) => Ok((names, true)), + _ => bail!("Expected SmallStatement::Import | SmallStatement::LazyImport"), } } pub(crate) fn match_import_from<'a, 'b>( statement: &'a mut Statement<'b>, -) -> Result<&'a mut ImportFrom<'b>> { - if let Statement::Simple(expr) = statement { - if let Some(SmallStatement::ImportFrom(expr)) = expr.body.first_mut() { - Ok(expr) - } else { - bail!("Expected SmallStatement::ImportFrom") - } - } else { +) -> Result<( + &'a mut Option>, + &'a mut ImportNames<'b>, + bool, +)> { + let Statement::Simple(statement) = statement else { bail!("Expected Statement::Simple") + }; + match statement.body.first_mut() { + Some(SmallStatement::ImportFrom(ImportFrom { module, names, .. })) => { + Ok((module, names, false)) + } + Some(SmallStatement::LazyImportFrom(LazyImportFrom { module, names, .. })) => { + Ok((module, names, true)) + } + _ => bail!("Expected SmallStatement::ImportFrom | SmallStatement::LazyImportFrom"), } } pub(crate) fn match_aliases<'a, 'b>( - import_from: &'a mut ImportFrom<'b>, + names: &'a mut ImportNames<'b>, ) -> Result<&'a mut Vec>> { - if let ImportNames::Aliases(aliases) = &mut import_from.names { + if let ImportNames::Aliases(aliases) = names { Ok(aliases) } else { bail!("Expected ImportNames::Aliases") diff --git a/crates/ruff_linter/src/fix/codemods.rs b/crates/ruff_linter/src/fix/codemods.rs index 3d181dfeb7..2a082a9cbe 100644 --- a/crates/ruff_linter/src/fix/codemods.rs +++ b/crates/ruff_linter/src/fix/codemods.rs @@ -4,8 +4,8 @@ use std::borrow::Cow; use anyhow::{Result, bail}; use libcst_native::{ - Codegen, CodegenState, Expression, ImportNames, NameOrAttribute, ParenthesizableWhitespace, - SmallStatement, Statement, + Codegen, CodegenState, Expression, Import, ImportFrom, ImportNames, LazyImport, LazyImportFrom, + NameOrAttribute, ParenthesizableWhitespace, SmallStatement, Statement, }; use rustc_hash::{FxHashMap, FxHashSet}; use smallvec::{SmallVec, smallvec}; @@ -53,11 +53,17 @@ pub(crate) fn remove_imports<'a>( }; let aliases = match body.body.first_mut() { - Some(SmallStatement::Import(import_body)) => &mut import_body.names, - Some(SmallStatement::ImportFrom(import_body)) => { - if let ImportNames::Aliases(names) = &mut import_body.names { - names - } else if let ImportNames::Star(..) = &import_body.names { + Some( + SmallStatement::Import(Import { names, .. }) + | SmallStatement::LazyImport(LazyImport { names, .. }), + ) => names, + Some( + SmallStatement::ImportFrom(ImportFrom { names, .. }) + | SmallStatement::LazyImportFrom(LazyImportFrom { names, .. }), + ) => { + if let ImportNames::Aliases(aliases) = names { + aliases + } else if let ImportNames::Star(..) = names { // Special-case: if the import is a `from ... import *`, then we delete the // entire statement. let mut found_star = false; @@ -76,7 +82,7 @@ pub(crate) fn remove_imports<'a>( bail!("Expected: ImportNames::Aliases | ImportNames::Star"); } } - _ => bail!("Expected: SmallStatement::ImportFrom | SmallStatement::Import"), + _ => bail!("Expected import statement"), }; // Preserve the trailing comma (or not) from the last entry. @@ -139,15 +145,21 @@ pub(crate) fn retain_imports( }; let aliases = match body.body.first_mut() { - Some(SmallStatement::Import(import_body)) => &mut import_body.names, - Some(SmallStatement::ImportFrom(import_body)) => { - if let ImportNames::Aliases(names) = &mut import_body.names { - names + Some( + SmallStatement::Import(Import { names, .. }) + | SmallStatement::LazyImport(LazyImport { names, .. }), + ) => names, + Some( + SmallStatement::ImportFrom(ImportFrom { names, .. }) + | SmallStatement::LazyImportFrom(LazyImportFrom { names, .. }), + ) => { + if let ImportNames::Aliases(aliases) = names { + aliases } else { bail!("Expected: ImportNames::Aliases"); } } - _ => bail!("Expected: SmallStatement::ImportFrom | SmallStatement::Import"), + _ => bail!("Expected import statement"), }; // Preserve the trailing comma (or not) from the last entry. diff --git a/crates/ruff_linter/src/importer/mod.rs b/crates/ruff_linter/src/importer/mod.rs index 3387a79a96..27abb8b1c0 100644 --- a/crates/ruff_linter/src/importer/mod.rs +++ b/crates/ruff_linter/src/importer/mod.rs @@ -66,14 +66,18 @@ impl<'a> Importer<'a> { self.type_checking_blocks.push(type_checking_block); } - /// Add an import statement to import the given module. + /// Add an import statement to import the given module, with an explicit `lazy` keyword if requested. /// /// If there are no existing imports, the new import will be added at the top /// of the file. If there are future imports, the new import will be added /// after the last future import. Otherwise, it will be added after the most /// recent top-level import statement. - pub(crate) fn add_import(&self, import: &NameImport, at: TextSize) -> Edit { - let required_import = import.to_string(); + pub(crate) fn add_import(&self, import: &NameImport, at: TextSize, is_lazy: bool) -> Edit { + let required_import = if is_lazy { + format!("lazy {import}") + } else { + import.to_string() + }; if let Some(stmt) = self.preceding_import(at) { // Insert after the last top-level import. Insertion::end_of_statement(stmt, self.source, self.stylist).into_edit(&required_import) @@ -420,6 +424,7 @@ impl<'a> Importer<'a> { symbol.module.to_string(), )), at, + false, ); Ok(( import_edit, @@ -443,6 +448,7 @@ impl<'a> Importer<'a> { symbol.member.to_string(), )), at, + false, ); Ok((import_edit, symbol.member.to_string())) } else { @@ -484,8 +490,8 @@ impl<'a> Importer<'a> { /// Add the given member to an existing `Stmt::ImportFrom` statement. fn add_member(&self, stmt: &Stmt, member: &str) -> Result { let mut statement = match_statement(&self.source[stmt.range()])?; - let import_from = match_import_from(&mut statement)?; - let aliases = match_aliases(import_from)?; + let (_, names, _) = match_import_from(&mut statement)?; + let aliases = match_aliases(names)?; aliases.push(cst::ImportAlias { name: cst::NameOrAttribute::N(Box::new(cst::Name { value: member, @@ -563,7 +569,7 @@ impl<'a> Importer<'a> { )); // Note that `TextSize::default` should ensure that the import is added at the very // beginning of the file via `Insertion::start_of_file`. - self.add_import(import, TextSize::default()) + self.add_import(import, TextSize::default(), false) } } diff --git a/crates/ruff_linter/src/rules/airflow/helpers.rs b/crates/ruff_linter/src/rules/airflow/helpers.rs index b2b0d554bb..b197570eee 100644 --- a/crates/ruff_linter/src/rules/airflow/helpers.rs +++ b/crates/ruff_linter/src/rules/airflow/helpers.rs @@ -7,7 +7,7 @@ use ruff_python_ast::helpers::map_callable; use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder}; use ruff_python_ast::statement_visitor::StatementVisitor; use ruff_python_ast::visitor::Visitor; -use ruff_python_ast::{Expr, ExprAttribute, ExprName, StmtFunctionDef, StmtTry}; +use ruff_python_ast::{Expr, ExprAttribute, ExprName, Stmt, StmtFunctionDef, StmtTry}; use ruff_python_semantic::Exceptions; use ruff_python_semantic::ScopeKind; use ruff_python_semantic::SemanticModel; @@ -259,12 +259,18 @@ pub(crate) fn generate_remove_and_runtime_import_edit( checker.indexer(), ) .ok()?; + let is_lazy = match stmt { + Stmt::Import(import) => import.is_lazy, + Stmt::ImportFrom(import) => import.is_lazy, + _ => false, + }; let import_edit = checker.importer().add_import( &NameImport::ImportFrom(MemberNameImport::member( - (*module).to_string(), + module.to_string(), name.to_string(), )), expr.start(), + is_lazy, ); Some(Fix::unsafe_edits(remove_edit, [import_edit])) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_in_call.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_in_call.rs index 1228122394..68169208cd 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_in_call.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_in_call.rs @@ -155,10 +155,6 @@ pub(crate) fn unnecessary_comprehension_in_call( } }; if args.len() == 1 { - if elt.is_starred_expr() { - // The LibCST-based fixer does not yet support PEP 798 unpacking comprehensions. - return; - } // If there's only one argument, remove the list or set brackets. diagnostic.try_set_fix(|| { fixes::fix_unnecessary_comprehension_in_call(expr, checker.locator(), checker.stylist()) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_list_call.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_list_call.rs index 15f44a61c2..099f378f8b 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_list_call.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_list_call.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall}; +use ruff_python_ast::{Arguments, Expr, ExprCall}; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_text_size::Ranged; @@ -80,13 +80,6 @@ pub(crate) fn unnecessary_list_call(checker: &Checker, expr: &Expr, call: &ExprC return; } let mut diagnostic = checker.report_diagnostic(UnnecessaryListCall, expr.range()); - if matches!( - argument, - Expr::ListComp(ast::ExprListComp { elt, .. }) if elt.is_starred_expr() - ) { - // The LibCST-based fixer does not yet support PEP 798 unpacking comprehensions. - return; - } diagnostic.try_set_fix(|| { fixes::fix_unnecessary_list_call(expr, checker.locator(), checker.stylist()) .map(Fix::unsafe_edit) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_dict_call.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_dict_call.rs index c77e6c62c4..64e38a2dfa 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_dict_call.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_dict_call.rs @@ -82,14 +82,6 @@ pub(crate) fn unnecessary_literal_within_dict_call(checker: &Checker, call: &ast call.range(), ); - if matches!( - argument, - Expr::DictComp(ast::ExprDictComp { key: None, .. }) - ) { - // The LibCST-based fixer does not yet support PEP 798 unpacking comprehensions. - return; - } - // Convert `dict({"a": 1})` to `{"a": 1}` diagnostic.set_fix({ // Delete from the start of the call to the start of the argument. diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-comprehension-in-call_C419_py315.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-comprehension-in-call_C419_py315.py.snap index 438cf0716e..1336b989a4 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-comprehension-in-call_C419_py315.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-comprehension-in-call_C419_py315.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs --- -C419 Unnecessary list comprehension +C419 [*] Unnecessary list comprehension --> C419_py315.py:3:5 | 1 | xs = [[1], [2]] @@ -11,11 +11,24 @@ C419 Unnecessary list comprehension 4 | any({*x for x in xs}) | help: Remove unnecessary comprehension + | +2 | + - all([*x for x in xs]) +3 + all(*x for x in xs) +4 | any({*x for x in xs}) + | +note: This is an unsafe fix and may change runtime behavior -C419 Unnecessary set comprehension +C419 [*] Unnecessary set comprehension --> C419_py315.py:4:5 | 3 | all([*x for x in xs]) 4 | any({*x for x in xs}) | ^^^^^^^^^^^^^^^^ help: Remove unnecessary comprehension + | +3 | all([*x for x in xs]) + - any({*x for x in xs}) +4 + any(*x for x in xs) + | +note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-list-call_C411_py315.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-list-call_C411_py315.py.snap index 91188754db..270918f8f2 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-list-call_C411_py315.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-list-call_C411_py315.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs --- -C411 Unnecessary `list()` call (remove the outer call to `list()`) +C411 [*] Unnecessary `list()` call (remove the outer call to `list()`) --> C411_py315.py:3:1 | 1 | xs = [[1], [2]] @@ -9,3 +9,9 @@ C411 Unnecessary `list()` call (remove the outer call to `list()`) 3 | list([*x for x in xs]) | ^^^^^^^^^^^^^^^^^^^^^^ help: Remove outer `list()` call + | +2 | + - list([*x for x in xs]) +3 + [*x for x in xs] + | +note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-literal-within-dict-call_C418_py315.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-literal-within-dict-call_C418_py315.py.snap index 3dc227f900..8953661460 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-literal-within-dict-call_C418_py315.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__unnecessary-literal-within-dict-call_C418_py315.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs --- -C418 Unnecessary dict comprehension passed to `dict()` (remove the outer call to `dict()`) +C418 [*] Unnecessary dict comprehension passed to `dict()` (remove the outer call to `dict()`) --> C418_py315.py:3:1 | 1 | dicts = [{"a": 1}, {"b": 2}] @@ -9,3 +9,9 @@ C418 Unnecessary dict comprehension passed to `dict()` (remove the outer call to 3 | dict({**d for d in dicts}) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Remove outer `dict()` call + | +2 | + - dict({**d for d in dicts}) +3 + {**d for d in dicts} + | +note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs b/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs index e14d3284bc..0ac06f80f9 100644 --- a/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs +++ b/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs @@ -129,8 +129,11 @@ fn add_required_import( TextRange::default(), ); diagnostic.set_fix(Fix::safe_edit( - Importer::new(parsed, locator.contents(), stylist) - .add_import(required_import, TextSize::default()), + Importer::new(parsed, locator.contents(), stylist).add_import( + required_import, + TextSize::default(), + false, + ), )); } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_mock_import.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_mock_import.rs index 7d55390635..6decf941ea 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_mock_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_mock_import.rs @@ -1,7 +1,7 @@ use anyhow::Result; use libcst_native::{ - AsName, AssignTargetExpression, Attribute, Dot, Expression, Import, ImportAlias, ImportFrom, - ImportNames, Name, NameOrAttribute, ParenthesizableWhitespace, + AsName, AssignTargetExpression, Attribute, Dot, Expression, ImportAlias, ImportNames, Name, + NameOrAttribute, ParenthesizableWhitespace, }; use log::debug; @@ -16,7 +16,7 @@ use ruff_text_size::Ranged; use crate::Locator; use crate::checkers::ast::Checker; use crate::codes::Category; -use crate::cst::matchers::{match_import, match_import_from, match_statement}; +use crate::cst::matchers::{match_aliases, match_import, match_import_from, match_statement}; use crate::fix::codemods::CodegenStylist; use crate::rules::pyupgrade::rules::is_import_required_by_isort; use crate::{Edit, Fix, FixAvailability, Violation}; @@ -128,7 +128,12 @@ fn clean_import_aliases(aliases: Vec) -> (Vec, Vec>, indent: &str, stylist: &Stylist) -> String { +fn format_mocks( + aliases: Vec>, + is_lazy: bool, + indent: &str, + stylist: &Stylist, +) -> String { let mut content = String::new(); for alias in aliases { match alias { @@ -137,6 +142,9 @@ fn format_mocks(aliases: Vec>, indent: &str, stylist: &Stylist) - content.push_str(&stylist.line_ending()); content.push_str(indent); } + if is_lazy { + content.push_str("lazy "); + } content.push_str("from unittest import mock"); } Some(as_name) => { @@ -145,6 +153,9 @@ fn format_mocks(aliases: Vec>, indent: &str, stylist: &Stylist) - content.push_str(&stylist.line_ending()); content.push_str(indent); } + if is_lazy { + content.push_str("lazy "); + } content.push_str("from unittest import mock as "); content.push_str(name.value); } @@ -163,20 +174,18 @@ fn format_import( ) -> Result { let module_text = locator.slice(stmt); let mut tree = match_statement(module_text)?; - let import = match_import(&mut tree)?; - - let Import { names, .. } = import.clone(); - let (clean_aliases, mock_aliases) = clean_import_aliases(names); + let (names, is_lazy) = match_import(&mut tree)?; + let (clean_aliases, mock_aliases) = clean_import_aliases(names.clone()); Ok(if clean_aliases.is_empty() { - format_mocks(mock_aliases, indent, stylist) + format_mocks(mock_aliases, is_lazy, indent, stylist) } else { - import.names = clean_aliases; + *names = clean_aliases; let mut content = tree.codegen_stylist(stylist); content.push_str(&stylist.line_ending()); content.push_str(indent); - content.push_str(&format_mocks(mock_aliases, indent, stylist)); + content.push_str(&format_mocks(mock_aliases, is_lazy, indent, stylist)); content }) } @@ -189,16 +198,12 @@ fn format_import_from( stylist: &Stylist, ) -> Result { let module_text = locator.slice(stmt); - let mut tree = match_statement(module_text).unwrap(); - let import = match_import_from(&mut tree)?; + let mut tree = match_statement(module_text)?; + let (module, names, is_lazy) = match_import_from(&mut tree)?; - if let ImportFrom { - names: ImportNames::Star(..), - .. - } = import - { + if let ImportNames::Star(..) = names { // Ex) `from mock import *` - import.module = Some(NameOrAttribute::A(Box::new(Attribute { + *module = Some(NameOrAttribute::A(Box::new(Attribute { value: Box::new(Expression::Name(Box::new(Name { value: "unittest", lpar: vec![], @@ -217,18 +222,15 @@ fn format_import_from( rpar: vec![], }))); Ok(tree.codegen_stylist(stylist)) - } else if let ImportFrom { - names: ImportNames::Aliases(aliases), - .. - } = import - { + } else { + let aliases = match_aliases(names)?; // Ex) `from mock import mock` let (clean_aliases, mock_aliases) = clean_import_aliases(aliases.clone()); Ok(if clean_aliases.is_empty() { - format_mocks(mock_aliases, indent, stylist) + format_mocks(mock_aliases, is_lazy, indent, stylist) } else { - import.names = ImportNames::Aliases(clean_aliases); - import.module = Some(NameOrAttribute::A(Box::new(Attribute { + *names = ImportNames::Aliases(clean_aliases); + *module = Some(NameOrAttribute::A(Box::new(Attribute { value: Box::new(Expression::Name(Box::new(Name { value: "unittest", lpar: vec![], @@ -251,12 +253,10 @@ fn format_import_from( if !mock_aliases.is_empty() { content.push_str(&stylist.line_ending()); content.push_str(indent); - content.push_str(&format_mocks(mock_aliases, indent, stylist)); + content.push_str(&format_mocks(mock_aliases, is_lazy, indent, stylist)); } content }) - } else { - panic!("Expected ImportNames::Aliases | ImportNames::Star"); } } @@ -296,9 +296,11 @@ pub(crate) fn deprecated_mock_import(checker: &Checker, stmt: &Stmt) { .iter() .any(|name| &name.name == "mock" || &name.name == "mock.mock") => { - // The CST-based fixer does not support explicit lazy import syntax. - let can_fix = !*is_lazy - && names + // Explicit `lazy` imports stay lazy because the fix preserves the keyword. + // Otherwise, changing the module must preserve whether `__lazy_modules__` + // makes the import lazy. + let can_fix = *is_lazy + || names .iter() .filter(|name| matches!(name.name.as_str(), "mock" | "mock.mock")) .all(|name| checker.import_rewrite_preserves_laziness(&name.name, "unittest")); @@ -370,8 +372,8 @@ pub(crate) fn deprecated_mock_import(checker: &Checker, stmt: &Stmt) { stmt.range(), ); diagnostic.add_primary_tag(ruff_db::diagnostic::DiagnosticTag::Deprecated); - let can_fix = !*is_lazy - && names.iter().all(|name| { + let can_fix = *is_lazy + || names.iter().all(|name| { let target = if name.name.as_str() == "mock" { "unittest" } else {