diff --git a/crates/red_knot/src/args.rs b/crates/red_knot/src/args.rs index df07863dbd1e4..9e8d0d66a5435 100644 --- a/crates/red_knot/src/args.rs +++ b/crates/red_knot/src/args.rs @@ -21,17 +21,22 @@ pub(crate) struct Args { #[derive(Debug, clap::Subcommand)] pub(crate) enum Command { /// Check a project for type errors. - Check(CheckArgs), + Check(CheckCommand), /// Start the language server Server, } #[derive(Debug, Parser)] -pub(crate) struct CheckArgs { - /// Run in watch mode by re-running whenever files change. - #[arg(long, short = 'W')] - pub(crate) watch: bool, +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, + /// as will the project's virtual environment (`.venv`) unless the `venv-path` option is set. + /// + /// Other command-line arguments (such as relative paths) will be resolved relative to the current working directory. + #[arg(long, value_name = "PROJECT")] + pub(crate) project: Option, /// Path to the virtual environment the project uses. /// @@ -53,22 +58,17 @@ pub(crate) struct CheckArgs { pub(crate) python_version: Option, #[clap(flatten)] - pub(crate) rules: RulesArg, - - /// 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, - /// as will the project's virtual environment (`.venv`) unless the `venv-path` option is set. - /// - /// Other command-line arguments (such as relative paths) will be resolved relative to the current working directory. - #[arg(long, value_name = "PROJECT")] - pub(crate) project: Option, + pub(crate) verbosity: Verbosity, #[clap(flatten)] - pub(crate) verbosity: Verbosity, + pub(crate) rules: RulesArg, + + /// Run in watch mode by re-running whenever files change. + #[arg(long, short = 'W')] + pub(crate) watch: bool, } -impl CheckArgs { +impl CheckCommand { pub(crate) fn into_options(self) -> Options { let rules = if self.rules.is_empty() { None diff --git a/crates/red_knot/src/main.rs b/crates/red_knot/src/main.rs index 628abe06d03dd..3f717434f42fe 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, CheckArgs, Command}; +use crate::args::{Args, CheckCommand, Command}; use crate::logging::setup_tracing; use anyhow::{anyhow, Context}; use clap::Parser; @@ -52,7 +52,7 @@ fn run() -> anyhow::Result { } } -fn run_check(args: CheckArgs) -> anyhow::Result { +fn run_check(args: CheckCommand) -> anyhow::Result { let verbosity = args.verbosity.level(); countme::enable(verbosity.is_trace()); let _guard = setup_tracing(verbosity)?; diff --git a/crates/red_knot/tests/cli.rs b/crates/red_knot/tests/cli.rs index 74d01d0bfb876..c114f24e3b722 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,14 +429,6 @@ 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).arg("check");