Skip to content

Commit

Permalink
Add rules table to configuration (#15645)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Jan 23, 2025
1 parent 23c2223 commit 7b17c9c
Show file tree
Hide file tree
Showing 13 changed files with 276 additions and 49 deletions.
91 changes: 91 additions & 0 deletions crates/red_knot/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down
10 changes: 3 additions & 7 deletions crates/red_knot_project/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,22 +27,18 @@ pub struct ProjectDatabase {
storage: salsa::Storage<ProjectDatabase>,
files: Files,
system: Arc<dyn System + Send + Sync + RefUnwindSafe>,
rule_selection: Arc<RuleSelection>,
}

impl ProjectDatabase {
pub fn new<S>(project_metadata: ProjectMetadata, system: S) -> anyhow::Result<Self>
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
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
64 changes: 52 additions & 12 deletions crates/red_knot_project/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand All @@ -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);
});
}
Expand All @@ -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.
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand All @@ -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<_>>(),
Expand All @@ -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<_>>(),
Expand Down
Loading

0 comments on commit 7b17c9c

Please sign in to comment.