Skip to content

Commit

Permalink
Create Unknown rule diagnostics with a source range (#15648)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Jan 23, 2025
1 parent 1e790d3 commit 05ea77b
Show file tree
Hide file tree
Showing 10 changed files with 341 additions and 115 deletions.
6 changes: 4 additions & 2 deletions crates/red_knot/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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| {
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 -----
");
Expand Down
16 changes: 10 additions & 6 deletions crates/red_knot/tests/file_watching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_project/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 60 additions & 24 deletions crates/red_knot_project/src/metadata/options.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()));
}
}
}
Expand All @@ -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,
Expand Down Expand Up @@ -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)]
Expand All @@ -197,6 +217,8 @@ pub struct OptionDiagnostic {
id: DiagnosticId,
message: String,
severity: Severity,
file: Option<File>,
range: Option<TextRange>,
}

impl OptionDiagnostic {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions crates/red_knot_project/src/metadata/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down
Loading

0 comments on commit 05ea77b

Please sign in to comment.