Add rules table to configuration (#15645)

This commit is contained in:
Micha Reiser 2025-01-23 10:56:58 +01:00 committed by GitHub
parent 23c222368e
commit 7b17c9c445
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 276 additions and 49 deletions

View file

@ -205,6 +205,97 @@ stat = add(10, 15)
Ok(())
}
/// The rule severity can be changed in the configuration file
#[test]
fn rule_severity() -> anyhow::Result<()> {
let tempdir = TempDir::new()?;
let project_dir = tempdir.path().canonicalize()?;
std::fs::write(
project_dir.join("test.py"),
r#"
y = 4 / 0
for a in range(0, y):
x = a
print(x) # possibly-unresolved-reference
"#,
)
.context("Failed to write `test.py`")?;
// Assert that there's a possibly unresolved reference diagnostic
// and that division-by-zero has a severity of error by default.
insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "<temp_dir>/")]}, {
assert_cmd_snapshot!(knot().current_dir(&project_dir), @r"
success: false
exit_code: 1
----- stdout -----
error[lint:division-by-zero] <temp_dir>/test.py:2:5 Cannot divide object of type `Literal[4]` by zero
warning[lint:possibly-unresolved-reference] <temp_dir>/test.py:7:7 Name `x` used when possibly not defined
----- stderr -----
");
});
std::fs::write(
project_dir.join("pyproject.toml"),
r#"
[tool.knot.rules]
division-by-zero = "warn" # demote to warn
possibly-unresolved-reference = "ignore"
"#,
)
.context("Failed to write `pyproject.toml`")?;
insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "<temp_dir>/")]}, {
assert_cmd_snapshot!(knot().current_dir(project_dir), @r"
success: false
exit_code: 1
----- stdout -----
warning[lint:division-by-zero] <temp_dir>/test.py:2:5 Cannot divide object of type `Literal[4]` by zero
----- stderr -----
");
});
Ok(())
}
/// Red Knot warns about unknown rules
#[test]
fn unknown_rules() -> anyhow::Result<()> {
let tempdir = TempDir::new()?;
let project_dir = tempdir.path().canonicalize()?;
std::fs::write(
project_dir.join("pyproject.toml"),
r#"
[tool.knot.rules]
division-by-zer = "warn" # incorrect rule name
"#,
)
.context("Failed to write `pyproject.toml`")?;
std::fs::write(project_dir.join("test.py"), r#"print(10)"#)
.context("Failed to write `test.py`")?;
insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "<temp_dir>/")]}, {
assert_cmd_snapshot!(knot().current_dir(project_dir), @r"
success: false
exit_code: 1
----- stdout -----
warning[unknown-rule] Unknown lint rule `division-by-zer`
----- stderr -----
");
});
Ok(())
}
fn knot() -> Command {
Command::new(get_cargo_bin("red_knot"))
}

View file

@ -2,7 +2,7 @@ use std::panic::RefUnwindSafe;
use std::sync::Arc;
use crate::DEFAULT_LINT_REGISTRY;
use crate::{check_file, Project, ProjectMetadata};
use crate::{Project, ProjectMetadata};
use red_knot_python_semantic::lint::{LintRegistry, RuleSelection};
use red_knot_python_semantic::{Db as SemanticDb, Program};
use ruff_db::diagnostic::Diagnostic;
@ -27,7 +27,6 @@ pub struct ProjectDatabase {
storage: salsa::Storage<ProjectDatabase>,
files: Files,
system: Arc<dyn System + Send + Sync + RefUnwindSafe>,
rule_selection: Arc<RuleSelection>,
}
impl ProjectDatabase {
@ -35,14 +34,11 @@ impl ProjectDatabase {
where
S: System + 'static + Send + Sync + RefUnwindSafe,
{
let rule_selection = RuleSelection::from_registry(&DEFAULT_LINT_REGISTRY);
let mut db = Self {
project: None,
storage: salsa::Storage::default(),
files: Files::default(),
system: Arc::new(system),
rule_selection: Arc::new(rule_selection),
};
// TODO: Use the `program_settings` to compute the key for the database's persistent
@ -66,7 +62,7 @@ impl ProjectDatabase {
pub fn check_file(&self, file: File) -> Result<Vec<Box<dyn Diagnostic>>, Cancelled> {
let _span = tracing::debug_span!("check_file", file=%file.path(self)).entered();
self.with_db(|db| check_file(db, file))
self.with_db(|db| self.project().check_file(db, file))
}
/// Returns a mutable reference to the system.
@ -119,7 +115,7 @@ impl SemanticDb for ProjectDatabase {
}
fn rule_selection(&self) -> &RuleSelection {
&self.rule_selection
self.project().rule_selection(self)
}
fn lint_registry(&self) -> &LintRegistry {

View file

@ -1,6 +1,10 @@
#![allow(clippy::ref_option)]
use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder};
use crate::metadata::options::OptionDiagnostic;
pub use db::{Db, ProjectDatabase};
use files::{Index, Indexed, IndexedFiles};
pub use metadata::{ProjectDiscoveryError, ProjectMetadata};
use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection};
use red_knot_python_semantic::register_lints;
use red_knot_python_semantic::types::check_types;
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, ParseDiagnostic, Severity};
@ -17,10 +21,6 @@ use salsa::Setter;
use std::borrow::Cow;
use std::sync::Arc;
pub use db::{Db, ProjectDatabase};
use files::{Index, Indexed, IndexedFiles};
pub use metadata::{ProjectDiscoveryError, ProjectMetadata};
pub mod combine;
mod db;
@ -68,6 +68,7 @@ pub struct Project {
pub metadata: ProjectMetadata,
}
#[salsa::tracked]
impl Project {
pub fn from_metadata(db: &dyn Db, metadata: ProjectMetadata) -> Self {
Project::builder(metadata)
@ -96,13 +97,34 @@ impl Project {
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 fn check(self, db: &ProjectDatabase) -> Vec<Box<dyn Diagnostic>> {
pub(crate) fn check(self, db: &ProjectDatabase) -> Vec<Box<dyn Diagnostic>> {
let project_span = tracing::debug_span!("Project::check");
let _span = project_span.enter();
tracing::debug!("Checking project '{name}'", name = self.name(db));
let result = Arc::new(std::sync::Mutex::new(Vec::new()));
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| {
let diagnostic: Box<dyn Diagnostic> = Box::new(diagnostic.clone());
diagnostic
}));
let result = Arc::new(std::sync::Mutex::new(diagnostics));
let inner_result = Arc::clone(&result);
let db = db.clone();
@ -119,7 +141,7 @@ impl Project {
let check_file_span = tracing::debug_span!(parent: &project_span, "check_file", file=%file.path(&db));
let _entered = check_file_span.entered();
let file_diagnostics = check_file(&db, file);
let file_diagnostics = check_file_impl(&db, file);
result.lock().unwrap().extend(file_diagnostics);
});
}
@ -128,6 +150,23 @@ impl Project {
Arc::into_inner(result).unwrap().into_inner().unwrap()
}
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
.iter()
.map(|diagnostic| {
let diagnostic: Box<dyn Diagnostic> = Box::new(diagnostic.clone());
diagnostic
})
.collect();
let check_diagnostics = check_file_impl(db, file);
file_diagnostics.extend(check_diagnostics);
file_diagnostics
}
/// Opens a file in the project.
///
/// This changes the behavior of `check` to only check the open files rather than all files in the project.
@ -265,8 +304,9 @@ impl Project {
}
}
pub(crate) fn check_file(db: &dyn Db, file: File) -> Vec<Box<dyn Diagnostic>> {
fn check_file_impl(db: &dyn Db, file: File) -> Vec<Box<dyn Diagnostic>> {
let mut diagnostics: Vec<Box<dyn Diagnostic>> = Vec::new();
// Abort checking if there are IO errors.
let source = source_text(db.upcast(), file);
@ -418,7 +458,7 @@ impl Diagnostic for IOErrorDiagnostic {
#[cfg(test)]
mod tests {
use crate::db::tests::TestDb;
use crate::{check_file, ProjectMetadata};
use crate::{check_file_impl, ProjectMetadata};
use red_knot_python_semantic::types::check_types;
use ruff_db::diagnostic::Diagnostic;
use ruff_db::files::system_path_to_file;
@ -442,7 +482,7 @@ mod tests {
assert_eq!(source_text(&db, file).as_str(), "");
assert_eq!(
check_file(&db, file)
check_file_impl(&db, file)
.into_iter()
.map(|diagnostic| diagnostic.message().into_owned())
.collect::<Vec<_>>(),
@ -458,7 +498,7 @@ mod tests {
assert_eq!(source_text(&db, file).as_str(), "");
assert_eq!(
check_file(&db, file)
check_file_impl(&db, file)
.into_iter()
.map(|diagnostic| diagnostic.message().into_owned())
.collect::<Vec<_>>(),

View file

@ -1,10 +1,17 @@
use crate::metadata::value::{RelativePathBuf, ValueSource, ValueSourceGuard};
use crate::Db;
use red_knot_python_semantic::lint::{GetLintError, Level, 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::system::{System, SystemPath};
use ruff_macros::Combine;
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize};
use std::borrow::Cow;
use std::fmt::Debug;
use thiserror::Error;
@ -12,9 +19,14 @@ use thiserror::Error;
#[derive(Debug, Default, Clone, PartialEq, Eq, Combine, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct Options {
#[serde(skip_serializing_if = "Option::is_none")]
pub environment: Option<EnvironmentOptions>,
#[serde(skip_serializing_if = "Option::is_none")]
pub src: Option<SrcOptions>,
#[serde(skip_serializing_if = "Option::is_none")]
pub rules: Option<Rules>,
}
impl Options {
@ -88,27 +100,75 @@ impl Options {
.unwrap_or(SitePackages::Known(vec![])),
}
}
#[must_use]
pub(crate) fn to_rule_selection(&self, db: &dyn Db) -> (RuleSelection, Vec<OptionDiagnostic>) {
let registry = db.lint_registry();
let mut diagnostics = Vec::new();
// Initialize the selection with the defaults
let mut selection = RuleSelection::from_registry(registry);
let rules = self
.rules
.as_ref()
.into_iter()
.flat_map(|rules| rules.inner.iter());
for (rule_name, level) in rules {
match registry.get(rule_name) {
Ok(lint) => {
if let Ok(severity) = Severity::try_from(*level) {
selection.enable(lint, severity);
} 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,
));
}
}
}
(selection, diagnostics)
}
}
#[derive(Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct EnvironmentOptions {
#[serde(skip_serializing_if = "Option::is_none")]
pub python_version: Option<PythonVersion>,
#[serde(skip_serializing_if = "Option::is_none")]
pub python_platform: Option<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,
/// or pyright's stubPath configuration setting.
#[serde(skip_serializing_if = "Option::is_none")]
pub extra_paths: Option<Vec<RelativePathBuf>>,
/// Optional path to a "typeshed" directory on disk for us to use for standard-library types.
/// If this is not provided, we will fallback to our vendored typeshed stubs for the stdlib,
/// bundled as a zip file in the binary
#[serde(skip_serializing_if = "Option::is_none")]
pub typeshed: Option<RelativePathBuf>,
// TODO: Rename to python, see https://github.com/astral-sh/ruff/issues/15530
/// The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed.
#[serde(skip_serializing_if = "Option::is_none")]
pub venv_path: Option<RelativePathBuf>,
}
@ -116,11 +176,57 @@ pub struct EnvironmentOptions {
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct SrcOptions {
/// The root of the project, used for finding first-party modules.
#[serde(skip_serializing_if = "Option::is_none")]
pub root: Option<RelativePathBuf>,
}
#[derive(Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", transparent)]
pub struct Rules {
inner: FxHashMap<String, Level>,
}
#[derive(Error, Debug)]
pub enum KnotTomlError {
#[error(transparent)]
TomlSyntax(#[from] toml::de::Error),
}
#[derive(Debug, PartialEq, Eq, Clone)]
pub struct OptionDiagnostic {
id: DiagnosticId,
message: String,
severity: Severity,
}
impl OptionDiagnostic {
pub fn new(id: DiagnosticId, message: String, severity: Severity) -> Self {
Self {
id,
message,
severity,
}
}
}
impl Diagnostic for OptionDiagnostic {
fn id(&self) -> DiagnosticId {
self.id
}
fn message(&self) -> Cow<str> {
Cow::Borrowed(&self.message)
}
fn file(&self) -> Option<File> {
None
}
fn range(&self) -> Option<TextRange> {
None
}
fn severity(&self) -> Severity {
self.severity
}
}

View file

@ -6,7 +6,6 @@ ProjectMetadata(
name: Name("project-root"),
root: "/app",
options: Options(
environment: None,
src: Some(SrcOptions(
root: Some("src"),
)),

View file

@ -6,7 +6,6 @@ ProjectMetadata(
name: Name("nested-project"),
root: "/app/packages/a",
options: Options(
environment: None,
src: Some(SrcOptions(
root: Some("src"),
)),

View file

@ -8,11 +8,6 @@ ProjectMetadata(
options: Options(
environment: Some(EnvironmentOptions(
r#python-version: Some("3.10"),
r#python-platform: None,
r#extra-paths: None,
typeshed: None,
r#venv-path: None,
)),
src: None,
),
)

View file

@ -5,8 +5,5 @@ expression: sub_project
ProjectMetadata(
name: Name("nested-project"),
root: "/app/packages/a",
options: Options(
environment: None,
src: None,
),
options: Options(),
)

View file

@ -6,7 +6,6 @@ ProjectMetadata(
name: Name("super-app"),
root: "/app",
options: Options(
environment: None,
src: Some(SrcOptions(
root: Some("src"),
)),

View file

@ -5,8 +5,5 @@ expression: project
ProjectMetadata(
name: Name("backend"),
root: "/app",
options: Options(
environment: None,
src: None,
),
options: Options(),
)

View file

@ -5,8 +5,5 @@ expression: project
ProjectMetadata(
name: Name("app"),
root: "/app",
options: Options(
environment: None,
src: None,
),
options: Options(),
)

View file

@ -31,6 +31,11 @@ pub struct LintMetadata {
}
#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize),
serde(rename_all = "kebab-case")
)]
pub enum Level {
/// The lint is disabled and should not run.
Ignore,
@ -404,7 +409,7 @@ impl From<&'static LintMetadata> for LintEntry {
}
}
#[derive(Debug, Clone, Default)]
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct RuleSelection {
/// Map with the severity for each enabled lint rule.
///

View file

@ -73,6 +73,9 @@ pub enum DiagnosticId {
/// A revealed type: Created by `reveal_type(expression)`.
RevealedType,
/// No rule with the given name exists.
UnknownRule,
}
impl DiagnosticId {
@ -112,15 +115,18 @@ impl DiagnosticId {
}
pub fn as_str(&self) -> Result<&str, DiagnosticAsStrError> {
match self {
DiagnosticId::Io => Ok("io"),
DiagnosticId::InvalidSyntax => Ok("invalid-syntax"),
DiagnosticId::Lint(name) => Err(DiagnosticAsStrError::Category {
category: "lint",
name: name.as_str(),
}),
DiagnosticId::RevealedType => Ok("revealed-type"),
}
Ok(match self {
DiagnosticId::Io => "io",
DiagnosticId::InvalidSyntax => "invalid-syntax",
DiagnosticId::Lint(name) => {
return Err(DiagnosticAsStrError::Category {
category: "lint",
name: name.as_str(),
})
}
DiagnosticId::RevealedType => "revealed-type",
DiagnosticId::UnknownRule => "unknown-rule",
})
}
}