Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement consider-using-from-import #1018

Merged
merged 1 commit into from
Dec 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 26 additions & 0 deletions resources/test/fixtures/pylint/import_aliasing.py
Original file line number Diff line number Diff line change
@@ -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]
5 changes: 5 additions & 0 deletions src/check_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 12 additions & 2 deletions src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,11 @@ pub enum CheckCode {
F841,
F901,
// pylint
PLR1701,
PLC3002,
PLR0206,
PLE1142,
PLR0206,
PLR0402,
PLR1701,
// flake8-builtins
A001,
A002,
Expand Down Expand Up @@ -576,6 +577,7 @@ pub enum CheckKind {
ConsiderMergingIsinstance(String, Vec<String>),
UnnecessaryDirectLambdaCall,
PropertyWithParameters,
ConsiderUsingFromImport(String, String),
AwaitOutsideAsync,
// flake8-builtins
BuiltinVariableShadowing(String),
Expand Down Expand Up @@ -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()])
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
}
Expand Down
15 changes: 13 additions & 2 deletions src/checks_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,9 @@ pub enum CheckCodePrefix {
PLR02,
PLR020,
PLR0206,
PLR04,
PLR040,
PLR0402,
PLR1,
PLR17,
PLR170,
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
16 changes: 15 additions & 1 deletion src/pylint/plugins.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down
27 changes: 27 additions & 0 deletions src/pylint/snapshots/ruff__pylint__tests__import_aliasing.py.snap
Original file line number Diff line number Diff line change
@@ -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: ~