Add `check` command (#15692)

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<Command>,
+    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 {
+impl CheckCommand {
-    pub(crate) fn to_options(&self) -> Options {
+    pub(crate) fn into_options(self) -> Options {
         let rules = if self.rules.is_empty() {
             None
         } else {
             Some(
                 self.rules
-                    .iter()
+                    .into_iter()
-                    .map(|(rule, level)| {
+                    .map(|(rule, level)| (RangedValue::cli(rule), RangedValue::cli(level)))
-                        (RangedValue::cli(rule.to_string()), 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),
+                venv_path: self.venv_path.map(RelativePathBuf::cli),
-                typeshed: self.typeshed.as_ref().map(RelativePathBuf::cli),
+                typeshed: self.typeshed.map(RelativePathBuf::cli),
-                extra_paths: self.extra_search_path.as_ref().map(|extra_search_paths| {
+                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<Item = (&str, lint::Level)> {
+    fn into_iter(self) -> impl Iterator<Item = (String, lint::Level)> {
-        self.0.iter().map(|(rule, level)| (rule.as_str(), *level))
+        self.0.into_iter()
     }
 }
 

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<ExitStatus> {
     let args = Args::parse_from(std::env::args());
 
-    if matches!(args.command, Some(Command::Server)) {
+    match args.command {
-        return run_server().map(|()| ExitStatus::Success);
+        Command::Server => run_server().map(|()| ExitStatus::Success),
+        Command::Check(check_args) => run_check(check_args),
+    }
 }
 
+fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
     let verbosity = args.verbosity.level();
     countme::enable(verbosity.is_trace());
     let _guard = setup_tracing(verbosity)?;
@@ -86,7 +83,8 @@ fn run() -> anyhow::Result<ExitStatus> {
         .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<ExitStatus> {
         }
     })?;
 
-    let exit_status = if args.watch {
+    let exit_status = if watch {
         main_loop.watch(&mut db)?
     } else {
         main_loop.run(&mut db)

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,7 +28,6 @@ fn config_override() -> anyhow::Result<()> {
         ),
     ])?;
 
-    case.insta_settings().bind(|| {
     assert_cmd_snapshot!(case.command(), @r"
         success: false
         exit_code: 1
@@ -45,7 +44,6 @@ fn config_override() -> anyhow::Result<()> {
 
         ----- stderr -----
     ");
-    });
 
     Ok(())
 }
@@ -92,7 +90,6 @@ 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
@@ -110,7 +107,6 @@ fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> {
 
         ----- stderr -----
     ");
-    });
 
     Ok(())
 }
@@ -156,7 +152,6 @@ 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
@@ -164,7 +159,6 @@ fn paths_in_configuration_files_are_relative_to_the_project_root() -> anyhow::Re
 
         ----- stderr -----
     ");
-    });
 
     Ok(())
 }
@@ -184,7 +178,6 @@ 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"
@@ -197,11 +190,14 @@ fn configuration_rule_severity() -> anyhow::Result<()> {
         ----- stderr -----
     ");
 
-        case.write_file("pyproject.toml", r#"
+    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
@@ -213,7 +209,6 @@ fn configuration_rule_severity() -> anyhow::Result<()> {
     ");
 
     Ok(())
-    })
 }
 
 /// The rule severity can be changed using `--ignore`, `--warn`, and `--error`
@@ -233,7 +228,6 @@ 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"
@@ -247,7 +241,6 @@ fn cli_rule_severity() -> anyhow::Result<()> {
         ----- stderr -----
     ");
 
-
     assert_cmd_snapshot!(
         case
             .command()
@@ -269,7 +262,6 @@ fn cli_rule_severity() -> anyhow::Result<()> {
     );
 
     Ok(())
-    })
 }
 
 /// The rule severity can be changed using `--ignore`, `--warn`, and `--error` and
@@ -288,7 +280,6 @@ 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"
@@ -301,7 +292,6 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> {
         ----- stderr -----
     ");
 
-
     assert_cmd_snapshot!(
         case
             .command()
@@ -309,9 +299,9 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> {
             .arg("possibly-unresolved-reference")
             .arg("--warn")
             .arg("division-by-zero")
+            // Override the error severity with warning
             .arg("--ignore")
             .arg("possibly-unresolved-reference"),
-                // Override the error severity with warning
         @r"
         success: false
         exit_code: 1
@@ -323,7 +313,6 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> {
     );
 
     Ok(())
-    })
 }
 
 /// Red Knot warns about unknown rules specified in a configuration file
@@ -340,7 +329,6 @@ 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
@@ -349,7 +337,6 @@ fn configuration_unknown_rules() -> anyhow::Result<()> {
 
         ----- stderr -----
     ");
-    });
 
     Ok(())
 }
@@ -359,7 +346,6 @@ 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"
         success: false
         exit_code: 1
@@ -368,13 +354,13 @@ fn cli_unknown_rules() -> anyhow::Result<()> {
 
         ----- 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), "<temp_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), "<temp_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
     }
 }

scripts/knot_benchmark/src/benchmark/cases.py

@@ -68,7 +68,7 @@ class Knot(Tool):
         )
 
     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"