Create Unknown rule diagnostics with a source range (#15648)

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

@@ -287,7 +287,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)]