Skip to content

Commit

Permalink
rule[flake8-import-conventions]: Add a rule for BannedImportAlias
Browse files Browse the repository at this point in the history
  • Loading branch information
Daniel committed Apr 10, 2023
1 parent 002caad commit 01ebca6
Show file tree
Hide file tree
Showing 12 changed files with 271 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import typing as t # unconventional
import typing as ty # unconventional

import numpy as nmp # unconventional
import tensorflow.keras.backend as K # unconventional
import torch.nn.functional as F # unconventional
from tensorflow.keras import backend as K # unconventional
from torch.nn import functional as F # unconventional

from typing import Any # conventional

import numpy as np # conventional
import tensorflow as tf # conventional
import torch.nn as nn # conventional
31 changes: 31 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,19 @@ where
}
}

if self.settings.rules.enabled(Rule::BannedImportAlias) {
if let Some(diagnostic) =
flake8_import_conventions::rules::check_banned_import(
stmt,
&alias.node.name,
alias.node.asname.as_deref(),
&self.settings.flake8_import_conventions.banned_aliases,
)
{
self.diagnostics.push(diagnostic);
}
}

if self
.settings
.rules
Expand Down Expand Up @@ -1339,6 +1352,24 @@ where
}
}

if self.settings.rules.enabled(Rule::BannedImportAlias) {
let full_name = helpers::format_import_from_member(
*level,
module.as_deref(),
&alias.node.name,
);
if let Some(diagnostic) =
flake8_import_conventions::rules::check_banned_import(
stmt,
&full_name,
alias.node.asname.as_deref(),
&self.settings.flake8_import_conventions.banned_aliases,
)
{
self.diagnostics.push(diagnostic);
}
}

if let Some(asname) = &alias.node.asname {
if self
.settings
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {

// flake8-import-conventions
(Flake8ImportConventions, "001") => Rule::UnconventionalImportAlias,
(Flake8ImportConventions, "002") => Rule::BannedImportAlias,

// flake8-datetimez
(Flake8Datetimez, "001") => Rule::CallDatetimeWithoutTzinfo,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ ruff_macros::register_rules!(
rules::flake8_unused_arguments::rules::UnusedLambdaArgument,
// flake8-import-conventions
rules::flake8_import_conventions::rules::UnconventionalImportAlias,
rules::flake8_import_conventions::rules::BannedImportAlias,
// flake8-datetimez
rules::flake8_datetimez::rules::CallDatetimeWithoutTzinfo,
rules::flake8_datetimez::rules::CallDatetimeToday,
Expand Down
28 changes: 28 additions & 0 deletions crates/ruff/src/rules/flake8_import_conventions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ mod tests {
("dask.array".to_string(), "da".to_string()),
("dask.dataframe".to_string(), "dd".to_string()),
])),
banned_aliases: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand All @@ -44,6 +45,30 @@ mod tests {
Ok(())
}

#[test]
fn custom_banned() -> Result<()> {
let diagnostics = test_path(
Path::new("flake8_import_conventions/custom_banned.py"),
&Settings {
flake8_import_conventions: super::settings::Options {
aliases: None,
extend_aliases: None,
banned_aliases: Some(FxHashMap::from_iter([
("typing".to_string(), "t".to_string()),
("typing".to_string(), "ty".to_string()),
("numpy".to_string(), "nmp".to_string()),
("tensorflow.keras.backend".to_string(), "K".to_string()),
("torch.nn.functional".to_string(), "F".to_string()),
])),
}
.into(),
..Settings::for_rule(Rule::BannedImportAlias)
},
)?;
assert_yaml_snapshot!("custom_banned", diagnostics);
Ok(())
}

#[test]
fn remove_defaults() -> Result<()> {
let diagnostics = test_path(
Expand All @@ -57,6 +82,7 @@ mod tests {
("seaborn".to_string(), "sns".to_string()),
])),
extend_aliases: None,
banned_aliases: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand All @@ -77,6 +103,7 @@ mod tests {
"numpy".to_string(),
"nmp".to_string(),
)])),
banned_aliases: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand All @@ -100,6 +127,7 @@ mod tests {
"pstr".to_string(),
),
])),
banned_aliases: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
use rustc_hash::FxHashMap;
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 imports that should not be using a non-standard convention,
/// like `import tensorflow.keras.backend as K`, and suggest avoiding such practice.
///
/// ## Why is this bad?
/// Consistency is good. Avoid using a non-standard convention for imports violating
/// PEP 8 principle to make your code more readable idiomatic.
///
/// For example, `import tensorflow.keras.backend as K` is an example of violating
/// PEP 8 principle, and users should typically avoid such imports in large codebases.
///
/// ## Example
/// ```python
/// import tensorflow.keras.backend as K
/// ```
///
/// Use instead, for example,:
/// ```python
/// import tensorflow as tf
/// ...
/// tf.keras.backend
/// ```
#[violation]
pub struct BannedImportAlias(pub String, pub String);

impl Violation for BannedImportAlias {
#[derive_message_formats]
fn message(&self) -> String {
let BannedImportAlias(name, asname) = self;
format!("`{name}` should not be imported as `{asname}`")
}
}

/// ICN002
pub fn check_banned_import(
import_from: &Stmt,
name: &str,
asname: Option<&str>,
banned_conventions: &FxHashMap<String, String>,
) -> Option<Diagnostic> {
let mut is_valid_import = true;
if let Some(expected_alias) = banned_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 {
return Some(Diagnostic::new(
BannedImportAlias(name.to_string(), expected_alias.to_string()),
Range::from(import_from),
));
}
}
None
}
5 changes: 5 additions & 0 deletions crates/ruff/src/rules/flake8_import_conventions/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub use check_banned_import::{check_banned_import, BannedImportAlias};
pub use check_conventional_import::{check_conventional_import, UnconventionalImportAlias};

mod check_banned_import;
mod check_conventional_import;
26 changes: 23 additions & 3 deletions crates/ruff/src/rules/flake8_import_conventions/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,23 @@ pub struct Options {
/// A mapping of modules to their conventional import aliases. These aliases
/// will be added to the `aliases` mapping.
pub extend_aliases: Option<FxHashMap<String, String>>,
#[option(
default = r#"{}"#,
value_type = "dict[str, str]",
example = r#"
[tool.ruff.flake8-import-conventions.banned-aliases]
# Declare the banned aliases.
"tensorflow.keras.backend" = "K"
"#
)]
/// A mapping of modules to their banned import aliases.
pub banned_aliases: Option<FxHashMap<String, String>>,
}

#[derive(Debug, CacheKey)]
pub struct Settings {
pub aliases: FxHashMap<String, String>,
pub banned_aliases: FxHashMap<String, String>,
}

fn default_aliases() -> FxHashMap<String, String> {
Expand All @@ -73,29 +85,36 @@ fn default_aliases() -> FxHashMap<String, String> {
.collect::<FxHashMap<_, _>>()
}

fn resolve_aliases(options: Options) -> FxHashMap<String, String> {
fn resolve_aliases(options: Options) -> (FxHashMap<String, String>, FxHashMap<String, String>) {
let mut aliases = match options.aliases {
Some(options_aliases) => options_aliases,
None => default_aliases(),
};
if let Some(extend_aliases) = options.extend_aliases {
aliases.extend(extend_aliases);
}
aliases
let banned_aliases = match options.banned_aliases {
Some(options_banned_aliases) => options_banned_aliases,
None => FxHashMap::default(),
};
(aliases, banned_aliases)
}

impl Default for Settings {
fn default() -> Self {
Self {
aliases: default_aliases(),
banned_aliases: FxHashMap::default(),
}
}
}

impl From<Options> for Settings {
fn from(options: Options) -> Self {
let (aliases, banned_aliases) = resolve_aliases(options);
Self {
aliases: resolve_aliases(options),
aliases,
banned_aliases,
}
}
}
Expand All @@ -105,6 +124,7 @@ impl From<Settings> for Options {
Self {
aliases: Some(settings.aliases),
extend_aliases: None,
banned_aliases: None,
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
---
source: crates/ruff/src/rules/flake8_import_conventions/mod.rs
expression: diagnostics
---
- kind:
name: BannedImportAlias
body: "`typing` should not be imported as `ty`"
suggestion: ~
fixable: false
location:
row: 2
column: 0
end_location:
row: 2
column: 19
fix:
edits: []
parent: ~
- kind:
name: BannedImportAlias
body: "`numpy` should not be imported as `nmp`"
suggestion: ~
fixable: false
location:
row: 4
column: 0
end_location:
row: 4
column: 19
fix:
edits: []
parent: ~
- kind:
name: BannedImportAlias
body: "`tensorflow.keras.backend` should not be imported as `K`"
suggestion: ~
fixable: false
location:
row: 5
column: 0
end_location:
row: 5
column: 36
fix:
edits: []
parent: ~
- kind:
name: BannedImportAlias
body: "`torch.nn.functional` should not be imported as `F`"
suggestion: ~
fixable: false
location:
row: 6
column: 0
end_location:
row: 6
column: 31
fix:
edits: []
parent: ~
- kind:
name: BannedImportAlias
body: "`tensorflow.keras.backend` should not be imported as `K`"
suggestion: ~
fixable: false
location:
row: 7
column: 0
end_location:
row: 7
column: 41
fix:
edits: []
parent: ~
- kind:
name: BannedImportAlias
body: "`torch.nn.functional` should not be imported as `F`"
suggestion: ~
fixable: false
location:
row: 8
column: 0
end_location:
row: 8
column: 36
fix:
edits: []
parent: ~

1 change: 1 addition & 0 deletions crates/ruff/src/settings/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ other-attribute = 1
"dask.dataframe".to_string(),
"dd".to_string(),
)])),
banned_aliases: None,
}),
mccabe: Some(mccabe::settings::Options {
max_complexity: Some(10),
Expand Down
Loading

0 comments on commit 01ebca6

Please sign in to comment.