diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index 5a3f80c9e6fec1..77425f1cb4ccf7 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -3543,6 +3543,36 @@ def func(x: int): ... func() # error ``` +## `missing-direct-dependency` + + +Default level: ignore · +Added in 0.0.64 · +Related issues · +View source + + + +**What it does** + +Checks for third-party imports that are used without a matching direct dependency +declaration. + +**Why is this bad?** + +Importing a package that is only available transitively can make the project break when +dependency resolution changes. + +**Rule status** + +This rule is disabled by default. + +**Examples** + +```python +import requests # requests is not declared as a direct dependency +``` + ## `missing-override-decorator` diff --git a/crates/ty/src/args.rs b/crates/ty/src/args.rs index d3a576211df13d..334019b0c89704 100644 --- a/crates/ty/src/args.rs +++ b/crates/ty/src/args.rs @@ -77,6 +77,10 @@ pub(crate) struct CheckCommand { #[arg(long, conflicts_with("fix"))] pub(crate) add_ignore: bool, + /// Path to a `uv workspace metadata` JSON snapshot used instead of automatic uv discovery. + #[arg(long, value_name = "PATH", hide = true)] + pub(crate) dependency_metadata: Option, + /// Run the command within the given project directory. /// /// All `pyproject.toml` files will be discovered by walking up the directory tree from the given project directory, diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index 2b47acdb009fba..8ab0023e31985b 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -7,7 +7,7 @@ mod version; use std::io::{BufWriter, Write}; use std::process::{ExitCode, Termination}; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; use anyhow::Result; use anyhow::{Context, anyhow}; @@ -24,10 +24,14 @@ use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf}; use ruff_db::{STACK_SIZE, max_parallelism}; use ruff_diagnostics::Applicability; use salsa::Database; +use ty_project::dependency_metadata::{ + enrich_dependency_metadata_with_editables, parse_uv_workspace_metadata, +}; use ty_project::metadata::settings::TerminalSettings; use ty_project::watch::ProjectWatcher; use ty_project::{CollectReporter, Db, watch}; use ty_project::{ProjectDatabase, ProjectMetadata}; +use ty_python_semantic::dependency::DependencyMetadata; use ty_python_semantic::{fix_all_diagnostics, suppress_all_diagnostics}; use ty_server::run_server; use ty_static::EnvVars; @@ -133,6 +137,10 @@ fn run_check(args: CheckCommand) -> anyhow::Result { .iter() .map(|path| SystemPath::absolute(path, &cwd)) .collect(); + let dependency_metadata_path = args + .dependency_metadata + .as_ref() + .map(|path| SystemPath::absolute(path, &cwd)); let mode = if args.fix { MainLoopMode::Fix(FixMode::ApplyFixes) @@ -170,15 +178,24 @@ fn run_check(args: CheckCommand) -> anyhow::Result { )); } + let dependency_metadata = match dependency_metadata_path.as_deref() { + Some(path) => Some(load_dependency_metadata(&system, path)?), + None => project_metadata.uv_dependency_metadata().cloned(), + }; + project_metadata.apply_configuration_files(&system)?; project_metadata.apply_override_options(args.into_options()); let mut db = ProjectDatabase::fallible(project_metadata, system)?; let project = db.project(); + let dependency_metadata = dependency_metadata.map(Arc::new); + let enriched_dependency_metadata = + enrich_dependency_metadata(&db, dependency_metadata.as_ref()); project.set_verbose(&mut db, verbosity >= VerbosityLevel::Verbose); project.set_force_exclude(&mut db, force_exclude); + project.set_dependency_metadata(&mut db, enriched_dependency_metadata.as_ref()); if !check_paths.is_empty() { project.set_included_paths(&mut db, check_paths); @@ -201,7 +218,8 @@ fn run_check(args: CheckCommand) -> anyhow::Result { db.freeze(); } - let (main_loop, main_loop_cancellation_token) = MainLoop::new(mode, printer); + let (main_loop, main_loop_cancellation_token) = + MainLoop::new(mode, dependency_metadata, printer); // Listen to Ctrl+C and abort the watch mode. let main_loop_cancellation_token = Mutex::new(Some(main_loop_cancellation_token)); @@ -245,6 +263,29 @@ fn run_check(args: CheckCommand) -> anyhow::Result { } } +fn load_dependency_metadata(system: &dyn System, path: &SystemPath) -> Result { + let source = system + .read_to_string(path) + .with_context(|| format!("Failed to read dependency metadata `{path}`"))?; + + let metadata = parse_uv_workspace_metadata(&source) + .with_context(|| format!("Failed to load dependency metadata `{path}`"))?; + + Ok(metadata) +} + +fn enrich_dependency_metadata( + db: &ProjectDatabase, + dependency_metadata: Option<&Arc>, +) -> Option> { + dependency_metadata.map(|metadata| { + Arc::new(enrich_dependency_metadata_with_editables( + db, + (**metadata).clone(), + )) + }) +} + #[derive(Copy, Clone)] pub enum ExitStatus { /// Checking was successful and there were no errors. @@ -291,6 +332,8 @@ struct MainLoop { /// Interface for displaying information to the user. printer: Printer, + dependency_metadata: Option>, + /// Cancellation token that gets set by Ctrl+C. /// Used for long-running operations on the main thread. Operations on background threads /// use Salsa's cancellation mechanism. @@ -298,7 +341,11 @@ struct MainLoop { } impl MainLoop { - fn new(mode: MainLoopMode, printer: Printer) -> (Self, MainLoopCancellationToken) { + fn new( + mode: MainLoopMode, + dependency_metadata: Option>, + printer: Printer, + ) -> (Self, MainLoopCancellationToken) { let (sender, receiver) = crossbeam_channel::bounded(10); let cancellation_token_source = CancellationTokenSource::new(); @@ -310,6 +357,7 @@ impl MainLoop { sender: sender.clone(), receiver, watcher: None, + dependency_metadata, printer, cancellation_token, }, @@ -481,6 +529,10 @@ impl MainLoop { revision += 1; // Automatically cancels any pending queries and waits for them to complete. db.apply_changes(&changes); + let dependency_metadata = + enrich_dependency_metadata(db, self.dependency_metadata.as_ref()); + db.project() + .set_dependency_metadata(db, dependency_metadata.as_ref()); if let Some(watcher) = self.watcher.as_mut() { watcher.update(db); } diff --git a/crates/ty/tests/cli/dependency_metadata.rs b/crates/ty/tests/cli/dependency_metadata.rs new file mode 100644 index 00000000000000..fac91db6ce5aa4 --- /dev/null +++ b/crates/ty/tests/cli/dependency_metadata.rs @@ -0,0 +1,583 @@ +use insta_cmd::assert_cmd_snapshot; + +use crate::CliTest; + +#[cfg(unix)] +fn write_uv(case: &CliTest, metadata: &str) -> anyhow::Result { + use std::os::unix::fs::PermissionsExt; + + let bin = case.root().join("bin"); + let uv = bin.join("uv"); + case.write_file( + "bin/uv", + &format!( + r#"#!/bin/sh +if [ "$*" != "workspace metadata --frozen --active" ]; then + echo "unexpected arguments: $*" >&2 + exit 2 +fi +cat <<'EOF' +{} +EOF +"#, + metadata.trim() + ), + )?; + std::fs::set_permissions(&uv, std::fs::Permissions::from_mode(0o755))?; + + Ok(uv) +} + +#[cfg(unix)] +fn write_failing_uv(case: &CliTest) -> anyhow::Result { + use std::os::unix::fs::PermissionsExt; + + let bin = case.root().join("bin"); + let uv = bin.join("uv"); + case.write_file( + "bin/uv", + r#"#!/bin/sh +echo "workspace metadata is unavailable" >&2 +exit 2 +"#, + )?; + std::fs::set_permissions(&uv, std::fs::Permissions::from_mode(0o755))?; + + Ok(uv) +} + +fn write_dependency_metadata(case: &CliTest, dependencies: &str) -> anyhow::Result<()> { + let root = format!("{:?}", case.root().to_str().unwrap()); + case.write_file( + "metadata.json", + &format!( + r#" + {{ + "schema": {{"version": "preview"}}, + "members": [ + {{"name": "app", "path": {root}, "id": "app"}} + ], + "resolution": {{ + "app": {{"name": "app", "dependencies": {dependencies}}}, + "requests==2.32.0@registry+https://pypi.org/simple": {{"name": "requests", "dependencies": []}} + }}, + "module_owners": {{ + "requests": [{{"package_id": "requests==2.32.0@registry+https://pypi.org/simple"}}] + }} + }} + "# + ), + ) +} + +fn uv_workspace_metadata(case: &CliTest) -> String { + let root = format!("{:?}", case.root().to_str().unwrap()); + let environment = format!("{:?}", case.root().join(".venv").to_str().unwrap()); + format!( + r#" + {{ + "schema": {{"version": "preview"}}, + "workspace_root": {root}, + "environment": {{"root": {environment}, "python": {{"version": "3.13.0"}}}}, + "requires_python": ">=3.8", + "members": [ + {{"name": "app", "path": {root}, "id": "app"}} + ], + "resolution": {{ + "app": {{"name": "app", "dependencies": []}}, + "requests==2.32.0@registry+https://pypi.org/simple": {{"name": "requests", "dependencies": []}} + }}, + "module_owners": {{ + "requests": [{{"package_id": "requests==2.32.0@registry+https://pypi.org/simple"}}] + }} + }} + "# + ) +} + +#[cfg(unix)] +#[test] +fn dependency_metadata_is_loaded_from_uv_workspace() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("test.py", "import requests"), + ("uv.lock", ""), + ( + ".venv/lib/python3.13/site-packages/requests/__init__.py", + "", + ), + ])?; + let uv = write_uv(&case, &uv_workspace_metadata(&case))?; + + assert_cmd_snapshot!(case.command() + .arg("--warn").arg("missing-direct-dependency") + .env("TY_UV", "1") + .env("UV", uv), @" + success: false + exit_code: 1 + ----- stdout ----- + warning[missing-direct-dependency]: Third-party import `requests` is used but no direct dependency on `requests` is declared + --> test.py:1:8 + | + 1 | import requests + | ^^^^^^^^ + | + + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_metadata_is_not_loaded_without_uv_integration() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("test.py", "import requests"), + ( + ".venv/lib/python3.13/site-packages/requests/__init__.py", + "", + ), + ])?; + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency"), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + Ok(()) +} + +#[cfg(unix)] +#[test] +fn dependency_metadata_uv_failure_does_not_fail_check() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("test.py", "import requests"), + ("uv.lock", ""), + ( + ".venv/lib/python3.13/site-packages/requests/__init__.py", + "", + ), + ])?; + let uv = write_failing_uv(&case)?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .env("TY_UV", "1") + .env("UV", uv), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + WARN `uv workspace metadata` failed with status exit status: 2: workspace metadata is unavailable + "); + + Ok(()) +} + +fn write_site_packages_file(case: &CliTest, path: &str, contents: &str) -> anyhow::Result<()> { + let site_packages = if cfg!(windows) { + ".venv/Lib/site-packages" + } else { + ".venv/lib/python3.13/site-packages" + }; + case.write_file(format!("{site_packages}/{path}"), contents) +} + +fn write_site_package(case: &CliTest, module: &str) -> anyhow::Result<()> { + write_site_packages_file(case, &format!("{module}/__init__.py"), "") +} + +fn write_dependency_group_metadata( + case: &CliTest, + group_dependencies: &str, + module_owners: &str, +) -> anyhow::Result<()> { + let root = format!("{:?}", case.root().to_str().unwrap()); + case.write_file( + "metadata.json", + &format!( + r#" + {{ + "schema": {{"version": "preview"}}, + "members": [ + {{"name": "app", "path": {root}, "id": "app"}} + ], + "resolution": {{ + "app": {{ + "name": "app", + "dependencies": [], + "dependency_groups": [ + {{"name": "dev", "id": "app:dev"}} + ] + }}, + "app:dev": {{"dependencies": {group_dependencies}}}, + "inline-snapshot==0.20.8@registry+https://pypi.org/simple": {{"name": "inline-snapshot", "dependencies": []}}, + "pyx-test==1.0.0@registry+https://pypi.org/simple": {{ + "name": "pyx-test", + "dependencies": [ + {{"id": "pytest==8.0.0@registry+https://pypi.org/simple"}} + ] + }}, + "pytest==8.0.0@registry+https://pypi.org/simple": {{"name": "pytest", "dependencies": []}} + }}, + "module_owners": {{ + {module_owners} + }} + }} + "# + ), + ) +} + +#[test] +fn missing_direct_dependency_is_disabled_by_default() -> anyhow::Result<()> { + let case = CliTest::with_file("test.py", "import requests")?; + write_site_package(&case, "requests")?; + write_dependency_metadata(&case, "[]")?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_metadata_supports_missing_direct_dependency_lint() -> anyhow::Result<()> { + let case = CliTest::with_file("test.py", "import requests")?; + write_site_package(&case, "requests")?; + + write_dependency_metadata(&case, "[]")?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: false + exit_code: 1 + ----- stdout ----- + warning[missing-direct-dependency]: Third-party import `requests` is used but no direct dependency on `requests` is declared + --> test.py:1:8 + | + 1 | import requests + | ^^^^^^^^ + | + + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_metadata_supports_concise_diagnostics() -> anyhow::Result<()> { + let case = CliTest::with_file("test.py", "import requests")?; + write_site_package(&case, "requests")?; + write_dependency_metadata(&case, "[]")?; + + let output = case + .command() + .arg("--python") + .arg(".venv") + .arg("--warn") + .arg("missing-direct-dependency") + .arg("--dependency-metadata") + .arg("metadata.json") + .arg("--output-format=concise") + .output()?; + + assert_eq!(output.status.code(), Some(1)); + assert!(String::from_utf8(output.stdout)?.contains( + "test.py:1:8: warning[missing-direct-dependency] Third-party import `requests` is used but no direct dependency on `requests` is declared" + )); + + Ok(()) +} + +#[test] +fn dependency_metadata_infers_editable_module_owners() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("test.py", "import lib_module"), + ("libs/lib/src/lib_module/__init__.py", ""), + ])?; + let root = format!("{:?}", case.root().to_str().unwrap()); + let lib = format!("{:?}", case.root().join("libs/lib").to_str().unwrap()); + let editable = case.root().join("libs/lib/src"); + let editable = editable.to_str().unwrap(); + + write_site_packages_file(&case, "_lib.pth", editable)?; + case.write_file( + "metadata.json", + &format!( + r#" + {{ + "schema": {{"version": "preview"}}, + "members": [ + {{"name": "app", "path": {root}, "id": "app"}}, + {{"name": "lib-project", "path": {lib}, "id": "lib-project"}} + ], + "resolution": {{ + "app": {{"name": "app", "dependencies": []}}, + "lib-project": {{"name": "lib-project", "dependencies": []}} + }}, + "module_owners": {{}} + }} + "# + ), + )?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: false + exit_code: 1 + ----- stdout ----- + warning[missing-direct-dependency]: Third-party import `lib_module` is used but no direct dependency on `lib-project` is declared + --> test.py:1:8 + | + 1 | import lib_module + | ^^^^^^^^^^ + | + + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_group_dependency_is_reported_in_package_code() -> anyhow::Result<()> { + let case = CliTest::with_files([("src/app/__init__.py", "import inline_snapshot")])?; + write_site_package(&case, "inline_snapshot")?; + write_dependency_group_metadata( + &case, + r#"[{"id": "inline-snapshot==0.20.8@registry+https://pypi.org/simple"}]"#, + r#" + "app": [{"package_id": "app"}], + "inline_snapshot": [{"package_id": "inline-snapshot==0.20.8@registry+https://pypi.org/simple"}] + "#, + )?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: false + exit_code: 1 + ----- stdout ----- + warning[missing-direct-dependency]: Third-party import `inline_snapshot` is used but no direct dependency on `inline-snapshot` is declared + --> src/app/__init__.py:1:8 + | + 1 | import inline_snapshot + | ^^^^^^^^^^^^^^^ + | + + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_group_dependency_is_allowed_in_non_package_file() -> anyhow::Result<()> { + let case = CliTest::with_files([("tests/test_app.py", "import inline_snapshot")])?; + write_site_package(&case, "inline_snapshot")?; + write_dependency_group_metadata( + &case, + r#"[{"id": "inline-snapshot==0.20.8@registry+https://pypi.org/simple"}]"#, + r#" + "app": [{"package_id": "app"}], + "inline_snapshot": [{"package_id": "inline-snapshot==0.20.8@registry+https://pypi.org/simple"}] + "#, + )?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_group_dependency_does_not_allow_transitive_dependency() -> anyhow::Result<()> { + let case = CliTest::with_files([("tests/test_app.py", "import pytest")])?; + write_site_package(&case, "pytest")?; + write_dependency_group_metadata( + &case, + r#"[{"id": "pyx-test==1.0.0@registry+https://pypi.org/simple"}]"#, + r#""pytest": [{"package_id": "pytest==8.0.0@registry+https://pypi.org/simple"}]"#, + )?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: false + exit_code: 1 + ----- stdout ----- + warning[missing-direct-dependency]: Third-party import `pytest` is used but no direct dependency on `pytest` is declared + --> tests/test_app.py:1:8 + | + 1 | import pytest + | ^^^^^^ + | + + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_group_dependency_is_reported_in_editable_package_code() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ( + "ty.toml", + r#" + [environment] + root = ["tests"] + "#, + ), + ("src/app/__init__.py", "import inline_snapshot"), + ("tests/__init__.py", ""), + ])?; + let editable = case.root().join("src"); + let editable = editable.to_str().unwrap(); + write_site_packages_file(&case, "_app.pth", editable)?; + write_site_package(&case, "inline_snapshot")?; + write_dependency_group_metadata( + &case, + r#"[{"id": "inline-snapshot==0.20.8@registry+https://pypi.org/simple"}]"#, + r#""inline_snapshot": [{"package_id": "inline-snapshot==0.20.8@registry+https://pypi.org/simple"}]"#, + )?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .arg("--dependency-metadata").arg("metadata.json") + .arg("src/app/__init__.py"), @" + success: false + exit_code: 1 + ----- stdout ----- + warning[missing-direct-dependency]: Third-party import `inline_snapshot` is used but no direct dependency on `inline-snapshot` is declared + --> src/app/__init__.py:1:8 + | + 1 | import inline_snapshot + | ^^^^^^^^^^^^^^^ + | + + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn declared_dependency_is_not_reported() -> anyhow::Result<()> { + let case = CliTest::with_file("test.py", "import requests")?; + write_site_package(&case, "requests")?; + + write_dependency_metadata( + &case, + r#"[{"id": "requests==2.32.0@registry+https://pypi.org/simple"}]"#, + )?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--warn").arg("missing-direct-dependency") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn dependency_metadata_applies_with_multiple_overrides() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ( + "pyproject.toml", + r#" + [tool.ty.rules] + division-by-zero = "error" + missing-direct-dependency = "warn" + + [[tool.ty.overrides]] + include = ["*.py"] + + [tool.ty.overrides.analysis] + respect-type-ignore-comments = true + + [[tool.ty.overrides]] + include = ["test.py"] + + [tool.ty.overrides.rules] + division-by-zero = "ignore" + "#, + ), + ("test.py", "import requests"), + ])?; + write_site_package(&case, "requests")?; + + write_dependency_metadata(&case, "[]")?; + + assert_cmd_snapshot!(case.command() + .arg("--python").arg(".venv") + .arg("--dependency-metadata").arg("metadata.json"), @" + success: false + exit_code: 1 + ----- stdout ----- + warning[missing-direct-dependency]: Third-party import `requests` is used but no direct dependency on `requests` is declared + --> test.py:1:8 + | + 1 | import requests + | ^^^^^^^^ + | + + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index 67d5672c380d3d..8b53ea34a91a40 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -1,5 +1,6 @@ mod analysis_options; mod config_option; +mod dependency_metadata; mod exit_code; mod file_selection; mod fixes; diff --git a/crates/ty_module_resolver/src/path.rs b/crates/ty_module_resolver/src/path.rs index efada7b3772e29..0b9df5660a4942 100644 --- a/crates/ty_module_resolver/src/path.rs +++ b/crates/ty_module_resolver/src/path.rs @@ -623,6 +623,11 @@ impl SearchPath { } } + /// Is the module in an editable install discovered from site-packages? + pub fn is_editable(&self) -> bool { + matches!(&*self.0, SearchPathInner::Editable(_)) + } + fn is_valid_extension(&self, extension: &str) -> bool { if self.is_standard_library() { extension == "pyi" diff --git a/crates/ty_project/src/dependency_metadata.rs b/crates/ty_project/src/dependency_metadata.rs new file mode 100644 index 00000000000000..9ae509ba59ae3c --- /dev/null +++ b/crates/ty_project/src/dependency_metadata.rs @@ -0,0 +1,555 @@ +use std::collections::{BTreeMap, BTreeSet}; + +use anyhow::{Context, Result, bail}; +use ruff_db::system::{SystemPath, SystemPathBuf}; +use serde::Deserialize; +use serde_json::{Map, Value}; +use ty_module_resolver::{Db as ModuleResolverDb, list_modules}; +use ty_python_semantic::dependency::{DependencyMetadata, DependencyProject, ModuleOwner}; + +#[derive(Debug, Deserialize)] +struct UvWorkspaceMetadata { + schema: UvSchema, + members: Vec, + #[serde(default)] + resolution: Map, + #[serde(default)] + module_owners: BTreeMap>, +} + +#[derive(Debug, Deserialize)] +struct UvModuleOwner { + package_id: String, +} + +#[derive(Debug, Deserialize)] +struct UvSchema { + version: String, +} + +#[derive(Debug, Deserialize)] +struct UvWorkspaceMember { + name: String, + path: String, + id: String, +} + +pub fn parse_uv_workspace_metadata(source: impl AsRef<[u8]>) -> Result { + let snapshot: UvWorkspaceMetadata = serde_json::from_slice(source.as_ref()) + .context("Failed to parse dependency metadata JSON")?; + + if snapshot.schema.version != "preview" { + bail!( + "Unsupported dependency metadata schema version `{}`; expected `preview`", + snapshot.schema.version + ); + } + + let mut projects = Vec::with_capacity(snapshot.members.len()); + for member in snapshot.members { + let package = snapshot + .resolution + .get(&member.id) + .with_context(|| format!("Missing resolution package node `{}`", member.id))?; + let direct_dependencies = direct_dependencies(package, &snapshot.resolution) + .with_context(|| format!("Failed to read dependencies for `{}`", member.name))?; + let dependency_group_dependencies = + dependency_group_dependencies(package, &snapshot.resolution).with_context(|| { + format!("Failed to read dependency groups for `{}`", member.name) + })?; + + projects.push(DependencyProject::new_with_dependency_groups( + member.name, + SystemPathBuf::from(member.path), + direct_dependencies, + dependency_group_dependencies, + )); + } + + let mut module_owners = Vec::with_capacity(snapshot.module_owners.len()); + for (module, owner_references) in snapshot.module_owners { + let owners = owner_references + .iter() + .map(|owner| { + resolve_package_reference(&owner.package_id, &snapshot.resolution).with_context( + || format!("Missing resolution package node `{}`", owner.package_id), + ) + }) + .collect::>>()?; + + let Some(module_owner) = ModuleOwner::from_module(&module, owners) else { + bail!("Invalid module owner `{module}`"); + }; + + module_owners.push(module_owner); + } + + Ok(DependencyMetadata::new(projects, module_owners)) +} + +pub fn enrich_dependency_metadata_with_editables( + db: &dyn ModuleResolverDb, + metadata: DependencyMetadata, +) -> DependencyMetadata { + let (projects, mut module_owners) = metadata.into_parts(); + let mut modules_with_owners: BTreeSet<_> = module_owners + .iter() + .map(|owner| owner.module().clone()) + .collect(); + + for module in list_modules(db) { + let Some(search_path) = module.search_path(db) else { + continue; + }; + + if !search_path.is_editable() { + continue; + } + + let Some(file) = module.file(db) else { + continue; + }; + + let Some(path) = file.path(db).as_system_path() else { + continue; + }; + + let Some(project) = project_for_path(&projects, path) else { + continue; + }; + + let module_name = module.name(db).clone(); + if !modules_with_owners.insert(module_name.clone()) { + continue; + } + + module_owners.push(ModuleOwner::new(module_name, [project.name().to_string()])); + } + + DependencyMetadata::new(projects, module_owners) +} + +fn project_for_path<'a>( + projects: &'a [DependencyProject], + path: &SystemPath, +) -> Option<&'a DependencyProject> { + projects + .iter() + .filter(|project| path.starts_with(project.path())) + .max_by_key(|project| project.path().as_str().len()) +} + +fn direct_dependencies(package: &Value, resolution: &Map) -> Result> { + let Some(dependencies) = package.get("dependencies") else { + return Ok(Vec::new()); + }; + + let dependencies = dependencies + .as_array() + .context("Expected `dependencies` to be an array")?; + + dependencies + .iter() + .map(|dependency| dependency_name(dependency, resolution)) + .collect() +} + +fn dependency_group_dependencies( + package: &Value, + resolution: &Map, +) -> Result> { + let Some(dependency_groups) = package.get("dependency_groups") else { + return Ok(Vec::new()); + }; + + let dependency_groups = dependency_groups + .as_array() + .context("Expected `dependency_groups` to be an array")?; + + let mut dependencies = Vec::new(); + for dependency_group in dependency_groups { + let dependency_group = dependency_group.as_object().with_context(|| { + format!("Expected dependency group entry to be an object, got `{dependency_group}`") + })?; + let group_id = dependency_group + .get("id") + .and_then(Value::as_str) + .context("Dependency group entry does not contain a group id")?; + let group = resolution + .get(group_id) + .with_context(|| format!("Missing resolution dependency group node `{group_id}`"))?; + dependencies.extend(direct_dependencies(group, resolution).with_context(|| { + format!("Failed to read dependencies for dependency group `{group_id}`") + })?); + } + + Ok(dependencies) +} + +fn dependency_name(dependency: &Value, resolution: &Map) -> Result { + match dependency { + Value::String(reference) => { + Ok(resolve_package_reference(reference, resolution) + .unwrap_or_else(|| reference.clone())) + } + Value::Object(object) => dependency_name_from_object(object, resolution), + _ => { + bail!("Expected dependency entry to be a string or object, got `{dependency}`") + } + } +} + +fn dependency_name_from_object( + dependency: &Map, + resolution: &Map, +) -> Result { + if let Some(name) = string_field(dependency, &["name", "package_name", "package-name"]) { + return Ok(name.to_string()); + } + + if let Some(reference) = string_field(dependency, &["id", "package_id", "package-id"]) { + return Ok(resolve_package_reference(reference, resolution) + .unwrap_or_else(|| reference.to_string())); + } + + if let Some(package) = dependency.get("package") { + return dependency_name(package, resolution); + } + + bail!("Dependency entry does not contain a dependency name or package id") +} + +fn resolve_package_reference(reference: &str, resolution: &Map) -> Option { + resolution + .get(reference) + .and_then(|package| package.get("name")) + .and_then(Value::as_str) + .map(ToString::to_string) +} + +fn string_field<'a>(object: &'a Map, fields: &[&str]) -> Option<&'a str> { + fields + .iter() + .find_map(|field| object.get(*field).and_then(Value::as_str)) +} + +#[cfg(test)] +mod tests { + use ruff_db::Db as _; + use ruff_db::system::{DbWithTestSystem, DbWithWritableSystem as _, SystemPathBuf}; + use ruff_python_ast::PythonVersion; + use ruff_python_ast::name::Name; + use ty_module_resolver::{ModuleName, SearchPathSettings}; + use ty_python_core::platform::PythonPlatform; + use ty_python_core::program::{FallibleStrategy, Program, ProgramSettings}; + use ty_python_semantic::PythonVersionWithSource; + + use crate::ProjectMetadata; + use crate::db::testing::TestDb; + + use super::{enrich_dependency_metadata_with_editables, parse_uv_workspace_metadata}; + use ty_python_semantic::dependency::{ + DependencyMetadata, DependencyProject, DistributionName, ModuleOwner, + }; + + fn setup_db(files: &[(&str, &str)]) -> anyhow::Result { + let project = + ProjectMetadata::new(Name::new_static("test"), SystemPathBuf::from("/workspace")); + let mut db = TestDb::new(project); + + db.memory_file_system() + .create_directory_all("/workspace/app")?; + db.memory_file_system() + .create_directory_all("/site-packages")?; + + for (path, contents) in files { + db.write_file(path, contents)?; + } + + let search_paths = SearchPathSettings { + src_roots: vec![SystemPathBuf::from("/workspace/app")], + site_packages_paths: vec![SystemPathBuf::from("/site-packages")], + ..SearchPathSettings::empty() + } + .to_search_paths(db.system(), db.vendored(), &FallibleStrategy)?; + + Program::from_settings( + &db, + ProgramSettings { + python_version: PythonVersionWithSource { + version: PythonVersion::latest_ty(), + source: ty_python_semantic::PythonVersionSource::Default, + }, + python_platform: PythonPlatform::default(), + search_paths, + }, + ); + + Ok(db) + } + + fn owner_names(metadata: &DependencyMetadata, module: &str) -> Vec { + let module = ModuleName::new(module).unwrap(); + + metadata + .module_owners() + .iter() + .filter(|owner| owner.module() == &module) + .flat_map(|owner| owner.owners().iter().map(ToString::to_string)) + .collect() + } + + fn dependency_names<'a>( + dependencies: impl Iterator, + ) -> Vec { + let mut dependencies = dependencies.map(ToString::to_string).collect::>(); + dependencies.sort(); + dependencies + } + + #[test] + fn parses_uv_workspace_metadata() { + let metadata = parse_uv_workspace_metadata( + r#" +{ + "schema": {"version": "preview"}, + "members": [ + { + "name": "app", + "path": "/workspace/app", + "id": "app==0.1.0@editable+/workspace/app/" + } + ], + "resolution": { + "app==0.1.0@editable+/workspace/app/": { + "name": "app", + "dependencies": [ + {"id": "requests==2.32.0@registry+https://pypi.org/simple"}, + "rich==13.0.0", + {"package": "attrs==24.0.0"} + ] + }, + "requests==2.32.0@registry+https://pypi.org/simple": {"name": "requests"}, + "rich==13.0.0": {"name": "rich"}, + "attrs==24.0.0": {"name": "attrs"} + }, + "module_owners": { + "requests": [{"package_id": "requests==2.32.0@registry+https://pypi.org/simple"}], + "rich": [{"package_id": "rich==13.0.0"}] + } +} +"#, + ) + .unwrap(); + + let debug = format!("{metadata:?}"); + assert!(debug.contains("requests")); + assert!(debug.contains("rich")); + assert!(debug.contains("attrs")); + } + + #[test] + fn parses_dependency_group_dependencies_separately() { + let metadata = parse_uv_workspace_metadata( + r#" +{ + "schema": {"version": "preview"}, + "members": [ + { + "name": "app", + "path": "/workspace/app", + "id": "app" + } + ], + "resolution": { + "app": { + "name": "app", + "dependencies": [ + {"id": "requests==2.32.0@registry+https://pypi.org/simple"} + ], + "dependency_groups": [ + {"name": "dev", "id": "app:dev"} + ] + }, + "app:dev": { + "dependencies": [ + {"id": "inline-snapshot==0.20.8@registry+https://pypi.org/simple"} + ] + }, + "requests==2.32.0@registry+https://pypi.org/simple": {"name": "requests"}, + "inline-snapshot==0.20.8@registry+https://pypi.org/simple": {"name": "inline-snapshot"} + }, + "module_owners": {} +} +"#, + ) + .unwrap(); + + let project = metadata.projects().first().unwrap(); + assert_eq!( + dependency_names(project.direct_dependencies()), + ["requests"] + ); + assert_eq!( + dependency_names(project.dependency_group_dependencies()), + ["inline-snapshot"] + ); + } + + #[test] + fn parses_nested_module_owner_map() { + let metadata = parse_uv_workspace_metadata( + r#" +{ + "schema": {"version": "preview"}, + "members": [], + "resolution": { + "gpu-a==0.1.0@path+/workspace/gpu_a-0.1.0-py3-none-any.whl": {"name": "gpu-a"}, + "gpu-b==0.1.0@path+/workspace/gpu_b-0.1.0-py3-none-any.whl": {"name": "gpu-b"} + }, + "module_owners": { + "gpu": [ + {"package_id": "gpu-a==0.1.0@path+/workspace/gpu_a-0.1.0-py3-none-any.whl"}, + {"package_id": "gpu-b==0.1.0@path+/workspace/gpu_b-0.1.0-py3-none-any.whl"} + ], + "gpu.a": [ + {"package_id": "gpu-a==0.1.0@path+/workspace/gpu_a-0.1.0-py3-none-any.whl"} + ], + "gpu.b": [ + {"package_id": "gpu-b==0.1.0@path+/workspace/gpu_b-0.1.0-py3-none-any.whl"} + ] + } +} +"#, + ) + .unwrap(); + + let debug = format!("{metadata:?}"); + assert!(debug.contains("gpu-a")); + assert!(debug.contains("gpu-b")); + assert!(debug.contains("gpu.a")); + assert!(debug.contains("gpu.b")); + assert!(!debug.contains("@path+")); + } + + #[test] + fn rejects_unsupported_schema_version() { + let error = parse_uv_workspace_metadata( + r#" +{ + "schema": {"version": "1"}, + "members": [], + "resolution": {} +} +"#, + ) + .unwrap_err(); + + assert!( + error + .to_string() + .contains("Unsupported dependency metadata schema version") + ); + } + + #[test] + fn infers_editable_module_owners() -> anyhow::Result<()> { + let db = setup_db(&[ + ("/site-packages/_lib.pth", "/workspace/lib/src"), + ("/workspace/lib/src/lib_module/__init__.py", ""), + ])?; + let metadata = DependencyMetadata::new( + vec![ + DependencyProject::new( + "app", + SystemPathBuf::from("/workspace/app"), + std::iter::empty::<&str>(), + ), + DependencyProject::new( + "lib-project", + SystemPathBuf::from("/workspace/lib"), + std::iter::empty::<&str>(), + ), + ], + vec![], + ); + + let metadata = enrich_dependency_metadata_with_editables(&db, metadata); + + assert_eq!(owner_names(&metadata, "lib_module"), ["lib-project"]); + Ok(()) + } + + #[test] + fn nested_workspace_member_uses_longest_matching_project_path() -> anyhow::Result<()> { + let db = setup_db(&[ + ("/site-packages/_parent.pth", "/workspace/parent"), + ("/workspace/parent/child/__init__.py", ""), + ])?; + let metadata = DependencyMetadata::new( + vec![ + DependencyProject::new( + "parent-project", + SystemPathBuf::from("/workspace/parent"), + std::iter::empty::<&str>(), + ), + DependencyProject::new( + "child-project", + SystemPathBuf::from("/workspace/parent/child"), + std::iter::empty::<&str>(), + ), + ], + vec![], + ); + + let metadata = enrich_dependency_metadata_with_editables(&db, metadata); + + assert_eq!(owner_names(&metadata, "child"), ["child-project"]); + Ok(()) + } + + #[test] + fn explicit_module_owners_are_preserved() -> anyhow::Result<()> { + let db = setup_db(&[ + ("/site-packages/_editable.pth", "/workspace/editable/src"), + ("/workspace/editable/src/editable/__init__.py", ""), + ])?; + let metadata = DependencyMetadata::new( + vec![DependencyProject::new( + "editable-project", + SystemPathBuf::from("/workspace/editable"), + std::iter::empty::<&str>(), + )], + vec![ModuleOwner::new( + ModuleName::new_static("editable").unwrap(), + ["explicit-owner"], + )], + ); + + let metadata = enrich_dependency_metadata_with_editables(&db, metadata); + + assert_eq!(owner_names(&metadata, "editable"), ["explicit-owner"]); + Ok(()) + } + + #[test] + fn namespace_editable_modules_are_skipped() -> anyhow::Result<()> { + let db = setup_db(&[ + ("/site-packages/_namespace.pth", "/workspace/namespace/src"), + ("/workspace/namespace/src/ns_pkg/submodule.py", ""), + ])?; + let metadata = DependencyMetadata::new( + vec![DependencyProject::new( + "namespace-project", + SystemPathBuf::from("/workspace/namespace"), + std::iter::empty::<&str>(), + )], + vec![], + ); + + let metadata = enrich_dependency_metadata_with_editables(&db, metadata); + + assert_eq!(owner_names(&metadata, "ns_pkg"), Vec::::new()); + Ok(()) + } +} diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index aa61f4fe3f825e..350447169090b7 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -27,9 +27,11 @@ use std::collections::{BTreeSet, hash_set}; use std::iter::FusedIterator; use std::panic::{AssertUnwindSafe, UnwindSafe}; use std::sync::Arc; +use ty_python_semantic::dependency::DependencyMetadata; use ty_python_semantic::lint::RuleSelection; mod db; +pub mod dependency_metadata; mod files; pub mod glob; pub mod metadata; @@ -248,7 +250,22 @@ impl Project { self.settings(db).to_rules() } - /// Returns whether `path` is part of the project and included (see `included_paths_list`). + pub fn set_dependency_metadata( + self, + db: &mut dyn Db, + dependency_metadata: Option<&Arc>, + ) { + let settings = self + .settings(db) + .clone() + .with_dependency_metadata(dependency_metadata); + + if self.settings(db) != &settings { + self.set_settings(db).to(Box::new(settings)); + } + } + + /// Returns `true` if `path` is both part of the project and included (see `included_paths_list`). /// /// Unlike [Self::files], this method does not respect `.gitignore` files. It only checks /// the project's include and exclude settings as well as the paths that were passed to `ty check `. diff --git a/crates/ty_project/src/metadata.rs b/crates/ty_project/src/metadata.rs index b87c086ff403f3..b68fbae6c9f56c 100644 --- a/crates/ty_project/src/metadata.rs +++ b/crates/ty_project/src/metadata.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use thiserror::Error; use ty_combine::Combine; use ty_python_core::program::{FallibleStrategy, MisconfigurationStrategy, ProgramSettings}; +use ty_python_semantic::dependency::DependencyMetadata; use ty_static::EnvVars; use crate::Db; @@ -456,6 +457,12 @@ impl ProjectMetadata { self.uv_workspace.is_some() } + pub fn uv_dependency_metadata(&self) -> Option<&DependencyMetadata> { + self.uv_workspace + .as_ref() + .and_then(uv::UvWorkspace::dependency_metadata) + } + /// Applies lower-precedence options to this project. /// /// Options applied later take precedence over options applied earlier, but all fallback options diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 8c586ea8ce9c62..23fe27a509e1e4 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -1656,6 +1656,7 @@ impl AnalysisOptions { respect_type_ignore_comments: respect_type_ignore_default, allowed_unresolved_imports: allowed_unresolved_imports_default, replace_imports_with_any: replace_imports_with_any_default, + dependency_metadata: dependency_metadata_default, } = AnalysisSettings::default(); let allowed_unresolved_imports = @@ -1687,6 +1688,7 @@ impl AnalysisOptions { .unwrap_or(respect_type_ignore_default), allowed_unresolved_imports, replace_imports_with_any, + dependency_metadata: dependency_metadata_default, } } } diff --git a/crates/ty_project/src/metadata/settings.rs b/crates/ty_project/src/metadata/settings.rs index 9ba362b57d27a1..06493244fffb18 100644 --- a/crates/ty_project/src/metadata/settings.rs +++ b/crates/ty_project/src/metadata/settings.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use ruff_db::files::File; use ty_combine::Combine; use ty_python_semantic::AnalysisSettings; +use ty_python_semantic::dependency::DependencyMetadata; use ty_python_semantic::lint::RuleSelection; use crate::metadata::options::{InnerOverrideOptions, Options, OutputFormat}; @@ -61,6 +62,32 @@ impl Settings { pub fn analysis(&self) -> &AnalysisSettings { &self.analysis } + + pub(crate) fn with_dependency_metadata( + mut self, + dependency_metadata: Option<&Arc>, + ) -> Self { + let dependency_metadata = dependency_metadata.cloned(); + self.analysis + .dependency_metadata + .clone_from(&dependency_metadata); + + self.overrides = self + .overrides + .into_iter() + .map(|mut override_| { + let mut settings = (*override_.settings).clone(); + settings + .analysis + .dependency_metadata + .clone_from(&dependency_metadata); + override_.settings = Arc::new(settings); + override_ + }) + .collect(); + + self + } } #[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)] @@ -240,7 +267,10 @@ fn merge_overrides(db: &dyn Db, overrides: Vec>, _: () // It's okay to ignore the errors here because the rules are eagerly validated // during `overrides.to_settings()`. let rules = rules.to_rule_selection(db, &mut Vec::new()); - let analysis = analysis.to_settings(db, &mut Vec::new()); + let mut analysis = analysis.to_settings(db, &mut Vec::new()); + analysis + .dependency_metadata + .clone_from(&db.project().settings(db).analysis().dependency_metadata); FileSettings::File(Arc::new(OverrideSettings { rules, analysis })) } diff --git a/crates/ty_project/src/metadata/uv.rs b/crates/ty_project/src/metadata/uv.rs index 1d46104be76ce9..1c80c97abcd1b7 100644 --- a/crates/ty_project/src/metadata/uv.rs +++ b/crates/ty_project/src/metadata/uv.rs @@ -5,8 +5,11 @@ use ruff_db::system::{System, SystemPath, SystemPathBuf}; use ruff_ranged_value::{RangedValue, ValueSource}; use serde::Deserialize; use thiserror::Error; +use ty_python_semantic::dependency::DependencyMetadata; use ty_static::EnvVars; +use crate::dependency_metadata::parse_uv_workspace_metadata; + use super::python_version::SupportedPythonVersion; #[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)] @@ -14,6 +17,7 @@ pub(super) struct UvWorkspace { root: SystemPathBuf, environment: Option, python_version: Option>, + dependency_metadata: Option, } impl UvWorkspace { @@ -46,12 +50,22 @@ impl UvWorkspace { } pub(super) fn from_metadata( - metadata: &[u8], + source: &[u8], system: &dyn System, ) -> Result { - let metadata = serde_json::from_slice::(metadata) + let metadata = serde_json::from_slice::(source) .map_err(UvWorkspaceError::InvalidMetadata)?; + let dependency_metadata = match parse_uv_workspace_metadata(source) { + Ok(metadata) => Some(metadata), + Err(error) => { + tracing::debug!( + "Failed to read dependency data from `uv workspace metadata` output: {error:#}" + ); + None + } + }; + let root = existing_directory(metadata.workspace_root, "workspace root", system)?; let (environment, python_version) = match metadata.environment { @@ -70,6 +84,7 @@ impl UvWorkspace { root, environment, python_version, + dependency_metadata, }) } @@ -84,6 +99,10 @@ impl UvWorkspace { pub(super) fn python_version(&self) -> Option<&RangedValue> { self.python_version.as_ref() } + + pub(super) fn dependency_metadata(&self) -> Option<&DependencyMetadata> { + self.dependency_metadata.as_ref() + } } fn resolve_python_version( diff --git a/crates/ty_python_semantic/src/db.rs b/crates/ty_python_semantic/src/db.rs index 16e02dc948a872..00b44b5c61b7c2 100644 --- a/crates/ty_python_semantic/src/db.rs +++ b/crates/ty_python_semantic/src/db.rs @@ -106,6 +106,14 @@ pub(crate) mod tests { pub(crate) fn clear_salsa_events(&mut self) { self.take_salsa_events(); } + + pub(crate) fn set_analysis_settings(&mut self, settings: AnalysisSettings) { + self.analysis_settings = Arc::new(settings); + } + + pub(crate) fn set_rule_selection(&mut self, rule_selection: RuleSelection) { + self.rule_selection = Arc::new(rule_selection); + } } impl DbWithTestSystem for TestDb { diff --git a/crates/ty_python_semantic/src/dependency.rs b/crates/ty_python_semantic/src/dependency.rs new file mode 100644 index 00000000000000..1a3af739d9c1d3 --- /dev/null +++ b/crates/ty_python_semantic/src/dependency.rs @@ -0,0 +1,959 @@ +use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticId, Severity, Span}; +use ruff_db::files::File; +use ruff_db::parsed::parsed_module; +use ruff_db::system::{SystemPath, SystemPathBuf}; +use ruff_python_ast::statement_visitor::{StatementVisitor, walk_stmt}; +use ruff_python_ast::{self as ast}; +use ruff_text_size::{Ranged, TextRange}; +use rustc_hash::FxHashSet; +use std::fmt; +use ty_module_resolver::{ + Module, ModuleName, ModuleNameResolutionError, SearchPath, file_to_module, resolve_module, +}; +use ty_python_core::scope::{FileScopeId, NodeWithScopeRef}; +use ty_python_core::{SemanticIndex, semantic_index}; + +use crate::lint::{Level, LintId, LintSource, LintStatus}; +use crate::reachability::is_range_reachable; +use crate::suppression::suppressions; +use crate::types::TypeCheckDiagnostics; +use crate::{Db, declare_lint}; + +declare_lint! { + /// ## What it does + /// Checks for third-party imports that are used without a matching direct dependency + /// declaration. + /// + /// ## Why is this bad? + /// Importing a package that is only available transitively can make the project break when + /// dependency resolution changes. + /// + /// ## Rule status + /// This rule is disabled by default. + /// + /// ## Examples + /// ```python + /// import requests # requests is not declared as a direct dependency + /// ``` + pub(crate) static MISSING_DIRECT_DEPENDENCY = { + summary: "detects third-party imports without direct dependency declarations", + status: LintStatus::stable("0.0.64"), + default_level: Level::Ignore, + } +} + +/// Dependency metadata supplied by the package manager. +/// +/// This is intentionally smaller than the full `uv workspace metadata` schema. `uv` can keep +/// exporting its normalized graph, while ty normalizes only the fields it needs for import-driven +/// dependency linting. +#[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)] +pub struct DependencyMetadata { + projects: Vec, + module_owners: Vec, +} + +impl DependencyMetadata { + pub fn new(projects: Vec, module_owners: Vec) -> Self { + Self { + projects, + module_owners, + } + } + + pub fn projects(&self) -> &[DependencyProject] { + &self.projects + } + + pub fn module_owners(&self) -> &[ModuleOwner] { + &self.module_owners + } + + pub fn into_parts(self) -> (Vec, Vec) { + (self.projects, self.module_owners) + } + + fn project_for_file(&self, db: &dyn Db, file: File) -> Option<&DependencyProject> { + let path = file.path(db).as_system_path()?; + + self.projects + .iter() + .filter(|project| path.starts_with(project.path())) + .max_by_key(|project| project.path().as_str().len()) + } + + fn ownership_for_module(&self, module: &ModuleName) -> ModuleOwnership<'_> { + let mut best_component_count = 0; + let mut best_owners: Option<&[DistributionName]> = None; + + for owner in &self.module_owners { + if !module.starts_with(owner.module()) { + continue; + } + + let component_count = owner.module().components().count(); + if component_count > best_component_count { + best_component_count = component_count; + best_owners = Some(owner.owners()); + } + } + + match best_owners { + None | Some([]) => ModuleOwnership::Unknown, + Some([owner]) => ModuleOwnership::Unique(owner), + Some(_) => ModuleOwnership::Ambiguous, + } + } + + fn dependency_context_for_file( + &self, + db: &dyn Db, + file: File, + project: &DependencyProject, + ) -> DependencyContext { + let Some(module) = file_to_module(db, file) else { + return DependencyContext::NonPackage; + }; + + match self.ownership_for_module(module.name(db)) { + ModuleOwnership::Unique(owner) if owner == project.name() => DependencyContext::Package, + ModuleOwnership::Unique(_) | ModuleOwnership::Ambiguous | ModuleOwnership::Unknown => { + DependencyContext::NonPackage + } + } + } +} + +/// A workspace member or project whose imports are checked against direct dependencies. +#[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)] +pub struct DependencyProject { + name: DistributionName, + path: SystemPathBuf, + direct_dependencies: FxHashSet, + dependency_group_dependencies: FxHashSet, +} + +impl DependencyProject { + pub fn new( + name: impl Into, + path: SystemPathBuf, + direct_dependencies: impl IntoIterator>, + ) -> Self { + Self::new_with_dependency_groups( + name, + path, + direct_dependencies, + std::iter::empty::(), + ) + } + + pub fn new_with_dependency_groups( + name: impl Into, + path: SystemPathBuf, + direct_dependencies: impl IntoIterator>, + dependency_group_dependencies: impl IntoIterator>, + ) -> Self { + Self { + name: DistributionName::new(name), + path, + direct_dependencies: dependency_set(direct_dependencies), + dependency_group_dependencies: dependency_set(dependency_group_dependencies), + } + } + + pub fn name(&self) -> &DistributionName { + &self.name + } + + pub fn path(&self) -> &SystemPath { + &self.path + } + + pub fn direct_dependencies(&self) -> impl Iterator { + self.direct_dependencies.iter() + } + + pub fn dependency_group_dependencies(&self) -> impl Iterator { + self.dependency_group_dependencies.iter() + } + + fn declares_dependency( + &self, + dependency: &DistributionName, + context: DependencyContext, + ) -> bool { + self.name == *dependency + || self.direct_dependencies.contains(dependency) + || (context == DependencyContext::NonPackage + && self.dependency_group_dependencies.contains(dependency)) + } +} + +fn dependency_set( + dependencies: impl IntoIterator>, +) -> FxHashSet { + dependencies + .into_iter() + .map(DistributionName::new) + .collect() +} + +/// A mapping from an importable module prefix to the distributions that provide it. +#[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)] +pub struct ModuleOwner { + module: ModuleName, + owners: Vec, +} + +impl ModuleOwner { + pub fn new(module: ModuleName, owners: impl IntoIterator>) -> Self { + let mut unique_owners = Vec::new(); + for owner in owners { + let owner = DistributionName::new(owner); + if !unique_owners.contains(&owner) { + unique_owners.push(owner); + } + } + + Self { + module, + owners: unique_owners, + } + } + + pub fn from_module( + module: &str, + owners: impl IntoIterator>, + ) -> Option { + Some(Self::new(ModuleName::new(module)?, owners)) + } + + pub fn module(&self) -> &ModuleName { + &self.module + } + + pub fn owners(&self) -> &[DistributionName] { + &self.owners + } +} + +#[derive(Clone, PartialEq, Eq, Hash, get_size2::GetSize)] +pub struct DistributionName { + normalized: String, + display: String, +} + +impl DistributionName { + pub fn new(name: impl Into) -> Self { + let display = name.into(); + Self { + normalized: normalize_distribution_name(&display), + display, + } + } +} + +impl fmt::Debug for DistributionName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.display.fmt(f) + } +} + +impl fmt::Display for DistributionName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.display.fmt(f) + } +} + +fn normalize_distribution_name(name: &str) -> String { + let mut normalized = String::with_capacity(name.len()); + let mut previous_was_separator = false; + + for char in name.chars() { + if matches!(char, '-' | '_' | '.') { + if !previous_was_separator { + normalized.push('-'); + previous_was_separator = true; + } + } else { + normalized.push(char.to_ascii_lowercase()); + previous_was_separator = false; + } + } + + normalized +} + +enum ModuleOwnership<'a> { + Unique(&'a DistributionName), + Ambiguous, + Unknown, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum DependencyContext { + Package, + NonPackage, +} + +#[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)] +pub(crate) struct ImportFact { + range: TextRange, + highlight_range: TextRange, + requested_module: Option, + imported_module: Option, + resolution: ImportResolution, + context: ImportContext, + kind: ImportFactKind, + scope: FileScopeId, +} + +impl ImportFact { + fn resolved_third_party_module(&self) -> Option<&ModuleName> { + match &self.resolution { + ImportResolution::Resolved { + source: ImportSource::ThirdParty, + } => self.imported_module.as_ref(), + ImportResolution::Resolved { .. } + | ImportResolution::Unresolved + | ImportResolution::InvalidSyntax => None, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, get_size2::GetSize)] +enum ImportFactKind { + Import, + ImportFrom { is_star: bool }, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, get_size2::GetSize)] +enum ImportContext { + Runtime, + TypeChecking, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, get_size2::GetSize)] +enum ImportResolution { + Resolved { source: ImportSource }, + Unresolved, + InvalidSyntax, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, get_size2::GetSize)] +enum ImportSource { + StandardLibrary, + FirstParty, + ThirdParty, + Other, +} + +#[salsa::tracked(returns(ref), no_eq, heap_size=ruff_memory_usage::heap_size)] +pub(crate) fn import_facts(db: &dyn Db, file: File) -> Vec { + let module = parsed_module(db, file).load(db); + let index = semantic_index(db, file); + let mut collector = ImportFactsCollector::new(db, file, index); + + collector.visit_body(module.suite()); + collector.finish() +} + +pub(crate) fn register_lints(registry: &mut crate::lint::LintRegistryBuilder) { + registry.register_lint(&MISSING_DIRECT_DEPENDENCY); +} + +pub(crate) fn check_dependency_lints(db: &dyn Db, file: File) -> TypeCheckDiagnostics { + let mut diagnostics = TypeCheckDiagnostics::default(); + let Some(metadata) = db.analysis_settings(file).dependency_metadata.as_deref() else { + return diagnostics; + }; + + let Some(project) = metadata.project_for_file(db, file) else { + return diagnostics; + }; + + let lint_id = LintId::of(&MISSING_DIRECT_DEPENDENCY); + let Some((severity, source)) = db.rule_selection(file).get(lint_id) else { + return diagnostics; + }; + + let suppressions = suppressions(db, file); + let index = semantic_index(db, file); + let mut reported = FxHashSet::default(); + let dependency_context = metadata.dependency_context_for_file(db, file, project); + + for fact in import_facts(db, file) { + if fact.context == ImportContext::TypeChecking { + continue; + } + + let Some(imported_module) = fact.resolved_third_party_module() else { + continue; + }; + + if !is_range_reachable(db, index, fact.scope, fact.range) { + continue; + } + + let ModuleOwnership::Unique(owner) = metadata.ownership_for_module(imported_module) else { + continue; + }; + + if project.declares_dependency(owner, dependency_context) { + continue; + } + + if let Some(suppression) = suppressions.find_suppression(fact.highlight_range, lint_id) { + diagnostics.mark_used(suppression.id()); + continue; + } + + if !reported.insert(owner.clone()) { + continue; + } + + diagnostics.push(missing_direct_dependency_diagnostic( + file, + fact, + owner, + severity, + source, + db.verbose(), + )); + } + + diagnostics +} + +fn missing_direct_dependency_diagnostic( + file: File, + fact: &ImportFact, + owner: &DistributionName, + severity: Severity, + source: LintSource, + verbose: bool, +) -> Diagnostic { + let module = fact + .imported_module + .as_ref() + .or(fact.requested_module.as_ref()) + .expect("missing-direct-dependency diagnostics require an imported module"); + + let mut diagnostic = Diagnostic::new( + DiagnosticId::Lint(MISSING_DIRECT_DEPENDENCY.name()), + severity, + format_args!( + "Third-party import `{module}` is used but no direct dependency on `{owner}` is declared" + ), + ); + + diagnostic.set_documentation_url(Some(MISSING_DIRECT_DEPENDENCY.documentation_url())); + diagnostic.annotate(Annotation::primary( + Span::from(file).with_range(fact.highlight_range), + )); + + if matches!(fact.kind, ImportFactKind::ImportFrom { is_star: true }) { + diagnostic.info("The import is a star import; dependency ownership was inferred from the imported module."); + } + + if verbose { + diagnostic.info(match source { + LintSource::Default => "rule `missing-direct-dependency` is enabled by default", + LintSource::Cli => "rule `missing-direct-dependency` was selected on the command line", + LintSource::File => { + "rule `missing-direct-dependency` was selected in the configuration file" + } + LintSource::Editor => { + "rule `missing-direct-dependency` was selected in the editor settings" + } + LintSource::UvWorkspace => { + "rule `missing-direct-dependency` was selected by uv workspace metadata" + } + }); + } + + diagnostic +} + +struct ImportFactsCollector<'db> { + db: &'db dyn Db, + file: File, + index: &'db SemanticIndex<'db>, + scope: FileScopeId, + facts: Vec, +} + +impl<'db> ImportFactsCollector<'db> { + fn new(db: &'db dyn Db, file: File, index: &'db SemanticIndex<'db>) -> Self { + Self { + db, + file, + index, + scope: FileScopeId::global(), + facts: Vec::new(), + } + } + + fn finish(self) -> Vec { + self.facts + } + + fn with_scope(&mut self, scope: FileScopeId, f: impl FnOnce(&mut Self)) { + let previous = self.scope; + self.scope = scope; + f(self); + self.scope = previous; + } + + fn import_context(&self, range: TextRange) -> ImportContext { + if self.index.is_in_type_checking_block(self.scope, range) { + ImportContext::TypeChecking + } else { + ImportContext::Runtime + } + } + + fn push_import_fact(&mut self, alias: &ast::Alias) { + let imported_module = ModuleName::new(&alias.name); + let resolution = imported_module + .as_ref() + .map(|module_name| self.resolve_import(module_name)) + .unwrap_or(ImportResolution::InvalidSyntax); + + self.facts.push(ImportFact { + range: alias.range(), + highlight_range: alias.range(), + requested_module: imported_module.clone(), + imported_module, + resolution, + context: self.import_context(alias.range()), + kind: ImportFactKind::Import, + scope: self.scope, + }); + } + + fn push_import_from_facts(&mut self, import_from: &ast::StmtImportFrom) { + let requested_module = + match ModuleName::from_import_statement(self.db, self.file, import_from) { + Ok(module_name) => Some(module_name), + Err(ModuleNameResolutionError::InvalidSyntax) => None, + Err( + ModuleNameResolutionError::TooManyDots + | ModuleNameResolutionError::UnknownCurrentModule, + ) => { + for alias in &import_from.names { + self.facts.push(ImportFact { + range: alias.range(), + highlight_range: import_from_highlight_range(import_from, alias), + requested_module: None, + imported_module: None, + resolution: ImportResolution::Unresolved, + context: self.import_context(alias.range()), + kind: ImportFactKind::ImportFrom { + is_star: alias.name.as_str() == "*", + }, + scope: self.scope, + }); + } + return; + } + }; + + let Some(requested_module) = requested_module else { + for alias in &import_from.names { + self.facts.push(ImportFact { + range: alias.range(), + highlight_range: import_from_highlight_range(import_from, alias), + requested_module: None, + imported_module: None, + resolution: ImportResolution::InvalidSyntax, + context: self.import_context(alias.range()), + kind: ImportFactKind::ImportFrom { + is_star: alias.name.as_str() == "*", + }, + scope: self.scope, + }); + } + return; + }; + + let base_resolution = self.resolve_import(&requested_module); + + for alias in &import_from.names { + let is_star = alias.name.as_str() == "*"; + let imported_module = if is_star { + requested_module.clone() + } else { + self.resolve_imported_member(&requested_module, &alias.name) + }; + + let resolution = if imported_module == requested_module { + base_resolution + } else { + self.resolve_import(&imported_module) + }; + + self.facts.push(ImportFact { + range: alias.range(), + highlight_range: import_from_highlight_range(import_from, alias), + requested_module: Some(requested_module.clone()), + imported_module: Some(imported_module), + resolution, + context: self.import_context(alias.range()), + kind: ImportFactKind::ImportFrom { is_star }, + scope: self.scope, + }); + } + } + + fn resolve_imported_member(&self, module_name: &ModuleName, member: &str) -> ModuleName { + let Some(member_name) = ModuleName::new(member) else { + return module_name.clone(); + }; + + let mut submodule_name = module_name.clone(); + submodule_name.extend(&member_name); + + if resolve_module(self.db, self.file, &submodule_name).is_some() { + submodule_name + } else { + module_name.clone() + } + } + + fn resolve_import(&self, module_name: &ModuleName) -> ImportResolution { + resolve_module(self.db, self.file, module_name) + .map(|module| ImportResolution::Resolved { + source: import_source(self.db, module), + }) + .unwrap_or(ImportResolution::Unresolved) + } +} + +impl<'ast> StatementVisitor<'ast> for ImportFactsCollector<'_> { + fn visit_stmt(&mut self, stmt: &'ast ast::Stmt) { + match stmt { + ast::Stmt::Import(import) => { + for alias in &import.names { + self.push_import_fact(alias); + } + } + ast::Stmt::ImportFrom(import_from) => { + self.push_import_from_facts(import_from); + } + ast::Stmt::FunctionDef(function) => { + let scope = self.index.node_scope(NodeWithScopeRef::Function(function)); + self.with_scope(scope, |collector| collector.visit_body(&function.body)); + } + ast::Stmt::ClassDef(class) => { + let scope = self.index.node_scope(NodeWithScopeRef::Class(class)); + self.with_scope(scope, |collector| collector.visit_body(&class.body)); + } + _ => walk_stmt(self, stmt), + } + } +} + +fn import_from_highlight_range(import_from: &ast::StmtImportFrom, alias: &ast::Alias) -> TextRange { + import_from + .module + .as_ref() + .map_or(alias.range(), |module| module.range) +} + +fn import_source(db: &dyn Db, module: Module) -> ImportSource { + let Some(search_path) = module.search_path(db) else { + return ImportSource::Other; + }; + + search_path_source(search_path) +} + +fn search_path_source(search_path: &SearchPath) -> ImportSource { + if search_path.is_standard_library() { + ImportSource::StandardLibrary + } else if search_path.is_first_party() { + ImportSource::FirstParty + } else if search_path.is_site_packages() || search_path.is_editable() { + ImportSource::ThirdParty + } else { + ImportSource::Other + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use ruff_db::Db as _; + use ruff_db::diagnostic::Diagnostic; + use ruff_db::files::system_path_to_file; + use ruff_db::system::{DbWithTestSystem, DbWithWritableSystem, SystemPathBuf}; + use ty_module_resolver::SearchPathSettings; + use ty_python_core::platform::PythonPlatform; + use ty_python_core::program::{FallibleStrategy, Program, ProgramSettings}; + use ty_site_packages::{PythonVersionSource, PythonVersionWithSource}; + + use super::*; + use crate::db::tests::TestDb; + use crate::lint::RuleSelection; + use crate::types::check_types; + use crate::{AnalysisSettings, default_lint_registry}; + + fn setup_db( + source: &str, + direct_dependencies: &[&str], + module_owners: Vec, + ) -> anyhow::Result<(TestDb, File)> { + setup_db_with_file( + "/src/app.py", + source, + direct_dependencies, + std::iter::empty::<&str>(), + module_owners, + ) + } + + fn setup_db_with_file( + path: &str, + source: &str, + direct_dependencies: &[&str], + dependency_group_dependencies: impl IntoIterator>, + module_owners: Vec, + ) -> anyhow::Result<(TestDb, File)> { + let mut db = TestDb::new(); + let mut rule_selection = RuleSelection::from_registry(default_lint_registry()); + rule_selection.enable( + LintId::of(&MISSING_DIRECT_DEPENDENCY), + Severity::Warning, + LintSource::File, + ); + db.set_rule_selection(rule_selection); + + let src_root = SystemPathBuf::from("/src"); + let site_packages = SystemPathBuf::from("/site-packages"); + + db.memory_file_system().create_directory_all(&src_root)?; + db.memory_file_system() + .create_directory_all(&site_packages)?; + db.write_file(path, source)?; + + Program::from_settings( + &db, + ProgramSettings { + python_version: PythonVersionWithSource { + version: ruff_python_ast::PythonVersion::default(), + source: PythonVersionSource::default(), + }, + python_platform: PythonPlatform::default(), + search_paths: SearchPathSettings { + src_roots: vec![src_root.clone()], + site_packages_paths: vec![site_packages], + ..SearchPathSettings::empty() + } + .to_search_paths(db.system(), db.vendored(), &FallibleStrategy)?, + }, + ); + + db.set_analysis_settings(AnalysisSettings { + dependency_metadata: Some(Arc::new(DependencyMetadata::new( + vec![DependencyProject::new_with_dependency_groups( + "app", + src_root, + direct_dependencies.iter().copied(), + dependency_group_dependencies, + )], + module_owners, + ))), + ..AnalysisSettings::default() + }); + + let file = system_path_to_file(&db, path)?; + Ok((db, file)) + } + + fn messages(diagnostics: &[Diagnostic]) -> Vec<&str> { + diagnostics + .iter() + .map(Diagnostic::primary_message) + .collect() + } + + fn primary_highlight<'a>(source: &'a str, diagnostic: &Diagnostic) -> &'a str { + source[diagnostic.expect_primary_span().range().unwrap()].as_ref() + } + + #[test] + fn missing_direct_dependency_for_third_party_import() -> anyhow::Result<()> { + let (mut db, file) = setup_db( + "import requests\n", + &[], + vec![ModuleOwner::new( + ModuleName::new_static("requests").unwrap(), + ["requests"], + )], + )?; + db.write_file("/site-packages/requests/__init__.py", "")?; + + let diagnostics = check_types(&db, file); + + assert_eq!( + messages(&diagnostics), + [ + "Third-party import `requests` is used but no direct dependency on `requests` is declared" + ] + ); + + Ok(()) + } + + #[test] + fn declared_direct_dependency_is_not_reported() -> anyhow::Result<()> { + let (mut db, file) = setup_db( + "import requests\n", + &["requests"], + vec![ModuleOwner::new( + ModuleName::new_static("requests").unwrap(), + ["requests"], + )], + )?; + db.write_file("/site-packages/requests/__init__.py", "")?; + + let diagnostics = check_types(&db, file); + + assert_eq!(messages(&diagnostics), Vec::<&str>::new()); + + Ok(()) + } + + #[test] + fn dependency_group_dependency_is_allowed_for_non_package_file() -> anyhow::Result<()> { + let (mut db, file) = setup_db_with_file( + "/src/test_app.py", + "import inline_snapshot\n", + &[], + ["inline-snapshot"], + vec![ModuleOwner::new( + ModuleName::new_static("inline_snapshot").unwrap(), + ["inline-snapshot"], + )], + )?; + db.write_file("/site-packages/inline_snapshot/__init__.py", "")?; + + let diagnostics = check_types(&db, file); + + assert_eq!(messages(&diagnostics), Vec::<&str>::new()); + + Ok(()) + } + + #[test] + fn dependency_group_dependency_is_reported_for_package_file() -> anyhow::Result<()> { + let (mut db, file) = setup_db_with_file( + "/src/app/__init__.py", + "import inline_snapshot\n", + &[], + ["inline-snapshot"], + vec![ + ModuleOwner::new(ModuleName::new_static("app").unwrap(), ["app"]), + ModuleOwner::new( + ModuleName::new_static("inline_snapshot").unwrap(), + ["inline-snapshot"], + ), + ], + )?; + db.write_file("/site-packages/inline_snapshot/__init__.py", "")?; + + let diagnostics = check_types(&db, file); + + assert_eq!( + messages(&diagnostics), + [ + "Third-party import `inline_snapshot` is used but no direct dependency on `inline-snapshot` is declared" + ] + ); + + Ok(()) + } + + #[test] + fn type_checking_import_is_not_reported_as_runtime_dependency() -> anyhow::Result<()> { + let (mut db, file) = setup_db( + r#" +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + import rich +"#, + &[], + vec![ModuleOwner::new( + ModuleName::new_static("rich").unwrap(), + ["rich"], + )], + )?; + db.write_file("/site-packages/rich/__init__.py", "")?; + + let diagnostics = check_types(&db, file); + + assert_eq!(messages(&diagnostics), Vec::<&str>::new()); + + Ok(()) + } + + #[test] + fn multiline_from_import_suppression_uses_highlighted_range() -> anyhow::Result<()> { + let (mut db, file) = setup_db( + r#" +from requests import ( # ty: ignore[missing-direct-dependency] + get, +) +"#, + &[], + vec![ModuleOwner::new( + ModuleName::new_static("requests").unwrap(), + ["requests"], + )], + )?; + db.write_file("/site-packages/requests/__init__.py", "")?; + + let diagnostics = check_dependency_lints(&db, file).into_diagnostics(); + + assert_eq!(messages(&diagnostics), Vec::<&str>::new()); + + Ok(()) + } + + #[test] + fn from_import_uses_resolved_submodule_owner() -> anyhow::Result<()> { + let source = "from google.cloud import storage\n"; + let (mut db, file) = setup_db( + source, + &[], + vec![ + ModuleOwner::new( + ModuleName::new_static("google.cloud").unwrap(), + ["google-cloud-core"], + ), + ModuleOwner::new( + ModuleName::new_static("google.cloud.storage").unwrap(), + ["google-cloud-storage"], + ), + ], + )?; + db.write_file("/site-packages/google/__init__.py", "")?; + db.write_file("/site-packages/google/cloud/__init__.py", "")?; + db.write_file("/site-packages/google/cloud/storage/__init__.py", "")?; + + let diagnostics = check_types(&db, file); + + assert_eq!( + messages(&diagnostics), + [ + "Third-party import `google.cloud.storage` is used but no direct dependency on `google-cloud-storage` is declared" + ] + ); + assert_eq!(primary_highlight(source, &diagnostics[0]), "google.cloud"); + + Ok(()) + } +} diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index 72ab6581abeaf7..4a0e855ab51943 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -49,6 +49,7 @@ pub use types::ide_support::{ pub use types::{DisplaySettings, TypeQualifiers}; mod db; +pub mod dependency; mod dunder_all; mod fixes; pub mod lint; @@ -81,6 +82,7 @@ pub fn default_lint_registry() -> &'static LintRegistry { /// Register all known semantic lints. pub fn register_lints(registry: &mut LintRegistryBuilder) { + dependency::register_lints(registry); types::register_lints(registry); registry.register_lint(&UNUSED_IGNORE_COMMENT); registry.register_lint(&UNUSED_TYPE_IGNORE_COMMENT); @@ -106,6 +108,8 @@ pub struct AnalysisSettings { pub allowed_unresolved_imports: ModuleGlobSet, pub replace_imports_with_any: ModuleGlobSet, + + pub dependency_metadata: Option>, } impl Default for AnalysisSettings { @@ -115,6 +119,7 @@ impl Default for AnalysisSettings { respect_type_ignore_comments: true, allowed_unresolved_imports: ModuleGlobSet::empty(), replace_imports_with_any: ModuleGlobSet::empty(), + dependency_metadata: None, } } } diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 0642eb5fc0962c..56b33d90e81e6e 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -53,6 +53,7 @@ pub use self::signatures::ParameterKind; pub(crate) use self::signatures::Signature; pub(crate) use self::subclass_of::{SubclassOfInner, SubclassOfType}; pub(crate) use self::type_expansion::expand_type; +use crate::dependency::check_dependency_lints; pub use crate::diagnostic::add_inferred_python_version_hint_to_diagnostic; use crate::place::{ DefinedPlace, Definedness, Place, PlaceAndQualifiers, Provenance, TypeOrigin, @@ -203,6 +204,8 @@ pub fn check_types(db: &dyn Db, file: File) -> Vec { .map(|error| Diagnostic::invalid_syntax(file, error, error)), ); + diagnostics.extend(&check_dependency_lints(db, file)); + let diagnostics = check_suppressions(db, file, diagnostics); let elapsed = start.elapsed(); diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 0c1db5eb0b8f16..730288bb5a9a3f 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -1317,12 +1317,12 @@ impl TypeCheckDiagnostics { self.diagnostics.push(diagnostic); } - pub(super) fn extend(&mut self, other: &TypeCheckDiagnostics) { + pub(crate) fn extend(&mut self, other: &TypeCheckDiagnostics) { self.diagnostics.extend_from_slice(&other.diagnostics); self.used_suppressions.extend(&other.used_suppressions); } - pub(super) fn extend_diagnostics(&mut self, diagnostics: impl IntoIterator) { + pub(crate) fn extend_diagnostics(&mut self, diagnostics: impl IntoIterator) { self.diagnostics.extend(diagnostics); } diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap index b570aef0b48c16..d642d430c5606f 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap @@ -173,6 +173,7 @@ Settings: Settings { regex_set: RegexSet([]), globs: [], }, + dependency_metadata: None, }, overrides: [], } diff --git a/crates/ty_test/src/db.rs b/crates/ty_test/src/db.rs index f4289e78994e63..6d9de26bbaf2a1 100644 --- a/crates/ty_test/src/db.rs +++ b/crates/ty_test/src/db.rs @@ -256,6 +256,7 @@ fn mdtest_analysis_settings(options: Option<&Analysis>) -> AnalysisSettings { respect_type_ignore_comments: respect_type_ignore_comments_default, allowed_unresolved_imports: allowed_unresolved_imports_default, replace_imports_with_any: replace_imports_with_any_default, + dependency_metadata: dependency_metadata_default, } = AnalysisSettings::default(); let allowed_unresolved_imports = @@ -293,6 +294,7 @@ fn mdtest_analysis_settings(options: Option<&Analysis>) -> AnalysisSettings { .unwrap_or(respect_type_ignore_comments_default), allowed_unresolved_imports, replace_imports_with_any, + dependency_metadata: dependency_metadata_default, } } diff --git a/ty.schema.json b/ty.schema.json index ef68c1c4b0a32b..ce762dbfaf6143 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -1127,6 +1127,16 @@ } ] }, + "missing-direct-dependency": { + "title": "detects third-party imports without direct dependency declarations", + "description": "## What it does\nChecks for third-party imports that are used without a matching direct dependency\ndeclaration.\n\n## Why is this bad?\nImporting a package that is only available transitively can make the project break when\ndependency resolution changes.\n\n## Rule status\nThis rule is disabled by default.\n\n## Examples\n```python\nimport requests # requests is not declared as a direct dependency\n```", + "default": "ignore", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, "missing-override-decorator": { "title": "detects methods that override a superclass member without an `@override` annotation", "description": "## What it does\n\nChecks for methods that override a method or attribute in a superclass but are not decorated with `@override`.\n\nThis rule is disabled by default. Enable it to opt in to strict `@override` enforcement for a project.\n\n## Exemptions\n\nOverriding `__init__`, `__new__`, `__init_subclass__`, or `__post_init__` does not require\n`@override`, even if the method is explicitly declared by a superclass.\n\n## Why is this bad?\n\nWithout an `@override` annotation, refactors can silently change whether a method is an override.\nRequiring `@override` on every override lets ty report when an intended override stops overriding\nanything, and when a method unexpectedly starts overriding a superclass member.\n\n## Example\n\n```toml\n[environment]\npython-version = \"3.12\"\n```\n\n```python\nfrom typing import override\n\n\nclass Parent:\n def method(self) -> int:\n return 1\n\n\nclass Child(Parent):\n # when the rule is enabled\n def method(self) -> int: # error\n return 2\n\n\nclass ExplicitChild(Parent):\n @override\n def method(self) -> int: # fine\n return 2\n```",