From f9f3115f52126ab1c9d1d861887b5584d3091f44 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 4 Dec 2022 12:35:08 +0900 Subject: [PATCH] Implement consider-using-from-import --- README.md | 5 ++-- .../test/fixtures/pylint/import_aliasing.py | 26 ++++++++++++++++++ src/check_ast.rs | 5 ++++ src/checks.rs | 14 ++++++++-- src/checks_gen.rs | 15 +++++++++-- src/pylint/mod.rs | 1 + src/pylint/plugins.rs | 16 ++++++++++- ...ff__pylint__tests__import_aliasing.py.snap | 27 +++++++++++++++++++ 8 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 resources/test/fixtures/pylint/import_aliasing.py create mode 100644 src/pylint/snapshots/ruff__pylint__tests__import_aliasing.py.snap diff --git a/README.md b/README.md index ffadb5eeca9ad..207a38842ea3c 100644 --- a/README.md +++ b/README.md @@ -767,10 +767,11 @@ For more, see [Pylint](https://pypi.org/project/pylint/2.15.7/) on PyPI. | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | -| PLR1701 | ConsiderMergingIsinstance | Consider merging these isinstance calls: `isinstance(..., (...))` | | | PLC3002 | UnnecessaryDirectLambdaCall | Lambda expression called directly. Execute the expression inline instead. | | -| PLR0206 | PropertyWithParameters | Cannot have defined parameters for properties | | | PLE1142 | AwaitOutsideAsync | `await` should be used within an async function | | +| PLR0206 | PropertyWithParameters | Cannot have defined parameters for properties | | +| PLR0402 | ConsiderUsingFromImport | Consider using `from ... import ...` | | +| PLR1701 | ConsiderMergingIsinstance | Consider merging these isinstance calls: `isinstance(..., (...))` | | ### Ruff-specific rules diff --git a/resources/test/fixtures/pylint/import_aliasing.py b/resources/test/fixtures/pylint/import_aliasing.py new file mode 100644 index 0000000000000..08194fe1b5fb0 --- /dev/null +++ b/resources/test/fixtures/pylint/import_aliasing.py @@ -0,0 +1,26 @@ +# pylint: disable=unused-import, missing-docstring, invalid-name, reimported, import-error, wrong-import-order, no-name-in-module, shadowed-import +# Functional tests for import aliasing +# 1. useless-import-alias +# 2. consider-using-from-import + +from collections import OrderedDict as OrderedDict # [useless-import-alias] +from collections import OrderedDict as o_dict +import os.path as path # [consider-using-from-import] +import os.path as p +import foo.bar.foobar as foobar # [consider-using-from-import] +import os +import os as OS +from sys import version +from . import bar as bar # [useless-import-alias] +from . import bar as Bar +from . import bar +from ..foo import bar as bar # [useless-import-alias] +from ..foo.bar import foobar as foobar # [useless-import-alias] +from ..foo.bar import foobar as anotherfoobar +from . import foo as foo, foo2 as bar2 # [useless-import-alias] +from . import foo as bar, foo2 as foo2 # [useless-import-alias] +from . import foo as bar, foo2 as bar2 +from foo.bar import foobar as foobar # [useless-import-alias] +from foo.bar import foobar as foo +from .foo.bar import f as foobar +from ............a import b # [relative-beyond-top-level] diff --git a/src/check_ast.rs b/src/check_ast.rs index 9d7015a6a05ed..13a57788ec4fc 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -668,6 +668,11 @@ where } } + // pylint + if self.settings.enabled.contains(&CheckCode::PLR0402) { + pylint::plugins::consider_using_from_import(self, alias); + } + if let Some(asname) = &alias.node.asname { for alias in names { if let Some(asname) = &alias.node.asname { diff --git a/src/checks.rs b/src/checks.rs index 4a8b753ad89be..130c44f1576ef 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -91,10 +91,11 @@ pub enum CheckCode { F841, F901, // pylint - PLR1701, PLC3002, - PLR0206, PLE1142, + PLR0206, + PLR0402, + PLR1701, // flake8-builtins A001, A002, @@ -576,6 +577,7 @@ pub enum CheckKind { ConsiderMergingIsinstance(String, Vec), UnnecessaryDirectLambdaCall, PropertyWithParameters, + ConsiderUsingFromImport(String, String), AwaitOutsideAsync, // flake8-builtins BuiltinVariableShadowing(String), @@ -872,6 +874,9 @@ impl CheckCode { // pylint CheckCode::PLC3002 => CheckKind::UnnecessaryDirectLambdaCall, CheckCode::PLE1142 => CheckKind::AwaitOutsideAsync, + CheckCode::PLR0402 => { + CheckKind::ConsiderUsingFromImport("...".to_string(), "...".to_string()) + } CheckCode::PLR0206 => CheckKind::PropertyWithParameters, CheckCode::PLR1701 => { CheckKind::ConsiderMergingIsinstance("...".to_string(), vec!["...".to_string()]) @@ -1299,6 +1304,7 @@ impl CheckCode { CheckCode::PLC3002 => CheckCategory::Pylint, CheckCode::PLE1142 => CheckCategory::Pylint, CheckCode::PLR0206 => CheckCategory::Pylint, + CheckCode::PLR0402 => CheckCategory::Pylint, CheckCode::PLR1701 => CheckCategory::Pylint, CheckCode::Q000 => CheckCategory::Flake8Quotes, CheckCode::Q001 => CheckCategory::Flake8Quotes, @@ -1423,6 +1429,7 @@ impl CheckKind { CheckKind::AwaitOutsideAsync => &CheckCode::PLE1142, CheckKind::ConsiderMergingIsinstance(..) => &CheckCode::PLR1701, CheckKind::PropertyWithParameters => &CheckCode::PLR0206, + CheckKind::ConsiderUsingFromImport(..) => &CheckCode::PLR0402, CheckKind::UnnecessaryDirectLambdaCall => &CheckCode::PLC3002, // flake8-builtins CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001, @@ -1815,6 +1822,9 @@ impl CheckKind { CheckKind::PropertyWithParameters => { "Cannot have defined parameters for properties".to_string() } + CheckKind::ConsiderUsingFromImport(module, name) => { + format!("Consider using `from {module} import {name}`") + } CheckKind::AwaitOutsideAsync => { "`await` should be used within an async function".to_string() } diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 9fddc6735ceaa..acad1e7b62e66 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -297,6 +297,9 @@ pub enum CheckCodePrefix { PLR02, PLR020, PLR0206, + PLR04, + PLR040, + PLR0402, PLR1, PLR17, PLR170, @@ -1191,11 +1194,16 @@ impl CheckCodePrefix { CheckCodePrefix::PLE11 => vec![CheckCode::PLE1142], CheckCodePrefix::PLE114 => vec![CheckCode::PLE1142], CheckCodePrefix::PLE1142 => vec![CheckCode::PLE1142], - CheckCodePrefix::PLR => vec![CheckCode::PLR1701, CheckCode::PLR0206], - CheckCodePrefix::PLR0 => vec![CheckCode::PLR0206], + CheckCodePrefix::PLR => { + vec![CheckCode::PLR0206, CheckCode::PLR0402, CheckCode::PLR1701] + } + CheckCodePrefix::PLR0 => vec![CheckCode::PLR0206, CheckCode::PLR0402], CheckCodePrefix::PLR02 => vec![CheckCode::PLR0206], CheckCodePrefix::PLR020 => vec![CheckCode::PLR0206], CheckCodePrefix::PLR0206 => vec![CheckCode::PLR0206], + CheckCodePrefix::PLR04 => vec![CheckCode::PLR0402], + CheckCodePrefix::PLR040 => vec![CheckCode::PLR0402], + CheckCodePrefix::PLR0402 => vec![CheckCode::PLR0402], CheckCodePrefix::PLR1 => vec![CheckCode::PLR1701], CheckCodePrefix::PLR17 => vec![CheckCode::PLR1701], CheckCodePrefix::PLR170 => vec![CheckCode::PLR1701], @@ -1719,6 +1727,9 @@ impl CheckCodePrefix { CheckCodePrefix::PLR02 => SuffixLength::Two, CheckCodePrefix::PLR020 => SuffixLength::Three, CheckCodePrefix::PLR0206 => SuffixLength::Four, + CheckCodePrefix::PLR04 => SuffixLength::Two, + CheckCodePrefix::PLR040 => SuffixLength::Three, + CheckCodePrefix::PLR0402 => SuffixLength::Four, CheckCodePrefix::PLR1 => SuffixLength::One, CheckCodePrefix::PLR17 => SuffixLength::Two, CheckCodePrefix::PLR170 => SuffixLength::Three, diff --git a/src/pylint/mod.rs b/src/pylint/mod.rs index f95ed6e6d20ef..1df73acfd3736 100644 --- a/src/pylint/mod.rs +++ b/src/pylint/mod.rs @@ -14,6 +14,7 @@ mod tests { #[test_case(CheckCode::PLC3002, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")] #[test_case(CheckCode::PLE1142, Path::new("await_outside_async.py"); "PLE1142")] #[test_case(CheckCode::PLR0206, Path::new("property_with_parameters.py"); "PLR0206")] + #[test_case(CheckCode::PLR0402, Path::new("import_aliasing.py"); "PLR0402")] #[test_case(CheckCode::PLR1701, Path::new("consider_merging_isinstance.py"); "PLR1701")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}", path.to_string_lossy()); diff --git a/src/pylint/plugins.rs b/src/pylint/plugins.rs index 4b0aa2891e592..6acbeaa6c62bd 100644 --- a/src/pylint/plugins.rs +++ b/src/pylint/plugins.rs @@ -1,6 +1,6 @@ use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; -use rustpython_ast::{Arguments, Boolop, Expr, ExprKind, Stmt}; +use rustpython_ast::{Alias, Arguments, Boolop, Expr, ExprKind, Stmt}; use crate::ast::types::{FunctionScope, Range, ScopeKind}; use crate::check_ast::Checker; @@ -65,6 +65,20 @@ pub fn property_with_parameters( } } +/// PLR0402 +pub fn consider_using_from_import(checker: &mut Checker, alias: &Alias) { + if let Some(asname) = &alias.node.asname { + if let Some((_, module)) = alias.node.name.rsplit_once('.') { + if module == asname { + checker.add_check(Check::new( + CheckKind::ConsiderUsingFromImport(module.to_string(), asname.to_string()), + Range::from_located(alias), + )); + } + } + } +} + /// PLR1701 pub fn consider_merging_isinstance( checker: &mut Checker, diff --git a/src/pylint/snapshots/ruff__pylint__tests__import_aliasing.py.snap b/src/pylint/snapshots/ruff__pylint__tests__import_aliasing.py.snap new file mode 100644 index 0000000000000..3d7dc6288b746 --- /dev/null +++ b/src/pylint/snapshots/ruff__pylint__tests__import_aliasing.py.snap @@ -0,0 +1,27 @@ +--- +source: src/pylint/mod.rs +expression: checks +--- +- kind: + ConsiderUsingFromImport: + - path + - path + location: + row: 8 + column: 7 + end_location: + row: 8 + column: 22 + fix: ~ +- kind: + ConsiderUsingFromImport: + - foobar + - foobar + location: + row: 10 + column: 7 + end_location: + row: 10 + column: 31 + fix: ~ +