diff --git a/README.md b/README.md index 530f0dd33e7e8..a90c2d773a91f 100644 --- a/README.md +++ b/README.md @@ -465,8 +465,8 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI. | U010 | UnnecessaryFutureImport | Unnecessary `__future__` import `...` for target Python version | 🛠 | | U011 | UnnecessaryLRUCacheParams | Unnecessary parameters to `functools.lru_cache` | 🛠 | | U012 | UnnecessaryEncodeUTF8 | Unnecessary call to `encode` as UTF-8 | 🛠 | -| U013 | ConvertTypedDictFunctionalToClass | Convert `TypedDict` functional syntax to class syntax | 🛠 | -| U014 | ConvertNamedTupleFunctionalToClass | Convert `NamedTuple` functional syntax to class syntax | 🛠 | +| U013 | ConvertTypedDictFunctionalToClass | Convert `...` from `TypedDict` functional to class syntax | 🛠 | +| U014 | ConvertNamedTupleFunctionalToClass | Convert `...` from `NamedTuple` functional to class syntax | 🛠 | ### pep8-naming @@ -664,6 +664,7 @@ For more, see [mccabe](https://pypi.org/project/mccabe/0.7.0/) on PyPI. | RUF001 | AmbiguousUnicodeCharacterString | String contains ambiguous unicode character '𝐁' (did you mean 'B'?) | 🛠 | | RUF002 | AmbiguousUnicodeCharacterDocstring | Docstring contains ambiguous unicode character '𝐁' (did you mean 'B'?) | 🛠 | | RUF003 | AmbiguousUnicodeCharacterComment | Comment contains ambiguous unicode character '𝐁' (did you mean 'B'?) | | +| RUF101 | ConvertExitToSysExit | `exit()` is only available in the interpreter, use `sys.exit()` instead | 🛠 | ### Meta rules diff --git a/resources/test/fixtures/RUF101_0.py b/resources/test/fixtures/RUF101_0.py new file mode 100644 index 0000000000000..c08a1bc248f29 --- /dev/null +++ b/resources/test/fixtures/RUF101_0.py @@ -0,0 +1,5 @@ +exit(0) + + +def main(): + exit(2) diff --git a/resources/test/fixtures/RUF101_1.py b/resources/test/fixtures/RUF101_1.py new file mode 100644 index 0000000000000..082ff77261937 --- /dev/null +++ b/resources/test/fixtures/RUF101_1.py @@ -0,0 +1,10 @@ +import sys + +exit(0) + + +def main(): + exit(1) + + +sys.exit(2) diff --git a/resources/test/fixtures/RUF101_2.py b/resources/test/fixtures/RUF101_2.py new file mode 100644 index 0000000000000..62562eca7ec48 --- /dev/null +++ b/resources/test/fixtures/RUF101_2.py @@ -0,0 +1,7 @@ +import sys as sys2 + +exit(0) + + +def main(): + exit(1) diff --git a/resources/test/fixtures/RUF101_3.py b/resources/test/fixtures/RUF101_3.py new file mode 100644 index 0000000000000..9d20a42996c69 --- /dev/null +++ b/resources/test/fixtures/RUF101_3.py @@ -0,0 +1,7 @@ +from sys import exit + +exit(0) + + +def main(): + exit(1) diff --git a/resources/test/fixtures/RUF101_4.py b/resources/test/fixtures/RUF101_4.py new file mode 100644 index 0000000000000..deabd19468278 --- /dev/null +++ b/resources/test/fixtures/RUF101_4.py @@ -0,0 +1,7 @@ +from sys import exit as exit2 + +exit(0) + + +def main(): + exit(1) diff --git a/resources/test/fixtures/RUF101_5.py b/resources/test/fixtures/RUF101_5.py new file mode 100644 index 0000000000000..62f8a6118eda4 --- /dev/null +++ b/resources/test/fixtures/RUF101_5.py @@ -0,0 +1,7 @@ +from sys import * + +exit(0) + + +def main(): + exit(1) diff --git a/resources/test/fixtures/RUF101_6.py b/resources/test/fixtures/RUF101_6.py new file mode 100644 index 0000000000000..150088b59fe1c --- /dev/null +++ b/resources/test/fixtures/RUF101_6.py @@ -0,0 +1,12 @@ +exit(0) + + +def exit(e): + pass + + +exit(1) + + +def main(): + exit(2) diff --git a/src/check_ast.rs b/src/check_ast.rs index 190f6113a1798..fa8ac707d9be9 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -37,7 +37,7 @@ use crate::visibility::{module_visibility, transition_scope, Modifier, Visibilit use crate::{ docstrings, flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_print, - flake8_tidy_imports, mccabe, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, + flake8_tidy_imports, mccabe, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, rules, }; const GLOBAL_SCOPE_INDEX: usize = 0; @@ -471,7 +471,7 @@ where } StmtKind::Return { .. } => { if self.settings.enabled.contains(&CheckCode::F706) { - if let Some(index) = self.scope_stack.last().cloned() { + if let Some(&index) = self.scope_stack.last() { if matches!( self.scopes[index].kind, ScopeKind::Class(_) | ScopeKind::Module @@ -1527,6 +1527,11 @@ where } } } + + // Ruff + if self.settings.enabled.contains(&CheckCode::RUF101) { + rules::plugins::convert_exit_to_sys_exit(self, func); + } } ExprKind::Dict { keys, .. } => { let check_repeated_literals = self.settings.enabled.contains(&CheckCode::F601); @@ -2169,6 +2174,10 @@ impl<'a> Checker<'a> { &self.scopes[*(self.scope_stack.last().expect("No current scope found."))] } + pub fn current_scopes(&self) -> impl Iterator { + self.scope_stack.iter().rev().map(|s| &self.scopes[*s]) + } + pub fn current_parent(&self) -> &'a Stmt { self.parents[*(self.parent_stack.last().expect("No parent found."))] } @@ -2188,7 +2197,7 @@ impl<'a> Checker<'a> { 'b: 'a, { if self.settings.enabled.contains(&CheckCode::F402) { - let scope = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; + let scope = self.current_scope(); if let Some(existing) = scope.values.get(&name) { if matches!(binding.kind, BindingKind::LoopVar) && matches!( @@ -2213,7 +2222,7 @@ impl<'a> Checker<'a> { // TODO(charlie): Don't treat annotations as assignments if there is an existing // value. - let scope = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; + let scope = self.current_scope(); let binding = match scope.values.get(&name) { None => binding, Some(existing) => Binding { @@ -2229,8 +2238,7 @@ impl<'a> Checker<'a> { fn handle_node_load(&mut self, expr: &Expr) { if let ExprKind::Name { id, .. } = &expr.node { - let scope_id = - self.scopes[*(self.scope_stack.last().expect("No current scope found."))].id; + let scope_id = self.current_scope().id; let mut first_iter = true; let mut in_generator = false; @@ -2385,7 +2393,7 @@ impl<'a> Checker<'a> { return; } - let current = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; + let current = self.current_scope(); if id == "__all__" && matches!(current.kind, ScopeKind::Module) && matches!( diff --git a/src/checks.rs b/src/checks.rs index 8470265a40fa6..9fb29ad201fec 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -249,6 +249,7 @@ pub enum CheckCode { RUF001, RUF002, RUF003, + RUF101, // Meta M001, } @@ -583,6 +584,7 @@ pub enum CheckKind { AmbiguousUnicodeCharacterString(char, char), AmbiguousUnicodeCharacterDocstring(char, char), AmbiguousUnicodeCharacterComment(char, char), + ConvertExitToSysExit, // Meta UnusedNOQA(Option>), } @@ -872,6 +874,7 @@ impl CheckCode { CheckCode::RUF001 => CheckKind::AmbiguousUnicodeCharacterString('𝐁', 'B'), CheckCode::RUF002 => CheckKind::AmbiguousUnicodeCharacterDocstring('𝐁', 'B'), CheckCode::RUF003 => CheckKind::AmbiguousUnicodeCharacterComment('𝐁', 'B'), + CheckCode::RUF101 => CheckKind::ConvertExitToSysExit, // Meta CheckCode::M001 => CheckKind::UnusedNOQA(None), } @@ -1082,6 +1085,7 @@ impl CheckCode { CheckCode::RUF001 => CheckCategory::Ruff, CheckCode::RUF002 => CheckCategory::Ruff, CheckCode::RUF003 => CheckCategory::Ruff, + CheckCode::RUF101 => CheckCategory::Ruff, CheckCode::M001 => CheckCategory::Meta, } } @@ -1313,6 +1317,7 @@ impl CheckKind { CheckKind::AmbiguousUnicodeCharacterString(..) => &CheckCode::RUF001, CheckKind::AmbiguousUnicodeCharacterDocstring(..) => &CheckCode::RUF002, CheckKind::AmbiguousUnicodeCharacterComment(..) => &CheckCode::RUF003, + CheckKind::ConvertExitToSysExit => &CheckCode::RUF101, // Meta CheckKind::UnusedNOQA(_) => &CheckCode::M001, } @@ -2002,6 +2007,9 @@ impl CheckKind { '{representant}'?)" ) } + CheckKind::ConvertExitToSysExit => "`exit()` is only available in the interpreter, \ + use `sys.exit()` instead" + .to_string(), // Meta CheckKind::UnusedNOQA(codes) => match codes { None => "Unused `noqa` directive".to_string(), @@ -2048,6 +2056,7 @@ impl CheckKind { self, CheckKind::AmbiguousUnicodeCharacterString(..) | CheckKind::AmbiguousUnicodeCharacterDocstring(..) + | CheckKind::ConvertExitToSysExit | CheckKind::BlankLineAfterLastSection(..) | CheckKind::BlankLineAfterSection(..) | CheckKind::BlankLineAfterSummary diff --git a/src/checks_gen.rs b/src/checks_gen.rs index c9b175a3058e6..259f74d03c5a5 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -267,6 +267,9 @@ pub enum CheckCodePrefix { RUF001, RUF002, RUF003, + RUF1, + RUF10, + RUF101, S, S1, S10, @@ -1058,12 +1061,20 @@ impl CheckCodePrefix { CheckCodePrefix::Q001 => vec![CheckCode::Q001], CheckCodePrefix::Q002 => vec![CheckCode::Q002], CheckCodePrefix::Q003 => vec![CheckCode::Q003], - CheckCodePrefix::RUF => vec![CheckCode::RUF001, CheckCode::RUF002, CheckCode::RUF003], + CheckCodePrefix::RUF => vec![ + CheckCode::RUF001, + CheckCode::RUF002, + CheckCode::RUF003, + CheckCode::RUF101, + ], CheckCodePrefix::RUF0 => vec![CheckCode::RUF001, CheckCode::RUF002, CheckCode::RUF003], CheckCodePrefix::RUF00 => vec![CheckCode::RUF001, CheckCode::RUF002, CheckCode::RUF003], CheckCodePrefix::RUF001 => vec![CheckCode::RUF001], CheckCodePrefix::RUF002 => vec![CheckCode::RUF002], CheckCodePrefix::RUF003 => vec![CheckCode::RUF003], + CheckCodePrefix::RUF1 => vec![CheckCode::RUF101], + CheckCodePrefix::RUF10 => vec![CheckCode::RUF101], + CheckCodePrefix::RUF101 => vec![CheckCode::RUF101], CheckCodePrefix::S => vec![ CheckCode::S101, CheckCode::S102, @@ -1471,6 +1482,9 @@ impl CheckCodePrefix { CheckCodePrefix::RUF001 => PrefixSpecificity::Explicit, CheckCodePrefix::RUF002 => PrefixSpecificity::Explicit, CheckCodePrefix::RUF003 => PrefixSpecificity::Explicit, + CheckCodePrefix::RUF1 => PrefixSpecificity::Hundreds, + CheckCodePrefix::RUF10 => PrefixSpecificity::Tens, + CheckCodePrefix::RUF101 => PrefixSpecificity::Explicit, CheckCodePrefix::S => PrefixSpecificity::Category, CheckCodePrefix::S1 => PrefixSpecificity::Hundreds, CheckCodePrefix::S10 => PrefixSpecificity::Tens, diff --git a/src/flake8_print/checks.rs b/src/flake8_print/checks.rs index 1fd6b98b1af78..00f371d95ec99 100644 --- a/src/flake8_print/checks.rs +++ b/src/flake8_print/checks.rs @@ -5,7 +5,6 @@ use crate::checks::{Check, CheckKind}; /// Check whether a function call is a `print` or `pprint` invocation pub fn print_call( - expr: &Expr, func: &Expr, check_print: bool, check_pprint: bool, @@ -13,7 +12,7 @@ pub fn print_call( ) -> Option { if let ExprKind::Name { id, .. } = &func.node { if check_print && id == "print" { - return Some(Check::new(CheckKind::PrintFound, Range::from_located(expr))); + return Some(Check::new(CheckKind::PrintFound, location)); } else if check_pprint && id == "pprint" { return Some(Check::new(CheckKind::PPrintFound, location)); } diff --git a/src/flake8_print/plugins/print_call.rs b/src/flake8_print/plugins/print_call.rs index 36f794eca4d17..e9f9cbda39584 100644 --- a/src/flake8_print/plugins/print_call.rs +++ b/src/flake8_print/plugins/print_call.rs @@ -10,7 +10,6 @@ use crate::flake8_print::checks; /// T201, T203 pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { if let Some(mut check) = checks::print_call( - expr, func, checker.settings.enabled.contains(&CheckCode::T201), checker.settings.enabled.contains(&CheckCode::T203), diff --git a/src/linter.rs b/src/linter.rs index 5d4646b30a87c..758dc73bc3390 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -526,6 +526,13 @@ mod tests { #[test_case(CheckCode::RUF001, Path::new("RUF001.py"); "RUF001")] #[test_case(CheckCode::RUF002, Path::new("RUF002.py"); "RUF002")] #[test_case(CheckCode::RUF003, Path::new("RUF003.py"); "RUF003")] + #[test_case(CheckCode::RUF101, Path::new("RUF101_0.py"); "RUF101_0")] + #[test_case(CheckCode::RUF101, Path::new("RUF101_1.py"); "RUF101_1")] + #[test_case(CheckCode::RUF101, Path::new("RUF101_2.py"); "RUF101_2")] + #[test_case(CheckCode::RUF101, Path::new("RUF101_3.py"); "RUF101_3")] + #[test_case(CheckCode::RUF101, Path::new("RUF101_4.py"); "RUF101_4")] + #[test_case(CheckCode::RUF101, Path::new("RUF101_5.py"); "RUF101_5")] + #[test_case(CheckCode::RUF101, Path::new("RUF101_6.py"); "RUF101_6")] #[test_case(CheckCode::YTT101, Path::new("YTT101.py"); "YTT101")] #[test_case(CheckCode::YTT102, Path::new("YTT102.py"); "YTT102")] #[test_case(CheckCode::YTT103, Path::new("YTT103.py"); "YTT103")] diff --git a/src/rules/mod.rs b/src/rules/mod.rs index d1dc200d73890..8a4c0e9efc60c 100644 --- a/src/rules/mod.rs +++ b/src/rules/mod.rs @@ -1,3 +1,4 @@ //! Module for Ruff-specific rules. pub mod checks; +pub mod plugins; diff --git a/src/rules/plugins/convert_exit_to_sys_exit.rs b/src/rules/plugins/convert_exit_to_sys_exit.rs new file mode 100644 index 0000000000000..5451e81556c27 --- /dev/null +++ b/src/rules/plugins/convert_exit_to_sys_exit.rs @@ -0,0 +1,92 @@ +use rustpython_ast::{Expr, ExprKind}; + +use crate::ast::types::{BindingKind, Range}; +use crate::autofix::Fix; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckKind}; + +/// Return `true` if the `module` was imported using a star import (e.g., `from +/// sys import *`). +fn is_module_star_imported(checker: &Checker, module: &str) -> bool { + checker.current_scopes().any(|scope| { + scope.values.values().any(|binding| { + if let BindingKind::StarImportation(_, name) = &binding.kind { + name.as_ref().map(|name| name == module).unwrap_or_default() + } else { + false + } + }) + }) +} + +/// Return `true` if `exit` is (still) bound as a built-in in the current scope. +fn has_builtin_exit_in_scope(checker: &Checker) -> bool { + !is_module_star_imported(checker, "sys") + && checker + .current_scopes() + .find_map(|scope| scope.values.get("exit")) + .map(|binding| matches!(binding.kind, BindingKind::Builtin)) + .unwrap_or_default() +} + +/// Return the appropriate `sys.exit` reference based on the current set of +/// imports, or `None` is `sys.exit` hasn't been imported. +fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) -> Option { + checker.current_scopes().find_map(|scope| { + scope + .values + .values() + .find_map(|binding| match &binding.kind { + // e.g. module=sys object=exit + // `import sys` -> `sys.exit` + // `import sys as sys2` -> `sys2.exit` + BindingKind::Importation(name, full_name, _) if full_name == module => { + Some(format!("{}.{}", name, member)) + } + // e.g. module=os.path object=join + // `from os.path import join` -> `join` + // `from os.path import join as join2` -> `join2` + BindingKind::FromImportation(name, full_name, _) + if full_name == &format!("{}.{}", module, member) => + { + Some(name.to_string()) + } + // e.g. module=os.path object=join + // `from os.path import *` -> `join` + BindingKind::StarImportation(_, name) + if name.as_ref().map(|name| name == module).unwrap_or_default() => + { + Some(member.to_string()) + } + // e.g. module=os.path object=join + // `import os.path ` -> `os.path.join` + BindingKind::SubmoduleImportation(_, full_name, _) if full_name == module => { + Some(format!("{}.{}", full_name, member)) + } + // Non-imports. + _ => None, + }) + }) +} + +/// RUF101 +pub fn convert_exit_to_sys_exit(checker: &mut Checker, func: &Expr) { + if let ExprKind::Name { id, .. } = &func.node { + if id == "exit" { + if has_builtin_exit_in_scope(checker) { + let mut check = + Check::new(CheckKind::ConvertExitToSysExit, Range::from_located(func)); + if checker.patch(check.kind.code()) { + if let Some(content) = get_member_import_name_alias(checker, "sys", "exit") { + check.amend(Fix::replacement( + content, + func.location, + func.end_location.unwrap(), + )) + } + } + checker.add_check(check); + } + } + } +} diff --git a/src/rules/plugins/mod.rs b/src/rules/plugins/mod.rs new file mode 100644 index 0000000000000..727d046cb6f7d --- /dev/null +++ b/src/rules/plugins/mod.rs @@ -0,0 +1,3 @@ +pub use convert_exit_to_sys_exit::convert_exit_to_sys_exit; + +mod convert_exit_to_sys_exit; diff --git a/src/snapshots/ruff__linter__tests__RUF101_RUF101_0.py.snap b/src/snapshots/ruff__linter__tests__RUF101_RUF101_0.py.snap new file mode 100644 index 0000000000000..06b637628078a --- /dev/null +++ b/src/snapshots/ruff__linter__tests__RUF101_RUF101_0.py.snap @@ -0,0 +1,21 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: ConvertExitToSysExit + location: + row: 1 + column: 0 + end_location: + row: 1 + column: 4 + fix: ~ +- kind: ConvertExitToSysExit + location: + row: 5 + column: 4 + end_location: + row: 5 + column: 8 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__RUF101_RUF101_1.py.snap b/src/snapshots/ruff__linter__tests__RUF101_RUF101_1.py.snap new file mode 100644 index 0000000000000..dbeecd8f4886a --- /dev/null +++ b/src/snapshots/ruff__linter__tests__RUF101_RUF101_1.py.snap @@ -0,0 +1,39 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: ConvertExitToSysExit + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 4 + fix: + patch: + content: sys.exit + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 4 + applied: false +- kind: ConvertExitToSysExit + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 8 + fix: + patch: + content: sys.exit + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 8 + applied: false + diff --git a/src/snapshots/ruff__linter__tests__RUF101_RUF101_2.py.snap b/src/snapshots/ruff__linter__tests__RUF101_RUF101_2.py.snap new file mode 100644 index 0000000000000..76371f26a3c30 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__RUF101_RUF101_2.py.snap @@ -0,0 +1,39 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: ConvertExitToSysExit + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 4 + fix: + patch: + content: sys2.exit + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 4 + applied: false +- kind: ConvertExitToSysExit + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 8 + fix: + patch: + content: sys2.exit + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 8 + applied: false + diff --git a/src/snapshots/ruff__linter__tests__RUF101_RUF101_3.py.snap b/src/snapshots/ruff__linter__tests__RUF101_RUF101_3.py.snap new file mode 100644 index 0000000000000..60c615f917981 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__RUF101_RUF101_3.py.snap @@ -0,0 +1,6 @@ +--- +source: src/linter.rs +expression: checks +--- +[] + diff --git a/src/snapshots/ruff__linter__tests__RUF101_RUF101_4.py.snap b/src/snapshots/ruff__linter__tests__RUF101_RUF101_4.py.snap new file mode 100644 index 0000000000000..1f0ad1f327a77 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__RUF101_RUF101_4.py.snap @@ -0,0 +1,39 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: ConvertExitToSysExit + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 4 + fix: + patch: + content: exit2 + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 4 + applied: false +- kind: ConvertExitToSysExit + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 8 + fix: + patch: + content: exit2 + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 8 + applied: false + diff --git a/src/snapshots/ruff__linter__tests__RUF101_RUF101_5.py.snap b/src/snapshots/ruff__linter__tests__RUF101_RUF101_5.py.snap new file mode 100644 index 0000000000000..60c615f917981 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__RUF101_RUF101_5.py.snap @@ -0,0 +1,6 @@ +--- +source: src/linter.rs +expression: checks +--- +[] + diff --git a/src/snapshots/ruff__linter__tests__RUF101_RUF101_6.py.snap b/src/snapshots/ruff__linter__tests__RUF101_RUF101_6.py.snap new file mode 100644 index 0000000000000..fa0b1c7c2156f --- /dev/null +++ b/src/snapshots/ruff__linter__tests__RUF101_RUF101_6.py.snap @@ -0,0 +1,13 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: ConvertExitToSysExit + location: + row: 1 + column: 0 + end_location: + row: 1 + column: 4 + fix: ~ +