diff --git a/crates/red_knot/src/args.rs b/crates/red_knot/src/args.rs index b2c30016b60f27..9e8d0d66a5435e 100644 --- a/crates/red_knot/src/args.rs +++ b/crates/red_knot/src/args.rs @@ -1,6 +1,5 @@ use crate::logging::Verbosity; use crate::python_version::PythonVersion; -use crate::Command; use clap::{ArgAction, ArgMatches, Error, Parser}; use red_knot_project::metadata::options::{EnvironmentOptions, Options}; use red_knot_project::metadata::value::{RangedValue, RelativePathBuf}; @@ -16,8 +15,20 @@ use ruff_db::system::SystemPathBuf; #[command(version)] pub(crate) struct Args { #[command(subcommand)] - pub(crate) command: Option, + pub(crate) command: Command, +} + +#[derive(Debug, clap::Subcommand)] +pub(crate) enum Command { + /// Check a project for type errors. + Check(CheckCommand), + /// Start the language server + Server, +} + +#[derive(Debug, Parser)] +pub(crate) struct CheckCommand { /// 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, @@ -57,17 +68,15 @@ pub(crate) struct Args { pub(crate) watch: bool, } -impl Args { - pub(crate) fn to_options(&self) -> Options { +impl CheckCommand { + pub(crate) fn into_options(self) -> Options { let rules = if self.rules.is_empty() { None } else { Some( self.rules - .iter() - .map(|(rule, level)| { - (RangedValue::cli(rule.to_string()), RangedValue::cli(level)) - }) + .into_iter() + .map(|(rule, level)| (RangedValue::cli(rule), RangedValue::cli(level))) .collect(), ) }; @@ -77,11 +86,11 @@ impl Args { python_version: self .python_version .map(|version| RangedValue::cli(version.into())), - venv_path: self.venv_path.as_ref().map(RelativePathBuf::cli), - typeshed: self.typeshed.as_ref().map(RelativePathBuf::cli), - extra_paths: self.extra_search_path.as_ref().map(|extra_search_paths| { + venv_path: self.venv_path.map(RelativePathBuf::cli), + typeshed: self.typeshed.map(RelativePathBuf::cli), + extra_paths: self.extra_search_path.map(|extra_search_paths| { extra_search_paths - .iter() + .into_iter() .map(RelativePathBuf::cli) .collect() }), @@ -105,8 +114,8 @@ impl RulesArg { self.0.is_empty() } - fn iter(&self) -> impl Iterator { - self.0.iter().map(|(rule, level)| (rule.as_str(), *level)) + fn into_iter(self) -> impl Iterator { + self.0.into_iter() } } diff --git a/crates/red_knot/src/main.rs b/crates/red_knot/src/main.rs index 83c0950996e668..3f717434f42feb 100644 --- a/crates/red_knot/src/main.rs +++ b/crates/red_knot/src/main.rs @@ -1,7 +1,7 @@ use std::process::{ExitCode, Termination}; use std::sync::Mutex; -use crate::args::Args; +use crate::args::{Args, CheckCommand, Command}; use crate::logging::setup_tracing; use anyhow::{anyhow, Context}; use clap::Parser; @@ -21,12 +21,6 @@ mod logging; mod python_version; mod verbosity; -#[derive(Debug, clap::Subcommand)] -pub enum Command { - /// Start the language server - Server, -} - #[allow(clippy::print_stdout, clippy::unnecessary_wraps, clippy::print_stderr)] pub fn main() -> ExitStatus { run().unwrap_or_else(|error| { @@ -52,10 +46,13 @@ pub fn main() -> ExitStatus { fn run() -> anyhow::Result { let args = Args::parse_from(std::env::args()); - if matches!(args.command, Some(Command::Server)) { - return run_server().map(|()| ExitStatus::Success); + match args.command { + Command::Server => run_server().map(|()| ExitStatus::Success), + Command::Check(check_args) => run_check(check_args), } +} +fn run_check(args: CheckCommand) -> anyhow::Result { let verbosity = args.verbosity.level(); countme::enable(verbosity.is_trace()); let _guard = setup_tracing(verbosity)?; @@ -86,7 +83,8 @@ fn run() -> anyhow::Result { .unwrap_or_else(|| cli_base_path.clone()); let system = OsSystem::new(cwd); - let cli_options = args.to_options(); + let watch = args.watch; + let cli_options = args.into_options(); let mut workspace_metadata = ProjectMetadata::discover(system.current_directory(), &system)?; workspace_metadata.apply_cli_options(cli_options.clone()); @@ -104,7 +102,7 @@ fn run() -> anyhow::Result { } })?; - let exit_status = if args.watch { + let exit_status = if watch { main_loop.watch(&mut db)? } else { main_loop.run(&mut db) diff --git a/crates/red_knot/tests/cli.rs b/crates/red_knot/tests/cli.rs index cadcb401bb0479..c114f24e3b7220 100644 --- a/crates/red_knot/tests/cli.rs +++ b/crates/red_knot/tests/cli.rs @@ -1,5 +1,5 @@ use anyhow::Context; -use insta::Settings; +use insta::internals::SettingsBindDropGuard; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; use std::path::{Path, PathBuf}; use std::process::Command; @@ -28,24 +28,22 @@ fn config_override() -> anyhow::Result<()> { ), ])?; - case.insta_settings().bind(|| { - assert_cmd_snapshot!(case.command(), @r" - success: false - exit_code: 1 - ----- stdout ----- - error[lint:unresolved-attribute] /test.py:5:7 Type `` has no attribute `last_exc` + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[lint:unresolved-attribute] /test.py:5:7 Type `` has no attribute `last_exc` - ----- stderr ----- - "); + ----- stderr ----- + "); - assert_cmd_snapshot!(case.command().arg("--python-version").arg("3.12"), @r" - success: true - exit_code: 0 - ----- stdout ----- + assert_cmd_snapshot!(case.command().arg("--python-version").arg("3.12"), @r" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- - "); - }); + ----- stderr ----- + "); Ok(()) } @@ -92,25 +90,23 @@ fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> { ), ])?; - case.insta_settings().bind(|| { - // Make sure that the CLI fails when the `libs` directory is not in the search path. - assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r#" - success: false - exit_code: 1 - ----- stdout ----- - error[lint:unresolved-import] /child/test.py:2:1 Cannot resolve import `utils` + // Make sure that the CLI fails when the `libs` directory is not in the search path. + assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r#" + success: false + exit_code: 1 + ----- stdout ----- + error[lint:unresolved-import] /child/test.py:2:1 Cannot resolve import `utils` - ----- stderr ----- - "#); + ----- stderr ----- + "#); - assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")).arg("--extra-search-path").arg("../libs"), @r" - success: true - exit_code: 0 - ----- stdout ----- + assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")).arg("--extra-search-path").arg("../libs"), @r" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- - "); - }); + ----- stderr ----- + "); Ok(()) } @@ -156,15 +152,13 @@ fn paths_in_configuration_files_are_relative_to_the_project_root() -> anyhow::Re ), ])?; - case.insta_settings().bind(|| { - assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r" - success: true - exit_code: 0 - ----- stdout ----- + assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- - "); - }); + ----- stderr ----- + "); Ok(()) } @@ -184,36 +178,37 @@ fn configuration_rule_severity() -> anyhow::Result<()> { "#, )?; - case.insta_settings().bind(|| { - // Assert that there's a possibly unresolved reference diagnostic - // and that division-by-zero has a severity of error by default. - assert_cmd_snapshot!(case.command(), @r" - success: false - exit_code: 1 - ----- stdout ----- - error[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero - warning[lint:possibly-unresolved-reference] /test.py:7:7 Name `x` used when possibly not defined + // Assert that there's a possibly unresolved reference diagnostic + // and that division-by-zero has a severity of error by default. + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero + warning[lint:possibly-unresolved-reference] /test.py:7:7 Name `x` used when possibly not defined - ----- stderr ----- - "); + ----- stderr ----- + "); - case.write_file("pyproject.toml", r#" - [tool.knot.rules] - division-by-zero = "warn" # demote to warn - possibly-unresolved-reference = "ignore" - "#)?; + case.write_file( + "pyproject.toml", + r#" + [tool.knot.rules] + division-by-zero = "warn" # demote to warn + possibly-unresolved-reference = "ignore" + "#, + )?; - assert_cmd_snapshot!(case.command(), @r" - success: false - exit_code: 1 - ----- stdout ----- - warning[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + warning[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero - ----- stderr ----- - "); + ----- stderr ----- + "); - Ok(()) - }) + Ok(()) } /// The rule severity can be changed using `--ignore`, `--warn`, and `--error` @@ -233,10 +228,9 @@ fn cli_rule_severity() -> anyhow::Result<()> { "#, )?; - case.insta_settings().bind(|| { - // Assert that there's a possibly unresolved reference diagnostic - // and that division-by-zero has a severity of error by default. - assert_cmd_snapshot!(case.command(), @r" + // Assert that there's a possibly unresolved reference diagnostic + // and that division-by-zero has a severity of error by default. + assert_cmd_snapshot!(case.command(), @r" success: false exit_code: 1 ----- stdout ----- @@ -245,31 +239,29 @@ fn cli_rule_severity() -> anyhow::Result<()> { warning[lint:possibly-unresolved-reference] /test.py:9:7 Name `x` used when possibly not defined ----- stderr ----- - "); - - - assert_cmd_snapshot!( - case - .command() - .arg("--ignore") - .arg("possibly-unresolved-reference") - .arg("--warn") - .arg("division-by-zero") - .arg("--warn") - .arg("unresolved-import"), - @r" - success: false - exit_code: 1 - ----- stdout ----- - warning[lint:unresolved-import] /test.py:2:8 Cannot resolve import `does_not_exit` - warning[lint:division-by-zero] /test.py:4:5 Cannot divide object of type `Literal[4]` by zero - - ----- stderr ----- - " - ); + "); + + assert_cmd_snapshot!( + case + .command() + .arg("--ignore") + .arg("possibly-unresolved-reference") + .arg("--warn") + .arg("division-by-zero") + .arg("--warn") + .arg("unresolved-import"), + @r" + success: false + exit_code: 1 + ----- stdout ----- + warning[lint:unresolved-import] /test.py:2:8 Cannot resolve import `does_not_exit` + warning[lint:division-by-zero] /test.py:4:5 Cannot divide object of type `Literal[4]` by zero + + ----- stderr ----- + " + ); - Ok(()) - }) + Ok(()) } /// The rule severity can be changed using `--ignore`, `--warn`, and `--error` and @@ -288,42 +280,39 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> { "#, )?; - case.insta_settings().bind(|| { - // Assert that there's a possibly unresolved reference diagnostic - // and that division-by-zero has a severity of error by default. - assert_cmd_snapshot!(case.command(), @r" - success: false - exit_code: 1 - ----- stdout ----- - error[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero - warning[lint:possibly-unresolved-reference] /test.py:7:7 Name `x` used when possibly not defined - - ----- stderr ----- - "); - - - assert_cmd_snapshot!( - case - .command() - .arg("--error") - .arg("possibly-unresolved-reference") - .arg("--warn") - .arg("division-by-zero") - .arg("--ignore") - .arg("possibly-unresolved-reference"), - // Override the error severity with warning - @r" - success: false - exit_code: 1 - ----- stdout ----- - warning[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero - - ----- stderr ----- - " - ); + // Assert that there's a possibly unresolved reference diagnostic + // and that division-by-zero has a severity of error by default. + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero + warning[lint:possibly-unresolved-reference] /test.py:7:7 Name `x` used when possibly not defined - Ok(()) - }) + ----- stderr ----- + "); + + assert_cmd_snapshot!( + case + .command() + .arg("--error") + .arg("possibly-unresolved-reference") + .arg("--warn") + .arg("division-by-zero") + // Override the error severity with warning + .arg("--ignore") + .arg("possibly-unresolved-reference"), + @r" + success: false + exit_code: 1 + ----- stdout ----- + warning[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero + + ----- stderr ----- + " + ); + + Ok(()) } /// Red Knot warns about unknown rules specified in a configuration file @@ -340,16 +329,14 @@ fn configuration_unknown_rules() -> anyhow::Result<()> { ("test.py", "print(10)"), ])?; - case.insta_settings().bind(|| { - assert_cmd_snapshot!(case.command(), @r" - success: false - exit_code: 1 - ----- stdout ----- - warning[unknown-rule] /pyproject.toml:3:1 Unknown lint rule `division-by-zer` + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + warning[unknown-rule] /pyproject.toml:3:1 Unknown lint rule `division-by-zer` - ----- stderr ----- - "); - }); + ----- stderr ----- + "); Ok(()) } @@ -359,22 +346,21 @@ fn configuration_unknown_rules() -> anyhow::Result<()> { fn cli_unknown_rules() -> anyhow::Result<()> { let case = TestCase::with_file("test.py", "print(10)")?; - case.insta_settings().bind(|| { - assert_cmd_snapshot!(case.command().arg("--ignore").arg("division-by-zer"), @r" + assert_cmd_snapshot!(case.command().arg("--ignore").arg("division-by-zer"), @r" success: false exit_code: 1 ----- stdout ----- warning[unknown-rule] Unknown lint rule `division-by-zer` ----- stderr ----- - "); - }); + "); Ok(()) } struct TestCase { _temp_dir: TempDir, + _settings_scope: SettingsBindDropGuard, project_dir: PathBuf, } @@ -389,9 +375,16 @@ impl TestCase { .canonicalize() .context("Failed to canonicalize project path")?; + let mut settings = insta::Settings::clone_current(); + settings.add_filter(&tempdir_filter(&project_dir), "/"); + settings.add_filter(r#"\\(\w\w|\s|\.|")"#, "/$1"); + + let settings_scope = settings.bind_to_scope(); + Ok(Self { project_dir, _temp_dir: temp_dir, + _settings_scope: settings_scope, }) } @@ -436,17 +429,9 @@ impl TestCase { &self.project_dir } - // Returns the insta filters to escape paths in snapshots - fn insta_settings(&self) -> Settings { - let mut settings = insta::Settings::clone_current(); - settings.add_filter(&tempdir_filter(&self.project_dir), "/"); - settings.add_filter(r#"\\(\w\w|\s|\.|")"#, "/$1"); - settings - } - fn command(&self) -> Command { let mut command = Command::new(get_cargo_bin("red_knot")); - command.current_dir(&self.project_dir); + command.current_dir(&self.project_dir).arg("check"); command } } diff --git a/scripts/knot_benchmark/src/benchmark/cases.py b/scripts/knot_benchmark/src/benchmark/cases.py index 25eecf59210b6d..e2ef9d17ff84b6 100644 --- a/scripts/knot_benchmark/src/benchmark/cases.py +++ b/scripts/knot_benchmark/src/benchmark/cases.py @@ -68,7 +68,7 @@ def __init__(self, *, path: Path | None = None): ) def cold_command(self, project: Project, venv: Venv) -> Command: - command = [str(self.path), "-v"] + command = [str(self.path), "check", "-v"] assert len(project.include) < 2, "Knot doesn't support multiple source folders"