Use RangedValue in Options

Author: MichaReiser

crates/red_knot/src/main.rs

@@ -7,7 +7,7 @@ use colored::Colorize;
 use crossbeam::channel as crossbeam_channel;
 use python_version::PythonVersion;
 use red_knot_project::metadata::options::{EnvironmentOptions, Options};
-use red_knot_project::metadata::value::RelativePathBuf;
+use red_knot_project::metadata::value::{RangedValue, RelativePathBuf};
 use red_knot_project::watch;
 use red_knot_project::watch::ProjectWatcher;
 use red_knot_project::{ProjectDatabase, ProjectMetadata};
@@ -73,7 +73,9 @@ impl Args {
     fn to_options(&self) -> Options {
         Options {
             environment: Some(EnvironmentOptions {
-                python_version: self.python_version.map(Into::into),
+                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| {

crates/red_knot/tests/cli.rs

@@ -255,7 +255,7 @@ division-by-zer = "warn" # incorrect rule name
         success: false
         exit_code: 1
         ----- stdout -----
-        warning[unknown-rule] Unknown lint rule `division-by-zer`
+        warning[unknown-rule] <temp_dir>/pyproject.toml:3:1 Unknown lint rule `division-by-zer`
 
         ----- stderr -----
         ");

crates/red_knot/tests/file_watching.rs

@@ -6,7 +6,7 @@ use std::time::{Duration, Instant};
 use anyhow::{anyhow, Context};
 use red_knot_project::metadata::options::{EnvironmentOptions, Options};
 use red_knot_project::metadata::pyproject::{PyProject, Tool};
-use red_knot_project::metadata::value::RelativePathBuf;
+use red_knot_project::metadata::value::{RangedValue, RelativePathBuf};
 use red_knot_project::watch::{directory_watcher, ChangeEvent, ProjectWatcher};
 use red_knot_project::{Db, ProjectDatabase, ProjectMetadata};
 use red_knot_python_semantic::{resolve_module, ModuleName, PythonPlatform, PythonVersion};
@@ -897,8 +897,10 @@ print(sys.last_exc, os.getegid())
         |_root_path, _project_path| {
             Some(Options {
                 environment: Some(EnvironmentOptions {
-                    python_version: Some(PythonVersion::PY311),
-                    python_platform: Some(PythonPlatform::Identifier("win32".to_string())),
+                    python_version: Some(RangedValue::cli(PythonVersion::PY311)),
+                    python_platform: Some(RangedValue::cli(PythonPlatform::Identifier(
+                        "win32".to_string(),
+                    ))),
                     ..EnvironmentOptions::default()
                 }),
                 ..Options::default()
@@ -921,8 +923,10 @@ print(sys.last_exc, os.getegid())
     // Change the python version
     case.update_options(Options {
         environment: Some(EnvironmentOptions {
-            python_version: Some(PythonVersion::PY312),
-            python_platform: Some(PythonPlatform::Identifier("linux".to_string())),
+            python_version: Some(RangedValue::cli(PythonVersion::PY312)),
+            python_platform: Some(RangedValue::cli(PythonPlatform::Identifier(
+                "linux".to_string(),
+            ))),
             ..EnvironmentOptions::default()
         }),
         ..Options::default()
@@ -1382,7 +1386,7 @@ mod unix {
                         extra_paths: Some(vec![RelativePathBuf::cli(
                             ".venv/lib/python3.12/site-packages",
                         )]),
-                        python_version: Some(PythonVersion::PY312),
+                        python_version: Some(RangedValue::cli(PythonVersion::PY312)),
                         ..EnvironmentOptions::default()
                     }),
                     ..Options::default()

crates/red_knot_project/src/metadata.rs

@@ -55,7 +55,7 @@ impl ProjectMetadata {
     ) -> Self {
         let name = project
             .and_then(|project| project.name.as_ref())
-            .map(|name| Name::new(&**name))
+            .map(|name| Name::new(&***name))
             .unwrap_or_else(|| Name::new(root.file_name().unwrap_or("root")));
 
         // TODO(https://github.com/astral-sh/ruff/issues/15491): Respect requires-python

crates/red_knot_project/src/metadata/options.rs

@@ -1,11 +1,11 @@
-use crate::metadata::value::{RelativePathBuf, ValueSource, ValueSourceGuard};
+use crate::metadata::value::{RangedValue, RelativePathBuf, ValueSource, ValueSourceGuard};
 use crate::Db;
-use red_knot_python_semantic::lint::{GetLintError, Level, RuleSelection};
+use red_knot_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection};
 use red_knot_python_semantic::{
     ProgramSettings, PythonPlatform, PythonVersion, SearchPathSettings, SitePackages,
 };
 use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
-use ruff_db::files::File;
+use ruff_db::files::{system_path_to_file, File};
 use ruff_db::system::{System, SystemPath};
 use ruff_macros::Combine;
 use ruff_text_size::TextRange;
@@ -44,7 +44,12 @@ impl Options {
         let (python_version, python_platform) = self
             .environment
             .as_ref()
-            .map(|env| (env.python_version, env.python_platform.as_ref()))
+            .map(|env| {
+                (
+                    env.python_version.as_deref().copied(),
+                    env.python_platform.as_deref(),
+                )
+            })
             .unwrap_or_default();
 
         ProgramSettings {
@@ -116,27 +121,42 @@ impl Options {
             .flat_map(|rules| rules.inner.iter());
 
         for (rule_name, level) in rules {
+            let source = rule_name.source();
             match registry.get(rule_name) {
                 Ok(lint) => {
-                    if let Ok(severity) = Severity::try_from(*level) {
-                        selection.enable(lint, severity);
+                    let lint_source = match source {
+                        ValueSource::File(_) => LintSource::File,
+                        ValueSource::Cli => LintSource::Cli,
+                    };
+                    if let Ok(severity) = Severity::try_from(**level) {
+                        selection.enable(lint, severity, lint_source);
                     } else {
                         selection.disable(lint);
                     }
                 }
-                Err(GetLintError::Unknown(_)) => {
-                    diagnostics.push(OptionDiagnostic::new(
-                        DiagnosticId::UnknownRule,
-                        format!("Unknown lint rule `{rule_name}`"),
-                        Severity::Warning,
-                    ));
-                }
-                Err(GetLintError::Removed(_)) => {
-                    diagnostics.push(OptionDiagnostic::new(
-                        DiagnosticId::UnknownRule,
-                        format!("The lint rule `{rule_name}` has been removed and is no longer supported"),
-                        Severity::Warning,
-                    ));
+                Err(error) => {
+                    // `system_path_to_file` can return `Err` if the file was deleted since the configuration
+                    // was read. This should be rare and it should be okay to default to not showing a configuration
+                    // file in that case.
+                    let file = source
+                        .file()
+                        .and_then(|path| system_path_to_file(db.upcast(), path).ok());
+
+                    // TODO: Add a note if the value was configured on the CLI
+                    let diagnostic = match error {
+                        GetLintError::Unknown(_) => OptionDiagnostic::new(
+                            DiagnosticId::UnknownRule,
+                            format!("Unknown lint rule `{rule_name}`"),
+                            Severity::Warning,
+                        ),
+                        GetLintError::Removed(_) => OptionDiagnostic::new(
+                            DiagnosticId::UnknownRule,
+                            format!("Unknown lint rule `{rule_name}`"),
+                            Severity::Warning,
+                        ),
+                    };
+
+                    diagnostics.push(diagnostic.with_file(file).with_range(rule_name.range()));
                 }
             }
         }
@@ -149,10 +169,10 @@ impl Options {
 #[serde(rename_all = "kebab-case", deny_unknown_fields)]
 pub struct EnvironmentOptions {
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub python_version: Option<PythonVersion>,
+    pub python_version: Option<RangedValue<PythonVersion>>,
 
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub python_platform: Option<PythonPlatform>,
+    pub python_platform: Option<RangedValue<PythonPlatform>>,
 
     /// List of user-provided paths that should take first priority in the module resolution.
     /// Examples in other type checkers are mypy's MYPYPATH environment variable,
@@ -183,7 +203,7 @@ pub struct SrcOptions {
 #[derive(Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case", transparent)]
 pub struct Rules {
-    inner: FxHashMap<String, Level>,
+    inner: FxHashMap<RangedValue<String>, RangedValue<Level>>,
 }
 
 #[derive(Error, Debug)]
@@ -197,6 +217,8 @@ pub struct OptionDiagnostic {
     id: DiagnosticId,
     message: String,
     severity: Severity,
+    file: Option<File>,
+    range: Option<TextRange>,
 }
 
 impl OptionDiagnostic {
@@ -205,8 +227,22 @@ impl OptionDiagnostic {
             id,
             message,
             severity,
+            file: None,
+            range: None,
         }
     }
+
+    #[must_use]
+    fn with_file(mut self, file: Option<File>) -> Self {
+        self.file = file;
+        self
+    }
+
+    #[must_use]
+    fn with_range(mut self, range: Option<TextRange>) -> Self {
+        self.range = range;
+        self
+    }
 }
 
 impl Diagnostic for OptionDiagnostic {
@@ -219,11 +255,11 @@ impl Diagnostic for OptionDiagnostic {
     }
 
     fn file(&self) -> Option<File> {
-        None
+        self.file
     }
 
     fn range(&self) -> Option<TextRange> {
-        None
+        self.range
     }
 
     fn severity(&self) -> Severity {

crates/red_knot_project/src/metadata/pyproject.rs

@@ -4,7 +4,7 @@ use std::ops::Deref;
 use thiserror::Error;
 
 use crate::metadata::options::Options;
-use crate::metadata::value::{ValueSource, ValueSourceGuard};
+use crate::metadata::value::{RangedValue, ValueSource, ValueSourceGuard};
 
 /// A `pyproject.toml` as specified in PEP 517.
 #[derive(Deserialize, Serialize, Debug, Default, Clone)]
@@ -48,11 +48,11 @@ pub struct Project {
     ///
     /// Note: Intentionally option to be more permissive during deserialization.
     /// `PackageMetadata::from_pyproject` reports missing names.
-    pub name: Option<PackageName>,
+    pub name: Option<RangedValue<PackageName>>,
     /// The version of the project
-    pub version: Option<Version>,
+    pub version: Option<RangedValue<Version>>,
     /// The Python versions this project is compatible with.
-    pub requires_python: Option<VersionSpecifiers>,
+    pub requires_python: Option<RangedValue<VersionSpecifiers>>,
 }
 
 #[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]

crates/red_knot_project/src/metadata/value.rs

@@ -23,6 +23,15 @@ pub enum ValueSource {
     Cli,
 }
 
+impl ValueSource {
+    pub fn file(&self) -> Option<&SystemPath> {
+        match self {
+            ValueSource::File(path) => Some(&**path),
+            ValueSource::Cli => None,
+        }
+    }
+}
+
 impl fmt::Display for ValueSource {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -62,7 +71,7 @@ impl Drop for ValueSourceGuard {
     }
 }
 
-/// A configuration value that tracks where it originates from and the range in the source document.
+/// A value that "remembers" where it comes from (source) and its range in source.
 ///
 /// ## Equality, Hash, and Ordering
 /// The equality, hash, and ordering are solely based on the value. They disregard the value's range
@@ -74,6 +83,11 @@ impl Drop for ValueSourceGuard {
 pub struct RangedValue<T> {
     value: T,
     source: ValueSource,
+
+    /// The byte range of `value` in `source`.
+    ///
+    /// Can be `None` because not all sources support a range.
+    /// For example, arguments provided on the CLI won't have a range attached.
     range: Option<TextRange>,
 }
 
@@ -98,6 +112,10 @@ impl<T> RangedValue<T> {
         self.range
     }
 
+    pub fn source(&self) -> &ValueSource {
+        &self.source
+    }
+
     #[must_use]
     pub fn with_source(mut self, source: ValueSource) -> Self {
         self.source = source;

crates/red_knot_python_semantic/src/lint.rs

@@ -414,7 +414,7 @@ pub struct RuleSelection {
     /// Map with the severity for each enabled lint rule.
     ///
     /// If a rule isn't present in this map, then it should be considered disabled.
-    lints: FxHashMap<LintId, Severity>,
+    lints: FxHashMap<LintId, (Severity, LintSource)>,
 }
 
 impl RuleSelection {
@@ -427,7 +427,7 @@ impl RuleSelection {
             .filter_map(|lint| {
                 Severity::try_from(lint.default_level())
                     .ok()
-                    .map(|severity| (*lint, severity))
+                    .map(|severity| (*lint, (severity, LintSource::Default)))
             })
             .collect();
 
@@ -441,12 +441,14 @@ impl RuleSelection {
 
     /// Returns an iterator over all enabled lints and their severity.
     pub fn iter(&self) -> impl ExactSizeIterator<Item = (LintId, Severity)> + '_ {
-        self.lints.iter().map(|(&lint, &severity)| (lint, severity))
+        self.lints
+            .iter()
+            .map(|(&lint, &(severity, _))| (lint, severity))
     }
 
     /// Returns the configured severity for the lint with the given id or `None` if the lint is disabled.
     pub fn severity(&self, lint: LintId) -> Option<Severity> {
-        self.lints.get(&lint).copied()
+        self.lints.get(&lint).map(|(severity, _)| *severity)
     }
 
     /// Returns `true` if the `lint` is enabled.
@@ -457,19 +459,25 @@ impl RuleSelection {
     /// Enables `lint` and configures with the given `severity`.
     ///
     /// Overrides any previous configuration for the lint.
-    pub fn enable(&mut self, lint: LintId, severity: Severity) {
-        self.lints.insert(lint, severity);
+    pub fn enable(&mut self, lint: LintId, severity: Severity, source: LintSource) {
+        self.lints.insert(lint, (severity, source));
     }
 
     /// Disables `lint` if it was previously enabled.
     pub fn disable(&mut self, lint: LintId) {
         self.lints.remove(&lint);
     }
+}
 
-    /// Merges the enabled lints from `other` into this selection.
-    ///
-    /// Lints from `other` will override any existing configuration.
-    pub fn merge(&mut self, other: &RuleSelection) {
-        self.lints.extend(other.iter());
-    }
+#[derive(Default, Copy, Clone, Debug, PartialEq, Eq)]
+pub enum LintSource {
+    /// The user didn't enable the rule explicitly, instead it's enabled by default.
+    #[default]
+    Default,
+
+    /// The rule was enabled by using a CLI argument
+    Cli,
+
+    /// The rule was enabled in a configuration file.
+    File,
 }

crates/red_knot_wasm/src/lib.rs

@@ -4,6 +4,7 @@ use js_sys::Error;
 use wasm_bindgen::prelude::*;
 
 use red_knot_project::metadata::options::{EnvironmentOptions, Options};
+use red_knot_project::metadata::value::RangedValue;
 use red_knot_project::ProjectMetadata;
 use red_knot_project::{Db, ProjectDatabase};
 use ruff_db::diagnostic::Diagnostic;
@@ -48,7 +49,7 @@ impl Workspace {
 
         workspace.apply_cli_options(Options {
             environment: Some(EnvironmentOptions {
-                python_version: Some(settings.python_version.into()),
+                python_version: Some(RangedValue::cli(settings.python_version.into())),
                 ..EnvironmentOptions::default()
             }),
             ..Options::default()