Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions crates/red_knot/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ pub(crate) struct CheckCommand {
#[arg(long, value_name = "PROJECT")]
pub(crate) project: Option<SystemPathBuf>,

/// Path to the virtual environment the project uses.
/// Path to the Python installation from which Red Knot resolves type information and third-party dependencies.
///
/// If provided, red-knot will use the `site-packages` directory of this virtual environment
/// to resolve type information for the project's third-party dependencies.
/// Red Knot will search in the path's `site-packages` directories for type information and
/// third-party imports.
///
/// This option is commonly used to specify the path to a virtual environment.
#[arg(long, value_name = "PATH")]
pub(crate) venv_path: Option<SystemPathBuf>,
pub(crate) python: Option<SystemPathBuf>,

/// Custom directory to use for stdlib typeshed stubs.
#[arg(long, value_name = "PATH", alias = "custom-typeshed-dir")]
Expand Down Expand Up @@ -97,7 +99,7 @@ impl CheckCommand {
python_version: self
.python_version
.map(|version| RangedValue::cli(version.into())),
venv_path: self.venv_path.map(RelativePathBuf::cli),
python: self.python.map(RelativePathBuf::cli),
typeshed: self.typeshed.map(RelativePathBuf::cli),
extra_paths: self.extra_search_path.map(|extra_search_paths| {
extra_search_paths
Expand Down
20 changes: 11 additions & 9 deletions crates/red_knot/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use red_knot_project::{watch, Db};
use red_knot_project::{ProjectDatabase, ProjectMetadata};
use red_knot_server::run_server;
use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, Severity};
use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf};
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
use salsa::plumbing::ZalsaDatabase;

mod args;
Expand Down Expand Up @@ -69,7 +69,7 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
let _guard = setup_tracing(verbosity)?;

// The base path to which all CLI arguments are relative to.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems out of date (or at least badly located) now?

Copy link
Member Author

@MichaReiser MichaReiser Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still true. We relativize the paths in RelativePathBuf::absolute. The main change here is that we don't use the project path as cwd when it is set (which is incorrect)

let cli_base_path = {
let cwd = {
let cwd = std::env::current_dir().context("Failed to get the current working directory")?;
SystemPathBuf::from_path_buf(cwd)
.map_err(|path| {
Expand All @@ -80,25 +80,27 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
})?
};

let cwd = args
let project_path = args
.project
.as_ref()
.map(|cwd| {
if cwd.as_std_path().is_dir() {
Ok(SystemPath::absolute(cwd, &cli_base_path))
.map(|project| {
if project.as_std_path().is_dir() {
Ok(SystemPath::absolute(project, &cwd))
} else {
Err(anyhow!("Provided project path `{cwd}` is not a directory"))
Err(anyhow!(
"Provided project path `{project}` is not a directory"
))
}
})
.transpose()?
.unwrap_or_else(|| cli_base_path.clone());
.unwrap_or_else(|| cwd.clone());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed while testing that relative path arguments on the CLI were incorrectly anchored to the project directory instead of the cwd. For example, the python path for --project ../test --python ../test/venv resolved to ../test/test/venv.


let system = OsSystem::new(cwd);
let watch = args.watch;
let exit_zero = args.exit_zero;

let cli_options = args.into_options();
let mut project_metadata = ProjectMetadata::discover(system.current_directory(), &system)?;
let mut project_metadata = ProjectMetadata::discover(&project_path, &system)?;
project_metadata.apply_cli_options(cli_options.clone());
project_metadata.apply_configuration_files(&system)?;

Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_project/src/combine.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{collections::HashMap, hash::BuildHasher};

use red_knot_python_semantic::{PythonPlatform, SitePackages};
use red_knot_python_semantic::{PythonPath, PythonPlatform};
use ruff_db::system::SystemPathBuf;
use ruff_python_ast::PythonVersion;

Expand Down Expand Up @@ -128,7 +128,7 @@ macro_rules! impl_noop_combine {

impl_noop_combine!(SystemPathBuf);
impl_noop_combine!(PythonPlatform);
impl_noop_combine!(SitePackages);
impl_noop_combine!(PythonPath);
impl_noop_combine!(PythonVersion);

// std types
Expand Down
22 changes: 13 additions & 9 deletions crates/red_knot_project/src/metadata/options.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::metadata::value::{RangedValue, RelativePathBuf, ValueSource, ValueSourceGuard};
use crate::Db;
use red_knot_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection};
use red_knot_python_semantic::{ProgramSettings, PythonPlatform, SearchPathSettings, SitePackages};
use red_knot_python_semantic::{ProgramSettings, PythonPath, PythonPlatform, SearchPathSettings};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span};
use ruff_db::files::system_path_to_file;
use ruff_db::system::{System, SystemPath};
Expand Down Expand Up @@ -90,7 +90,7 @@ impl Options {
.map(|env| {
(
env.extra_paths.clone(),
env.venv_path.clone(),
env.python.clone(),
env.typeshed.clone(),
)
})
Expand All @@ -104,11 +104,11 @@ impl Options {
.collect(),
src_roots,
custom_typeshed: typeshed.map(|path| path.absolute(project_root, system)),
site_packages: python
.map(|venv_path| SitePackages::Derived {
venv_path: venv_path.absolute(project_root, system),
python_path: python
.map(|python_path| {
PythonPath::SysPrefix(python_path.absolute(project_root, system))
})
.unwrap_or(SitePackages::Known(vec![])),
.unwrap_or(PythonPath::KnownSitePackages(vec![])),
}
}

Expand Down Expand Up @@ -236,10 +236,14 @@ pub struct EnvironmentOptions {
#[serde(skip_serializing_if = "Option::is_none")]
pub typeshed: Option<RelativePathBuf>,

// TODO: Rename to python, see https://github.com/astral-sh/ruff/issues/15530
/// The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed.
/// Path to the Python installation from which Red Knot resolves type information and third-party dependencies.
///
/// Red Knot will search in the path's `site-packages` directories for type information and
/// third-party imports.
///
/// This option is commonly used to specify the path to a virtual environment.
#[serde(skip_serializing_if = "Option::is_none")]
pub venv_path: Option<RelativePathBuf>,
pub python: Option<RelativePathBuf>,
}

#[derive(Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize)]
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::suppression::{INVALID_IGNORE_COMMENT, UNKNOWN_RULE, UNUSED_IGNORE_COM
pub use db::Db;
pub use module_name::ModuleName;
pub use module_resolver::{resolve_module, system_module_search_paths, KnownModule, Module};
pub use program::{Program, ProgramSettings, SearchPathSettings, SitePackages};
pub use program::{Program, ProgramSettings, PythonPath, SearchPathSettings};
pub use python_platform::PythonPlatform;
pub use semantic_model::{HasType, SemanticModel};

Expand Down
16 changes: 8 additions & 8 deletions crates/red_knot_python_semantic/src/module_resolver/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::db::Db;
use crate::module_name::ModuleName;
use crate::module_resolver::typeshed::{vendored_typeshed_versions, TypeshedVersions};
use crate::site_packages::VirtualEnvironment;
use crate::{Program, SearchPathSettings, SitePackages};
use crate::{Program, PythonPath, SearchPathSettings};

use super::module::{Module, ModuleKind};
use super::path::{ModulePath, SearchPath, SearchPathValidationError};
Expand Down Expand Up @@ -171,7 +171,7 @@ impl SearchPaths {
extra_paths,
src_roots,
custom_typeshed: typeshed,
site_packages: site_packages_paths,
python_path,
} = settings;

let system = db.system();
Expand Down Expand Up @@ -222,16 +222,16 @@ impl SearchPaths {

static_paths.push(stdlib_path);

let site_packages_paths = match site_packages_paths {
SitePackages::Derived { venv_path } => {
let site_packages_paths = match python_path {
PythonPath::SysPrefix(sys_prefix) => {
// TODO: We may want to warn here if the venv's python version is older
// than the one resolved in the program settings because it indicates
// that the `target-version` is incorrectly configured or that the
// venv is out of date.
VirtualEnvironment::new(venv_path, system)
VirtualEnvironment::new(sys_prefix, system)
.and_then(|venv| venv.site_packages_directories(system))?
}
SitePackages::Known(paths) => paths
PythonPath::KnownSitePackages(paths) => paths
.iter()
.map(|path| canonicalize(path, system))
.collect(),
Expand Down Expand Up @@ -1310,7 +1310,7 @@ mod tests {
extra_paths: vec![],
src_roots: vec![src.clone()],
custom_typeshed: Some(custom_typeshed),
site_packages: SitePackages::Known(vec![site_packages]),
python_path: PythonPath::KnownSitePackages(vec![site_packages]),
},
},
)
Expand Down Expand Up @@ -1816,7 +1816,7 @@ not_a_directory
extra_paths: vec![],
src_roots: vec![SystemPathBuf::from("/src")],
custom_typeshed: None,
site_packages: SitePackages::Known(vec![
python_path: PythonPath::KnownSitePackages(vec![
venv_site_packages,
system_site_packages,
]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ruff_python_ast::PythonVersion;

use crate::db::tests::TestDb;
use crate::program::{Program, SearchPathSettings};
use crate::{ProgramSettings, PythonPlatform, SitePackages};
use crate::{ProgramSettings, PythonPath, PythonPlatform};

/// A test case for the module resolver.
///
Expand Down Expand Up @@ -239,7 +239,7 @@ impl TestCaseBuilder<MockedTypeshed> {
extra_paths: vec![],
src_roots: vec![src.clone()],
custom_typeshed: Some(typeshed.clone()),
site_packages: SitePackages::Known(vec![site_packages.clone()]),
python_path: PythonPath::KnownSitePackages(vec![site_packages.clone()]),
},
},
)
Expand Down Expand Up @@ -294,7 +294,7 @@ impl TestCaseBuilder<VendoredTypeshed> {
python_version,
python_platform,
search_paths: SearchPathSettings {
site_packages: SitePackages::Known(vec![site_packages.clone()]),
python_path: PythonPath::KnownSitePackages(vec![site_packages.clone()]),
..SearchPathSettings::new(vec![src.clone()])
},
},
Expand Down
34 changes: 25 additions & 9 deletions crates/red_knot_python_semantic/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ pub struct SearchPathSettings {
/// bundled as a zip file in the binary
pub custom_typeshed: Option<SystemPathBuf>,

/// The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed.
pub site_packages: SitePackages,
/// Path to the Python installation from which Red Knot resolves third party dependencies
/// and their type information.
pub python_path: PythonPath,
}

impl SearchPathSettings {
Expand All @@ -120,17 +121,32 @@ impl SearchPathSettings {
src_roots,
extra_paths: vec![],
custom_typeshed: None,
site_packages: SitePackages::Known(vec![]),
python_path: PythonPath::KnownSitePackages(vec![]),
}
}
}

#[derive(Debug, Clone, Eq, PartialEq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum SitePackages {
Derived {
venv_path: SystemPathBuf,
},
/// Resolved site packages paths
Known(Vec<SystemPathBuf>),
pub enum PythonPath {
/// A path that represents the value of [`sys.prefix`] at runtime in Python
/// for a given Python executable.
///
/// For the case of a virtual environment, where a
/// Python binary is at `/.venv/bin/python`, `sys.prefix` is the path to
/// the virtual environment the Python binary lies inside, i.e. `/.venv`,
/// and `site-packages` will be at `.venv/lib/python3.X/site-packages`.
/// System Python installations generally work the same way: if a system
/// Python installation lies at `/opt/homebrew/bin/python`, `sys.prefix`
/// will be `/opt/homebrew`, and `site-packages` will be at
/// `/opt/homebrew/lib/python3.X/site-packages`.
///
/// [`sys.prefix`]: https://docs.python.org/3/library/sys.html#sys.prefix
SysPrefix(SystemPathBuf),

/// Resolved site packages paths.
///
/// This variant is mainly intended for testing where we want to skip resolving `site-packages`
/// because it would unnecessarily complicate the test setup.
KnownSitePackages(Vec<SystemPathBuf>),
}
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/src/site_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,11 @@ System site-packages will not be used for module resolution.",

#[derive(Debug, thiserror::Error)]
pub(crate) enum SitePackagesDiscoveryError {
#[error("Invalid --venv-path argument: {0} could not be canonicalized")]
#[error("Invalid --python argument: `{0}` could not be canonicalized")]
VenvDirCanonicalizationError(SystemPathBuf, #[source] io::Error),
#[error("Invalid --venv-path argument: {0} does not point to a directory on disk")]
#[error("Invalid --python argument: `{0}` does not point to a directory on disk")]
VenvDirIsNotADirectory(SystemPathBuf),
#[error("--venv-path points to a broken venv with no pyvenv.cfg file")]
#[error("--python points to a broken venv with no pyvenv.cfg file")]
NoPyvenvCfgFile(#[source] io::Error),
#[error("Failed to parse the pyvenv.cfg file at {0} because {1}")]
PyvenvCfgParseError(SystemPathBuf, PyvenvCfgParseErrorKind),
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use camino::Utf8Path;
use colored::Colorize;
use parser as test_parser;
use red_knot_python_semantic::types::check_types;
use red_knot_python_semantic::{Program, ProgramSettings, SearchPathSettings, SitePackages};
use red_knot_python_semantic::{Program, ProgramSettings, PythonPath, SearchPathSettings};
use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, ParseDiagnostic};
use ruff_db::files::{system_path_to_file, File, Files};
use ruff_db::panic::catch_unwind;
Expand Down Expand Up @@ -180,7 +180,7 @@ fn run_test(
src_roots: vec![src_path],
extra_paths: vec![],
custom_typeshed: custom_typeshed_path,
site_packages: SitePackages::Known(vec![]),
python_path: PythonPath::KnownSitePackages(vec![]),
},
},
)
Expand Down
14 changes: 7 additions & 7 deletions knot.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@
"type": "string"
}
},
"python": {
"description": "Path to the Python installation from which Red Knot resolves type information and third-party dependencies.\n\nRed Knot will search in the path's `site-packages` directories for type information and third-party imports.\n\nThis option is commonly used to specify the path to a virtual environment.",
"type": [
"string",
"null"
]
},
"python-platform": {
"description": "Specifies the target platform that will be used to execute the source code. If specified, Red Knot will tailor its use of type stub files, which conditionalize type definitions based on the platform.\n\nIf no platform is specified, knot will use `all` or the current platform in the LSP use case.",
"anyOf": [
Expand Down Expand Up @@ -90,13 +97,6 @@
"string",
"null"
]
},
"venv-path": {
"description": "The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed.",
"type": [
"string",
"null"
]
}
},
"additionalProperties": false
Expand Down
2 changes: 1 addition & 1 deletion scripts/knot_benchmark/src/benchmark/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def cold_command(self, project: Project, venv: Venv) -> Command:
if project.include:
command.extend(["--project", project.include[0]])

command.extend(["--venv-path", str(venv.path)])
command.extend(["--python", str(venv.path)])

return Command(
name="knot",
Expand Down
Loading