From 6bcc11a90f50bc6a88dcd0c8a4cc30823804bc5c Mon Sep 17 00:00:00 2001 From: Chammika Mannakkara Date: Sun, 13 Nov 2022 01:28:05 +0900 Subject: [PATCH] add fixes for __future__ import removal (#682) --- README.md | 4 +- resources/test/fixtures/U010.py | 14 +- src/check_ast.rs | 14 +- src/checks.rs | 17 +- src/pyupgrade/checks.rs | 40 ---- src/pyupgrade/fixes.rs | 67 ++++++- .../plugins/unnecessary_future_import.rs | 76 +++++++- .../ruff__linter__tests__U010_U010.py.snap | 176 +++++++++++++++--- 8 files changed, 316 insertions(+), 92 deletions(-) diff --git a/README.md b/README.md index 8d2842f52f..7927287dea 100644 --- a/README.md +++ b/README.md @@ -441,8 +441,8 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI. | U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | 🛠 | | U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | 🛠 | | U009 | PEP3120UnnecessaryCodingComment | utf-8 encoding declaration is unnecessary | 🛠 | -| U010 | UnnecessaryFutureImport | Unnessary __future__ import `...` for target Python version | | -| U011 | UnnecessaryLRUCacheParams | Unnessary parameters to functools.lru_cache | 🛠 | +| U010 | UnnecessaryFutureImport | Unnecessary `__future__` import `...` for target Python version | 🛠 | +| U011 | UnnecessaryLRUCacheParams | Unnecessary parameters to functools.lru_cache | 🛠 | ### pep8-naming diff --git a/resources/test/fixtures/U010.py b/resources/test/fixtures/U010.py index 79084e165b..452a50ea5c 100644 --- a/resources/test/fixtures/U010.py +++ b/resources/test/fixtures/U010.py @@ -1,5 +1,13 @@ -from __future__ import annotations, nested_scopes, generators - +from __future__ import nested_scopes, generators +from __future__ import with_statement, unicode_literals from __future__ import absolute_import, division - from __future__ import generator_stop +from __future__ import print_function, generator_stop +from __future__ import invalid_module, generators + +if True: + from __future__ import generator_stop + +if True: + from __future__ import generator_stop + from __future__ import invalid_module, generators diff --git a/src/check_ast.rs b/src/check_ast.rs index 0caf1f9b6a..1be705a966 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -606,6 +606,12 @@ where } } + if let Some("__future__") = module.as_deref() { + if self.settings.enabled.contains(&CheckCode::U010) { + pyupgrade::plugins::unnecessary_future_import(self, stmt, names); + } + } + for alias in names { if let Some("__future__") = module.as_deref() { let name = alias.node.asname.as_ref().unwrap_or(&alias.node.name); @@ -638,14 +644,6 @@ where } } - if self.settings.enabled.contains(&CheckCode::U010) { - pyupgrade::plugins::unnecessary_future_import( - self, - stmt, - &alias.node.name, - ); - } - if self.settings.enabled.contains(&CheckCode::F404) && !self.futures_allowed { self.add_check(Check::new( diff --git a/src/checks.rs b/src/checks.rs index a4451b7a3a..ef224d21b4 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -442,7 +442,7 @@ pub enum CheckKind { UsePEP604Annotation, SuperCallWithParameters, PEP3120UnnecessaryCodingComment, - UnnecessaryFutureImport(String), + UnnecessaryFutureImport(Vec), UnnecessaryLRUCacheParams, // pydocstyle BlankLineAfterLastSection(String), @@ -689,7 +689,7 @@ impl CheckCode { CheckCode::U007 => CheckKind::UsePEP604Annotation, CheckCode::U008 => CheckKind::SuperCallWithParameters, CheckCode::U009 => CheckKind::PEP3120UnnecessaryCodingComment, - CheckCode::U010 => CheckKind::UnnecessaryFutureImport("...".to_string()), + CheckCode::U010 => CheckKind::UnnecessaryFutureImport(vec!["...".to_string()]), CheckCode::U011 => CheckKind::UnnecessaryLRUCacheParams, // pydocstyle CheckCode::D100 => CheckKind::PublicModule, @@ -1595,11 +1595,17 @@ impl CheckKind { CheckKind::SuperCallWithParameters => { "Use `super()` instead of `super(__class__, self)`".to_string() } - CheckKind::UnnecessaryFutureImport(name) => { - format!("Unnessary __future__ import `{name}` for target Python version") + CheckKind::UnnecessaryFutureImport(names) => { + if names.len() == 1 { + let import = &names[0]; + format!("Unnecessary `__future__` import `{import}` for target Python version") + } else { + let imports = names.iter().map(|name| format!("`{name}`")).join(", "); + format!("Unnecessary `__future__` imports {imports} for target Python version") + } } CheckKind::UnnecessaryLRUCacheParams => { - "Unnessary parameters to functools.lru_cache".to_string() + "Unnecessary parameters to functools.lru_cache".to_string() } // pydocstyle CheckKind::FitsOnOneLine => "One-line docstring should fit on one line".to_string(), @@ -1867,6 +1873,7 @@ impl CheckKind { | CheckKind::UnnecessaryAbspath | CheckKind::UnnecessaryCollectionCall(_) | CheckKind::UnnecessaryComprehension(_) + | CheckKind::UnnecessaryFutureImport(_) | CheckKind::UnnecessaryGeneratorDict | CheckKind::UnnecessaryGeneratorList | CheckKind::UnnecessaryGeneratorSet diff --git a/src/pyupgrade/checks.rs b/src/pyupgrade/checks.rs index 4e859623a0..34cfccce9f 100644 --- a/src/pyupgrade/checks.rs +++ b/src/pyupgrade/checks.rs @@ -9,29 +9,6 @@ use crate::checks::{Check, CheckKind}; use crate::pyupgrade::types::Primitive; use crate::settings::types::PythonVersion; -pub const PY33_PLUS_REMOVE_FUTURES: &[&str] = &[ - "nested_scopes", - "generators", - "with_statement", - "division", - "absolute_import", - "with_statement", - "print_function", - "unicode_literals", -]; - -pub const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[ - "nested_scopes", - "generators", - "with_statement", - "division", - "absolute_import", - "with_statement", - "print_function", - "unicode_literals", - "generator_stop", -]; - /// U008 pub fn super_args( scope: &Scope, @@ -183,23 +160,6 @@ pub fn type_of_primitive(func: &Expr, args: &[Expr], location: Range) -> Option< None } -/// U010 -pub fn unnecessary_future_import( - version: PythonVersion, - name: &str, - location: Range, -) -> Option { - if (version >= PythonVersion::Py33 && PY33_PLUS_REMOVE_FUTURES.contains(&name)) - || (version >= PythonVersion::Py37 && PY37_PLUS_REMOVE_FUTURES.contains(&name)) - { - return Some(Check::new( - CheckKind::UnnecessaryFutureImport(name.to_string()), - location, - )); - } - None -} - /// U011 pub fn unnecessary_lru_cache_params( decorator_list: &[Expr], diff --git a/src/pyupgrade/fixes.rs b/src/pyupgrade/fixes.rs index 30347a2cad..195742149d 100644 --- a/src/pyupgrade/fixes.rs +++ b/src/pyupgrade/fixes.rs @@ -1,11 +1,13 @@ -use libcst_native::{Codegen, Expression, SmallStatement, Statement}; -use rustpython_ast::{Expr, Keyword, Location}; +use anyhow::Result; +use libcst_native::{Codegen, Expression, ImportNames, SmallStatement, Statement}; +use rustpython_ast::{Expr, Keyword, Location, Stmt}; use rustpython_parser::lexer; use rustpython_parser::lexer::Tok; use crate::ast::helpers; use crate::ast::types::Range; -use crate::autofix::Fix; +use crate::autofix::{self, Fix}; +use crate::cst::matchers::match_module; use crate::source_code_locator::SourceCodeLocator; /// Generate a fix to remove a base from a ClassDef statement. @@ -41,7 +43,7 @@ pub fn remove_class_def_base( } return match (fix_start, fix_end) { - (Some(start), Some(end)) => Some(Fix::replacement("".to_string(), start, end)), + (Some(start), Some(end)) => Some(Fix::deletion(start, end)), _ => None, }; } @@ -133,6 +135,63 @@ pub fn remove_super_arguments(locator: &SourceCodeLocator, expr: &Expr) -> Optio None } +/// U010 +pub fn remove_unnecessary_future_import( + locator: &SourceCodeLocator, + removable: &[usize], + stmt: &Stmt, + parent: Option<&Stmt>, + deleted: &[&Stmt], +) -> Result { + // TODO(charlie): DRY up with pyflakes::fixes::remove_unused_import_froms. + let module_text = locator.slice_source_code_range(&Range::from_located(stmt)); + let mut tree = match_module(&module_text)?; + + let body = if let Some(Statement::Simple(body)) = tree.body.first_mut() { + body + } else { + return Err(anyhow::anyhow!("Expected node to be: Statement::Simple")); + }; + let body = if let Some(SmallStatement::ImportFrom(body)) = body.body.first_mut() { + body + } else { + return Err(anyhow::anyhow!( + "Expected node to be: SmallStatement::ImportFrom" + )); + }; + + let aliases = if let ImportNames::Aliases(aliases) = &mut body.names { + aliases + } else { + return Err(anyhow::anyhow!("Expected node to be: Aliases")); + }; + + // Preserve the trailing comma (or not) from the last entry. + let trailing_comma = aliases.last().and_then(|alias| alias.comma.clone()); + + // TODO(charlie): This is quadratic. + for index in removable.iter().rev() { + aliases.remove(*index); + } + + if let Some(alias) = aliases.last_mut() { + alias.comma = trailing_comma; + } + + if aliases.is_empty() { + autofix::helpers::remove_stmt(stmt, parent, deleted) + } else { + let mut state = Default::default(); + tree.codegen(&mut state); + + Ok(Fix::replacement( + state.to_string(), + stmt.location, + stmt.end_location.unwrap(), + )) + } +} + /// U011 pub fn remove_unnecessary_lru_cache_params( locator: &SourceCodeLocator, diff --git a/src/pyupgrade/plugins/unnecessary_future_import.rs b/src/pyupgrade/plugins/unnecessary_future_import.rs index b5f76cc2b6..b994b05095 100644 --- a/src/pyupgrade/plugins/unnecessary_future_import.rs +++ b/src/pyupgrade/plugins/unnecessary_future_import.rs @@ -1,15 +1,77 @@ +use std::collections::BTreeSet; + +use rustpython_ast::{AliasData, Located}; use rustpython_parser::ast::Stmt; use crate::ast::types::Range; use crate::check_ast::Checker; -use crate::pyupgrade::checks; +use crate::checks::{Check, CheckKind}; +use crate::pyupgrade::fixes; +use crate::settings::types::PythonVersion; -pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, name: &str) { - if let Some(check) = checks::unnecessary_future_import( - checker.settings.target_version, - name, - Range::from_located(stmt), - ) { +const PY33_PLUS_REMOVE_FUTURES: &[&str] = &[ + "nested_scopes", + "generators", + "with_statement", + "division", + "absolute_import", + "with_statement", + "print_function", + "unicode_literals", +]; + +const PY37_PLUS_REMOVE_FUTURES: &[&str] = &[ + "nested_scopes", + "generators", + "with_statement", + "division", + "absolute_import", + "with_statement", + "print_function", + "unicode_literals", + "generator_stop", +]; + +/// U010 +pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Located]) { + let target_version = checker.settings.target_version; + + let mut removable_index: Vec = vec![]; + let mut removable_names: BTreeSet<&str> = BTreeSet::new(); + for (index, alias) in names.iter().enumerate() { + let name = alias.node.name.as_str(); + if (target_version >= PythonVersion::Py33 && PY33_PLUS_REMOVE_FUTURES.contains(&name)) + || (target_version >= PythonVersion::Py37 && PY37_PLUS_REMOVE_FUTURES.contains(&name)) + { + removable_index.push(index); + removable_names.insert(name); + } + } + + if !removable_index.is_empty() { + let mut check = Check::new( + CheckKind::UnnecessaryFutureImport( + removable_names.into_iter().map(String::from).collect(), + ), + Range::from_located(stmt), + ); + if checker.patch() { + let context = checker.binding_context(); + let deleted: Vec<&Stmt> = checker + .deletions + .iter() + .map(|index| checker.parents[*index]) + .collect(); + if let Ok(fix) = fixes::remove_unnecessary_future_import( + checker.locator, + &removable_index, + checker.parents[context.defined_by], + context.defined_in.map(|index| checker.parents[index]), + &deleted, + ) { + check.amend(fix); + } + } checker.add_check(check); } } diff --git a/src/snapshots/ruff__linter__tests__U010_U010.py.snap b/src/snapshots/ruff__linter__tests__U010_U010.py.snap index fbc8ce88b5..dfc2aca003 100644 --- a/src/snapshots/ruff__linter__tests__U010_U010.py.snap +++ b/src/snapshots/ruff__linter__tests__U010_U010.py.snap @@ -3,48 +3,178 @@ source: src/linter.rs expression: checks --- - kind: - UnnecessaryFutureImport: nested_scopes + UnnecessaryFutureImport: + - generators + - nested_scopes location: row: 1 column: 0 end_location: row: 1 - column: 61 - fix: ~ + column: 48 + fix: + patch: + content: "" + location: + row: 1 + column: 0 + end_location: + row: 2 + column: 0 + applied: false - kind: - UnnecessaryFutureImport: generators + UnnecessaryFutureImport: + - unicode_literals + - with_statement location: - row: 1 + row: 2 column: 0 end_location: - row: 1 - column: 61 - fix: ~ + row: 2 + column: 55 + fix: + patch: + content: "" + location: + row: 2 + column: 0 + end_location: + row: 3 + column: 0 + applied: false - kind: - UnnecessaryFutureImport: absolute_import + UnnecessaryFutureImport: + - absolute_import + - division location: row: 3 column: 0 end_location: row: 3 column: 48 - fix: ~ + fix: + patch: + content: "" + location: + row: 3 + column: 0 + end_location: + row: 4 + column: 0 + applied: false - kind: - UnnecessaryFutureImport: division + UnnecessaryFutureImport: + - generator_stop location: - row: 3 + row: 4 column: 0 end_location: - row: 3 - column: 48 - fix: ~ -- kind: - UnnecessaryFutureImport: generator_stop - location: - row: 5 - column: 0 - end_location: - row: 5 + row: 4 column: 37 - fix: ~ + fix: + patch: + content: "" + location: + row: 4 + column: 0 + end_location: + row: 5 + column: 0 + applied: false +- kind: + UnnecessaryFutureImport: + - generator_stop + - print_function + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 53 + fix: + patch: + content: "" + location: + row: 5 + column: 0 + end_location: + row: 6 + column: 0 + applied: false +- kind: + UnnecessaryFutureImport: + - generators + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 49 + fix: + patch: + content: from __future__ import invalid_module + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 49 + applied: false +- kind: + UnnecessaryFutureImport: + - generator_stop + location: + row: 9 + column: 4 + end_location: + row: 9 + column: 41 + fix: + patch: + content: pass + location: + row: 9 + column: 4 + end_location: + row: 9 + column: 41 + applied: false +- kind: + UnnecessaryFutureImport: + - generator_stop + location: + row: 12 + column: 4 + end_location: + row: 12 + column: 41 + fix: + patch: + content: "" + location: + row: 12 + column: 0 + end_location: + row: 13 + column: 0 + applied: false +- kind: + UnnecessaryFutureImport: + - generators + location: + row: 13 + column: 4 + end_location: + row: 13 + column: 53 + fix: + patch: + content: from __future__ import invalid_module + location: + row: 13 + column: 4 + end_location: + row: 13 + column: 53 + applied: false