Skip to content

Commit

Permalink
Implement TID251 (banning modules & module members) (astral-sh#1436)
Browse files Browse the repository at this point in the history
  • Loading branch information
not-my-profile authored Dec 30, 2022
1 parent bde12c3 commit 9d34da2
Show file tree
Hide file tree
Showing 14 changed files with 514 additions and 10 deletions.
22 changes: 22 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,7 @@ For more, see [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| TID251 | BannedApi | `...` is banned: ... | |
| TID252 | BannedRelativeImport | Relative imports are banned | |
### flake8-unused-arguments (ARG)
Expand Down Expand Up @@ -2481,6 +2482,27 @@ ban-relative-imports = "all"
---
#### [`banned-api`](#banned-api)
Specific modules or module members that may not be imported or accessed.
Note that this check is only meant to flag accidental uses,
and can be circumvented via `eval` or `importlib`.
**Default value**: `{}`
**Type**: `HashMap<String, BannedApi>`
**Example usage**:
```toml
[tool.ruff.flake8-tidy-imports]
[tool.ruff.flake8-tidy-imports.banned-api]
"cgi".msg = "The cgi module is deprecated, see https://peps.python.org/pep-0594/#cgi."
"typing.TypedDict".msg = "Use typing_extensions.TypedDict instead."
```
---
### `flake8-unused-arguments`
#### [`ignore-variadic-names`](#ignore-variadic-names)
Expand Down
33 changes: 33 additions & 0 deletions resources/test/fixtures/flake8_tidy_imports/TID251.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
## Banned modules ##
import cgi

from cgi import *

from cgi import a, b, c

# banning a module also bans any submodules
import cgi.foo.bar

from cgi.foo import bar

from cgi.foo.bar import *

## Banned module members ##

from typing import TypedDict

import typing

# attribute access is checked
typing.TypedDict

typing.TypedDict.anything

# function calls are checked
typing.TypedDict()

typing.TypedDict.anything()

# import aliases are resolved
import typing as totally_not_typing
totally_not_typing.TypedDict
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# module members cannot be imported with that syntax
import typing.TypedDict

# we don't track reassignments
import typing, other
typing = other
typing.TypedDict()

# yet another false positive
def foo(typing):
typing.TypedDict()
4 changes: 4 additions & 0 deletions resources/test/fixtures/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ staticmethod-decorators = ["staticmethod"]
[tool.ruff.flake8-tidy-imports]
ban-relative-imports = "parents"

[tool.ruff.flake8-tidy-imports.banned-api]
"cgi".msg = "The cgi module is deprecated."
"typing.TypedDict".msg = "Use typing_extensions.TypedDict instead."

[tool.ruff.flake8-errmsg]
max-string-length = 20

Expand Down
24 changes: 24 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,19 @@
},
"additionalProperties": false,
"definitions": {
"BannedApi": {
"type": "object",
"required": [
"msg"
],
"properties": {
"msg": {
"description": "The message to display when the API is used.",
"type": "string"
}
},
"additionalProperties": false
},
"CheckCodePrefix": {
"type": "string",
"enum": [
Expand Down Expand Up @@ -842,6 +855,7 @@
"TID",
"TID2",
"TID25",
"TID251",
"TID252",
"U",
"U0",
Expand Down Expand Up @@ -1075,6 +1089,9 @@
},
"Flake8TidyImportsOptions": {
"type": "object",
"required": [
"banned-api"
],
"properties": {
"ban-relative-imports": {
"description": "Whether to ban all relative imports (`\"all\"`), or only those imports that extend into the parent module or beyond (`\"parents\"`).",
Expand All @@ -1086,6 +1103,13 @@
"type": "null"
}
]
},
"banned-api": {
"description": "Specific modules or module members that may not be imported or accessed. Note that this check is only meant to flag accidental uses, and can be circumvented via `eval` or `importlib`.",
"type": "object",
"additionalProperties": {
"$ref": "#/definitions/BannedApi"
}
}
},
"additionalProperties": false
Expand Down
41 changes: 41 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,17 @@ where
}
}

// flake8_tidy_imports
if self.settings.enabled.contains(&CheckCode::TID251) {
if let Some(check) = flake8_tidy_imports::checks::name_or_parent_is_banned(
alias,
&alias.node.name,
&self.settings.flake8_tidy_imports.banned_api,
) {
self.add_check(check);
}
}

// pylint
if self.settings.enabled.contains(&CheckCode::PLC0414) {
pylint::plugins::useless_import_alias(self, alias);
Expand Down Expand Up @@ -854,6 +865,27 @@ where
}
}

if self.settings.enabled.contains(&CheckCode::TID251) {
if let Some(module) = module {
for name in names {
if let Some(check) = flake8_tidy_imports::checks::name_is_banned(
module,
name,
&self.settings.flake8_tidy_imports.banned_api,
) {
self.add_check(check);
}
}
if let Some(check) = flake8_tidy_imports::checks::name_or_parent_is_banned(
stmt,
module,
&self.settings.flake8_tidy_imports.banned_api,
) {
self.add_check(check);
}
}
}

for alias in names {
if let Some("__future__") = module.as_deref() {
let name = alias.node.asname.as_ref().unwrap_or(&alias.node.name);
Expand Down Expand Up @@ -1586,6 +1618,15 @@ where
};
}
}

if self.settings.enabled.contains(&CheckCode::TID251) {
flake8_tidy_imports::checks::banned_attribute_access(
self,
&dealias_call_path(collect_call_paths(expr), &self.import_aliases),
expr,
&self.settings.flake8_tidy_imports.banned_api,
);
}
}
ExprKind::Call {
func,
Expand Down
9 changes: 9 additions & 0 deletions src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ pub enum CheckCode {
// mccabe
C901,
// flake8-tidy-imports
TID251,
TID252,
// flake8-return
RET501,
Expand Down Expand Up @@ -784,6 +785,7 @@ pub enum CheckKind {
// flake8-debugger
Debugger(DebuggerUsingType),
// flake8-tidy-imports
BannedApi { name: String, message: String },
BannedRelativeImport(Strictness),
// flake8-return
UnnecessaryReturnNone,
Expand Down Expand Up @@ -1164,6 +1166,10 @@ impl CheckCode {
// flake8-debugger
CheckCode::T100 => CheckKind::Debugger(DebuggerUsingType::Import("...".to_string())),
// flake8-tidy-imports
CheckCode::TID251 => CheckKind::BannedApi {
name: "...".to_string(),
message: "...".to_string(),
},
CheckCode::TID252 => CheckKind::BannedRelativeImport(Strictness::All),
// flake8-return
CheckCode::RET501 => CheckKind::UnnecessaryReturnNone,
Expand Down Expand Up @@ -1570,6 +1576,7 @@ impl CheckCode {
CheckCode::FBT002 => CheckCategory::Flake8BooleanTrap,
CheckCode::FBT003 => CheckCategory::Flake8BooleanTrap,
CheckCode::I001 => CheckCategory::Isort,
CheckCode::TID251 => CheckCategory::Flake8TidyImports,
CheckCode::TID252 => CheckCategory::Flake8TidyImports,
CheckCode::ICN001 => CheckCategory::Flake8ImportConventions,
CheckCode::N801 => CheckCategory::PEP8Naming,
Expand Down Expand Up @@ -1816,6 +1823,7 @@ impl CheckKind {
// flake8-debugger
CheckKind::Debugger(..) => &CheckCode::T100,
// flake8-tidy-imports
CheckKind::BannedApi { .. } => &CheckCode::TID251,
CheckKind::BannedRelativeImport(..) => &CheckCode::TID252,
// flake8-return
CheckKind::UnnecessaryReturnNone => &CheckCode::RET501,
Expand Down Expand Up @@ -2444,6 +2452,7 @@ impl CheckKind {
DebuggerUsingType::Import(name) => format!("Import for `{name}` found"),
},
// flake8-tidy-imports
CheckKind::BannedApi { name, message } => format!("`{name}` is banned: {message}"),
CheckKind::BannedRelativeImport(strictness) => match strictness {
Strictness::Parents => {
"Relative imports from parent modules are banned".to_string()
Expand Down
14 changes: 9 additions & 5 deletions src/checks_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ pub enum CheckCodePrefix {
TID,
TID2,
TID25,
TID251,
TID252,
U,
U0,
Expand Down Expand Up @@ -706,6 +707,7 @@ impl CheckCodePrefix {
CheckCode::C417,
CheckCode::T100,
CheckCode::C901,
CheckCode::TID251,
CheckCode::TID252,
CheckCode::RET501,
CheckCode::RET502,
Expand Down Expand Up @@ -1664,7 +1666,7 @@ impl CheckCodePrefix {
":".bold(),
"`I2` has been remapped to `TID2`".bold()
);
vec![CheckCode::TID252]
vec![CheckCode::TID251, CheckCode::TID252]
}
CheckCodePrefix::I25 => {
one_time_warning!(
Expand All @@ -1673,7 +1675,7 @@ impl CheckCodePrefix {
":".bold(),
"`I25` has been remapped to `TID25`".bold()
);
vec![CheckCode::TID252]
vec![CheckCode::TID251, CheckCode::TID252]
}
CheckCodePrefix::I252 => {
one_time_warning!(
Expand Down Expand Up @@ -2407,9 +2409,10 @@ impl CheckCodePrefix {
CheckCodePrefix::T20 => vec![CheckCode::T201, CheckCode::T203],
CheckCodePrefix::T201 => vec![CheckCode::T201],
CheckCodePrefix::T203 => vec![CheckCode::T203],
CheckCodePrefix::TID => vec![CheckCode::TID252],
CheckCodePrefix::TID2 => vec![CheckCode::TID252],
CheckCodePrefix::TID25 => vec![CheckCode::TID252],
CheckCodePrefix::TID => vec![CheckCode::TID251, CheckCode::TID252],
CheckCodePrefix::TID2 => vec![CheckCode::TID251, CheckCode::TID252],
CheckCodePrefix::TID25 => vec![CheckCode::TID251, CheckCode::TID252],
CheckCodePrefix::TID251 => vec![CheckCode::TID251],
CheckCodePrefix::TID252 => vec![CheckCode::TID252],
CheckCodePrefix::U => {
one_time_warning!(
Expand Down Expand Up @@ -3287,6 +3290,7 @@ impl CheckCodePrefix {
CheckCodePrefix::TID => SuffixLength::Zero,
CheckCodePrefix::TID2 => SuffixLength::One,
CheckCodePrefix::TID25 => SuffixLength::Two,
CheckCodePrefix::TID251 => SuffixLength::Three,
CheckCodePrefix::TID252 => SuffixLength::Three,
CheckCodePrefix::U => SuffixLength::Zero,
CheckCodePrefix::U0 => SuffixLength::One,
Expand Down
Loading

0 comments on commit 9d34da2

Please sign in to comment.