[red-knot] Add --ignore, --warn, and --error CLI arguments (#15689)

This commit is contained in:
Micha Reiser 2025-01-24 16:20:15 +01:00 committed by GitHub
parent ab2e1905c4
commit 4e3982cf95
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 396 additions and 83 deletions

2
.gitignore vendored
View file

@ -30,7 +30,7 @@ tracing-flamechart.svg
tracing-flamegraph.svg
# insta
.rs.pending-snap
*.rs.pending-snap
###

181
crates/red_knot/src/args.rs Normal file
View file

@ -0,0 +1,181 @@
use crate::logging::Verbosity;
use crate::python_version::PythonVersion;
use crate::Command;
use clap::{ArgAction, ArgMatches, Error, Parser};
use red_knot_project::metadata::options::{EnvironmentOptions, Options};
use red_knot_project::metadata::value::{RangedValue, RelativePathBuf};
use red_knot_python_semantic::lint;
use ruff_db::system::SystemPathBuf;
#[derive(Debug, Parser)]
#[command(
author,
name = "red-knot",
about = "An extremely fast Python type checker."
)]
#[command(version)]
pub(crate) struct Args {
#[command(subcommand)]
pub(crate) command: Option<Command>,
/// Run the command within the given project directory.
///
/// All `pyproject.toml` files will be discovered by walking up the directory tree from the given project directory,
/// as will the project's virtual environment (`.venv`) unless the `venv-path` option is set.
///
/// Other command-line arguments (such as relative paths) will be resolved relative to the current working directory.
#[arg(long, value_name = "PROJECT")]
pub(crate) project: Option<SystemPathBuf>,
/// Path to the virtual environment the project uses.
///
/// If provided, red-knot will use the `site-packages` directory of this virtual environment
/// to resolve type information for the project's third-party dependencies.
#[arg(long, value_name = "PATH")]
pub(crate) venv_path: Option<SystemPathBuf>,
/// Custom directory to use for stdlib typeshed stubs.
#[arg(long, value_name = "PATH", alias = "custom-typeshed-dir")]
pub(crate) typeshed: Option<SystemPathBuf>,
/// Additional path to use as a module-resolution source (can be passed multiple times).
#[arg(long, value_name = "PATH")]
pub(crate) extra_search_path: Option<Vec<SystemPathBuf>>,
/// Python version to assume when resolving types.
#[arg(long, value_name = "VERSION", alias = "target-version")]
pub(crate) python_version: Option<PythonVersion>,
#[clap(flatten)]
pub(crate) verbosity: Verbosity,
#[clap(flatten)]
pub(crate) rules: RulesArg,
/// Run in watch mode by re-running whenever files change.
#[arg(long, short = 'W')]
pub(crate) watch: bool,
}
impl Args {
pub(crate) fn to_options(&self) -> Options {
let rules = if self.rules.is_empty() {
None
} else {
Some(
self.rules
.iter()
.map(|(rule, level)| {
(RangedValue::cli(rule.to_string()), RangedValue::cli(level))
})
.collect(),
)
};
Options {
environment: Some(EnvironmentOptions {
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| {
extra_search_paths
.iter()
.map(RelativePathBuf::cli)
.collect()
}),
..EnvironmentOptions::default()
}),
rules,
..Default::default()
}
}
}
/// A list of rules to enable or disable with a given severity.
///
/// This type is used to parse the `--error`, `--warn`, and `--ignore` arguments
/// while preserving the order in which they were specified (arguments last override previous severities).
#[derive(Debug)]
pub(crate) struct RulesArg(Vec<(String, lint::Level)>);
impl RulesArg {
fn is_empty(&self) -> bool {
self.0.is_empty()
}
fn iter(&self) -> impl Iterator<Item = (&str, lint::Level)> {
self.0.iter().map(|(rule, level)| (rule.as_str(), *level))
}
}
impl clap::FromArgMatches for RulesArg {
fn from_arg_matches(matches: &ArgMatches) -> Result<Self, Error> {
let mut rules = Vec::new();
for (level, arg_id) in [
(lint::Level::Ignore, "ignore"),
(lint::Level::Warn, "warn"),
(lint::Level::Error, "error"),
] {
let indices = matches.indices_of(arg_id).into_iter().flatten();
let levels = matches.get_many::<String>(arg_id).into_iter().flatten();
rules.extend(
indices
.zip(levels)
.map(|(index, rule)| (index, rule, level)),
);
}
// Sort by their index so that values specified later override earlier ones.
rules.sort_by_key(|(index, _, _)| *index);
Ok(Self(
rules
.into_iter()
.map(|(_, rule, level)| (rule.to_owned(), level))
.collect(),
))
}
fn update_from_arg_matches(&mut self, matches: &ArgMatches) -> Result<(), Error> {
self.0 = Self::from_arg_matches(matches)?.0;
Ok(())
}
}
impl clap::Args for RulesArg {
fn augment_args(cmd: clap::Command) -> clap::Command {
const HELP_HEADING: &str = "Enabling / disabling rules";
cmd.arg(
clap::Arg::new("error")
.long("error")
.action(ArgAction::Append)
.help("Treat the given rule as having severity 'error'. Can be specified multiple times.")
.value_name("RULE")
.help_heading(HELP_HEADING),
)
.arg(
clap::Arg::new("warn")
.long("warn")
.action(ArgAction::Append)
.help("Treat the given rule as having severity 'warn'. Can be specified multiple times.")
.value_name("RULE")
.help_heading(HELP_HEADING),
)
.arg(
clap::Arg::new("ignore")
.long("ignore")
.action(ArgAction::Append)
.help("Disables the rule. Can be specified multiple times.")
.value_name("RULE")
.help_heading(HELP_HEADING),
)
}
fn augment_args_for_update(cmd: clap::Command) -> clap::Command {
Self::augment_args(cmd)
}
}

View file

@ -1,13 +1,13 @@
use std::process::{ExitCode, Termination};
use std::sync::Mutex;
use crate::args::Args;
use crate::logging::setup_tracing;
use anyhow::{anyhow, Context};
use clap::Parser;
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::{RangedValue, RelativePathBuf};
use red_knot_project::metadata::options::Options;
use red_knot_project::watch;
use red_knot_project::watch::ProjectWatcher;
use red_knot_project::{ProjectDatabase, ProjectMetadata};
@ -16,81 +16,11 @@ use ruff_db::diagnostic::Diagnostic;
use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf};
use salsa::plumbing::ZalsaDatabase;
use crate::logging::{setup_tracing, Verbosity};
mod args;
mod logging;
mod python_version;
mod verbosity;
#[derive(Debug, Parser)]
#[command(
author,
name = "red-knot",
about = "An extremely fast Python type checker."
)]
#[command(version)]
struct Args {
#[command(subcommand)]
pub(crate) command: Option<Command>,
/// Run the command within the given project directory.
///
/// All `pyproject.toml` files will be discovered by walking up the directory tree from the given project directory,
/// as will the project's virtual environment (`.venv`) unless the `venv-path` option is set.
///
/// Other command-line arguments (such as relative paths) will be resolved relative to the current working directory.
#[arg(long, value_name = "PROJECT")]
project: Option<SystemPathBuf>,
/// Path to the virtual environment the project uses.
///
/// If provided, red-knot will use the `site-packages` directory of this virtual environment
/// to resolve type information for the project's third-party dependencies.
#[arg(long, value_name = "PATH")]
venv_path: Option<SystemPathBuf>,
/// Custom directory to use for stdlib typeshed stubs.
#[arg(long, value_name = "PATH", alias = "custom-typeshed-dir")]
typeshed: Option<SystemPathBuf>,
/// Additional path to use as a module-resolution source (can be passed multiple times).
#[arg(long, value_name = "PATH")]
extra_search_path: Option<Vec<SystemPathBuf>>,
/// Python version to assume when resolving types.
#[arg(long, value_name = "VERSION", alias = "target-version")]
python_version: Option<PythonVersion>,
#[clap(flatten)]
verbosity: Verbosity,
/// Run in watch mode by re-running whenever files change.
#[arg(long, short = 'W')]
watch: bool,
}
impl Args {
fn to_options(&self) -> Options {
Options {
environment: Some(EnvironmentOptions {
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| {
extra_search_paths
.iter()
.map(RelativePathBuf::cli)
.collect()
}),
..EnvironmentOptions::default()
}),
..Default::default()
}
}
}
#[derive(Debug, clap::Subcommand)]
pub enum Command {
/// Start the language server

View file

@ -171,7 +171,7 @@ fn paths_in_configuration_files_are_relative_to_the_project_root() -> anyhow::Re
/// The rule severity can be changed in the configuration file
#[test]
fn rule_severity() -> anyhow::Result<()> {
fn configuration_rule_severity() -> anyhow::Result<()> {
let case = TestCase::with_file(
"test.py",
r#"
@ -216,9 +216,119 @@ fn rule_severity() -> anyhow::Result<()> {
})
}
/// Red Knot warns about unknown rules
/// The rule severity can be changed using `--ignore`, `--warn`, and `--error`
#[test]
fn unknown_rules() -> anyhow::Result<()> {
fn cli_rule_severity() -> anyhow::Result<()> {
let case = TestCase::with_file(
"test.py",
r#"
import does_not_exit
y = 4 / 0
for a in range(0, y):
x = a
print(x) # possibly-unresolved-reference
"#,
)?;
case.insta_settings().bind(|| {
// Assert that there's a possibly unresolved reference diagnostic
// and that division-by-zero has a severity of error by default.
assert_cmd_snapshot!(case.command(), @r"
success: false
exit_code: 1
----- stdout -----
error[lint:unresolved-import] <temp_dir>/test.py:2:8 Cannot resolve import `does_not_exit`
error[lint:division-by-zero] <temp_dir>/test.py:4:5 Cannot divide object of type `Literal[4]` by zero
warning[lint:possibly-unresolved-reference] <temp_dir>/test.py:9:7 Name `x` used when possibly not defined
----- stderr -----
");
assert_cmd_snapshot!(
case
.command()
.arg("--ignore")
.arg("possibly-unresolved-reference")
.arg("--warn")
.arg("division-by-zero")
.arg("--warn")
.arg("unresolved-import"),
@r"
success: false
exit_code: 1
----- stdout -----
warning[lint:unresolved-import] <temp_dir>/test.py:2:8 Cannot resolve import `does_not_exit`
warning[lint:division-by-zero] <temp_dir>/test.py:4:5 Cannot divide object of type `Literal[4]` by zero
----- stderr -----
"
);
Ok(())
})
}
/// The rule severity can be changed using `--ignore`, `--warn`, and `--error` and
/// values specified last override previous severities.
#[test]
fn cli_rule_severity_precedence() -> anyhow::Result<()> {
let case = TestCase::with_file(
"test.py",
r#"
y = 4 / 0
for a in range(0, y):
x = a
print(x) # possibly-unresolved-reference
"#,
)?;
case.insta_settings().bind(|| {
// Assert that there's a possibly unresolved reference diagnostic
// and that division-by-zero has a severity of error by default.
assert_cmd_snapshot!(case.command(), @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 -----
");
assert_cmd_snapshot!(
case
.command()
.arg("--error")
.arg("possibly-unresolved-reference")
.arg("--warn")
.arg("division-by-zero")
.arg("--ignore")
.arg("possibly-unresolved-reference"),
// Override the error severity with warning
@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 specified in a configuration file
#[test]
fn configuration_unknown_rules() -> anyhow::Result<()> {
let case = TestCase::with_files([
(
"pyproject.toml",
@ -244,6 +354,25 @@ fn unknown_rules() -> anyhow::Result<()> {
Ok(())
}
/// Red Knot warns about unknown rules specified in a CLI argument
#[test]
fn cli_unknown_rules() -> anyhow::Result<()> {
let case = TestCase::with_file("test.py", "print(10)")?;
case.insta_settings().bind(|| {
assert_cmd_snapshot!(case.command().arg("--ignore").arg("division-by-zer"), @r"
success: false
exit_code: 1
----- stdout -----
warning[unknown-rule] Unknown lint rule `division-by-zer`
----- stderr -----
");
});
Ok(())
}
struct TestCase {
_temp_dir: TempDir,
project_dir: PathBuf,

View file

@ -149,6 +149,16 @@ impl Options {
format!("Unknown lint rule `{rule_name}`"),
Severity::Warning,
),
GetLintError::PrefixedWithCategory { suggestion, .. } => {
OptionDiagnostic::new(
DiagnosticId::UnknownRule,
format!(
"Unknown lint rule `{rule_name}`. Did you mean `{suggestion}`?"
),
Severity::Warning,
)
}
GetLintError::Removed(_) => OptionDiagnostic::new(
DiagnosticId::UnknownRule,
format!("Unknown lint rule `{rule_name}`"),
@ -206,6 +216,16 @@ pub struct Rules {
inner: FxHashMap<RangedValue<String>, RangedValue<Level>>,
}
impl FromIterator<(RangedValue<String>, RangedValue<Level>)> for Rules {
fn from_iter<T: IntoIterator<Item = (RangedValue<String>, RangedValue<Level>)>>(
iter: T,
) -> Self {
Self {
inner: iter.into_iter().collect(),
}
}
}
#[derive(Error, Debug)]
pub enum KnotTomlError {
#[error(transparent)]

View file

@ -180,3 +180,11 @@ a = 4 / 0 # error: [division-by-zero]
# error: [unknown-rule] "Unknown rule `is-equal-14`"
a = 10 + 4 # knot: ignore[is-equal-14]
```
## Code with `lint:` prefix
```py
# error:[unknown-rule] "Unknown rule `lint:division-by-zero`. Did you mean `division-by-zero`?"
# error: [division-by-zero]
a = 10 / 0 # knot: ignore[lint:division-by-zero]
```

View file

@ -1,5 +1,5 @@
use itertools::Itertools;
use ruff_db::diagnostic::{LintName, Severity};
use ruff_db::diagnostic::{DiagnosticId, LintName, Severity};
use rustc_hash::FxHashMap;
use std::hash::Hasher;
use thiserror::Error;
@ -345,7 +345,18 @@ impl LintRegistry {
}
}
Some(LintEntry::Removed(lint)) => Err(GetLintError::Removed(lint.name())),
None => Err(GetLintError::Unknown(code.to_string())),
None => {
if let Some(without_prefix) = DiagnosticId::strip_category(code) {
if let Some(entry) = self.by_name.get(without_prefix) {
return Err(GetLintError::PrefixedWithCategory {
prefixed: code.to_string(),
suggestion: entry.id().name.to_string(),
});
}
}
Err(GetLintError::Unknown(code.to_string()))
}
}
}
@ -382,12 +393,20 @@ impl LintRegistry {
#[derive(Error, Debug, Clone, PartialEq, Eq)]
pub enum GetLintError {
/// The name maps to this removed lint.
#[error("lint {0} has been removed")]
#[error("lint `{0}` has been removed")]
Removed(LintName),
/// No lint with the given name is known.
#[error("unknown lint {0}")]
#[error("unknown lint `{0}`")]
Unknown(String),
/// The name uses the full qualified diagnostic id `lint:<rule>` instead of just `rule`.
/// The String is the name without the `lint:` category prefix.
#[error("unknown lint `{prefixed}`. Did you mean `{suggestion}`?")]
PrefixedWithCategory {
prefixed: String,
suggestion: String,
},
}
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
@ -399,6 +418,16 @@ pub enum LintEntry {
Alias(LintId),
}
impl LintEntry {
fn id(self) -> LintId {
match self {
LintEntry::Lint(id) => id,
LintEntry::Removed(id) => id,
LintEntry::Alias(id) => id,
}
}
}
impl From<&'static LintMetadata> for LintEntry {
fn from(metadata: &'static LintMetadata) -> Self {
if metadata.status.is_removed() {

View file

@ -163,6 +163,17 @@ fn check_unknown_rule(context: &mut CheckSuppressionsContext) {
format_args!("Unknown rule `{rule}`"),
);
}
GetLintError::PrefixedWithCategory {
prefixed,
suggestion,
} => {
context.report_lint(
&UNKNOWN_RULE,
unknown.range,
format_args!("Unknown rule `{prefixed}`. Did you mean `{suggestion}`?"),
);
}
};
}
}
@ -765,8 +776,9 @@ impl<'src> SuppressionParser<'src> {
fn eat_word(&mut self) -> bool {
if self.cursor.eat_if(char::is_alphabetic) {
// Allow `:` for better error recovery when someone uses `lint:code` instead of just `code`.
self.cursor
.eat_while(|c| c.is_alphanumeric() || matches!(c, '_' | '-'));
.eat_while(|c| c.is_alphanumeric() || matches!(c, '_' | '-' | ':'));
true
} else {
false

View file

@ -94,6 +94,10 @@ impl DiagnosticId {
matches!(self, DiagnosticId::Lint(self_name) if self_name == name)
}
pub fn strip_category(code: &str) -> Option<&str> {
code.split_once(':').map(|(_, rest)| rest)
}
/// Returns `true` if this `DiagnosticId` matches the given name.
///
/// ## Examples