Add strictness setting for `flake8-typing-imports` (#2221)

This commit is contained in:
Charlie Marsh 2023-01-26 16:04:21 -05:00 committed by GitHub
parent f15c562a1c
commit 5f8810e987
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 371 additions and 21 deletions

View File

@ -3106,6 +3106,27 @@ and can be circumvented via `eval` or `importlib`.
---
### `flake8-type-checking`
#### [`strict`](#strict)
Enforce TC001, TC002, and TC003 rules even when valid runtime imports
are present for the same module.
See: https://github.com/snok/flake8-type-checking#strict.
**Default value**: `false`
**Type**: `bool`
**Example usage**:
```toml
[tool.ruff.flake8-type-checking]
strict = true
```
---
### `flake8-unused-arguments`
#### [`ignore-variadic-names`](#ignore-variadic-names)

View File

@ -0,0 +1,54 @@
def f():
# Even in strict mode, this shouldn't rase an error, since `pkg` is used at runtime,
# and implicitly imports `pkg.bar`.
import pkg
import pkg.bar
def test(value: pkg.bar.A):
return pkg.B()
def f():
# Even in strict mode, this shouldn't rase an error, since `pkg.bar` is used at
# runtime, and implicitly imports `pkg`.
import pkg
import pkg.bar
def test(value: pkg.A):
return pkg.bar.B()
def f():
# In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
import pkg
from pkg import A
def test(value: A):
return pkg.B()
def f():
# In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
from pkg import A, B
def test(value: A):
return B()
def f():
# Even in strict mode, this shouldn't rase an error, since `pkg.baz` is used at
# runtime, and implicitly imports `pkg.bar`.
import pkg.bar
import pkg.baz
def test(value: pkg.bar.A):
return pkg.baz.B()
def f():
# In un-strict mode, this _should_ rase an error, since `pkg` is used at runtime.
import pkg
from pkg.bar import A
def test(value: A):
return pkg.B()

View File

@ -230,6 +230,17 @@
}
]
},
"flake8-type-checking": {
"description": "Options for the `flake8-type-checking` plugin.",
"anyOf": [
{
"$ref": "#/definitions/Flake8TypeCheckingOptions"
},
{
"type": "null"
}
]
},
"flake8-unused-arguments": {
"description": "Options for the `flake8-unused-arguments` plugin.",
"anyOf": [
@ -822,6 +833,19 @@
},
"additionalProperties": false
},
"Flake8TypeCheckingOptions": {
"type": "object",
"properties": {
"strict": {
"description": "Enforce TC001, TC002, and TC003 rules even when valid runtime imports are present for the same module. See: https://github.com/snok/flake8-type-checking#strict.",
"type": [
"boolean",
"null"
]
}
},
"additionalProperties": false
},
"Flake8UnusedArgumentsOptions": {
"type": "object",
"properties": {

View File

@ -1,5 +1,6 @@
//! Lint rules based on AST traversal.
use std::iter;
use std::path::Path;
use itertools::Itertools;
@ -80,7 +81,7 @@ pub struct Checker<'a> {
pub(crate) exprs: Vec<RefEquality<'a, Expr>>,
pub(crate) scopes: Vec<Scope<'a>>,
pub(crate) scope_stack: Vec<usize>,
pub(crate) dead_scopes: Vec<usize>,
pub(crate) dead_scopes: Vec<(usize, Vec<usize>)>,
deferred_string_type_definitions: Vec<(Range, &'a str, bool, DeferralContext<'a>)>,
deferred_type_definitions: Vec<(&'a Expr, bool, DeferralContext<'a>)>,
deferred_functions: Vec<(&'a Stmt, DeferralContext<'a>, VisibleScope)>,
@ -3687,11 +3688,12 @@ impl<'a> Checker<'a> {
}
fn pop_scope(&mut self) {
self.dead_scopes.push(
self.dead_scopes.push((
self.scope_stack
.pop()
.expect("Attempted to pop without scope"),
);
self.scope_stack.clone(),
));
}
fn bind_builtins(&mut self) {
@ -4363,13 +4365,51 @@ impl<'a> Checker<'a> {
return;
}
let mut diagnostics: Vec<Diagnostic> = vec![];
for scope in self
.dead_scopes
.iter()
.rev()
.map(|index| &self.scopes[*index])
// Identify any valid runtime imports. If a module is imported at runtime, and
// used at runtime, then by default, we avoid flagging any other
// imports from that model as typing-only.
let runtime_imports: Vec<Vec<&Binding>> = if !self.settings.flake8_type_checking.strict
&& (self
.settings
.rules
.enabled(&Rule::RuntimeImportInTypeCheckingBlock)
|| self
.settings
.rules
.enabled(&Rule::TypingOnlyFirstPartyImport)
|| self
.settings
.rules
.enabled(&Rule::TypingOnlyThirdPartyImport)
|| self
.settings
.rules
.enabled(&Rule::TypingOnlyStandardLibraryImport))
{
self.scopes
.iter()
.map(|scope| {
scope
.values
.values()
.map(|index| &self.bindings[*index])
.filter(|binding| {
flake8_type_checking::helpers::is_valid_runtime_import(
binding,
&self.type_checking_blocks,
)
})
.collect::<Vec<_>>()
})
.collect::<Vec<_>>()
} else {
vec![]
};
let mut diagnostics: Vec<Diagnostic> = vec![];
for (index, stack) in self.dead_scopes.iter().rev() {
let scope = &self.scopes[*index];
// PLW0602
if self
.settings
@ -4508,6 +4548,16 @@ impl<'a> Checker<'a> {
.rules
.enabled(&Rule::TypingOnlyStandardLibraryImport)
{
let runtime_imports: Vec<&Binding> = if self.settings.flake8_type_checking.strict {
vec![]
} else {
stack
.iter()
.chain(iter::once(index))
.flat_map(|index| runtime_imports[*index].iter())
.copied()
.collect()
};
for (.., index) in &scope.values {
let binding = &self.bindings[*index];
@ -4523,6 +4573,7 @@ impl<'a> Checker<'a> {
flake8_type_checking::rules::typing_only_runtime_import(
binding,
&self.type_checking_blocks,
&runtime_imports,
self.package,
self.settings,
)

View File

@ -11,8 +11,8 @@ use crate::registry::Rule;
use crate::rules::{
flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins, flake8_errmsg,
flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes,
flake8_tidy_imports, flake8_unused_arguments, isort, mccabe, pep8_naming, pycodestyle,
pydocstyle, pylint, pyupgrade,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming,
pycodestyle, pydocstyle, pylint, pyupgrade,
};
use crate::rustpython_helpers::tokenize;
use crate::settings::configuration::Configuration;
@ -141,13 +141,14 @@ pub fn defaultSettings() -> Result<JsValue, JsValue> {
flake8_errmsg: Some(flake8_errmsg::settings::Settings::default().into()),
flake8_pytest_style: Some(flake8_pytest_style::settings::Settings::default().into()),
flake8_quotes: Some(flake8_quotes::settings::Settings::default().into()),
flake8_tidy_imports: Some(flake8_tidy_imports::Settings::default().into()),
flake8_implicit_str_concat: Some(
flake8_implicit_str_concat::settings::Settings::default().into(),
),
flake8_import_conventions: Some(
flake8_import_conventions::settings::Settings::default().into(),
),
flake8_tidy_imports: Some(flake8_tidy_imports::Settings::default().into()),
flake8_type_checking: Some(flake8_type_checking::settings::Settings::default().into()),
flake8_unused_arguments: Some(
flake8_unused_arguments::settings::Settings::default().into(),
),

View File

@ -1,5 +1,6 @@
use rustpython_ast::Expr;
use rustpython_ast::{Expr, Stmt};
use crate::ast::types::{Binding, BindingKind, Range};
use crate::checkers::ast::Checker;
pub fn is_type_checking_block(checker: &Checker, test: &Expr) -> bool {
@ -7,3 +8,19 @@ pub fn is_type_checking_block(checker: &Checker, test: &Expr) -> bool {
call_path.as_slice() == ["typing", "TYPE_CHECKING"]
})
}
pub fn is_valid_runtime_import(binding: &Binding, blocks: &[&Stmt]) -> bool {
if matches!(
binding.kind,
BindingKind::Importation(..)
| BindingKind::FromImportation(..)
| BindingKind::SubmoduleImportation(..)
) {
if binding.runtime_usage.is_some() {
return !blocks
.iter()
.any(|block| Range::from_located(block).contains(&binding.range));
}
}
false
}

View File

@ -1,6 +1,7 @@
//! Rules from [flake8-type-checking](https://pypi.org/project/flake8-type-checking/).
pub(crate) mod helpers;
pub(crate) mod rules;
pub mod settings;
#[cfg(test)]
mod tests {
@ -26,6 +27,7 @@ mod tests {
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_7.py"); "TCH004_7")]
#[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_8.py"); "TCH004_8")]
#[test_case(Rule::EmptyTypeCheckingBlock, Path::new("TCH005.py"); "TCH005")]
#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"); "strict")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
let diagnostics = test_path(
@ -37,4 +39,19 @@ mod tests {
assert_yaml_snapshot!(snapshot, diagnostics);
Ok(())
}
#[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"); "strict")]
fn strict(rule_code: Rule, path: &Path) -> Result<()> {
let diagnostics = test_path(
Path::new("./resources/test/fixtures/flake8_type_checking")
.join(path)
.as_path(),
&settings::Settings {
flake8_type_checking: super::settings::Settings { strict: true },
..settings::Settings::for_rule(rule_code)
},
)?;
assert_yaml_snapshot!(diagnostics);
Ok(())
}
}

View File

@ -55,13 +55,64 @@ impl Violation for TypingOnlyStandardLibraryImport {
}
}
/// Return `true` if `this` is implicitly loaded via importing `that`.
fn is_implicit_import(this: &Binding, that: &Binding) -> bool {
match &this.kind {
BindingKind::Importation(.., this_name)
| BindingKind::SubmoduleImportation(this_name, ..) => match &that.kind {
BindingKind::FromImportation(.., that_name) => {
// Ex) `pkg.A` vs. `pkg`
this_name
.rfind('.')
.map_or(false, |i| this_name[..i] == *that_name)
}
BindingKind::Importation(.., that_name)
| BindingKind::SubmoduleImportation(that_name, ..) => {
// Ex) `pkg.A` vs. `pkg.B`
this_name == that_name
}
_ => false,
},
BindingKind::FromImportation(.., this_name) => match &that.kind {
BindingKind::Importation(.., that_name)
| BindingKind::SubmoduleImportation(that_name, ..) => {
// Ex) `pkg.A` vs. `pkg`
this_name
.rfind('.')
.map_or(false, |i| &this_name[..i] == *that_name)
}
BindingKind::FromImportation(.., that_name) => {
// Ex) `pkg.A` vs. `pkg.B`
this_name.rfind('.').map_or(false, |i| {
that_name
.rfind('.')
.map_or(false, |j| this_name[..i] == that_name[..j])
})
}
_ => false,
},
_ => false,
}
}
/// TCH001
pub fn typing_only_runtime_import(
binding: &Binding,
blocks: &[&Stmt],
runtime_imports: &[&Binding],
package: Option<&Path>,
settings: &Settings,
) -> Option<Diagnostic> {
// If we're in un-strict mode, don't flag typing-only imports that are
// implicitly loaded by way of a valid runtime import.
if !settings.flake8_type_checking.strict
&& runtime_imports
.iter()
.any(|import| is_implicit_import(binding, import))
{
return None;
}
let full_name = match &binding.kind {
BindingKind::Importation(.., full_name) => full_name,
BindingKind::FromImportation(.., full_name) => full_name.as_str(),

View File

@ -0,0 +1,48 @@
//! Settings for the `flake8-type-checking` plugin.
use ruff_macros::ConfigurationOptions;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
#[derive(
Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions, JsonSchema,
)]
#[serde(
deny_unknown_fields,
rename_all = "kebab-case",
rename = "Flake8TypeCheckingOptions"
)]
pub struct Options {
#[option(
default = "false",
value_type = "bool",
example = r#"
strict = true
"#
)]
/// Enforce TC001, TC002, and TC003 rules even when valid runtime imports
/// are present for the same module.
/// See: https://github.com/snok/flake8-type-checking#strict.
pub strict: Option<bool>,
}
#[derive(Debug, Hash, Default)]
pub struct Settings {
pub strict: bool,
}
impl From<Options> for Settings {
fn from(options: Options) -> Self {
Self {
strict: options.strict.unwrap_or_default(),
}
}
}
impl From<Settings> for Options {
fn from(settings: Settings) -> Self {
Self {
strict: Some(settings.strict),
}
}
}

View File

@ -0,0 +1,38 @@
---
source: src/rules/flake8_type_checking/mod.rs
expression: diagnostics
---
- kind:
TypingOnlyThirdPartyImport:
full_name: pkg.A
location:
row: 24
column: 20
end_location:
row: 24
column: 21
fix: ~
parent: ~
- kind:
TypingOnlyThirdPartyImport:
full_name: pkg.A
location:
row: 32
column: 20
end_location:
row: 32
column: 21
fix: ~
parent: ~
- kind:
TypingOnlyThirdPartyImport:
full_name: pkg.bar.A
location:
row: 51
column: 24
end_location:
row: 51
column: 25
fix: ~
parent: ~

View File

@ -0,0 +1,16 @@
---
source: src/rules/flake8_type_checking/mod.rs
expression: diagnostics
---
- kind:
TypingOnlyThirdPartyImport:
full_name: pkg.bar.A
location:
row: 51
column: 24
end_location:
row: 51
column: 25
fix: ~
parent: ~

View File

@ -17,8 +17,8 @@ use crate::rule_selector::RuleSelector;
use crate::rules::{
flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins, flake8_errmsg,
flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes,
flake8_tidy_imports, flake8_unused_arguments, isort, mccabe, pep8_naming, pycodestyle,
pydocstyle, pylint, pyupgrade,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming,
pycodestyle, pydocstyle, pylint, pyupgrade,
};
use crate::settings::options::Options;
use crate::settings::pyproject::load_options;
@ -70,6 +70,7 @@ pub struct Configuration {
pub flake8_pytest_style: Option<flake8_pytest_style::settings::Options>,
pub flake8_quotes: Option<flake8_quotes::settings::Options>,
pub flake8_tidy_imports: Option<flake8_tidy_imports::options::Options>,
pub flake8_type_checking: Option<flake8_type_checking::settings::Options>,
pub flake8_unused_arguments: Option<flake8_unused_arguments::settings::Options>,
pub isort: Option<isort::settings::Options>,
pub mccabe: Option<mccabe::settings::Options>,
@ -178,6 +179,7 @@ impl Configuration {
flake8_pytest_style: options.flake8_pytest_style,
flake8_quotes: options.flake8_quotes,
flake8_tidy_imports: options.flake8_tidy_imports,
flake8_type_checking: options.flake8_type_checking,
flake8_unused_arguments: options.flake8_unused_arguments,
isort: options.isort,
mccabe: options.mccabe,
@ -251,6 +253,7 @@ impl Configuration {
flake8_pytest_style: self.flake8_pytest_style.or(config.flake8_pytest_style),
flake8_quotes: self.flake8_quotes.or(config.flake8_quotes),
flake8_tidy_imports: self.flake8_tidy_imports.or(config.flake8_tidy_imports),
flake8_type_checking: self.flake8_type_checking.or(config.flake8_type_checking),
flake8_unused_arguments: self
.flake8_unused_arguments
.or(config.flake8_unused_arguments),

View File

@ -11,8 +11,8 @@ use crate::rule_selector::{prefix_to_selector, RuleSelector};
use crate::rules::{
flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins, flake8_errmsg,
flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes,
flake8_tidy_imports, flake8_unused_arguments, isort, mccabe, pep8_naming, pycodestyle,
pydocstyle, pylint, pyupgrade,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming,
pycodestyle, pydocstyle, pylint, pyupgrade,
};
pub const PREFIXES: &[RuleSelector] = &[
@ -84,6 +84,7 @@ impl Default for Settings {
flake8_pytest_style: flake8_pytest_style::settings::Settings::default(),
flake8_quotes: flake8_quotes::settings::Settings::default(),
flake8_tidy_imports: flake8_tidy_imports::Settings::default(),
flake8_type_checking: flake8_type_checking::settings::Settings::default(),
flake8_unused_arguments: flake8_unused_arguments::settings::Settings::default(),
isort: isort::settings::Settings::default(),
mccabe: mccabe::settings::Settings::default(),

View File

@ -16,8 +16,8 @@ use crate::rule_selector::{RuleSelector, Specificity};
use crate::rules::{
flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins, flake8_errmsg,
flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes,
flake8_tidy_imports, flake8_unused_arguments, isort, mccabe, pep8_naming, pycodestyle,
pydocstyle, pylint, pyupgrade,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming,
pycodestyle, pydocstyle, pylint, pyupgrade,
};
use crate::settings::configuration::Configuration;
use crate::settings::types::{PerFileIgnore, PythonVersion, SerializationFormat};
@ -111,6 +111,7 @@ pub struct Settings {
pub flake8_pytest_style: flake8_pytest_style::settings::Settings,
pub flake8_quotes: flake8_quotes::settings::Settings,
pub flake8_tidy_imports: flake8_tidy_imports::Settings,
pub flake8_type_checking: flake8_type_checking::settings::Settings,
pub flake8_unused_arguments: flake8_unused_arguments::settings::Settings,
pub isort: isort::settings::Settings,
pub mccabe: mccabe::settings::Settings,
@ -198,6 +199,10 @@ impl Settings {
.flake8_tidy_imports
.map(Into::into)
.unwrap_or_default(),
flake8_type_checking: config
.flake8_type_checking
.map(Into::into)
.unwrap_or_default(),
flake8_unused_arguments: config
.flake8_unused_arguments
.map(Into::into)

View File

@ -9,8 +9,8 @@ use crate::rule_selector::RuleSelector;
use crate::rules::{
flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins, flake8_errmsg,
flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes,
flake8_tidy_imports, flake8_unused_arguments, isort, mccabe, pep8_naming, pycodestyle,
pydocstyle, pylint, pyupgrade,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming,
pycodestyle, pydocstyle, pylint, pyupgrade,
};
use crate::settings::types::{PythonVersion, SerializationFormat, Version};
@ -441,6 +441,9 @@ pub struct Options {
/// Options for the `flake8-tidy-imports` plugin.
pub flake8_tidy_imports: Option<flake8_tidy_imports::options::Options>,
#[option_group]
/// Options for the `flake8-type-checking` plugin.
pub flake8_type_checking: Option<flake8_type_checking::settings::Options>,
#[option_group]
/// Options for the `flake8-implicit-str-concat` plugin.
pub flake8_implicit_str_concat: Option<flake8_implicit_str_concat::settings::Options>,
#[option_group]