diff --git a/README.md b/README.md index f84fc2525e..5747e4128b 100644 --- a/README.md +++ b/README.md @@ -471,6 +471,7 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI. | U012 | UnnecessaryEncodeUTF8 | Unnecessary call to `encode` as UTF-8 | 🛠 | | U013 | ConvertTypedDictFunctionalToClass | Convert `...` from `TypedDict` functional to class syntax | 🛠 | | U014 | ConvertNamedTupleFunctionalToClass | Convert `...` from `NamedTuple` functional to class syntax | 🛠 | +| U015 | RedundantOpenModes | Unnecessary open mode parameters | 🛠 | ### pep8-naming @@ -826,7 +827,7 @@ including: - [`flake8-boolean-trap`](https://pypi.org/project/flake8-boolean-trap/) - [`mccabe`](https://pypi.org/project/mccabe/) - [`isort`](https://pypi.org/project/isort/) -- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (15/33) +- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (16/33) - [`autoflake`](https://pypi.org/project/autoflake/) (1/7) Beyond rule-set parity, Ruff suffers from the following limitations vis-à-vis Flake8: @@ -858,7 +859,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`mccabe`](https://pypi.org/project/mccabe/) Ruff can also replace [`isort`](https://pypi.org/project/isort/), [`yesqa`](https://github.com/asottile/yesqa), -and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (15/33). +and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (16/33). If you're looking to use Ruff, but rely on an unsupported Flake8 plugin, free to file an Issue. diff --git a/resources/test/fixtures/U015.py b/resources/test/fixtures/U015.py new file mode 100644 index 0000000000..9e276f28c9 --- /dev/null +++ b/resources/test/fixtures/U015.py @@ -0,0 +1,38 @@ +open("foo", "U") +open("foo", "Ur") +open("foo", "Ub") +open("foo", "rUb") +open("foo", "r") +open("foo", "rt") +open("f", "r", encoding="UTF-8") +open("f", "wt") + +with open("foo", "U") as f: + pass +with open("foo", "Ur") as f: + pass +with open("foo", "Ub") as f: + pass +with open("foo", "rUb") as f: + pass +with open("foo", "r") as f: + pass +with open("foo", "rt") as f: + pass +with open("foo", "r", encoding="UTF-8") as f: + pass +with open("foo", "wt") as f: + pass + +open(f("a", "b", "c"), "U") +open(f("a", "b", "c"), "Ub") + +with open(f("a", "b", "c"), "U") as f: + pass +with open(f("a", "b", "c"), "Ub") as f: + pass + +with open("foo", "U") as fa, open("bar", "U") as fb: + pass +with open("foo", "Ub") as fa, open("bar", "Ub") as fb: + pass diff --git a/src/check_ast.rs b/src/check_ast.rs index 8d398cd399..44f8952747 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1516,6 +1516,10 @@ where pyupgrade::plugins::type_of_primitive(self, expr, func, args); } + if self.settings.enabled.contains(&CheckCode::U015) { + pyupgrade::plugins::redundant_open_modes(self, expr); + } + // flake8-boolean-trap if self.settings.enabled.contains(&CheckCode::FBT003) { flake8_boolean_trap::plugins::check_boolean_positional_value_in_function_call( diff --git a/src/checks.rs b/src/checks.rs index c03e673b68..7087b238e6 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -171,6 +171,7 @@ pub enum CheckCode { U012, U013, U014, + U015, // pydocstyle D100, D101, @@ -504,6 +505,7 @@ pub enum CheckKind { UnnecessaryEncodeUTF8, ConvertTypedDictFunctionalToClass(String), ConvertNamedTupleFunctionalToClass(String), + RedundantOpenModes, // pydocstyle BlankLineAfterLastSection(String), BlankLineAfterSection(String), @@ -779,6 +781,7 @@ impl CheckCode { CheckCode::U012 => CheckKind::UnnecessaryEncodeUTF8, CheckCode::U013 => CheckKind::ConvertTypedDictFunctionalToClass("...".to_string()), CheckCode::U014 => CheckKind::ConvertNamedTupleFunctionalToClass("...".to_string()), + CheckCode::U015 => CheckKind::RedundantOpenModes, // pydocstyle CheckCode::D100 => CheckKind::PublicModule, CheckCode::D101 => CheckKind::PublicClass, @@ -1013,6 +1016,7 @@ impl CheckCode { CheckCode::U012 => CheckCategory::Pyupgrade, CheckCode::U013 => CheckCategory::Pyupgrade, CheckCode::U014 => CheckCategory::Pyupgrade, + CheckCode::U015 => CheckCategory::Pyupgrade, CheckCode::D100 => CheckCategory::Pydocstyle, CheckCode::D101 => CheckCategory::Pydocstyle, CheckCode::D102 => CheckCategory::Pydocstyle, @@ -1238,6 +1242,7 @@ impl CheckKind { CheckKind::UnnecessaryEncodeUTF8 => &CheckCode::U012, CheckKind::ConvertTypedDictFunctionalToClass(_) => &CheckCode::U013, CheckKind::ConvertNamedTupleFunctionalToClass(_) => &CheckCode::U014, + CheckKind::RedundantOpenModes => &CheckCode::U015, // pydocstyle CheckKind::BlankLineAfterLastSection(_) => &CheckCode::D413, CheckKind::BlankLineAfterSection(_) => &CheckCode::D410, @@ -1787,6 +1792,7 @@ impl CheckKind { "Unnecessary parameters to `functools.lru_cache`".to_string() } CheckKind::UnnecessaryEncodeUTF8 => "Unnecessary call to `encode` as UTF-8".to_string(), + CheckKind::RedundantOpenModes => "Unnecessary open mode parameters".to_string(), CheckKind::ConvertTypedDictFunctionalToClass(name) => { format!("Convert `{name}` from `TypedDict` functional to class syntax") } @@ -2104,6 +2110,7 @@ impl CheckKind { | CheckKind::UnnecessaryComprehension(..) | CheckKind::UnnecessaryEncodeUTF8 | CheckKind::UnnecessaryFutureImport(..) + | CheckKind::RedundantOpenModes | CheckKind::UnnecessaryGeneratorDict | CheckKind::UnnecessaryGeneratorList | CheckKind::UnnecessaryGeneratorSet diff --git a/src/checks_gen.rs b/src/checks_gen.rs index b52833f54e..3ce5c8e197 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -301,6 +301,7 @@ pub enum CheckCodePrefix { U012, U013, U014, + U015, W, W2, W29, @@ -1125,6 +1126,7 @@ impl CheckCodePrefix { CheckCode::U012, CheckCode::U013, CheckCode::U014, + CheckCode::U015, ], CheckCodePrefix::U0 => vec![ CheckCode::U001, @@ -1140,6 +1142,7 @@ impl CheckCodePrefix { CheckCode::U012, CheckCode::U013, CheckCode::U014, + CheckCode::U015, ], CheckCodePrefix::U00 => vec![ CheckCode::U001, @@ -1165,12 +1168,14 @@ impl CheckCodePrefix { CheckCode::U012, CheckCode::U013, CheckCode::U014, + CheckCode::U015, ], CheckCodePrefix::U010 => vec![CheckCode::U010], CheckCodePrefix::U011 => vec![CheckCode::U011], CheckCodePrefix::U012 => vec![CheckCode::U012], CheckCodePrefix::U013 => vec![CheckCode::U013], CheckCodePrefix::U014 => vec![CheckCode::U014], + CheckCodePrefix::U015 => vec![CheckCode::U015], CheckCodePrefix::W => vec![CheckCode::W292, CheckCode::W605], CheckCodePrefix::W2 => vec![CheckCode::W292], CheckCodePrefix::W29 => vec![CheckCode::W292], @@ -1518,6 +1523,7 @@ impl CheckCodePrefix { CheckCodePrefix::U012 => PrefixSpecificity::Explicit, CheckCodePrefix::U013 => PrefixSpecificity::Explicit, CheckCodePrefix::U014 => PrefixSpecificity::Explicit, + CheckCodePrefix::U015 => PrefixSpecificity::Explicit, CheckCodePrefix::W => PrefixSpecificity::Category, CheckCodePrefix::W2 => PrefixSpecificity::Hundreds, CheckCodePrefix::W29 => PrefixSpecificity::Tens, diff --git a/src/linter.rs b/src/linter.rs index 6f25f846d7..d0a7cdf99e 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -518,6 +518,7 @@ mod tests { #[test_case(CheckCode::U012, Path::new("U012.py"); "U012")] #[test_case(CheckCode::U013, Path::new("U013.py"); "U013")] #[test_case(CheckCode::U014, Path::new("U014.py"); "U014")] + #[test_case(CheckCode::U015, Path::new("U015.py"); "U015")] #[test_case(CheckCode::W292, Path::new("W292_0.py"); "W292_0")] #[test_case(CheckCode::W292, Path::new("W292_1.py"); "W292_1")] #[test_case(CheckCode::W292, Path::new("W292_2.py"); "W292_2")] diff --git a/src/pyupgrade/plugins/mod.rs b/src/pyupgrade/plugins/mod.rs index 4bb314bbad..a016633697 100644 --- a/src/pyupgrade/plugins/mod.rs +++ b/src/pyupgrade/plugins/mod.rs @@ -1,6 +1,7 @@ pub use convert_named_tuple_functional_to_class::convert_named_tuple_functional_to_class; pub use convert_typed_dict_functional_to_class::convert_typed_dict_functional_to_class; pub use deprecated_unittest_alias::deprecated_unittest_alias; +pub use redundant_open_modes::redundant_open_modes; pub use super_call_with_parameters::super_call_with_parameters; pub use type_of_primitive::type_of_primitive; pub use unnecessary_encode_utf8::unnecessary_encode_utf8; @@ -14,6 +15,7 @@ pub use useless_object_inheritance::useless_object_inheritance; mod convert_named_tuple_functional_to_class; mod convert_typed_dict_functional_to_class; mod deprecated_unittest_alias; +mod redundant_open_modes; mod super_call_with_parameters; mod type_of_primitive; mod unnecessary_encode_utf8; diff --git a/src/pyupgrade/plugins/redundant_open_modes.rs b/src/pyupgrade/plugins/redundant_open_modes.rs new file mode 100644 index 0000000000..7e4f3a809f --- /dev/null +++ b/src/pyupgrade/plugins/redundant_open_modes.rs @@ -0,0 +1,155 @@ +use std::str::FromStr; + +use anyhow::{anyhow, Result}; +use log::error; +use rustpython_ast::{Constant, Expr, ExprKind, Located, Location}; +use rustpython_parser::lexer; +use rustpython_parser::token::Tok; + +use crate::ast::helpers::{self, match_name_or_attr}; +use crate::ast::types::Range; +use crate::autofix::Fix; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckCode, CheckKind}; +use crate::source_code_locator::SourceCodeLocator; + +const OPEN_FUNC_NAME: &str = "open"; + +enum OpenMode { + U, + Ur, + Ub, + RUb, + R, + Rt, + Wt, +} + +impl FromStr for OpenMode { + type Err = anyhow::Error; + + fn from_str(string: &str) -> Result { + match string { + "U" => Ok(OpenMode::U), + "Ur" => Ok(OpenMode::Ur), + "Ub" => Ok(OpenMode::Ub), + "rUb" => Ok(OpenMode::RUb), + "r" => Ok(OpenMode::R), + "rt" => Ok(OpenMode::Rt), + "wt" => Ok(OpenMode::Wt), + _ => Err(anyhow!("Unknown open mode: {}", string)), + } + } +} + +impl OpenMode { + fn replacement_value(&self) -> Option { + match *self { + OpenMode::U => None, + OpenMode::Ur => None, + OpenMode::Ub => Some(String::from("\"rb\"")), + OpenMode::RUb => Some(String::from("\"rb\"")), + OpenMode::R => None, + OpenMode::Rt => None, + OpenMode::Wt => Some(String::from("\"w\"")), + } + } +} + +fn match_open(expr: &Expr) -> Option<&Expr> { + if let ExprKind::Call { func, args, .. } = &expr.node { + // TODO(andberger): Verify that "open" is still bound to the built-in function. + if match_name_or_attr(func, OPEN_FUNC_NAME) { + // Return the "open mode" parameter. + return args.get(1); + } + } + None +} + +fn create_check( + expr: &Expr, + mode_param: &Expr, + replacement_value: Option, + locator: &SourceCodeLocator, + patch: bool, +) -> Check { + let mut check = Check::new(CheckKind::RedundantOpenModes, Range::from_located(expr)); + if patch { + if let Some(content) = replacement_value { + check.amend(Fix::replacement( + content, + mode_param.location, + mode_param.end_location.unwrap(), + )) + } else { + match create_remove_param_fix(locator, expr, mode_param) { + Ok(fix) => check.amend(fix), + Err(e) => error!("Failed to remove parameter: {}", e), + } + } + } + check +} + +fn create_remove_param_fix( + locator: &SourceCodeLocator, + expr: &Expr, + mode_param: &Expr, +) -> Result { + let content = locator.slice_source_code_range(&Range { + location: expr.location, + end_location: expr.end_location.unwrap(), + }); + // Find the last comma before mode_param + // and delete that comma as well as mode_param. + let mut fix_start: Option = None; + let mut fix_end: Option = None; + for (start, tok, end) in lexer::make_tokenizer(&content).flatten() { + let start = helpers::to_absolute(start, expr.location); + let end = helpers::to_absolute(end, expr.location); + if start == mode_param.location { + fix_end = Some(end); + break; + } + if matches!(tok, Tok::Comma) { + fix_start = Some(start); + } + } + match (fix_start, fix_end) { + (Some(start), Some(end)) => Ok(Fix::deletion(start, end)), + _ => Err(anyhow::anyhow!( + "Failed to locate start and end parentheses." + )), + } +} + +/// U015 +pub fn redundant_open_modes(checker: &mut Checker, expr: &Expr) { + // TODO(andberger): Add "mode" keyword argument handling to handle invocations + // on the following formats: + // - `open("foo", mode="U")` + // - `open(name="foo", mode="U")` + // - `open(mode="U", name="foo")` + if let Some(mode_param) = match_open(expr) { + if let Located { + node: + ExprKind::Constant { + value: Constant::Str(mode_param_value), + .. + }, + .. + } = mode_param + { + if let Ok(mode) = OpenMode::from_str(mode_param_value.as_str()) { + checker.add_check(create_check( + expr, + mode_param, + mode.replacement_value(), + checker.locator, + checker.patch(&CheckCode::U015), + )); + } + } + } +} diff --git a/src/snapshots/ruff__linter__tests__U015_U015.py.snap b/src/snapshots/ruff__linter__tests__U015_U015.py.snap new file mode 100644 index 0000000000..dfd299f257 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__U015_U015.py.snap @@ -0,0 +1,413 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: RedundantOpenModes + location: + row: 1 + column: 0 + end_location: + row: 1 + column: 16 + fix: + patch: + content: "" + location: + row: 1 + column: 10 + end_location: + row: 1 + column: 15 + applied: false +- kind: RedundantOpenModes + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 17 + fix: + patch: + content: "" + location: + row: 2 + column: 10 + end_location: + row: 2 + column: 16 + applied: false +- kind: RedundantOpenModes + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 17 + fix: + patch: + content: "\"rb\"" + location: + row: 3 + column: 12 + end_location: + row: 3 + column: 16 + applied: false +- kind: RedundantOpenModes + location: + row: 4 + column: 0 + end_location: + row: 4 + column: 18 + fix: + patch: + content: "\"rb\"" + location: + row: 4 + column: 12 + end_location: + row: 4 + column: 17 + applied: false +- kind: RedundantOpenModes + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 16 + fix: + patch: + content: "" + location: + row: 5 + column: 10 + end_location: + row: 5 + column: 15 + applied: false +- kind: RedundantOpenModes + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 17 + fix: + patch: + content: "" + location: + row: 6 + column: 10 + end_location: + row: 6 + column: 16 + applied: false +- kind: RedundantOpenModes + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 32 + fix: + patch: + content: "" + location: + row: 7 + column: 8 + end_location: + row: 7 + column: 13 + applied: false +- kind: RedundantOpenModes + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 15 + fix: + patch: + content: "\"w\"" + location: + row: 8 + column: 10 + end_location: + row: 8 + column: 14 + applied: false +- kind: RedundantOpenModes + location: + row: 10 + column: 5 + end_location: + row: 10 + column: 21 + fix: + patch: + content: "" + location: + row: 10 + column: 15 + end_location: + row: 10 + column: 20 + applied: false +- kind: RedundantOpenModes + location: + row: 12 + column: 5 + end_location: + row: 12 + column: 22 + fix: + patch: + content: "" + location: + row: 12 + column: 15 + end_location: + row: 12 + column: 21 + applied: false +- kind: RedundantOpenModes + location: + row: 14 + column: 5 + end_location: + row: 14 + column: 22 + fix: + patch: + content: "\"rb\"" + location: + row: 14 + column: 17 + end_location: + row: 14 + column: 21 + applied: false +- kind: RedundantOpenModes + location: + row: 16 + column: 5 + end_location: + row: 16 + column: 23 + fix: + patch: + content: "\"rb\"" + location: + row: 16 + column: 17 + end_location: + row: 16 + column: 22 + applied: false +- kind: RedundantOpenModes + location: + row: 18 + column: 5 + end_location: + row: 18 + column: 21 + fix: + patch: + content: "" + location: + row: 18 + column: 15 + end_location: + row: 18 + column: 20 + applied: false +- kind: RedundantOpenModes + location: + row: 20 + column: 5 + end_location: + row: 20 + column: 22 + fix: + patch: + content: "" + location: + row: 20 + column: 15 + end_location: + row: 20 + column: 21 + applied: false +- kind: RedundantOpenModes + location: + row: 22 + column: 5 + end_location: + row: 22 + column: 39 + fix: + patch: + content: "" + location: + row: 22 + column: 15 + end_location: + row: 22 + column: 20 + applied: false +- kind: RedundantOpenModes + location: + row: 24 + column: 5 + end_location: + row: 24 + column: 22 + fix: + patch: + content: "\"w\"" + location: + row: 24 + column: 17 + end_location: + row: 24 + column: 21 + applied: false +- kind: RedundantOpenModes + location: + row: 27 + column: 0 + end_location: + row: 27 + column: 27 + fix: + patch: + content: "" + location: + row: 27 + column: 21 + end_location: + row: 27 + column: 26 + applied: false +- kind: RedundantOpenModes + location: + row: 28 + column: 0 + end_location: + row: 28 + column: 28 + fix: + patch: + content: "\"rb\"" + location: + row: 28 + column: 23 + end_location: + row: 28 + column: 27 + applied: false +- kind: RedundantOpenModes + location: + row: 30 + column: 5 + end_location: + row: 30 + column: 32 + fix: + patch: + content: "" + location: + row: 30 + column: 26 + end_location: + row: 30 + column: 31 + applied: false +- kind: RedundantOpenModes + location: + row: 32 + column: 5 + end_location: + row: 32 + column: 33 + fix: + patch: + content: "\"rb\"" + location: + row: 32 + column: 28 + end_location: + row: 32 + column: 32 + applied: false +- kind: RedundantOpenModes + location: + row: 35 + column: 5 + end_location: + row: 35 + column: 21 + fix: + patch: + content: "" + location: + row: 35 + column: 15 + end_location: + row: 35 + column: 20 + applied: false +- kind: RedundantOpenModes + location: + row: 35 + column: 29 + end_location: + row: 35 + column: 45 + fix: + patch: + content: "" + location: + row: 35 + column: 39 + end_location: + row: 35 + column: 44 + applied: false +- kind: RedundantOpenModes + location: + row: 37 + column: 5 + end_location: + row: 37 + column: 22 + fix: + patch: + content: "\"rb\"" + location: + row: 37 + column: 17 + end_location: + row: 37 + column: 21 + applied: false +- kind: RedundantOpenModes + location: + row: 37 + column: 30 + end_location: + row: 37 + column: 47 + fix: + patch: + content: "\"rb\"" + location: + row: 37 + column: 42 + end_location: + row: 37 + column: 46 + applied: false +