Skip to content

Commit

Permalink
[flake8-import-conventions] Implement new rule ICN003 to ban `fro…
Browse files Browse the repository at this point in the history
…m ... import ...` for selected modules (#4040)
  • Loading branch information
edgarrmondragon authored Apr 23, 2023
1 parent f5cd659 commit cfc7d8a
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from logging.config import BaseConfigurator # banned
from typing import Any, Dict # banned
from typing import * # banned

from pandas import DataFrame # banned
from pandas import * # banned

import logging.config # ok
import typing # ok
import pandas # ok
12 changes: 12 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1457,6 +1457,18 @@ 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,
)
{
self.diagnostics.push(diagnostic);
}
}
}
StmtKind::Raise { exc, .. } => {
if self.settings.rules.enabled(Rule::RaiseNotImplemented) {
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 @@ -534,6 +534,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
// flake8-import-conventions
(Flake8ImportConventions, "001") => Rule::UnconventionalImportAlias,
(Flake8ImportConventions, "002") => Rule::BannedImportAlias,
(Flake8ImportConventions, "003") => Rule::BannedImportFrom,

// 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 @@ -491,6 +491,7 @@ ruff_macros::register_rules!(
// flake8-import-conventions
rules::flake8_import_conventions::rules::UnconventionalImportAlias,
rules::flake8_import_conventions::rules::BannedImportAlias,
rules::flake8_import_conventions::rules::BannedImportFrom,
// flake8-datetimez
rules::flake8_datetimez::rules::CallDatetimeWithoutTzinfo,
rules::flake8_datetimez::rules::CallDatetimeToday,
Expand Down
30 changes: 29 additions & 1 deletion crates/ruff/src/rules/flake8_import_conventions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mod tests {
use crate::assert_messages;
use anyhow::Result;

use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};

use crate::registry::Rule;
use crate::settings::Settings;
Expand Down Expand Up @@ -37,6 +37,7 @@ mod tests {
("dask.dataframe".to_string(), "dd".to_string()),
])),
banned_aliases: None,
banned_from: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand Down Expand Up @@ -69,6 +70,7 @@ mod tests {
),
("torch.nn.functional".to_string(), vec!["F".to_string()]),
])),
banned_from: None,
}
.into(),
..Settings::for_rule(Rule::BannedImportAlias)
Expand All @@ -78,6 +80,29 @@ mod tests {
Ok(())
}

#[test]
fn custom_banned_from() -> Result<()> {
let diagnostics = test_path(
Path::new("flake8_import_conventions/custom_banned_from.py"),
&Settings {
flake8_import_conventions: super::settings::Options {
aliases: None,
extend_aliases: None,
banned_aliases: None,
banned_from: Some(FxHashSet::from_iter([
"logging.config".to_string(),
"typing".to_string(),
"pandas".to_string(),
])),
}
.into(),
..Settings::for_rule(Rule::BannedImportFrom)
},
)?;
assert_messages!("custom_banned_from", diagnostics);
Ok(())
}

#[test]
fn remove_defaults() -> Result<()> {
let diagnostics = test_path(
Expand All @@ -92,6 +117,7 @@ mod tests {
])),
extend_aliases: None,
banned_aliases: None,
banned_from: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand All @@ -113,6 +139,7 @@ mod tests {
"nmp".to_string(),
)])),
banned_aliases: None,
banned_from: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand All @@ -137,6 +164,7 @@ mod tests {
),
])),
banned_aliases: None,
banned_from: None,
}
.into(),
..Settings::for_rule(Rule::UnconventionalImportAlias)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
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
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_import_conventions/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +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};

mod check_banned_import;
mod check_banned_import_from;
mod check_conventional_import;
44 changes: 24 additions & 20 deletions crates/ruff/src/rules/flake8_import_conventions/settings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Settings for import conventions.
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -70,12 +70,23 @@ pub struct Options {
)]
/// A mapping from module to its banned import aliases.
pub banned_aliases: Option<FxHashMap<String, Vec<String>>>,
#[option(
default = r#"[]"#,
value_type = "list[str]",
example = r#"
# Declare the banned `from` imports.
banned-from = ["typing"]
"#
)]
/// A list of modules that are allowed to be imported from
pub banned_from: Option<FxHashSet<String>>,
}

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

fn default_aliases() -> FxHashMap<String, String> {
Expand All @@ -85,38 +96,30 @@ fn default_aliases() -> FxHashMap<String, String> {
.collect::<FxHashMap<_, _>>()
}

fn resolve_aliases(
options: Options,
) -> (FxHashMap<String, String>, FxHashMap<String, Vec<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);
}
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(),
banned_from: FxHashSet::default(),
}
}
}

impl From<Options> for Settings {
fn from(options: Options) -> Self {
let (aliases, banned_aliases) = resolve_aliases(options);
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);
}

Self {
aliases,
banned_aliases,
banned_aliases: options.banned_aliases.unwrap_or_default(),
banned_from: options.banned_from.unwrap_or_default(),
}
}
}
Expand All @@ -127,6 +130,7 @@ impl From<Settings> for Options {
aliases: Some(settings.aliases),
extend_aliases: None,
banned_aliases: None,
banned_from: None,
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
source: crates/ruff/src/rules/flake8_import_conventions/mod.rs
---
custom_banned_from.py:1:1: ICN003 Members of `logging.config` should not be imported explicitly
|
1 | from logging.config import BaseConfigurator # banned
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ICN003
2 | from typing import Any, Dict # banned
3 | from typing import * # banned
|

custom_banned_from.py:2:1: ICN003 Members of `typing` should not be imported explicitly
|
2 | from logging.config import BaseConfigurator # banned
3 | from typing import Any, Dict # banned
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ICN003
4 | from typing import * # banned
|

custom_banned_from.py:3:1: ICN003 Members of `typing` should not be imported explicitly
|
3 | from logging.config import BaseConfigurator # banned
4 | from typing import Any, Dict # banned
5 | from typing import * # banned
| ^^^^^^^^^^^^^^^^^^^^ ICN003
6 |
7 | from pandas import DataFrame # banned
|

custom_banned_from.py:5:1: ICN003 Members of `pandas` should not be imported explicitly
|
5 | from typing import * # banned
6 |
7 | from pandas import DataFrame # banned
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ICN003
8 | from pandas import * # banned
|

custom_banned_from.py:6:1: ICN003 Members of `pandas` should not be imported explicitly
|
6 | from pandas import DataFrame # banned
7 | from pandas import * # banned
| ^^^^^^^^^^^^^^^^^^^^ ICN003
8 |
9 | import logging.config # ok
|


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

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit cfc7d8a

Please sign in to comment.