[red-knot] Resolve Options to Settings

Author: MichaReiser

crates/red_knot/src/args.rs

@@ -1,7 +1,7 @@
 use crate::logging::Verbosity;
 use crate::python_version::PythonVersion;
 use clap::{ArgAction, ArgMatches, Error, Parser};
-use red_knot_project::metadata::options::{EnvironmentOptions, Options};
+use red_knot_project::metadata::options::{EnvironmentOptions, Options, TerminalOptions};
 use red_knot_project::metadata::value::{RangedValue, RelativePathBuf};
 use red_knot_python_semantic::lint;
 use ruff_db::system::SystemPathBuf;
@@ -67,8 +67,8 @@ pub(crate) struct CheckCommand {
     pub(crate) rules: RulesArg,
 
     /// Use exit code 1 if there are any warning-level diagnostics.
-    #[arg(long, conflicts_with = "exit_zero")]
-    pub(crate) error_on_warning: bool,
+    #[arg(long, conflicts_with = "exit_zero", default_missing_value = "true", num_args=0..1)]
+    pub(crate) error_on_warning: Option<bool>,
 
     /// Always use exit code 0, even when there are error-level diagnostics.
     #[arg(long)]
@@ -107,6 +107,9 @@ impl CheckCommand {
                 }),
                 ..EnvironmentOptions::default()
             }),
+            terminal: Some(TerminalOptions {
+                error_on_warning: self.error_on_warning,
+            }),
             rules,
             ..Default::default()
         }

crates/red_knot/src/main.rs

@@ -11,8 +11,8 @@ use clap::Parser;
 use colored::Colorize;
 use crossbeam::channel as crossbeam_channel;
 use red_knot_project::metadata::options::Options;
-use red_knot_project::watch;
 use red_knot_project::watch::ProjectWatcher;
+use red_knot_project::{watch, Db};
 use red_knot_project::{ProjectDatabase, ProjectMetadata};
 use red_knot_server::run_server;
 use ruff_db::diagnostic::{Diagnostic, Severity};
@@ -97,19 +97,14 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
     let system = OsSystem::new(cwd);
     let watch = args.watch;
     let exit_zero = args.exit_zero;
-    let min_error_severity = if args.error_on_warning {
-        Severity::Warning
-    } else {
-        Severity::Error
-    };
 
     let cli_options = args.into_options();
     let mut workspace_metadata = ProjectMetadata::discover(system.current_directory(), &system)?;
     workspace_metadata.apply_cli_options(cli_options.clone());
 
     let mut db = ProjectDatabase::new(workspace_metadata, system)?;
 
-    let (main_loop, main_loop_cancellation_token) = MainLoop::new(cli_options, min_error_severity);
+    let (main_loop, main_loop_cancellation_token) = MainLoop::new(cli_options);
 
     // Listen to Ctrl+C and abort the watch mode.
     let main_loop_cancellation_token = Mutex::new(Some(main_loop_cancellation_token));
@@ -167,18 +162,10 @@ struct MainLoop {
     watcher: Option<ProjectWatcher>,
 
     cli_options: Options,
-
-    /// The minimum severity to consider an error when deciding the exit status.
-    ///
-    /// TODO(micha): Get from the terminal settings.
-    min_error_severity: Severity,
 }
 
 impl MainLoop {
-    fn new(
-        cli_options: Options,
-        min_error_severity: Severity,
-    ) -> (Self, MainLoopCancellationToken) {
+    fn new(cli_options: Options) -> (Self, MainLoopCancellationToken) {
         let (sender, receiver) = crossbeam_channel::bounded(10);
 
         (
@@ -187,7 +174,6 @@ impl MainLoop {
                 receiver,
                 watcher: None,
                 cli_options,
-                min_error_severity,
             },
             MainLoopCancellationToken { sender },
         )
@@ -245,9 +231,16 @@ impl MainLoop {
                     result,
                     revision: check_revision,
                 } => {
+                    let min_error_severity =
+                        if db.project().settings(db).terminal(db).error_on_warning {
+                            Severity::Warning
+                        } else {
+                            Severity::Error
+                        };
+
                     let failed = result
                         .iter()
-                        .any(|diagnostic| diagnostic.severity() >= self.min_error_severity);
+                        .any(|diagnostic| diagnostic.severity() >= min_error_severity);
 
                     if check_revision == revision {
                         #[allow(clippy::print_stdout)]

crates/red_knot/tests/cli.rs

@@ -575,6 +575,37 @@ fn exit_code_no_errors_but_error_on_warning_is_true() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[test]
+fn exit_code_no_errors_but_error_on_warning_is_enabled_in_configuration() -> anyhow::Result<()> {
+    let case = TestCase::with_files([
+        ("test.py", r"print(x)  # [unresolved-reference]"),
+        (
+            "knot.toml",
+            r#"
+            [terminal]
+            error-on-warning = true
+        "#,
+        ),
+    ])?;
+
+    assert_cmd_snapshot!(case.command(), @r###"
+    success: false
+    exit_code: 1
+    ----- stdout -----
+    warning: lint:unresolved-reference
+     --> <temp_dir>/test.py:1:7
+      |
+    1 | print(x)  # [unresolved-reference]
+      |       - Name `x` used when not defined
+      |
+
+
+    ----- stderr -----
+    "###);
+
+    Ok(())
+}
+
 #[test]
 fn exit_code_both_warnings_and_errors() -> anyhow::Result<()> {
     let case = TestCase::with_file(

crates/red_knot_project/src/db.rs

@@ -115,7 +115,7 @@ impl SemanticDb for ProjectDatabase {
     }
 
     fn rule_selection(&self) -> &RuleSelection {
-        self.project().rule_selection(self)
+        self.project().settings(self).rules(self)
     }
 
     fn lint_registry(&self) -> &LintRegistry {

crates/red_knot_project/src/lib.rs

@@ -3,8 +3,9 @@
 use crate::metadata::options::OptionDiagnostic;
 pub use db::{Db, ProjectDatabase};
 use files::{Index, Indexed, IndexedFiles};
+use metadata::settings::Settings;
 pub use metadata::{ProjectDiscoveryError, ProjectMetadata};
-use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection};
+use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder};
 use red_knot_python_semantic::register_lints;
 use red_knot_python_semantic::types::check_types;
 use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity};
@@ -66,12 +67,22 @@ pub struct Project {
     /// The metadata describing the project, including the unresolved options.
     #[return_ref]
     pub metadata: ProjectMetadata,
+
+    /// The resolved project settings.
+    #[return_ref]
+    pub settings: Settings,
+
+    /// Diagnostics that were generated when resolving the project settings.
+    #[return_ref]
+    settings_diagnostics: Vec<OptionDiagnostic>,
 }
 
 #[salsa::tracked]
 impl Project {
     pub fn from_metadata(db: &dyn Db, metadata: ProjectMetadata) -> Self {
-        Project::builder(metadata)
+        let (settings, settings_diagnostics) = metadata.options().to_settings(db);
+
+        Project::builder(metadata, settings, settings_diagnostics)
             .durability(Durability::MEDIUM)
             .open_fileset_durability(Durability::LOW)
             .file_set_durability(Durability::LOW)
@@ -91,25 +102,22 @@ impl Project {
         assert_eq!(self.root(db), metadata.root());
 
         if &metadata != self.metadata(db) {
+            let (settings, settings_diagnostics) = metadata.options().to_settings(db);
+
+            if self.settings(db) != &settings {
+                self.set_settings(db).to(settings);
+            }
+
+            if self.settings_diagnostics(db) != &settings_diagnostics {
+                self.set_settings_diagnostics(db).to(settings_diagnostics);
+            }
+
             self.set_metadata(db).to(metadata);
         }
 
         self.reload_files(db);
     }
 
-    pub fn rule_selection(self, db: &dyn Db) -> &RuleSelection {
-        let (selection, _) = self.rule_selection_with_diagnostics(db);
-        selection
-    }
-
-    #[salsa::tracked(return_ref)]
-    fn rule_selection_with_diagnostics(
-        self,
-        db: &dyn Db,
-    ) -> (RuleSelection, Vec<OptionDiagnostic>) {
-        self.metadata(db).options().to_rule_selection(db)
-    }
-
     /// Checks all open files in the project and its dependencies.
     pub(crate) fn check(self, db: &ProjectDatabase) -> Vec<Box<dyn Diagnostic>> {
         let project_span = tracing::debug_span!("Project::check");
@@ -118,8 +126,7 @@ impl Project {
         tracing::debug!("Checking project '{name}'", name = self.name(db));
 
         let mut diagnostics: Vec<Box<dyn Diagnostic>> = Vec::new();
-        let (_, options_diagnostics) = self.rule_selection_with_diagnostics(db);
-        diagnostics.extend(options_diagnostics.iter().map(|diagnostic| {
+        diagnostics.extend(self.settings_diagnostics(db).iter().map(|diagnostic| {
             let diagnostic: Box<dyn Diagnostic> = Box::new(diagnostic.clone());
             diagnostic
         }));
@@ -151,9 +158,8 @@ impl Project {
     }
 
     pub(crate) fn check_file(self, db: &dyn Db, file: File) -> Vec<Box<dyn Diagnostic>> {
-        let (_, options_diagnostics) = self.rule_selection_with_diagnostics(db);
-
-        let mut file_diagnostics: Vec<_> = options_diagnostics
+        let mut file_diagnostics: Vec<_> = self
+            .settings_diagnostics(db)
             .iter()
             .map(|diagnostic| {
                 let diagnostic: Box<dyn Diagnostic> = Box::new(diagnostic.clone());

crates/red_knot_project/src/metadata.rs

@@ -12,6 +12,7 @@ use options::Options;
 
 pub mod options;
 pub mod pyproject;
+pub mod settings;
 pub mod value;
 
 #[derive(Debug, PartialEq, Eq)]

crates/red_knot_project/src/metadata/options.rs

@@ -10,11 +10,14 @@ use ruff_db::system::{System, SystemPath};
 use ruff_macros::Combine;
 use ruff_text_size::TextRange;
 use rustc_hash::FxHashMap;
+use salsa::Durability;
 use serde::{Deserialize, Serialize};
 use std::borrow::Cow;
 use std::fmt::Debug;
 use thiserror::Error;
 
+use super::settings::{Settings, TerminalSettings};
+
 /// The options for the project.
 #[derive(Debug, Default, Clone, PartialEq, Eq, Combine, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case", deny_unknown_fields)]
@@ -27,6 +30,9 @@ pub struct Options {
 
     #[serde(skip_serializing_if = "Option::is_none")]
     pub rules: Option<Rules>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub terminal: Option<TerminalOptions>,
 }
 
 impl Options {
@@ -107,7 +113,22 @@ impl Options {
     }
 
     #[must_use]
-    pub(crate) fn to_rule_selection(&self, db: &dyn Db) -> (RuleSelection, Vec<OptionDiagnostic>) {
+    pub(crate) fn to_settings(&self, db: &dyn Db) -> (Settings, Vec<OptionDiagnostic>) {
+        let (rules, diagnostics) = self.to_rule_selection(db);
+
+        let mut settings = Settings::builder(rules).durability(Durability::MEDIUM);
+
+        if let Some(terminal) = self.terminal.as_ref() {
+            settings = settings.terminal(TerminalSettings {
+                error_on_warning: terminal.error_on_warning.unwrap_or_default(),
+            });
+        }
+
+        (settings.new(db), diagnostics)
+    }
+
+    #[must_use]
+    fn to_rule_selection(&self, db: &dyn Db) -> (RuleSelection, Vec<OptionDiagnostic>) {
         let registry = db.lint_registry();
         let mut diagnostics = Vec::new();
 
@@ -226,6 +247,15 @@ impl FromIterator<(RangedValue<String>, RangedValue<Level>)> for Rules {
     }
 }
 
+#[derive(Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub struct TerminalOptions {
+    /// Use exit code 1 if there are any warning-level diagnostics.
+    ///
+    /// Defaults to `false`.
+    pub error_on_warning: Option<bool>,
+}
+
 #[derive(Error, Debug)]
 pub enum KnotTomlError {
     #[error(transparent)]

crates/red_knot_project/src/metadata/settings.rs

@@ -0,0 +1,26 @@
+use red_knot_python_semantic::lint::RuleSelection;
+
+/// The resolved [`Options`] for the project.
+///
+/// Unlike [`Options`], the struct has default values filled in and
+/// uses representations that are optimized for reads (instead of preserving the source representation).
+///
+/// The struct is a salsa-input for better caching. Changing the
+/// terminal settings shouldn't invalidate queries that only read
+/// the rule selection.
+///
+/// Settings that are part of [`ProgramSettings`] are not included here.
+#[salsa::input]
+pub struct Settings {
+    #[return_ref]
+    pub rules: RuleSelection,
+
+    #[default]
+    #[return_ref]
+    pub terminal: TerminalSettings,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Default)]
+pub struct TerminalSettings {
+    pub error_on_warning: bool,
+}