mirror of https://github.com/astral-sh/ruff
`analyze`: Add option to skip over imports in `TYPE_CHECKING` blocks (#21472)
Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
parent
f5fb5c388a
commit
665f68036c
|
|
@ -167,6 +167,7 @@ pub enum AnalyzeCommand {
|
|||
}
|
||||
|
||||
#[derive(Clone, Debug, clap::Parser)]
|
||||
#[expect(clippy::struct_excessive_bools)]
|
||||
pub struct AnalyzeGraphCommand {
|
||||
/// List of files or directories to include.
|
||||
#[clap(help = "List of files or directories to include [default: .]")]
|
||||
|
|
@ -193,6 +194,12 @@ pub struct AnalyzeGraphCommand {
|
|||
/// Path to a virtual environment to use for resolving additional dependencies
|
||||
#[arg(long)]
|
||||
python: Option<PathBuf>,
|
||||
/// Include imports that are only used for type checking (i.e., imports within `if TYPE_CHECKING:` blocks).
|
||||
/// Use `--no-type-checking-imports` to exclude imports that are only used for type checking.
|
||||
#[arg(long, overrides_with("no_type_checking_imports"))]
|
||||
type_checking_imports: bool,
|
||||
#[arg(long, overrides_with("type_checking_imports"), hide = true)]
|
||||
no_type_checking_imports: bool,
|
||||
}
|
||||
|
||||
// The `Parser` derive is for ruff_dev, for ruff `Args` would be sufficient
|
||||
|
|
@ -839,6 +846,10 @@ impl AnalyzeGraphCommand {
|
|||
string_imports_min_dots: self.min_dots,
|
||||
preview: resolve_bool_arg(self.preview, self.no_preview).map(PreviewMode::from),
|
||||
target_version: self.target_version.map(ast::PythonVersion::from),
|
||||
type_checking_imports: resolve_bool_arg(
|
||||
self.type_checking_imports,
|
||||
self.no_type_checking_imports,
|
||||
),
|
||||
..ExplicitConfigOverrides::default()
|
||||
};
|
||||
|
||||
|
|
@ -1335,6 +1346,7 @@ struct ExplicitConfigOverrides {
|
|||
extension: Option<Vec<ExtensionPair>>,
|
||||
detect_string_imports: Option<bool>,
|
||||
string_imports_min_dots: Option<usize>,
|
||||
type_checking_imports: Option<bool>,
|
||||
}
|
||||
|
||||
impl ConfigurationTransformer for ExplicitConfigOverrides {
|
||||
|
|
@ -1425,6 +1437,9 @@ impl ConfigurationTransformer for ExplicitConfigOverrides {
|
|||
if let Some(string_imports_min_dots) = &self.string_imports_min_dots {
|
||||
config.analyze.string_imports_min_dots = Some(*string_imports_min_dots);
|
||||
}
|
||||
if let Some(type_checking_imports) = &self.type_checking_imports {
|
||||
config.analyze.type_checking_imports = Some(*type_checking_imports);
|
||||
}
|
||||
|
||||
config
|
||||
}
|
||||
|
|
|
|||
|
|
@ -105,6 +105,7 @@ pub(crate) fn analyze_graph(
|
|||
let settings = resolver.resolve(path);
|
||||
let string_imports = settings.analyze.string_imports;
|
||||
let include_dependencies = settings.analyze.include_dependencies.get(path).cloned();
|
||||
let type_checking_imports = settings.analyze.type_checking_imports;
|
||||
|
||||
// Skip excluded files.
|
||||
if (settings.file_resolver.force_exclude || !resolved_file.is_root())
|
||||
|
|
@ -167,6 +168,7 @@ pub(crate) fn analyze_graph(
|
|||
&path,
|
||||
package.as_deref(),
|
||||
string_imports,
|
||||
type_checking_imports,
|
||||
)
|
||||
.unwrap_or_else(|err| {
|
||||
warn!("Failed to generate import map for {path}: {err}");
|
||||
|
|
|
|||
|
|
@ -0,0 +1,157 @@
|
|||
use insta_cmd::assert_cmd_snapshot;
|
||||
|
||||
use crate::CliTest;
|
||||
|
||||
#[test]
|
||||
fn type_checking_imports() -> anyhow::Result<()> {
|
||||
let test = CliTest::with_files([
|
||||
("ruff/__init__.py", ""),
|
||||
(
|
||||
"ruff/a.py",
|
||||
r#"
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import ruff.b
|
||||
|
||||
if TYPE_CHECKING:
|
||||
import ruff.c
|
||||
"#,
|
||||
),
|
||||
(
|
||||
"ruff/b.py",
|
||||
r#"
|
||||
if TYPE_CHECKING:
|
||||
from ruff import c
|
||||
"#,
|
||||
),
|
||||
("ruff/c.py", ""),
|
||||
])?;
|
||||
|
||||
assert_cmd_snapshot!(test.analyze_graph_command(), @r###"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
{
|
||||
"ruff/__init__.py": [],
|
||||
"ruff/a.py": [
|
||||
"ruff/b.py",
|
||||
"ruff/c.py"
|
||||
],
|
||||
"ruff/b.py": [
|
||||
"ruff/c.py"
|
||||
],
|
||||
"ruff/c.py": []
|
||||
}
|
||||
|
||||
----- stderr -----
|
||||
"###);
|
||||
|
||||
assert_cmd_snapshot!(
|
||||
test.analyze_graph_command()
|
||||
.arg("--no-type-checking-imports"),
|
||||
@r###"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
{
|
||||
"ruff/__init__.py": [],
|
||||
"ruff/a.py": [
|
||||
"ruff/b.py"
|
||||
],
|
||||
"ruff/b.py": [],
|
||||
"ruff/c.py": []
|
||||
}
|
||||
|
||||
----- stderr -----
|
||||
"###
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn type_checking_imports_from_config() -> anyhow::Result<()> {
|
||||
let test = CliTest::with_files([
|
||||
("ruff/__init__.py", ""),
|
||||
(
|
||||
"ruff/a.py",
|
||||
r#"
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import ruff.b
|
||||
|
||||
if TYPE_CHECKING:
|
||||
import ruff.c
|
||||
"#,
|
||||
),
|
||||
(
|
||||
"ruff/b.py",
|
||||
r#"
|
||||
if TYPE_CHECKING:
|
||||
from ruff import c
|
||||
"#,
|
||||
),
|
||||
("ruff/c.py", ""),
|
||||
(
|
||||
"ruff.toml",
|
||||
r#"
|
||||
[analyze]
|
||||
type-checking-imports = false
|
||||
"#,
|
||||
),
|
||||
])?;
|
||||
|
||||
assert_cmd_snapshot!(test.analyze_graph_command(), @r###"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
{
|
||||
"ruff/__init__.py": [],
|
||||
"ruff/a.py": [
|
||||
"ruff/b.py"
|
||||
],
|
||||
"ruff/b.py": [],
|
||||
"ruff/c.py": []
|
||||
}
|
||||
|
||||
----- stderr -----
|
||||
"###);
|
||||
|
||||
test.write_file(
|
||||
"ruff.toml",
|
||||
r#"
|
||||
[analyze]
|
||||
type-checking-imports = true
|
||||
"#,
|
||||
)?;
|
||||
|
||||
assert_cmd_snapshot!(test.analyze_graph_command(), @r###"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
{
|
||||
"ruff/__init__.py": [],
|
||||
"ruff/a.py": [
|
||||
"ruff/b.py",
|
||||
"ruff/c.py"
|
||||
],
|
||||
"ruff/b.py": [
|
||||
"ruff/c.py"
|
||||
],
|
||||
"ruff/c.py": []
|
||||
}
|
||||
|
||||
----- stderr -----
|
||||
"###
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
impl CliTest {
|
||||
fn analyze_graph_command(&self) -> std::process::Command {
|
||||
let mut command = self.command();
|
||||
command.arg("analyze").arg("graph").arg("--preview");
|
||||
command
|
||||
}
|
||||
}
|
||||
|
|
@ -15,6 +15,7 @@ use std::{
|
|||
};
|
||||
use tempfile::TempDir;
|
||||
|
||||
mod analyze_graph;
|
||||
mod format;
|
||||
mod lint;
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ info:
|
|||
- concise
|
||||
- "--show-settings"
|
||||
- test.py
|
||||
snapshot_kind: text
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
|
|
@ -284,5 +283,6 @@ analyze.target_version = 3.10
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -12,7 +12,6 @@ info:
|
|||
- UP007
|
||||
- test.py
|
||||
- "-"
|
||||
snapshot_kind: text
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
|
|
@ -286,5 +285,6 @@ analyze.target_version = 3.11
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -13,7 +13,6 @@ info:
|
|||
- UP007
|
||||
- test.py
|
||||
- "-"
|
||||
snapshot_kind: text
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
|
|
@ -288,5 +287,6 @@ analyze.target_version = 3.11
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -14,7 +14,6 @@ info:
|
|||
- py310
|
||||
- test.py
|
||||
- "-"
|
||||
snapshot_kind: text
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
|
|
@ -288,5 +287,6 @@ analyze.target_version = 3.10
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ info:
|
|||
- "--select"
|
||||
- UP007
|
||||
- foo/test.py
|
||||
snapshot_kind: text
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
|
|
@ -285,5 +284,6 @@ analyze.target_version = 3.11
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ info:
|
|||
- "--select"
|
||||
- UP007
|
||||
- foo/test.py
|
||||
snapshot_kind: text
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
|
|
@ -285,5 +284,6 @@ analyze.target_version = 3.10
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -283,5 +283,6 @@ analyze.target_version = 3.10
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -283,5 +283,6 @@ analyze.target_version = 3.10
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ info:
|
|||
- concise
|
||||
- test.py
|
||||
- "--show-settings"
|
||||
snapshot_kind: text
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
|
|
@ -284,5 +283,6 @@ analyze.target_version = 3.11
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -396,5 +396,6 @@ analyze.target_version = 3.7
|
|||
analyze.string_imports = disabled
|
||||
analyze.extension = ExtensionMapping({})
|
||||
analyze.include_dependencies = {}
|
||||
analyze.type_checking_imports = true
|
||||
|
||||
----- stderr -----
|
||||
|
|
|
|||
|
|
@ -14,14 +14,21 @@ pub(crate) struct Collector<'a> {
|
|||
string_imports: StringImports,
|
||||
/// The collected imports from the Python AST.
|
||||
imports: Vec<CollectedImport>,
|
||||
/// Whether to detect type checking imports
|
||||
type_checking_imports: bool,
|
||||
}
|
||||
|
||||
impl<'a> Collector<'a> {
|
||||
pub(crate) fn new(module_path: Option<&'a [String]>, string_imports: StringImports) -> Self {
|
||||
pub(crate) fn new(
|
||||
module_path: Option<&'a [String]>,
|
||||
string_imports: StringImports,
|
||||
type_checking_imports: bool,
|
||||
) -> Self {
|
||||
Self {
|
||||
module_path,
|
||||
string_imports,
|
||||
imports: Vec::new(),
|
||||
type_checking_imports,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -91,10 +98,25 @@ impl<'ast> SourceOrderVisitor<'ast> for Collector<'_> {
|
|||
}
|
||||
}
|
||||
}
|
||||
Stmt::If(ast::StmtIf {
|
||||
test,
|
||||
body,
|
||||
elif_else_clauses,
|
||||
range: _,
|
||||
node_index: _,
|
||||
}) => {
|
||||
// Skip TYPE_CHECKING blocks if not requested
|
||||
if self.type_checking_imports || !is_type_checking_condition(test) {
|
||||
self.visit_body(body);
|
||||
}
|
||||
|
||||
for clause in elif_else_clauses {
|
||||
self.visit_elif_else_clause(clause);
|
||||
}
|
||||
}
|
||||
Stmt::FunctionDef(_)
|
||||
| Stmt::ClassDef(_)
|
||||
| Stmt::While(_)
|
||||
| Stmt::If(_)
|
||||
| Stmt::With(_)
|
||||
| Stmt::Match(_)
|
||||
| Stmt::Try(_)
|
||||
|
|
@ -152,6 +174,30 @@ impl<'ast> SourceOrderVisitor<'ast> for Collector<'_> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Check if an expression is a `TYPE_CHECKING` condition.
|
||||
///
|
||||
/// Returns `true` for:
|
||||
/// - `TYPE_CHECKING`
|
||||
/// - `typing.TYPE_CHECKING`
|
||||
///
|
||||
/// NOTE: Aliased `TYPE_CHECKING`, i.e. `import typing.TYPE_CHECKING as TC; if TC: ...`
|
||||
/// will not be detected!
|
||||
fn is_type_checking_condition(expr: &Expr) -> bool {
|
||||
match expr {
|
||||
// `if TYPE_CHECKING:`
|
||||
Expr::Name(ast::ExprName { id, .. }) => id.as_str() == "TYPE_CHECKING",
|
||||
// `if typing.TYPE_CHECKING:`
|
||||
Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => {
|
||||
attr.as_str() == "TYPE_CHECKING"
|
||||
&& matches!(
|
||||
value.as_ref(),
|
||||
Expr::Name(ast::ExprName { id, .. }) if id.as_str() == "typing"
|
||||
)
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) enum CollectedImport {
|
||||
/// The import was part of an `import` statement.
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ impl ModuleImports {
|
|||
path: &SystemPath,
|
||||
package: Option<&SystemPath>,
|
||||
string_imports: StringImports,
|
||||
type_checking_imports: bool,
|
||||
) -> Result<Self> {
|
||||
// Parse the source code.
|
||||
let parsed = parse(source, ParseOptions::from(source_type))?;
|
||||
|
|
@ -38,8 +39,12 @@ impl ModuleImports {
|
|||
package.and_then(|package| to_module_path(package.as_std_path(), path.as_std_path()));
|
||||
|
||||
// Collect the imports.
|
||||
let imports =
|
||||
Collector::new(module_path.as_deref(), string_imports).collect(parsed.syntax());
|
||||
let imports = Collector::new(
|
||||
module_path.as_deref(),
|
||||
string_imports,
|
||||
type_checking_imports,
|
||||
)
|
||||
.collect(parsed.syntax());
|
||||
|
||||
// Resolve the imports.
|
||||
let mut resolved_imports = ModuleImports::default();
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use std::collections::BTreeMap;
|
|||
use std::fmt;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Debug, Default, Clone, CacheKey)]
|
||||
#[derive(Debug, Clone, CacheKey)]
|
||||
pub struct AnalyzeSettings {
|
||||
pub exclude: FilePatternSet,
|
||||
pub preview: PreviewMode,
|
||||
|
|
@ -14,6 +14,21 @@ pub struct AnalyzeSettings {
|
|||
pub string_imports: StringImports,
|
||||
pub include_dependencies: BTreeMap<PathBuf, (PathBuf, Vec<String>)>,
|
||||
pub extension: ExtensionMapping,
|
||||
pub type_checking_imports: bool,
|
||||
}
|
||||
|
||||
impl Default for AnalyzeSettings {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
exclude: FilePatternSet::default(),
|
||||
preview: PreviewMode::default(),
|
||||
target_version: PythonVersion::default(),
|
||||
string_imports: StringImports::default(),
|
||||
include_dependencies: BTreeMap::default(),
|
||||
extension: ExtensionMapping::default(),
|
||||
type_checking_imports: true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for AnalyzeSettings {
|
||||
|
|
@ -29,6 +44,7 @@ impl fmt::Display for AnalyzeSettings {
|
|||
self.string_imports,
|
||||
self.extension | debug,
|
||||
self.include_dependencies | debug,
|
||||
self.type_checking_imports,
|
||||
]
|
||||
}
|
||||
Ok(())
|
||||
|
|
|
|||
|
|
@ -232,6 +232,9 @@ impl Configuration {
|
|||
include_dependencies: analyze
|
||||
.include_dependencies
|
||||
.unwrap_or(analyze_defaults.include_dependencies),
|
||||
type_checking_imports: analyze
|
||||
.type_checking_imports
|
||||
.unwrap_or(analyze_defaults.type_checking_imports),
|
||||
};
|
||||
|
||||
let lint = self.lint;
|
||||
|
|
@ -1277,6 +1280,7 @@ pub struct AnalyzeConfiguration {
|
|||
pub detect_string_imports: Option<bool>,
|
||||
pub string_imports_min_dots: Option<usize>,
|
||||
pub include_dependencies: Option<BTreeMap<PathBuf, (PathBuf, Vec<String>)>>,
|
||||
pub type_checking_imports: Option<bool>,
|
||||
}
|
||||
|
||||
impl AnalyzeConfiguration {
|
||||
|
|
@ -1303,6 +1307,7 @@ impl AnalyzeConfiguration {
|
|||
})
|
||||
.collect::<BTreeMap<_, _>>()
|
||||
}),
|
||||
type_checking_imports: options.type_checking_imports,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -1317,6 +1322,7 @@ impl AnalyzeConfiguration {
|
|||
.string_imports_min_dots
|
||||
.or(config.string_imports_min_dots),
|
||||
include_dependencies: self.include_dependencies.or(config.include_dependencies),
|
||||
type_checking_imports: self.type_checking_imports.or(config.type_checking_imports),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3892,6 +3892,18 @@ pub struct AnalyzeOptions {
|
|||
"#
|
||||
)]
|
||||
pub include_dependencies: Option<BTreeMap<PathBuf, Vec<String>>>,
|
||||
/// Whether to include imports that are only used for type checking (i.e., imports within `if TYPE_CHECKING:` blocks).
|
||||
/// When enabled (default), type-checking-only imports are included in the import graph.
|
||||
/// When disabled, they are excluded.
|
||||
#[option(
|
||||
default = "true",
|
||||
value_type = "bool",
|
||||
example = r#"
|
||||
# Exclude type-checking-only imports from the graph
|
||||
type-checking-imports = false
|
||||
"#
|
||||
)]
|
||||
pub type_checking_imports: Option<bool>,
|
||||
}
|
||||
|
||||
/// Like [`LintCommonOptions`], but with any `#[serde(flatten)]` fields inlined. This leads to far,
|
||||
|
|
|
|||
|
|
@ -818,6 +818,13 @@
|
|||
],
|
||||
"format": "uint",
|
||||
"minimum": 0
|
||||
},
|
||||
"type-checking-imports": {
|
||||
"description": "Whether to include imports that are only used for type checking (i.e., imports within `if TYPE_CHECKING:` blocks).\nWhen enabled (default), type-checking-only imports are included in the import graph.\nWhen disabled, they are excluded.",
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"additionalProperties": false
|
||||
|
|
|
|||
Loading…
Reference in New Issue