Misc. small clean-up of `flake8-import-conventions` rules (#4069)

This commit is contained in:
Charlie Marsh 2023-04-22 22:57:15 -06:00 committed by GitHub
parent cfc7d8a2b5
commit 0e7914010f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 99 additions and 72 deletions

View File

@ -1057,7 +1057,7 @@ where
if self.settings.rules.enabled(Rule::UnconventionalImportAlias) {
if let Some(diagnostic) =
flake8_import_conventions::rules::check_conventional_import(
flake8_import_conventions::rules::conventional_import_alias(
stmt,
&alias.node.name,
alias.node.asname.as_deref(),
@ -1071,7 +1071,7 @@ where
if self.settings.rules.enabled(Rule::BannedImportAlias) {
if let Some(asname) = &alias.node.asname {
if let Some(diagnostic) =
flake8_import_conventions::rules::check_banned_import(
flake8_import_conventions::rules::banned_import_alias(
stmt,
&alias.node.name,
asname,
@ -1332,7 +1332,7 @@ where
&alias.node.name,
);
if let Some(diagnostic) =
flake8_import_conventions::rules::check_conventional_import(
flake8_import_conventions::rules::conventional_import_alias(
stmt,
&full_name,
alias.node.asname.as_deref(),
@ -1351,7 +1351,7 @@ where
&alias.node.name,
);
if let Some(diagnostic) =
flake8_import_conventions::rules::check_banned_import(
flake8_import_conventions::rules::banned_import_alias(
stmt,
&full_name,
asname,
@ -1459,13 +1459,11 @@ where
}
if self.settings.rules.enabled(Rule::BannedImportFrom) {
if let Some(diagnostic) =
flake8_import_conventions::rules::check_banned_import_from(
stmt,
&helpers::format_import_from(*level, module.as_deref()),
&self.settings.flake8_import_conventions.banned_from,
)
{
if let Some(diagnostic) = flake8_import_conventions::rules::banned_import_from(
stmt,
&helpers::format_import_from(*level, module.as_deref()),
&self.settings.flake8_import_conventions.banned_from,
) {
self.diagnostics.push(diagnostic);
}
}

View File

@ -28,19 +28,22 @@ use ruff_python_ast::types::Range;
/// tf.keras.backend
/// ```
#[violation]
pub struct BannedImportAlias(pub String, pub String);
pub struct BannedImportAlias {
pub name: String,
pub asname: String,
}
impl Violation for BannedImportAlias {
#[derive_message_formats]
fn message(&self) -> String {
let BannedImportAlias(name, asname) = self;
let BannedImportAlias { name, asname } = self;
format!("`{name}` should not be imported as `{asname}`")
}
}
/// ICN002
pub fn check_banned_import(
import_from: &Stmt,
pub fn banned_import_alias(
stmt: &Stmt,
name: &str,
asname: &str,
banned_conventions: &FxHashMap<String, Vec<String>>,
@ -51,8 +54,11 @@ pub fn check_banned_import(
.any(|banned_alias| banned_alias == asname)
{
return Some(Diagnostic::new(
BannedImportAlias(name.to_string(), asname.to_string()),
Range::from(import_from),
BannedImportAlias {
name: name.to_string(),
asname: asname.to_string(),
},
Range::from(stmt),
));
}
}

View File

@ -0,0 +1,59 @@
use rustc_hash::FxHashSet;
use rustpython_parser::ast::Stmt;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;
/// ## What it does
/// Checks for member imports that should instead be accessed by importing the
/// module.
///
/// ## Why is this bad?
/// Consistency is good. Use a common convention for imports to make your code
/// more readable and idiomatic.
///
/// For example, it's common to import `pandas` as `pd`, and then access
/// members like `Series` via `pd.Series`, rather than importing `Series`
/// directly.
///
/// ## Example
/// ```python
/// from pandas import Series
/// ```
///
/// Use instead:
/// ```python
/// import pandas as pd
///
/// pd.Series
/// ```
#[violation]
pub struct BannedImportFrom {
pub name: String,
}
impl Violation for BannedImportFrom {
#[derive_message_formats]
fn message(&self) -> String {
let BannedImportFrom { name } = self;
format!("Members of `{name}` should not be imported explicitly")
}
}
/// ICN003
pub fn banned_import_from(
stmt: &Stmt,
name: &str,
banned_conventions: &FxHashSet<String>,
) -> Option<Diagnostic> {
if banned_conventions.contains(name) {
return Some(Diagnostic::new(
BannedImportFrom {
name: name.to_string(),
},
Range::from(stmt),
));
}
None
}

View File

@ -1,32 +0,0 @@
use rustc_hash::FxHashSet;
use rustpython_parser::ast::Stmt;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;
#[violation]
pub struct BannedImportFrom(pub String);
impl Violation for BannedImportFrom {
#[derive_message_formats]
fn message(&self) -> String {
let BannedImportFrom(name) = self;
format!("Members of `{name}` should not be imported explicitly")
}
}
/// ICN003
pub fn check_banned_import_from(
import_from: &Stmt,
name: &str,
banned_conventions: &FxHashSet<String>,
) -> Option<Diagnostic> {
if banned_conventions.contains(name) {
return Some(Diagnostic::new(
BannedImportFrom(name.to_string()),
Range::from(import_from),
));
}
None
}

View File

@ -27,38 +27,34 @@ use ruff_python_ast::types::Range;
/// import pandas as pd
/// ```
#[violation]
pub struct UnconventionalImportAlias(pub String, pub String);
pub struct UnconventionalImportAlias {
pub name: String,
pub asname: String,
}
impl Violation for UnconventionalImportAlias {
#[derive_message_formats]
fn message(&self) -> String {
let UnconventionalImportAlias(name, asname) = self;
let UnconventionalImportAlias { name, asname } = self;
format!("`{name}` should be imported as `{asname}`")
}
}
/// ICN001
pub fn check_conventional_import(
import_from: &Stmt,
pub fn conventional_import_alias(
stmt: &Stmt,
name: &str,
asname: Option<&str>,
conventions: &FxHashMap<String, String>,
) -> Option<Diagnostic> {
let mut is_valid_import = true;
if let Some(expected_alias) = conventions.get(name) {
if !expected_alias.is_empty() {
if let Some(alias) = asname {
if expected_alias != alias {
is_valid_import = false;
}
} else {
is_valid_import = false;
}
}
if !is_valid_import {
if asname != Some(expected_alias) {
return Some(Diagnostic::new(
UnconventionalImportAlias(name.to_string(), expected_alias.to_string()),
Range::from(import_from),
UnconventionalImportAlias {
name: name.to_string(),
asname: expected_alias.to_string(),
},
Range::from(stmt),
));
}
}

View File

@ -1,7 +1,7 @@
pub use check_banned_import::{check_banned_import, BannedImportAlias};
pub use check_banned_import_from::{check_banned_import_from, BannedImportFrom};
pub use check_conventional_import::{check_conventional_import, UnconventionalImportAlias};
pub use banned_import_alias::{banned_import_alias, BannedImportAlias};
pub use banned_import_from::{banned_import_from, BannedImportFrom};
pub use conventional_import_alias::{conventional_import_alias, UnconventionalImportAlias};
mod check_banned_import;
mod check_banned_import_from;
mod check_conventional_import;
mod banned_import_alias;
mod banned_import_from;
mod conventional_import_alias;