From 98cb2a7d336a9a8b06cbba66fcaea683541e3e3e Mon Sep 17 00:00:00 2001 From: Pieter Hooimeijer Date: Fri, 24 Oct 2025 22:39:44 -0700 Subject: [PATCH 1/2] [ruff][ext-lint] 1: external lint type defintions and serde config --- crates/ruff/tests/cli/lint.rs | 2 +- crates/ruff_linter/src/checkers/noqa.rs | 22 +- crates/ruff_linter/src/codes.rs | 2 +- .../src/external/ast/definition.rs | 26 + crates/ruff_linter/src/external/ast/loader.rs | 420 ++++++++++++++++ crates/ruff_linter/src/external/ast/mod.rs | 6 + .../ruff_linter/src/external/ast/registry.rs | 163 +++++++ crates/ruff_linter/src/external/ast/rule.rs | 203 ++++++++ .../ruff_linter/src/external/ast/runtime.rs | 21 + crates/ruff_linter/src/external/ast/target.rs | 449 ++++++++++++++++++ crates/ruff_linter/src/external/error.rs | 78 +++ crates/ruff_linter/src/external/mod.rs | 26 + crates/ruff_linter/src/lib.rs | 1 + crates/ruff_linter/src/rule_selector.rs | 81 +++- .../src/rules/ruff/rules/external_ast.rs | 37 ++ .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + crates/ruff_server/src/session/options.rs | 2 +- ruff.schema.json | 3 + 18 files changed, 1520 insertions(+), 24 deletions(-) create mode 100644 crates/ruff_linter/src/external/ast/definition.rs create mode 100644 crates/ruff_linter/src/external/ast/loader.rs create mode 100644 crates/ruff_linter/src/external/ast/mod.rs create mode 100644 crates/ruff_linter/src/external/ast/registry.rs create mode 100644 crates/ruff_linter/src/external/ast/rule.rs create mode 100644 crates/ruff_linter/src/external/ast/runtime.rs create mode 100644 crates/ruff_linter/src/external/ast/target.rs create mode 100644 crates/ruff_linter/src/external/error.rs create mode 100644 crates/ruff_linter/src/external/mod.rs create mode 100644 crates/ruff_linter/src/rules/ruff/rules/external_ast.rs diff --git a/crates/ruff/tests/cli/lint.rs b/crates/ruff/tests/cli/lint.rs index 25500ed346..9ad806b4a4 100644 --- a/crates/ruff/tests/cli/lint.rs +++ b/crates/ruff/tests/cli/lint.rs @@ -775,7 +775,7 @@ fn valid_toml_but_nonexistent_option_provided_via_config_argument() { Could not parse the supplied argument as a `ruff.toml` configuration option: - Unknown rule selector: `F481` + Unknown rule selector: `F481`. External rule selectors are not supported yet. For more information, try '--help'. "); diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index 7cf58a5def..ee87989bf2 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -5,6 +5,8 @@ use std::path::Path; use itertools::Itertools; use rustc_hash::FxHashSet; +use ruff_db::diagnostic::SecondaryCode; + use ruff_python_trivia::CommentRanges; use ruff_text_size::{Ranged, TextRange}; @@ -77,7 +79,7 @@ pub(crate) fn check_noqa( { let suppressed = match &directive_line.directive { Directive::All(_) => { - let Ok(rule) = Rule::from_code(code) else { + let Some(rule) = resolve_rule_for_noqa(code, settings) else { debug_assert!(false, "Invalid secondary code `{code}`"); continue; }; @@ -87,7 +89,7 @@ pub(crate) fn check_noqa( } Directive::Codes(directive) => { if directive.includes(code) { - let Ok(rule) = Rule::from_code(code) else { + let Some(rule) = resolve_rule_for_noqa(code, settings) else { debug_assert!(false, "Invalid secondary code `{code}`"); continue; }; @@ -258,3 +260,19 @@ pub(crate) fn check_noqa( ignored_diagnostics.sort_unstable(); ignored_diagnostics } + +fn resolve_rule_for_noqa(code: &SecondaryCode, settings: &LinterSettings) -> Option { + if let Ok(rule) = Rule::from_code(code.as_str()) { + return Some(rule); + } + + if settings + .external + .iter() + .any(|external| code.as_str().starts_with(external)) + { + return Some(Rule::ExternalLinter); + } + + None +} diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ebec5f4acc..db46fde2b6 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1065,6 +1065,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "102") => rules::ruff::rules::InvalidRuleCode, (Ruff, "200") => rules::ruff::rules::InvalidPyprojectToml, + (Ruff, "300") => rules::ruff::rules::ExternalLinter, #[cfg(any(feature = "test-rules", test))] (Ruff, "900") => rules::ruff::rules::StableTestRule, #[cfg(any(feature = "test-rules", test))] @@ -1092,7 +1093,6 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { #[cfg(any(feature = "test-rules", test))] (Ruff, "990") => rules::ruff::rules::PanicyTestRule, - // flake8-django (Flake8Django, "001") => rules::flake8_django::rules::DjangoNullableModelStringField, (Flake8Django, "003") => rules::flake8_django::rules::DjangoLocalsInRenderFunction, diff --git a/crates/ruff_linter/src/external/ast/definition.rs b/crates/ruff_linter/src/external/ast/definition.rs new file mode 100644 index 0000000000..26b4e6565c --- /dev/null +++ b/crates/ruff_linter/src/external/ast/definition.rs @@ -0,0 +1,26 @@ +use serde::Deserialize; + +use crate::external::ast::rule::ExternalAstRuleSpec; + +fn default_true() -> bool { + true +} + +#[derive(Debug, Clone, Deserialize)] +pub struct ExternalAstLinterFile { + #[serde(default = "default_true")] + pub enabled: bool, + #[serde(default)] + pub name: Option, + #[serde(default)] + pub description: Option, + #[serde(default)] + #[serde(rename = "rule")] + pub rules: Vec, +} + +#[allow(dead_code)] +pub fn _assert_specs_send_sync() { + fn assert_send_sync() {} + assert_send_sync::(); +} diff --git a/crates/ruff_linter/src/external/ast/loader.rs b/crates/ruff_linter/src/external/ast/loader.rs new file mode 100644 index 0000000000..8cbebc4880 --- /dev/null +++ b/crates/ruff_linter/src/external/ast/loader.rs @@ -0,0 +1,420 @@ +use std::collections::HashSet; +use std::fs; +use std::path::{Path, PathBuf}; + +use crate::external::PyprojectExternalLinterEntry; +use crate::external::ast::definition::ExternalAstLinterFile; +use crate::external::ast::registry::ExternalLintRegistry; +use crate::external::ast::rule::{ + CallCalleeMatcher, ExternalAstLinter, ExternalAstRule, ExternalAstRuleSpec, ExternalRuleCode, + ExternalRuleCodeError, ExternalRuleScript, +}; +use crate::external::ast::target::{AstTarget, AstTargetSpec, ExprKind}; +use crate::external::error::ExternalLinterError; + +pub fn load_linter_into_registry( + registry: &mut ExternalLintRegistry, + id: &str, + entry: &PyprojectExternalLinterEntry, +) -> Result<(), ExternalLinterError> { + let linter = load_linter_from_entry(id, entry)?; + registry.insert_linter(linter) +} + +pub fn load_linter_from_entry( + id: &str, + entry: &PyprojectExternalLinterEntry, +) -> Result { + let definition = load_definition_file(&entry.toml_path)?; + build_linter(id, entry, &definition) +} + +fn load_definition_file(path: &Path) -> Result { + let contents = fs::read_to_string(path).map_err(|source| ExternalLinterError::Io { + path: path.to_path_buf(), + source, + })?; + toml::from_str(&contents).map_err(|source| ExternalLinterError::Parse { + path: path.to_path_buf(), + source, + }) +} + +fn build_linter( + id: &str, + entry: &PyprojectExternalLinterEntry, + linter_file: &ExternalAstLinterFile, +) -> Result { + if linter_file.rules.is_empty() { + return Err(ExternalLinterError::EmptyLinter { id: id.to_string() }); + } + + let resolved_dir = entry + .toml_path + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| PathBuf::from(".")); + + let mut codes = HashSet::new(); + let mut rules = Vec::with_capacity(linter_file.rules.len()); + + for rule_spec in &linter_file.rules { + let rule = build_rule(id, &resolved_dir, rule_spec)?; + + if !codes.insert(rule.code.as_str().to_string()) { + return Err(ExternalLinterError::DuplicateRule { + linter: id.to_string(), + code: rule.code.as_str().to_string(), + }); + } + + rules.push(rule); + } + + let linter = ExternalAstLinter::new( + id, + linter_file.name.clone().unwrap_or_else(|| id.to_string()), + linter_file.description.clone(), + entry.enabled && linter_file.enabled, + rules, + ); + + Ok(linter) +} + +fn build_rule( + linter_id: &str, + base_dir: &Path, + spec: &ExternalAstRuleSpec, +) -> Result { + let code = ExternalRuleCode::new(&spec.code).map_err(|error| match error { + ExternalRuleCodeError::Empty | ExternalRuleCodeError::InvalidCharacters(_) => { + ExternalLinterError::InvalidRuleCode { + linter: linter_id.to_string(), + code: spec.code.clone(), + } + } + })?; + + if spec.targets.is_empty() { + return Err(ExternalLinterError::MissingTargets { + linter: linter_id.to_string(), + rule: spec.name.clone(), + }); + } + + let mut resolved_targets = Vec::with_capacity(spec.targets.len()); + for target in &spec.targets { + let parsed = parse_target(target).map_err(|source| ExternalLinterError::UnknownTarget { + linter: linter_id.to_string(), + rule: spec.name.clone(), + target: target.raw().to_string(), + source, + })?; + resolved_targets.push(parsed); + } + + let script = resolve_script(linter_id, &spec.name, base_dir, &spec.script)?; + let call_callee = if let Some(pattern) = spec.call_callee_regex.as_ref() { + if !resolved_targets + .iter() + .any(|target| matches!(target, AstTarget::Expr(ExprKind::Call))) + { + return Err(ExternalLinterError::CallCalleeRegexWithoutCallTarget { + linter: linter_id.to_string(), + rule: spec.name.clone(), + }); + } + + Some(CallCalleeMatcher::new(pattern.clone()).map_err(|source| { + ExternalLinterError::InvalidCallCalleeRegex { + linter: linter_id.to_string(), + rule: spec.name.clone(), + pattern: pattern.clone(), + source, + } + })?) + } else { + None + }; + + Ok(ExternalAstRule::new( + code, + spec.name.clone(), + spec.summary.clone(), + resolved_targets, + script, + call_callee, + )) +} + +fn resolve_script( + linter_id: &str, + rule_name: &str, + base_dir: &Path, + script_path: &Path, +) -> Result { + let resolved = if script_path.is_absolute() { + script_path.to_path_buf() + } else { + base_dir.join(script_path) + }; + let contents = + fs::read_to_string(&resolved).map_err(|source| ExternalLinterError::ScriptIo { + linter: linter_id.to_string(), + rule: rule_name.to_string(), + path: resolved.clone(), + source, + })?; + if contents.trim().is_empty() { + return Err(ExternalLinterError::MissingScriptBody { + linter: linter_id.to_string(), + rule: rule_name.to_string(), + }); + } + Ok(ExternalRuleScript::file(resolved, contents)) +} + +fn parse_target( + spec: &AstTargetSpec, +) -> Result { + spec.parse() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::external::ast::target::{ExprKind, StmtKind}; + use anyhow::Result; + use std::path::Path; + use tempfile::tempdir; + + fn write(path: &Path, contents: &str) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::write(path, contents)?; + Ok(()) + } + + #[test] + fn load_linter_from_entry_resolves_relative_paths() -> Result<()> { + let temp = tempdir()?; + let linter_path = temp.path().join("linters/my_linter.toml"); + let script_path = temp.path().join("linters/rules/example.py"); + let call_script_path = temp.path().join("linters/rules/call.py"); + + write( + &script_path, + r#" +def check(): + # placeholder body + pass +"#, + )?; + + write( + &call_script_path, + r#" +def check(): + pass +"#, + )?; + + write( + &linter_path, + r#" +name = "Example External Linter" +description = "Demonstrates external AST configuration" + +[[rule]] +code = "EXT001" +name = "ExampleRule" +summary = "Flags demo targets" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" + +[[rule]] +code = "EXT100" +name = "CallRule" +targets = ["expr:Call"] +call-callee-regex = "^logging\\." +script = "rules/call.py" +"#, + )?; + + let entry = PyprojectExternalLinterEntry { + toml_path: linter_path, + enabled: true, + }; + + let linter = load_linter_from_entry("example", &entry)?; + assert!(linter.enabled); + assert_eq!(linter.id.as_str(), "example"); + assert_eq!(linter.name.as_str(), "Example External Linter"); + assert_eq!( + linter.description.as_deref(), + Some("Demonstrates external AST configuration") + ); + assert_eq!(linter.rules.len(), 2); + + let example_rule = &linter.rules[0]; + assert_eq!(example_rule.code.as_str(), "EXT001"); + assert_eq!(example_rule.name.as_str(), "ExampleRule"); + assert_eq!(example_rule.summary.as_deref(), Some("Flags demo targets")); + assert_eq!(example_rule.targets.len(), 1); + assert_eq!( + example_rule.targets[0], + AstTarget::Stmt(StmtKind::FunctionDef) + ); + assert_eq!(example_rule.script.path(), script_path.as_path()); + assert!(example_rule.script.body().contains("placeholder body")); + + let call_rule = &linter.rules[1]; + assert_eq!(call_rule.code.as_str(), "EXT100"); + assert_eq!(call_rule.name.as_str(), "CallRule"); + assert_eq!(call_rule.targets[0], AstTarget::Expr(ExprKind::Call)); + let call_callee = call_rule + .call_callee() + .expect("expected call callee matcher to be present"); + assert_eq!(call_callee.pattern(), "^logging\\."); + assert!(call_callee.regex().is_match("logging.info")); + + Ok(()) + } + + #[test] + fn load_linter_rejects_call_regex_without_call_target() -> Result<()> { + let temp = tempdir()?; + let linter_path = temp.path().join("linters/invalid-call.toml"); + let script_path = temp.path().join("linters/rules/invalid.py"); + + write( + &script_path, + r#" +def check(): + pass +"#, + )?; + + write( + &linter_path, + r#" +[[rule]] +code = "EXT101" +name = "InvalidCallRule" +targets = ["expr:Name"] +call-callee-regex = "^logging\\." +script = "rules/invalid.py" +"#, + )?; + + let entry = PyprojectExternalLinterEntry { + toml_path: linter_path, + enabled: true, + }; + + let err = load_linter_from_entry("invalid-call", &entry).unwrap_err(); + let ExternalLinterError::CallCalleeRegexWithoutCallTarget { linter, rule } = err else { + panic!("expected call regex without target error"); + }; + assert_eq!(linter, "invalid-call"); + assert_eq!(rule, "InvalidCallRule"); + + Ok(()) + } + + #[test] + fn load_linter_rejects_invalid_call_regex() -> Result<()> { + let temp = tempdir()?; + let linter_path = temp.path().join("linters/bad-regex.toml"); + let script_path = temp.path().join("linters/rules/bad.py"); + + write( + &script_path, + r#" +def check(): + pass +"#, + )?; + + write( + &linter_path, + r#" +[[rule]] +code = "EXT102" +name = "BadRegexRule" +targets = ["expr:Call"] +call-callee-regex = "[" +script = "rules/bad.py" +"#, + )?; + + let entry = PyprojectExternalLinterEntry { + toml_path: linter_path, + enabled: true, + }; + + let err = load_linter_from_entry("bad-regex", &entry).unwrap_err(); + let ExternalLinterError::InvalidCallCalleeRegex { + linter, + rule, + pattern, + .. + } = err + else { + panic!("expected invalid call regex error"); + }; + assert_eq!(linter, "bad-regex"); + assert_eq!(rule, "BadRegexRule"); + assert_eq!(pattern, "["); + + Ok(()) + } + + #[test] + fn load_linter_into_registry_marks_disabled_linters() -> Result<()> { + let temp = tempdir()?; + let linter_path = temp.path().join("linters/disabled.toml"); + let script_path = temp.path().join("linters/rules/unused.py"); + + write( + &script_path, + r#" +def check(): + pass +"#, + )?; + + write( + &linter_path, + r#" +enabled = false + +[[rule]] +code = "EXT002" +name = "DisabledRule" +targets = ["stmt:Expr"] +script = "rules/unused.py" +"#, + )?; + + let entry = PyprojectExternalLinterEntry { + toml_path: linter_path, + enabled: true, + }; + + let mut registry = ExternalLintRegistry::new(); + load_linter_into_registry(&mut registry, "disabled", &entry)?; + + assert_eq!(registry.linters().len(), 1); + + let linter = ®istry.linters()[0]; + assert!(!linter.enabled); + + // Disabled linters should not be discoverable by rule code lookup. + assert!(registry.find_rule_by_code("EXT002").is_none()); + + Ok(()) + } +} diff --git a/crates/ruff_linter/src/external/ast/mod.rs b/crates/ruff_linter/src/external/ast/mod.rs new file mode 100644 index 0000000000..db244eb75d --- /dev/null +++ b/crates/ruff_linter/src/external/ast/mod.rs @@ -0,0 +1,6 @@ +pub mod definition; +pub mod loader; +pub mod registry; +pub mod rule; +pub mod runtime; +pub mod target; diff --git a/crates/ruff_linter/src/external/ast/registry.rs b/crates/ruff_linter/src/external/ast/registry.rs new file mode 100644 index 0000000000..ee0435f68c --- /dev/null +++ b/crates/ruff_linter/src/external/ast/registry.rs @@ -0,0 +1,163 @@ +use std::hash::Hasher; + +use crate::external::ast::rule::{CallCalleeMatcher, ExternalAstLinter, ExternalAstRule}; +use crate::external::ast::target::{AstTarget, ExprKind, StmtKind}; +use crate::external::error::ExternalLinterError; +use rustc_hash::FxHashMap; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct RuleLocator { + pub linter_index: usize, + pub rule_index: usize, +} + +impl RuleLocator { + pub const fn new(linter_index: usize, rule_index: usize) -> Self { + Self { + linter_index, + rule_index, + } + } +} + +#[derive(Debug, Default, Clone)] +pub struct ExternalLintRegistry { + linters: Vec, + index_by_code: FxHashMap, + stmt_index: FxHashMap>, + expr_index: FxHashMap>, +} + +impl ExternalLintRegistry { + pub fn new() -> Self { + Self::default() + } + + pub fn is_empty(&self) -> bool { + self.linters.is_empty() + } + + pub fn linters(&self) -> &[ExternalAstLinter] { + &self.linters + } + + pub fn insert_linter(&mut self, linter: ExternalAstLinter) -> Result<(), ExternalLinterError> { + if self.linters.iter().any(|existing| existing.id == linter.id) { + return Err(ExternalLinterError::DuplicateLinter { id: linter.id }); + } + + let linter_index = self.linters.len(); + for (rule_index, rule) in linter.rules.iter().enumerate() { + let code = rule.code.as_str().to_string(); + if self.index_by_code.contains_key(&code) { + return Err(ExternalLinterError::DuplicateRule { + linter: linter.id.clone(), + code, + }); + } + self.index_by_code + .insert(code, RuleLocator::new(linter_index, rule_index)); + + if !linter.enabled { + continue; + } + + for target in &rule.targets { + match target { + AstTarget::Stmt(kind) => self + .stmt_index + .entry(*kind) + .or_default() + .push(RuleLocator::new(linter_index, rule_index)), + AstTarget::Expr(kind) => self + .expr_index + .entry(*kind) + .or_default() + .push(RuleLocator::new(linter_index, rule_index)), + } + } + } + self.linters.push(linter); + Ok(()) + } + + pub fn get_rule(&self, locator: RuleLocator) -> Option<&ExternalAstRule> { + self.linters + .get(locator.linter_index) + .and_then(|linter| linter.rules.get(locator.rule_index)) + } + + pub fn get_linter(&self, locator: RuleLocator) -> Option<&ExternalAstLinter> { + self.linters.get(locator.linter_index) + } + + pub fn find_rule_by_code( + &self, + code: &str, + ) -> Option<(RuleLocator, &ExternalAstRule, &ExternalAstLinter)> { + let locator = *self.index_by_code.get(code)?; + let linter = self.linters.get(locator.linter_index)?; + if !linter.enabled { + return None; + } + let rule = linter.rules.get(locator.rule_index)?; + Some((locator, rule, linter)) + } + + pub fn rules_for_stmt(&self, kind: StmtKind) -> impl Iterator + '_ { + self.stmt_index.get(&kind).into_iter().flatten().copied() + } + + pub fn rules_for_expr(&self, kind: ExprKind) -> impl Iterator + '_ { + self.expr_index.get(&kind).into_iter().flatten().copied() + } + + pub fn rule_entry( + &self, + locator: RuleLocator, + ) -> Option<(&ExternalAstRule, &ExternalAstLinter)> { + let linter = self.linters.get(locator.linter_index)?; + let rule = linter.rules.get(locator.rule_index)?; + Some((rule, linter)) + } +} + +impl ruff_cache::CacheKey for ExternalLintRegistry { + fn cache_key(&self, key: &mut ruff_cache::CacheKeyHasher) { + key.write_usize(self.linters.len()); + for linter in &self.linters { + linter.id.as_str().cache_key(key); + linter.enabled.cache_key(key); + linter.name.as_str().cache_key(key); + linter.description.as_deref().cache_key(key); + key.write_usize(linter.rules.len()); + for rule in &linter.rules { + rule.code.as_str().cache_key(key); + rule.name.as_str().cache_key(key); + rule.summary.as_deref().cache_key(key); + rule.call_callee() + .map(CallCalleeMatcher::pattern) + .cache_key(key); + key.write_usize(rule.targets.len()); + for target in &rule.targets { + match target { + AstTarget::Stmt(kind) => { + key.write_u8(0); + key.write_u16(*kind as u16); + } + AstTarget::Expr(kind) => { + key.write_u8(1); + key.write_u16(*kind as u16); + } + } + } + let path_str = rule.script.path().to_string_lossy(); + key.write_usize(path_str.len()); + key.write(path_str.as_bytes()); + let contents_str = rule.script.body(); + key.write_usize(contents_str.len()); + key.write(contents_str.as_bytes()); + } + } + } +} diff --git a/crates/ruff_linter/src/external/ast/rule.rs b/crates/ruff_linter/src/external/ast/rule.rs new file mode 100644 index 0000000000..45c7f9f711 --- /dev/null +++ b/crates/ruff_linter/src/external/ast/rule.rs @@ -0,0 +1,203 @@ +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +use regex::Regex; +use ruff_db::diagnostic::SecondaryCode; +use serde::Deserialize; +use thiserror::Error; + +use crate::external::ast::target::{AstTarget, AstTargetSpec}; + +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct ExternalRuleCode(Box); + +impl ExternalRuleCode { + pub fn new>(code: S) -> Result { + let code_ref = code.as_ref(); + if code_ref.is_empty() { + return Err(ExternalRuleCodeError::Empty); + } + if !Self::matches_format(code_ref) { + return Err(ExternalRuleCodeError::InvalidCharacters( + code_ref.to_string(), + )); + } + Ok(Self(code_ref.into())) + } + + pub fn as_str(&self) -> &str { + &self.0 + } + + pub fn to_secondary_code(&self) -> SecondaryCode { + SecondaryCode::new(self.as_str().to_string()) + } + + fn pattern() -> &'static Regex { + static PATTERN: OnceLock = OnceLock::new(); + PATTERN.get_or_init(|| Regex::new(r"^[A-Z]+[0-9]+$").expect("valid external rule regex")) + } + + pub(crate) fn matches_format(code: &str) -> bool { + Self::pattern().is_match(code) + } +} + +impl std::fmt::Display for ExternalRuleCode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +#[derive(Debug, Error)] +pub enum ExternalRuleCodeError { + #[error("external rule codes must not be empty")] + Empty, + #[error("external rule codes must contain only uppercase ASCII letters and digits: `{0}`")] + InvalidCharacters(String), +} + +/// Fully resolved script content that can be handed to the runtime for compilation. +#[derive(Debug, Clone)] +pub struct ExternalRuleScript { + path: PathBuf, + contents: String, +} + +impl ExternalRuleScript { + pub fn file(path: PathBuf, contents: impl Into) -> Self { + Self { + path, + contents: contents.into(), + } + } + + pub fn path(&self) -> &Path { + &self.path + } + + pub fn body(&self) -> &str { + &self.contents + } +} + +/// User-facing metadata describing an external AST rule before targets are resolved. +#[derive(Debug, Clone, Deserialize)] +pub struct ExternalAstRuleSpec { + pub code: String, + pub name: String, + #[serde(default)] + pub summary: Option, + #[serde(default)] + pub targets: Vec, + #[serde(default, rename = "call-callee-regex")] + pub call_callee_regex: Option, + pub script: PathBuf, +} + +/// A validated, ready-to-run external AST rule definition. +#[derive(Debug, Clone)] +pub struct ExternalAstRule { + pub code: ExternalRuleCode, + pub name: String, + pub summary: Option, + pub targets: Box<[AstTarget]>, + pub script: ExternalRuleScript, + pub call_callee: Option, +} + +impl ExternalAstRule { + #[allow(clippy::too_many_arguments)] + pub fn new( + code: ExternalRuleCode, + name: impl Into, + summary: Option>, + targets: Vec, + script: ExternalRuleScript, + call_callee: Option, + ) -> Self { + let targets = targets.into_boxed_slice(); + Self { + code, + name: name.into(), + summary: summary.map(Into::into), + targets, + script, + call_callee, + } + } + + pub fn call_callee(&self) -> Option<&CallCalleeMatcher> { + self.call_callee.as_ref() + } +} + +/// Metadata about a collection of external AST rules loaded from a user-defined linter file. +#[derive(Debug, Clone)] +pub struct ExternalAstLinter { + pub id: String, + pub name: String, + pub description: Option, + pub enabled: bool, + pub rules: Vec, +} + +impl ExternalAstLinter { + pub fn new( + id: impl Into, + name: impl Into, + description: Option>, + enabled: bool, + rules: Vec, + ) -> Self { + Self { + id: id.into(), + name: name.into(), + description: description.map(Into::into), + enabled, + rules, + } + } +} + +impl std::fmt::Display for ExternalAstLinter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + writeln!( + f, + "{}{}", + self.id, + if self.enabled { "" } else { " (disabled)" } + )?; + writeln!(f, " name: {}", self.name)?; + if let Some(description) = &self.description { + writeln!(f, " description: {description}")?; + } + writeln!(f, " rules:")?; + for rule in &self.rules { + writeln!(f, " - {} ({})", rule.code.as_str(), rule.name)?; + } + writeln!(f) + } +} + +#[derive(Debug, Clone)] +pub struct CallCalleeMatcher { + pattern: String, + regex: Regex, +} + +impl CallCalleeMatcher { + pub fn new(pattern: impl Into) -> Result { + let pattern = pattern.into(); + let regex = Regex::new(pattern.as_ref())?; + Ok(Self { pattern, regex }) + } + + pub fn pattern(&self) -> &str { + &self.pattern + } + + pub fn regex(&self) -> &Regex { + &self.regex + } +} diff --git a/crates/ruff_linter/src/external/ast/runtime.rs b/crates/ruff_linter/src/external/ast/runtime.rs new file mode 100644 index 0000000000..7d87ff8075 --- /dev/null +++ b/crates/ruff_linter/src/external/ast/runtime.rs @@ -0,0 +1,21 @@ +use std::sync::Arc; + +use crate::external::ast::registry::ExternalLintRegistry; + +/// Shareable handle to the external lint runtime state. +#[derive(Clone, Debug, Default)] +pub struct ExternalLintRuntimeHandle { + registry: Arc, +} + +impl ExternalLintRuntimeHandle { + pub fn new(registry: ExternalLintRegistry) -> Self { + Self { + registry: Arc::new(registry), + } + } + + pub fn registry(&self) -> &ExternalLintRegistry { + &self.registry + } +} diff --git a/crates/ruff_linter/src/external/ast/target.rs b/crates/ruff_linter/src/external/ast/target.rs new file mode 100644 index 0000000000..bc63412ff2 --- /dev/null +++ b/crates/ruff_linter/src/external/ast/target.rs @@ -0,0 +1,449 @@ +use std::fmt; +use std::str::FromStr; + +use ruff_python_ast::{Expr, Stmt}; +use serde::Deserialize; +use thiserror::Error; + +/// An AST node selector identifying which nodes a scripted rule should run against. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum AstTarget { + Stmt(StmtKind), + Expr(ExprKind), +} + +impl AstTarget { + pub const fn kind(&self) -> AstNodeClass { + match self { + AstTarget::Stmt(..) => AstNodeClass::Stmt, + AstTarget::Expr(..) => AstNodeClass::Expr, + } + } + + pub const fn name(&self) -> &'static str { + match self { + AstTarget::Stmt(kind) => kind.as_str(), + AstTarget::Expr(kind) => kind.as_str(), + } + } +} + +impl fmt::Display for AstTarget { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AstTarget::Stmt(kind) => write!(f, "stmt:{}", kind.as_str()), + AstTarget::Expr(kind) => write!(f, "expr:{}", kind.as_str()), + } + } +} + +impl FromStr for AstTarget { + type Err = AstTargetParseError; + + fn from_str(s: &str) -> Result { + parse_target(s) + } +} + +/// Convenience wrapper that enables parsing `AstTarget` values directly from configuration. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize)] +#[serde(transparent)] +pub struct AstTargetSpec(String); + +impl AstTargetSpec { + pub fn parse(&self) -> Result { + self.0.as_str().parse() + } + + pub fn raw(&self) -> &str { + &self.0 + } +} + +/// Broad AST node classes supported by scripted rules. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum AstNodeClass { + Stmt, + Expr, +} + +/// Statement kinds supported by scripted rules. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum StmtKind { + FunctionDef, + ClassDef, + Return, + Delete, + TypeAlias, + Assign, + AugAssign, + AnnAssign, + For, + While, + If, + With, + Match, + Raise, + Try, + Assert, + Import, + ImportFrom, + Global, + Nonlocal, + Expr, + Pass, + Break, + Continue, + IpyEscapeCommand, +} + +impl StmtKind { + pub const fn as_str(self) -> &'static str { + match self { + StmtKind::FunctionDef => "FunctionDef", + StmtKind::ClassDef => "ClassDef", + StmtKind::Return => "Return", + StmtKind::Delete => "Delete", + StmtKind::TypeAlias => "TypeAlias", + StmtKind::Assign => "Assign", + StmtKind::AugAssign => "AugAssign", + StmtKind::AnnAssign => "AnnAssign", + StmtKind::For => "For", + StmtKind::While => "While", + StmtKind::If => "If", + StmtKind::With => "With", + StmtKind::Match => "Match", + StmtKind::Raise => "Raise", + StmtKind::Try => "Try", + StmtKind::Assert => "Assert", + StmtKind::Import => "Import", + StmtKind::ImportFrom => "ImportFrom", + StmtKind::Global => "Global", + StmtKind::Nonlocal => "Nonlocal", + StmtKind::Expr => "Expr", + StmtKind::Pass => "Pass", + StmtKind::Break => "Break", + StmtKind::Continue => "Continue", + StmtKind::IpyEscapeCommand => "IpyEscapeCommand", + } + } + + pub fn matches(self, stmt: &Stmt) -> bool { + matches!( + (self, stmt), + (StmtKind::FunctionDef, Stmt::FunctionDef(_)) + | (StmtKind::ClassDef, Stmt::ClassDef(_)) + | (StmtKind::Return, Stmt::Return(_)) + | (StmtKind::Delete, Stmt::Delete(_)) + | (StmtKind::TypeAlias, Stmt::TypeAlias(_)) + | (StmtKind::Assign, Stmt::Assign(_)) + | (StmtKind::AugAssign, Stmt::AugAssign(_)) + | (StmtKind::AnnAssign, Stmt::AnnAssign(_)) + | (StmtKind::For, Stmt::For(_)) + | (StmtKind::While, Stmt::While(_)) + | (StmtKind::If, Stmt::If(_)) + | (StmtKind::With, Stmt::With(_)) + | (StmtKind::Match, Stmt::Match(_)) + | (StmtKind::Raise, Stmt::Raise(_)) + | (StmtKind::Try, Stmt::Try(_)) + | (StmtKind::Assert, Stmt::Assert(_)) + | (StmtKind::Import, Stmt::Import(_)) + | (StmtKind::ImportFrom, Stmt::ImportFrom(_)) + | (StmtKind::Global, Stmt::Global(_)) + | (StmtKind::Nonlocal, Stmt::Nonlocal(_)) + | (StmtKind::Expr, Stmt::Expr(_)) + | (StmtKind::Pass, Stmt::Pass(_)) + | (StmtKind::Break, Stmt::Break(_)) + | (StmtKind::Continue, Stmt::Continue(_)) + | (StmtKind::IpyEscapeCommand, Stmt::IpyEscapeCommand(_)) + ) + } +} + +impl fmt::Display for StmtKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl From<&Stmt> for StmtKind { + fn from(value: &Stmt) -> Self { + match value { + Stmt::FunctionDef(_) => StmtKind::FunctionDef, + Stmt::ClassDef(_) => StmtKind::ClassDef, + Stmt::Return(_) => StmtKind::Return, + Stmt::Delete(_) => StmtKind::Delete, + Stmt::TypeAlias(_) => StmtKind::TypeAlias, + Stmt::Assign(_) => StmtKind::Assign, + Stmt::AugAssign(_) => StmtKind::AugAssign, + Stmt::AnnAssign(_) => StmtKind::AnnAssign, + Stmt::For(_) => StmtKind::For, + Stmt::While(_) => StmtKind::While, + Stmt::If(_) => StmtKind::If, + Stmt::With(_) => StmtKind::With, + Stmt::Match(_) => StmtKind::Match, + Stmt::Raise(_) => StmtKind::Raise, + Stmt::Try(_) => StmtKind::Try, + Stmt::Assert(_) => StmtKind::Assert, + Stmt::Import(_) => StmtKind::Import, + Stmt::ImportFrom(_) => StmtKind::ImportFrom, + Stmt::Global(_) => StmtKind::Global, + Stmt::Nonlocal(_) => StmtKind::Nonlocal, + Stmt::Expr(_) => StmtKind::Expr, + Stmt::Pass(_) => StmtKind::Pass, + Stmt::Break(_) => StmtKind::Break, + Stmt::Continue(_) => StmtKind::Continue, + Stmt::IpyEscapeCommand(_) => StmtKind::IpyEscapeCommand, + } + } +} + +/// Expression kinds supported by scripted rules. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum ExprKind { + Attribute, + Await, + BinOp, + BoolOp, + BooleanLiteral, + BytesLiteral, + Call, + Compare, + Dict, + DictComp, + EllipsisLiteral, + FString, + Generator, + If, + IpyEscapeCommand, + Lambda, + List, + ListComp, + Name, + Named, + NoneLiteral, + NumberLiteral, + Set, + SetComp, + Slice, + Starred, + StringLiteral, + Subscript, + Tuple, + UnaryOp, + Yield, + YieldFrom, +} + +impl ExprKind { + pub const fn as_str(self) -> &'static str { + match self { + ExprKind::Attribute => "Attribute", + ExprKind::Await => "Await", + ExprKind::BinOp => "BinOp", + ExprKind::BoolOp => "BoolOp", + ExprKind::BooleanLiteral => "BooleanLiteral", + ExprKind::BytesLiteral => "BytesLiteral", + ExprKind::Call => "Call", + ExprKind::Compare => "Compare", + ExprKind::Dict => "Dict", + ExprKind::DictComp => "DictComp", + ExprKind::EllipsisLiteral => "EllipsisLiteral", + ExprKind::FString => "FString", + ExprKind::Generator => "Generator", + ExprKind::If => "If", + ExprKind::IpyEscapeCommand => "IpyEscapeCommand", + ExprKind::Lambda => "Lambda", + ExprKind::List => "List", + ExprKind::ListComp => "ListComp", + ExprKind::Name => "Name", + ExprKind::Named => "Named", + ExprKind::NoneLiteral => "NoneLiteral", + ExprKind::NumberLiteral => "NumberLiteral", + ExprKind::Set => "Set", + ExprKind::SetComp => "SetComp", + ExprKind::Slice => "Slice", + ExprKind::Starred => "Starred", + ExprKind::StringLiteral => "StringLiteral", + ExprKind::Subscript => "Subscript", + ExprKind::Tuple => "Tuple", + ExprKind::UnaryOp => "UnaryOp", + ExprKind::Yield => "Yield", + ExprKind::YieldFrom => "YieldFrom", + } + } + + pub fn matches(self, expr: &Expr) -> bool { + match self { + ExprKind::Attribute => matches!(expr, Expr::Attribute(_)), + ExprKind::Await => matches!(expr, Expr::Await(_)), + ExprKind::BinOp => matches!(expr, Expr::BinOp(_)), + ExprKind::BoolOp => matches!(expr, Expr::BoolOp(_)), + ExprKind::BooleanLiteral => matches!(expr, Expr::BooleanLiteral(_)), + ExprKind::BytesLiteral => matches!(expr, Expr::BytesLiteral(_)), + ExprKind::Call => matches!(expr, Expr::Call(_)), + ExprKind::Compare => matches!(expr, Expr::Compare(_)), + ExprKind::Dict => matches!(expr, Expr::Dict(_)), + ExprKind::DictComp => matches!(expr, Expr::DictComp(_)), + ExprKind::EllipsisLiteral => matches!(expr, Expr::EllipsisLiteral(_)), + ExprKind::FString => matches!(expr, Expr::FString(_) | Expr::TString(_)), + ExprKind::Generator => matches!(expr, Expr::Generator(_)), + ExprKind::If => matches!(expr, Expr::If(_)), + ExprKind::IpyEscapeCommand => matches!(expr, Expr::IpyEscapeCommand(_)), + ExprKind::Lambda => matches!(expr, Expr::Lambda(_)), + ExprKind::List => matches!(expr, Expr::List(_)), + ExprKind::ListComp => matches!(expr, Expr::ListComp(_)), + ExprKind::Name => matches!(expr, Expr::Name(_)), + ExprKind::Named => matches!(expr, Expr::Named(_)), + ExprKind::NoneLiteral => matches!(expr, Expr::NoneLiteral(_)), + ExprKind::NumberLiteral => matches!(expr, Expr::NumberLiteral(_)), + ExprKind::Set => matches!(expr, Expr::Set(_)), + ExprKind::SetComp => matches!(expr, Expr::SetComp(_)), + ExprKind::Slice => matches!(expr, Expr::Slice(_)), + ExprKind::Starred => matches!(expr, Expr::Starred(_)), + ExprKind::StringLiteral => matches!(expr, Expr::StringLiteral(_)), + ExprKind::Subscript => matches!(expr, Expr::Subscript(_)), + ExprKind::Tuple => matches!(expr, Expr::Tuple(_)), + ExprKind::UnaryOp => matches!(expr, Expr::UnaryOp(_)), + ExprKind::Yield => matches!(expr, Expr::Yield(_)), + ExprKind::YieldFrom => matches!(expr, Expr::YieldFrom(_)), + } + } +} + +impl fmt::Display for ExprKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl From<&Expr> for ExprKind { + fn from(value: &Expr) -> Self { + match value { + Expr::Attribute(_) => ExprKind::Attribute, + Expr::Await(_) => ExprKind::Await, + Expr::BinOp(_) => ExprKind::BinOp, + Expr::BoolOp(_) => ExprKind::BoolOp, + Expr::BooleanLiteral(_) => ExprKind::BooleanLiteral, + Expr::BytesLiteral(_) => ExprKind::BytesLiteral, + Expr::Call(_) => ExprKind::Call, + Expr::Compare(_) => ExprKind::Compare, + Expr::Dict(_) => ExprKind::Dict, + Expr::DictComp(_) => ExprKind::DictComp, + Expr::EllipsisLiteral(_) => ExprKind::EllipsisLiteral, + Expr::FString(_) => ExprKind::FString, + Expr::TString(_) => ExprKind::FString, + Expr::Generator(_) => ExprKind::Generator, + Expr::If(_) => ExprKind::If, + Expr::IpyEscapeCommand(_) => ExprKind::IpyEscapeCommand, + Expr::Lambda(_) => ExprKind::Lambda, + Expr::List(_) => ExprKind::List, + Expr::ListComp(_) => ExprKind::ListComp, + Expr::Name(_) => ExprKind::Name, + Expr::Named(_) => ExprKind::Named, + Expr::NoneLiteral(_) => ExprKind::NoneLiteral, + Expr::NumberLiteral(_) => ExprKind::NumberLiteral, + Expr::Set(_) => ExprKind::Set, + Expr::SetComp(_) => ExprKind::SetComp, + Expr::Slice(_) => ExprKind::Slice, + Expr::Starred(_) => ExprKind::Starred, + Expr::StringLiteral(_) => ExprKind::StringLiteral, + Expr::Subscript(_) => ExprKind::Subscript, + Expr::Tuple(_) => ExprKind::Tuple, + Expr::UnaryOp(_) => ExprKind::UnaryOp, + Expr::Yield(_) => ExprKind::Yield, + Expr::YieldFrom(_) => ExprKind::YieldFrom, + } + } +} + +#[derive(Debug, Error)] +pub enum AstTargetParseError { + #[error("expected `stmt:` or `expr:` target selector")] + MissingPrefix, + #[error("unknown statement selector `{0}`")] + UnknownStmtKind(String), + #[error("unknown expression selector `{0}`")] + UnknownExprKind(String), +} + +fn parse_target(raw: &str) -> Result { + let (prefix, name) = raw + .split_once(':') + .ok_or(AstTargetParseError::MissingPrefix)?; + match prefix { + "stmt" => Ok(AstTarget::Stmt(parse_stmt_kind(name)?)), + "expr" => Ok(AstTarget::Expr(parse_expr_kind(name)?)), + _ => Err(AstTargetParseError::MissingPrefix), + } +} + +fn parse_stmt_kind(name: &str) -> Result { + match name { + "FunctionDef" => Ok(StmtKind::FunctionDef), + "ClassDef" => Ok(StmtKind::ClassDef), + "Return" => Ok(StmtKind::Return), + "Delete" => Ok(StmtKind::Delete), + "TypeAlias" => Ok(StmtKind::TypeAlias), + "Assign" => Ok(StmtKind::Assign), + "AugAssign" => Ok(StmtKind::AugAssign), + "AnnAssign" => Ok(StmtKind::AnnAssign), + "For" => Ok(StmtKind::For), + "While" => Ok(StmtKind::While), + "If" => Ok(StmtKind::If), + "With" => Ok(StmtKind::With), + "Match" => Ok(StmtKind::Match), + "Raise" => Ok(StmtKind::Raise), + "Try" => Ok(StmtKind::Try), + "Assert" => Ok(StmtKind::Assert), + "Import" => Ok(StmtKind::Import), + "ImportFrom" => Ok(StmtKind::ImportFrom), + "Global" => Ok(StmtKind::Global), + "Nonlocal" => Ok(StmtKind::Nonlocal), + "Expr" => Ok(StmtKind::Expr), + "Pass" => Ok(StmtKind::Pass), + "Break" => Ok(StmtKind::Break), + "Continue" => Ok(StmtKind::Continue), + "IpyEscapeCommand" => Ok(StmtKind::IpyEscapeCommand), + other => Err(AstTargetParseError::UnknownStmtKind(other.to_string())), + } +} + +fn parse_expr_kind(name: &str) -> Result { + match name { + "Attribute" => Ok(ExprKind::Attribute), + "Await" => Ok(ExprKind::Await), + "BinOp" => Ok(ExprKind::BinOp), + "BoolOp" => Ok(ExprKind::BoolOp), + "BooleanLiteral" => Ok(ExprKind::BooleanLiteral), + "BytesLiteral" => Ok(ExprKind::BytesLiteral), + "Call" => Ok(ExprKind::Call), + "Compare" => Ok(ExprKind::Compare), + "Dict" => Ok(ExprKind::Dict), + "DictComp" => Ok(ExprKind::DictComp), + "EllipsisLiteral" => Ok(ExprKind::EllipsisLiteral), + "FString" => Ok(ExprKind::FString), + "Generator" => Ok(ExprKind::Generator), + "If" => Ok(ExprKind::If), + "IpyEscapeCommand" => Ok(ExprKind::IpyEscapeCommand), + "Lambda" => Ok(ExprKind::Lambda), + "List" => Ok(ExprKind::List), + "ListComp" => Ok(ExprKind::ListComp), + "Name" => Ok(ExprKind::Name), + "Named" => Ok(ExprKind::Named), + "NoneLiteral" => Ok(ExprKind::NoneLiteral), + "NumberLiteral" => Ok(ExprKind::NumberLiteral), + "Set" => Ok(ExprKind::Set), + "SetComp" => Ok(ExprKind::SetComp), + "Slice" => Ok(ExprKind::Slice), + "Starred" => Ok(ExprKind::Starred), + "StringLiteral" => Ok(ExprKind::StringLiteral), + "Subscript" => Ok(ExprKind::Subscript), + "Tuple" => Ok(ExprKind::Tuple), + "TString" => Ok(ExprKind::FString), + "UnaryOp" => Ok(ExprKind::UnaryOp), + "Yield" => Ok(ExprKind::Yield), + "YieldFrom" => Ok(ExprKind::YieldFrom), + other => Err(AstTargetParseError::UnknownExprKind(other.to_string())), + } +} diff --git a/crates/ruff_linter/src/external/error.rs b/crates/ruff_linter/src/external/error.rs new file mode 100644 index 0000000000..dd4ec23f5a --- /dev/null +++ b/crates/ruff_linter/src/external/error.rs @@ -0,0 +1,78 @@ +use std::path::PathBuf; + +use thiserror::Error; + +use crate::external::ast::target::AstTargetParseError; + +#[derive(Debug, Error)] +pub enum ExternalLinterError { + #[error("failed to read external linter definition `{path}`: {source}")] + Io { + path: PathBuf, + #[source] + source: std::io::Error, + }, + + #[error("failed to parse external linter definition `{path}`: {source}")] + Parse { + path: PathBuf, + #[source] + source: toml::de::Error, + }, + + #[error("invalid rule code `{code}` for external linter `{linter}`")] + InvalidRuleCode { linter: String, code: String }, + + #[error("unknown AST target `{target}` for external rule `{rule}` in linter `{linter}`")] + // Targets must expand to one of the supported StmtKind or ExprKind enums; anything else is rejected. + UnknownTarget { + linter: String, + rule: String, + target: String, + #[source] + source: AstTargetParseError, + }, + + #[error("duplicate rule code `{code}` in external linter `{linter}`")] + DuplicateRule { linter: String, code: String }, + + #[error("duplicate external linter identifier `{id}`")] + DuplicateLinter { id: String }, + + #[error("external linter `{id}` defines no rules")] + EmptyLinter { id: String }, + + #[error("external rule `{rule}` in linter `{linter}` must declare at least one AST target")] + MissingTargets { linter: String, rule: String }, + + #[error( + "failed to read script `{path}` for external rule `{rule}` in linter `{linter}`: {source}" + )] + ScriptIo { + linter: String, + rule: String, + path: PathBuf, + #[source] + source: std::io::Error, + }, + + #[error("no script body provided for external rule `{rule}` in linter `{linter}`")] + // Raised when we read a script file but it is empty or whitespace-only. + MissingScriptBody { linter: String, rule: String }, + + #[error( + "invalid `call-callee-regex` `{pattern}` for external rule `{rule}` in linter `{linter}`: {source}" + )] + InvalidCallCalleeRegex { + linter: String, + rule: String, + pattern: String, + #[source] + source: regex::Error, + }, + + #[error( + "external rule `{rule}` in linter `{linter}` declares `call-callee-regex` but does not target `expr:Call` nodes" + )] + CallCalleeRegexWithoutCallTarget { linter: String, rule: String }, +} diff --git a/crates/ruff_linter/src/external/mod.rs b/crates/ruff_linter/src/external/mod.rs new file mode 100644 index 0000000000..acccb64147 --- /dev/null +++ b/crates/ruff_linter/src/external/mod.rs @@ -0,0 +1,26 @@ +pub mod ast; +pub mod error; + +use serde::Deserialize; +use std::path::PathBuf; + +fn default_true() -> bool { + true +} + +#[derive(Debug, Clone, Deserialize)] +pub struct PyprojectExternalLinterEntry { + pub toml_path: PathBuf, + #[serde(default = "default_true")] + pub enabled: bool, +} + +pub use ast::definition::ExternalAstLinterFile; +pub use ast::loader::{load_linter_from_entry, load_linter_into_registry}; +pub use ast::registry::{ExternalLintRegistry, RuleLocator}; +pub use ast::rule::{ + ExternalAstLinter, ExternalAstRule, ExternalAstRuleSpec, ExternalRuleCode, ExternalRuleScript, +}; +pub use ast::runtime::ExternalLintRuntimeHandle; +pub use ast::target::{AstNodeClass, AstTarget, AstTargetSpec, ExprKind, StmtKind}; +pub use error::ExternalLinterError; diff --git a/crates/ruff_linter/src/lib.rs b/crates/ruff_linter/src/lib.rs index eaafd7a526..58d9248441 100644 --- a/crates/ruff_linter/src/lib.rs +++ b/crates/ruff_linter/src/lib.rs @@ -26,6 +26,7 @@ mod cst; pub mod directives; mod doc_lines; mod docstrings; +pub mod external; mod fix; pub mod fs; mod importer; diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index b3eee0d837..6203ca39f8 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -7,10 +7,15 @@ use strum_macros::EnumIter; use crate::codes::RuleIter; use crate::codes::{RuleCodePrefix, RuleGroup}; +use crate::external::ast::rule::ExternalRuleCode; use crate::registry::{Linter, Rule, RuleNamespace}; use crate::rule_redirects::get_redirect; use crate::settings::types::PreviewMode; +fn looks_like_external_rule_code(s: &str) -> bool { + ExternalRuleCode::matches_format(s) +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum RuleSelector { /// Select all rules (includes rules in preview if enabled) @@ -33,6 +38,8 @@ pub enum RuleSelector { prefix: RuleCodePrefix, redirected_from: Option<&'static str>, }, + /// Select an external rule code. + External { code: Box }, } impl From for RuleSelector { @@ -70,8 +77,16 @@ impl FromStr for RuleSelector { None => (s, None), }; - let (linter, code) = - Linter::parse_code(s).ok_or_else(|| ParseError::Unknown(s.to_string()))?; + let Some((linter, code)) = Linter::parse_code(s) else { + if looks_like_external_rule_code(s) { + if ExternalRuleCode::new(s).is_ok() { + return Ok(Self::External { code: s.into() }); + } + + return Err(ParseError::External(s.to_string())); + } + return Err(ParseError::Unknown(s.to_string())); + }; if code.is_empty() { return Ok(Self::Linter(linter)); @@ -119,10 +134,12 @@ pub enum ParseError { // TODO(martin): tell the user how to discover rule codes via the CLI once such a command is // implemented (but that should of course be done only in ruff and not here) Unknown(String), + #[error("External rule selector `{0}` is not supported yet.")] + External(String), } impl RuleSelector { - pub fn prefix_and_code(&self) -> (&'static str, &'static str) { + pub fn prefix_and_code(&self) -> (&str, &str) { match self { RuleSelector::All => ("", "ALL"), RuleSelector::C => ("", "C"), @@ -131,6 +148,7 @@ impl RuleSelector { (prefix.linter().common_prefix(), prefix.short_code()) } RuleSelector::Linter(l) => (l.common_prefix(), ""), + RuleSelector::External { code } => ("", code.as_ref()), } } } @@ -174,7 +192,14 @@ impl Visitor<'_> for SelectorVisitor { where E: de::Error, { - FromStr::from_str(v).map_err(de::Error::custom) + match FromStr::from_str(v) { + Ok(value) => Ok(value), + Err(err @ ParseError::External(_)) => Err(de::Error::custom(err.to_string())), + Err(err) if looks_like_external_rule_code(v) => Err(de::Error::custom(format!( + "{err}. External rule selectors are not supported yet." + ))), + Err(err) => Err(de::Error::custom(err)), + } } } @@ -198,6 +223,9 @@ impl RuleSelector { RuleSelector::Prefix { prefix, .. } | RuleSelector::Rule { prefix, .. } => { RuleSelectorIter::Vec(prefix.clone().rules()) } + RuleSelector::External { .. } => { + RuleSelectorIter::Vec(vec![Rule::ExternalLinter].into_iter()) + } } } @@ -224,7 +252,7 @@ impl RuleSelector { /// Returns true if this selector is exact i.e. selects a single rule by code pub fn is_exact(&self) -> bool { - matches!(self, Self::Rule { .. }) + matches!(self, Self::Rule { .. } | Self::External { .. }) } } @@ -337,6 +365,7 @@ impl RuleSelector { RuleSelector::C => Specificity::LinterGroup, RuleSelector::Linter(..) => Specificity::Linter, RuleSelector::Rule { .. } => Specificity::Rule, + RuleSelector::External { .. } => Specificity::Rule, RuleSelector::Prefix { prefix, .. } => { let prefix: &'static str = prefix.short_code(); match prefix.len() { @@ -360,8 +389,16 @@ impl RuleSelector { "C" => Ok(Self::C), "T" => Ok(Self::T), _ => { - let (linter, code) = - Linter::parse_code(s).ok_or_else(|| ParseError::Unknown(s.to_string()))?; + let Some((linter, code)) = Linter::parse_code(s) else { + if looks_like_external_rule_code(s) { + if ExternalRuleCode::new(s).is_ok() { + return Ok(Self::External { code: s.into() }); + } + + return Err(ParseError::External(s.to_string())); + } + return Err(ParseError::Unknown(s.to_string())); + }; if code.is_empty() { return Ok(Self::Linter(linter)); @@ -415,7 +452,7 @@ pub mod clap_completion { RuleSelector, codes::RuleCodePrefix, registry::{Linter, RuleNamespace}, - rule_selector::is_single_rule_selector, + rule_selector::{ParseError, is_single_rule_selector}, }; #[derive(Clone)] @@ -442,20 +479,26 @@ pub mod clap_completion { .to_str() .ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?; - value.parse().map_err(|_| { - let mut error = - clap::Error::new(clap::error::ErrorKind::ValueValidation).with_cmd(cmd); - if let Some(arg) = arg { + value.parse().map_err(|err| match err { + ParseError::External(code) => clap::Error::raw( + clap::error::ErrorKind::ValueValidation, + format!("External rule selector `{code}` is not supported yet."), + ), + ParseError::Unknown(_) => { + let mut error = + clap::Error::new(clap::error::ErrorKind::ValueValidation).with_cmd(cmd); + if let Some(arg) = arg { + error.insert( + clap::error::ContextKind::InvalidArg, + clap::error::ContextValue::String(arg.to_string()), + ); + } error.insert( - clap::error::ContextKind::InvalidArg, - clap::error::ContextValue::String(arg.to_string()), + clap::error::ContextKind::InvalidValue, + clap::error::ContextValue::String(value.to_string()), ); + error } - error.insert( - clap::error::ContextKind::InvalidValue, - clap::error::ContextValue::String(value.to_string()), - ); - error }) } diff --git a/crates/ruff_linter/src/rules/ruff/rules/external_ast.rs b/crates/ruff_linter/src/rules/ruff/rules/external_ast.rs new file mode 100644 index 0000000000..25ac617815 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/external_ast.rs @@ -0,0 +1,37 @@ +use ruff_macros::{CacheKey, ViolationMetadata, derive_message_formats}; + +/// Diagnostics surfaced by external AST linters +/// +/// ## What it does +/// +/// This is a meta rule that represents any/all external rules implemented +/// in Python. See more at TODO documentation link +/// +/// ## Why is this bad? +/// +/// Depends on the rule. +/// +#[derive(Debug, Clone, PartialEq, Eq, CacheKey, ViolationMetadata)] +#[violation_metadata(stable_since = "v0.0.0")] +pub(crate) struct ExternalLinter { + pub rule_name: String, + pub message: String, +} + +impl ExternalLinter { + #[allow(dead_code)] + pub(crate) fn new(rule_name: impl Into, message: String) -> Self { + Self { + rule_name: rule_name.into(), + message, + } + } +} + +impl crate::Violation for ExternalLinter { + #[derive_message_formats] + fn message(&self) -> String { + let ExternalLinter { rule_name, message } = self; + format!("{rule_name}: {message}") + } +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 206a504e74..feb878b65a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -9,6 +9,7 @@ pub(crate) use dataclass_enum::*; pub(crate) use decimal_from_float_literal::*; pub(crate) use default_factory_kwarg::*; pub(crate) use explicit_f_string_type_conversion::*; +pub(crate) use external_ast::*; pub(crate) use falsy_dict_get_fallback::*; pub(crate) use function_call_in_dataclass_default::*; pub(crate) use if_key_in_dict_del::*; @@ -74,6 +75,7 @@ mod dataclass_enum; mod decimal_from_float_literal; mod default_factory_kwarg; mod explicit_f_string_type_conversion; +mod external_ast; mod falsy_dict_get_fallback; mod function_call_in_dataclass_default; mod if_key_in_dict_del; diff --git a/crates/ruff_server/src/session/options.rs b/crates/ruff_server/src/session/options.rs index dba88c99ae..19c221aa1e 100644 --- a/crates/ruff_server/src/session/options.rs +++ b/crates/ruff_server/src/session/options.rs @@ -187,7 +187,7 @@ impl ClientOptions { for rule in rules { match RuleSelector::from_str(rule) { Ok(selector) => known.push(selector), - Err(ParseError::Unknown(_)) => unknown.push(rule), + Err(ParseError::Unknown(_) | ParseError::External(_)) => unknown.push(rule), } } if !unknown.is_empty() { diff --git a/ruff.schema.json b/ruff.schema.json index 1c8a092042..d8f6b34df1 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4053,6 +4053,9 @@ "RUF2", "RUF20", "RUF200", + "RUF3", + "RUF30", + "RUF300", "S", "S1", "S10", From 7da4ae6323b838c0e66e4d276960d2ddb73b60b4 Mon Sep 17 00:00:00 2001 From: Pieter Hooimeijer Date: Fri, 24 Oct 2025 22:41:26 -0700 Subject: [PATCH 2/2] [ruff][ext-lint] 2: external lint rule selection --- crates/ruff/src/args.rs | 163 +++- crates/ruff/src/commands/check.rs | 29 + crates/ruff/src/commands/format.rs | 2 +- crates/ruff/src/external.rs | 298 +++++++ crates/ruff/src/lib.rs | 195 ++++- crates/ruff/tests/cli/lint.rs | 805 +++++++++++++++++- .../ruff_linter/src/external/ast/registry.rs | 37 + crates/ruff_linter/src/external/error.rs | 21 + crates/ruff_linter/src/rule_selector.rs | 23 +- crates/ruff_linter/src/settings/mod.rs | 10 + crates/ruff_workspace/src/configuration.rs | 277 +++++- crates/ruff_workspace/src/options.rs | 73 ++ crates/ruff_workspace/src/resolver.rs | 17 + docs/configuration.md | 16 + ruff.schema.json | 114 +++ 15 files changed, 2068 insertions(+), 12 deletions(-) create mode 100644 crates/ruff/src/external.rs diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 370186a0b4..ca35cc0ae7 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -28,7 +28,7 @@ use ruff_options_metadata::{OptionEntry, OptionsMetadata}; use ruff_python_ast as ast; use ruff_source_file::{LineIndex, OneIndexed, PositionEncoding}; use ruff_text_size::TextRange; -use ruff_workspace::configuration::{Configuration, RuleSelection}; +use ruff_workspace::configuration::{Configuration, ExternalRuleSelection, RuleSelection}; use ruff_workspace::options::{Options, PycodestyleOptions}; use ruff_workspace::resolver::ConfigurationTransformer; use rustc_hash::FxHashMap; @@ -469,6 +469,53 @@ pub struct CheckCommand { conflicts_with = "watch", )] pub show_settings: bool, + /// List configured external AST linters and exit. + #[arg( + long, + help_heading = "External linter options", + conflicts_with = "add_noqa" + )] + pub list_external_linters: bool, + /// Restrict linting to the given external linter IDs. + #[arg( + long = "select-external", + value_name = "LINTER", + action = clap::ArgAction::Append, + help_heading = "External linter options", + )] + pub select_external: Vec, + /// Enable additional external linter IDs or rule codes without replacing existing selections. + #[arg( + long = "extend-select-external", + value_name = "LINTER", + action = clap::ArgAction::Append, + help_heading = "External linter options", + )] + pub extend_select_external: Vec, + /// Disable the given external linter IDs or rule codes. + #[arg( + long = "ignore-external", + value_name = "LINTER", + action = clap::ArgAction::Append, + help_heading = "External linter options", + )] + pub ignore_external: Vec, + /// Disable additional external linter IDs or rule codes without replacing existing ignores. + #[arg( + long = "extend-ignore-external", + value_name = "LINTER", + action = clap::ArgAction::Append, + help_heading = "External linter options", + )] + pub extend_ignore_external: Vec, + /// Validate external linter definitions without running lint checks. + #[arg( + long = "verify-external-linters", + help_heading = "External linter options", + conflicts_with = "add_noqa", + conflicts_with = "list_external_linters" + )] + pub verify_external_linters: bool, } #[derive(Clone, Debug, clap::Parser)] @@ -666,6 +713,14 @@ impl ConfigArguments { self.config_file.as_deref() } + pub(crate) fn has_cli_external_selection(&self) -> bool { + self.per_flag_overrides + .select_external + .as_ref() + .is_some_and(|selection| !selection.is_empty()) + || !self.per_flag_overrides.extend_select_external.is_empty() + } + fn from_cli_arguments( global_options: GlobalConfigArgs, per_flag_overrides: ExplicitConfigOverrides, @@ -737,6 +792,70 @@ impl CheckCommand { self, global_options: GlobalConfigArgs, ) -> anyhow::Result<(CheckArguments, ConfigArguments)> { + if let Some(invalid) = self + .select_external + .iter() + .find(|selector| is_builtin_rule_selector(selector)) + { + anyhow::bail!( + "Internal rule `{invalid}` cannot be enabled with `--select-external`; use `--select` instead." + ); + } + if let Some(selector) = self.select.as_ref().and_then(|selectors| { + selectors.iter().find_map(|selector| { + if let RuleSelector::External { code } = selector { + Some(code.as_ref().to_string()) + } else { + None + } + }) + }) { + anyhow::bail!( + "External rule `{selector}` cannot be enabled with `--select`; use `--select-external` instead." + ); + } + if let Some(selector) = self.extend_select.as_ref().and_then(|selectors| { + selectors.iter().find_map(|selector| { + if let RuleSelector::External { code } = selector { + Some(code.as_ref().to_string()) + } else { + None + } + }) + }) { + anyhow::bail!( + "External rule `{selector}` cannot be enabled with `--extend-select`; use `--extend-select-external` instead." + ); + } + if let Some(invalid) = self + .extend_select_external + .iter() + .find(|selector| is_builtin_rule_selector(selector)) + { + anyhow::bail!( + "Internal rule `{invalid}` cannot be enabled with `--extend-select-external`; use `--extend-select` instead." + ); + } + if let Some(invalid) = self + .ignore_external + .iter() + .chain(self.extend_ignore_external.iter()) + .find(|selector| is_builtin_rule_selector(selector)) + { + anyhow::bail!( + "Internal rule `{invalid}` cannot be disabled with `--ignore-external`; use `--ignore` instead." + ); + } + + let select_external_override = if self.select_external.is_empty() { + None + } else { + Some(self.select_external.clone()) + }; + let extend_select_external_override = self.extend_select_external.clone(); + let ignore_external_override = self.ignore_external.clone(); + let extend_ignore_external_override = self.extend_ignore_external.clone(); + let check_arguments = CheckArguments { add_noqa: self.add_noqa, diff: self.diff, @@ -748,6 +867,12 @@ impl CheckCommand { output_file: self.output_file, show_files: self.show_files, show_settings: self.show_settings, + list_external_linters: self.list_external_linters, + select_external: self.select_external, + extend_select_external: self.extend_select_external, + ignore_external: self.ignore_external, + extend_ignore_external: self.extend_ignore_external, + verify_external_linters: self.verify_external_linters, statistics: self.statistics, stdin_filename: self.stdin_filename, watch: self.watch, @@ -781,6 +906,10 @@ impl CheckCommand { output_format: self.output_format, show_fixes: resolve_bool_arg(self.show_fixes, self.no_show_fixes), extension: self.extension, + select_external: select_external_override, + extend_select_external: extend_select_external_override, + ignore_external: ignore_external_override, + extend_ignore_external: extend_ignore_external_override, ..ExplicitConfigOverrides::default() }; @@ -1083,6 +1212,12 @@ pub struct CheckArguments { pub output_file: Option, pub show_files: bool, pub show_settings: bool, + pub list_external_linters: bool, + pub select_external: Vec, + pub extend_select_external: Vec, + pub ignore_external: Vec, + pub extend_ignore_external: Vec, + pub verify_external_linters: bool, pub statistics: bool, pub stdin_filename: Option, pub watch: bool, @@ -1347,6 +1482,10 @@ struct ExplicitConfigOverrides { detect_string_imports: Option, string_imports_min_dots: Option, type_checking_imports: Option, + select_external: Option>, + extend_select_external: Vec, + ignore_external: Vec, + extend_ignore_external: Vec, } impl ConfigurationTransformer for ExplicitConfigOverrides { @@ -1440,11 +1579,33 @@ impl ConfigurationTransformer for ExplicitConfigOverrides { if let Some(type_checking_imports) = &self.type_checking_imports { config.analyze.type_checking_imports = Some(*type_checking_imports); } + if self.select_external.is_some() + || !self.extend_select_external.is_empty() + || !self.ignore_external.is_empty() + || !self.extend_ignore_external.is_empty() + { + config + .lint + .external_rule_selections + .push(ExternalRuleSelection { + select: self.select_external.clone(), + extend_select: self.extend_select_external.clone(), + ignore: self.ignore_external.clone(), + extend_ignore: self.extend_ignore_external.clone(), + }); + } config } } +fn is_builtin_rule_selector(selector: &str) -> bool { + matches!( + RuleSelector::from_str(selector), + Ok(RuleSelector::Linter(_) | RuleSelector::Prefix { .. } | RuleSelector::Rule { .. }) + ) +} + /// Convert a list of `PatternPrefixPair` structs to `PerFileIgnore`. pub fn collect_per_file_ignores(pairs: Vec) -> Vec { let mut per_file_ignores: FxHashMap> = FxHashMap::default(); diff --git a/crates/ruff/src/commands/check.rs b/crates/ruff/src/commands/check.rs index dcdd0f9b18..887e30dbfa 100644 --- a/crates/ruff/src/commands/check.rs +++ b/crates/ruff/src/commands/check.rs @@ -21,6 +21,7 @@ use ruff_linter::settings::{LinterSettings, flags}; use ruff_linter::{IOError, Violation, fs, warn_user_once}; use ruff_source_file::SourceFileBuilder; use ruff_text_size::TextRange; +use ruff_workspace::Settings; use ruff_workspace::resolver::{ PyprojectConfig, ResolvedFile, match_exclusion, python_files_in_path, }; @@ -28,6 +29,7 @@ use ruff_workspace::resolver::{ use crate::args::ConfigArguments; use crate::cache::{Cache, PackageCacheMap, PackageCaches}; use crate::diagnostics::Diagnostics; +use crate::{apply_external_linter_selection_to_settings, compute_external_selection_state}; /// Run the linter over a collection of files. pub(crate) fn check( @@ -41,6 +43,20 @@ pub(crate) fn check( ) -> Result { // Collect all the Python files to check. let start = Instant::now(); + let apply_external_selection = |settings: &mut Settings| -> Result<()> { + let state = compute_external_selection_state( + &settings.linter.selected_external, + &settings.linter.ignored_external, + &[], + &[], + &[], + &[], + ); + settings.linter.selected_external = state.effective.iter().cloned().collect(); + settings.linter.ignored_external = state.ignored.iter().cloned().collect(); + apply_external_linter_selection_to_settings(settings, &state.effective, &state.ignored)?; + Ok(()) + }; let (paths, resolver) = python_files_in_path(files, pyproject_config, config_arguments)?; debug!("Identified files to lint in: {:?}", start.elapsed()); @@ -49,6 +65,19 @@ pub(crate) fn check( return Ok(Diagnostics::default()); } + let resolver = resolver.transform_settings(apply_external_selection)?; + let selection_from_cli = config_arguments.has_cli_external_selection(); + let any_external_selection = selection_from_cli + || resolver + .settings() + .any(|settings| !settings.linter.selected_external.is_empty()); + let any_external_registry = resolver + .settings() + .any(|settings| settings.linter.external_ast.is_some()); + if any_external_selection && !any_external_registry { + anyhow::bail!("No external AST linters are configured in this workspace."); + } + // Discover the package root for each Python file. let package_roots = resolver.package_roots( &paths diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 0e245efa8c..57d34512da 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -596,7 +596,7 @@ impl<'a> FormatResults<'a> { .iter() .map(Diagnostic::from) .chain(self.to_diagnostics(&mut notebook_index)) - .sorted_unstable_by(Diagnostic::ruff_start_ordering) + .sorted_by(Diagnostic::ruff_start_ordering) .collect(); let context = EmitterContext::new(¬ebook_index); diff --git a/crates/ruff/src/external.rs b/crates/ruff/src/external.rs new file mode 100644 index 0000000000..ec1325bb99 --- /dev/null +++ b/crates/ruff/src/external.rs @@ -0,0 +1,298 @@ +use std::io::{self, Write}; + +use anyhow::Result; +use ruff_linter::external::ExternalLintRegistry; +use ruff_linter::external::ast::rule::{ExternalAstLinter, ExternalAstRule}; +use ruff_linter::registry::Rule; +use ruff_workspace::Settings; +use rustc_hash::FxHashSet; + +#[derive(Debug)] +pub(crate) struct ExternalSelectionState { + pub ignored: FxHashSet, + pub effective: FxHashSet, +} + +pub(crate) fn compute_external_selection_state( + base_selected: &[String], + base_ignored: &[String], + cli_select: &[String], + cli_extend_select: &[String], + cli_ignore: &[String], + cli_extend_ignore: &[String], +) -> ExternalSelectionState { + let mut selected: FxHashSet = if cli_select.is_empty() { + base_selected.iter().cloned().collect() + } else { + FxHashSet::default() + }; + selected.extend(cli_select.iter().cloned()); + selected.extend(cli_extend_select.iter().cloned()); + + let mut ignored: FxHashSet = base_ignored.iter().cloned().collect(); + ignored.extend(cli_ignore.iter().cloned()); + ignored.extend(cli_extend_ignore.iter().cloned()); + + let effective = selected + .iter() + .filter(|code| !ignored.contains(*code)) + .cloned() + .collect(); + + ExternalSelectionState { ignored, effective } +} + +pub(crate) fn apply_external_linter_selection_to_settings( + settings: &mut Settings, + selected: &FxHashSet, + ignored: &FxHashSet, +) -> Result { + let linter = &mut settings.linter; + + if selected.is_empty() { + if linter.rules.enabled(Rule::ExternalLinter) { + linter.rules.disable(Rule::ExternalLinter); + } + linter.selected_external.clear(); + linter.external_ast = None; + return Ok(true); + } + + if !linter.rules.enabled(Rule::ExternalLinter) { + linter.selected_external.clear(); + linter.external_ast = None; + return Ok(false); + } + + if let Some(registry) = linter.external_ast.take() { + let selection = select_external_linters(®istry, selected, ignored); + if !selection.missing.is_empty() { + anyhow::bail!( + "Unknown external linter or rule selector(s): {}", + selection.missing.join(", ") + ); + } + + let mut filtered = ExternalLintRegistry::new(); + for matched in &selection.matches { + filtered.insert_linter(matched.clone_selected())?; + } + + linter.selected_external = selected.iter().cloned().collect(); + let codes: Vec = filtered + .iter_enabled_rules() + .map(|rule| rule.code.as_str().to_string()) + .collect(); + + if filtered.is_empty() { + linter.rules.disable(Rule::ExternalLinter); + linter.external_ast = None; + linter.selected_external.clear(); + } else { + linter.rules.enable(Rule::ExternalLinter, false); + linter.external_ast = Some(filtered); + let external_codes = &mut linter.external; + for code in codes { + if !external_codes.iter().any(|existing| existing == &code) { + external_codes.push(code); + } + } + } + + Ok(true) + } else { + Ok(false) + } +} + +pub(crate) fn select_external_linters<'a>( + registry: &'a ExternalLintRegistry, + selected: &FxHashSet, + ignored: &FxHashSet, +) -> SelectedExternalLinters<'a> { + let mut matches = Vec::new(); + let mut missing = Vec::new(); + + let enabled_linters: Vec<&'a ExternalAstLinter> = registry + .linters() + .iter() + .filter(|linter| linter.enabled) + .collect(); + + if selected.is_empty() { + matches.extend( + enabled_linters + .iter() + .copied() + .map(SelectedExternalLinter::all_rules), + ); + return SelectedExternalLinters { matches, missing }; + } + + let mut satisfied: FxHashSet<&'a str> = FxHashSet::default(); + let mut available_linter_ids: FxHashSet<&'a str> = FxHashSet::default(); + + for linter in &enabled_linters { + available_linter_ids.insert(linter.id.as_str()); + } + + for linter in enabled_linters { + let selected_linter = selected.contains(linter.id.as_str()); + + if selected_linter && ignored.is_empty() { + matches.push(SelectedExternalLinter::all_rules(linter)); + satisfied.insert(linter.id.as_str()); + continue; + } + + let included: Vec<_> = linter + .rules + .iter() + .filter(|rule| !ignored.contains(rule.code.as_str())) + .collect(); + + if selected_linter { + if included.is_empty() { + missing.push(linter.id.clone()); + continue; + } + + satisfied.insert(linter.id.as_str()); + for rule in &included { + if selected.contains(rule.code.as_str()) { + satisfied.insert(rule.code.as_str()); + } + } + + matches.push(SelectedExternalLinter::subset(linter, included)); + continue; + } + + let matched_rules: Vec<_> = included + .iter() + .copied() + .filter(|rule| selected.contains(rule.code.as_str())) + .collect(); + + if matched_rules.is_empty() { + continue; + } + + for rule in &matched_rules { + satisfied.insert(rule.code.as_str()); + } + + matches.push(SelectedExternalLinter::subset(linter, matched_rules)); + } + + for selector in selected { + let selector = selector.as_str(); + if ignored.contains(selector) || satisfied.contains(selector) { + continue; + } + + if available_linter_ids.contains(selector) { + continue; + } + + if registry.find_rule_by_code(selector).is_some() { + continue; + } + + missing.push(selector.to_string()); + } + + SelectedExternalLinters { matches, missing } +} + +#[derive(Debug)] +pub(crate) struct SelectedExternalLinters<'a> { + pub matches: Vec>, + pub missing: Vec, +} + +pub(crate) fn print_external_linters( + registry: &ExternalLintRegistry, + linters: &[SelectedExternalLinter<'_>], + mut writer: impl Write, +) -> io::Result<()> { + match (registry.is_empty(), linters.is_empty()) { + (true, _) => writeln!(writer, "No external AST linters configured.")?, + (false, true) => writeln!(writer, "No matching external AST linters found.")?, + (false, false) => { + for selected in linters { + selected.print(&mut writer)?; + } + } + } + Ok(()) +} + +#[derive(Debug)] +pub(crate) struct SelectedExternalLinter<'a> { + linter: &'a ExternalAstLinter, + selection: SelectedRules<'a>, +} + +impl<'a> SelectedExternalLinter<'a> { + fn all_rules(linter: &'a ExternalAstLinter) -> Self { + Self { + linter, + selection: SelectedRules::All, + } + } + + fn subset(linter: &'a ExternalAstLinter, rules: Vec<&'a ExternalAstRule>) -> Self { + debug_assert!(!rules.is_empty()); + Self { + linter, + selection: SelectedRules::Subset(rules), + } + } + + fn clone_selected(&self) -> ExternalAstLinter { + match &self.selection { + SelectedRules::All => self.linter.clone(), + SelectedRules::Subset(rules) => ExternalAstLinter { + id: self.linter.id.clone(), + name: self.linter.name.clone(), + description: self.linter.description.clone(), + enabled: self.linter.enabled, + rules: rules.iter().map(|&rule| rule.clone()).collect(), + }, + } + } + + fn print(&self, writer: &mut impl Write) -> io::Result<()> { + match &self.selection { + SelectedRules::All => write!(writer, "{}", self.linter), + SelectedRules::Subset(rules) => { + writeln!( + writer, + "{}{}", + self.linter.id, + if self.linter.enabled { + "" + } else { + " (disabled)" + } + )?; + writeln!(writer, " name: {}", self.linter.name)?; + if let Some(description) = &self.linter.description { + writeln!(writer, " description: {description}")?; + } + writeln!(writer, " rules:")?; + for rule in rules { + writeln!(writer, " - {} ({})", rule.code.as_str(), rule.name)?; + } + writeln!(writer) + } + } + } +} + +#[derive(Debug)] +enum SelectedRules<'a> { + All, + Subset(Vec<&'a ExternalAstRule>), +} diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index 3ea0d94fad..47a4b64d91 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -14,26 +14,35 @@ use notify::{RecursiveMode, Watcher, recommended_watcher}; use args::{GlobalConfigArgs, ServerCommand}; use ruff_db::diagnostic::{Diagnostic, Severity}; +use ruff_linter::external::ExternalLintRegistry; use ruff_linter::logging::{LogLevel, set_up_logging}; use ruff_linter::settings::flags::FixMode; use ruff_linter::settings::types::OutputFormat; use ruff_linter::{fs, warn_user, warn_user_once}; use ruff_workspace::Settings; +use ruff_workspace::resolver::PyprojectConfig; +use rustc_hash::FxHashSet; -use crate::args::{ - AnalyzeCommand, AnalyzeGraphCommand, Args, CheckCommand, Command, FormatCommand, +use crate::{ + args::{AnalyzeCommand, AnalyzeGraphCommand, Args, CheckCommand, Command, FormatCommand}, + printer::{Flags as PrinterFlags, Printer}, }; -use crate::printer::{Flags as PrinterFlags, Printer}; pub mod args; mod cache; mod commands; mod diagnostics; +mod external; mod printer; pub mod resolve; mod stdin; mod version; +pub(crate) use external::{ + apply_external_linter_selection_to_settings, compute_external_selection_state, + print_external_linters, select_external_linters, +}; + #[derive(Copy, Clone)] pub enum ExitStatus { /// Linting was successful and there were no linting errors. @@ -125,6 +134,14 @@ fn resolve_default_files(files: Vec, is_stdin: bool) -> Vec { } } +fn apply_external_linter_selection( + pyproject_config: &mut PyprojectConfig, + selected: &FxHashSet, + ignored: &FxHashSet, +) -> Result { + apply_external_linter_selection_to_settings(&mut pyproject_config.settings, selected, ignored) +} + pub fn run( Args { command, @@ -238,7 +255,7 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result = match cli.output_file { Some(path) if !cli.watch => { @@ -256,6 +273,48 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result Result Result ExternalLintRegistry { + let mut registry = ExternalLintRegistry::new(); + + let rules = vec![ + ExternalAstRule::new( + ExternalRuleCode::new("EXT001").unwrap(), + "FirstRule", + None::<&str>, + vec![AstTarget::Stmt(StmtKind::FunctionDef)], + ExternalRuleScript::file( + PathBuf::from("ext001.py"), + "def check_stmt(node, ctx):\n pass\n", + ), + None, + ), + ExternalAstRule::new( + ExternalRuleCode::new("EXT002").unwrap(), + "SecondRule", + None::<&str>, + vec![AstTarget::Stmt(StmtKind::FunctionDef)], + ExternalRuleScript::file( + PathBuf::from("ext002.py"), + "def check_stmt(node, ctx):\n pass\n", + ), + None, + ), + ]; + + let linter = ExternalAstLinter::new("demo", "Demo", None::<&str>, true, rules); + registry.insert_linter(linter).unwrap(); + registry + } + + #[test] + fn selecting_linter_respects_ignored_rule_codes() { + let registry = make_registry(); + + let mut settings = Settings::default(); + settings.linter.external_ast = Some(registry); + settings.linter.selected_external = vec!["demo".to_string()]; + settings.linter.ignored_external = vec!["EXT002".to_string()]; + settings + .linter + .rules + .enable(ruff_linter::registry::Rule::ExternalLinter, false); + + let selected: FxHashSet = + settings.linter.selected_external.iter().cloned().collect(); + let ignored: FxHashSet = settings.linter.ignored_external.iter().cloned().collect(); + apply_external_linter_selection_to_settings(&mut settings, &selected, &ignored).unwrap(); + + let filtered = settings + .linter + .external_ast + .expect("external registry should remain configured"); + assert!( + filtered.find_rule_by_code("EXT002").is_none(), + "ignored rule code should be excluded when selecting the entire linter" + ); + assert!( + filtered.find_rule_by_code("EXT001").is_some(), + "other rules should remain enabled" + ); + } +} + #[cfg(test)] mod test_file_change_detector { use std::path::PathBuf; diff --git a/crates/ruff/tests/cli/lint.rs b/crates/ruff/tests/cli/lint.rs index 9ad806b4a4..6afcd0ed06 100644 --- a/crates/ruff/tests/cli/lint.rs +++ b/crates/ruff/tests/cli/lint.rs @@ -21,6 +21,44 @@ impl CliTest { } } +fn toml_path(path: &std::path::Path) -> String { + // Escape backslashes so Windows paths stay valid inside TOML strings. + path.to_string_lossy().replace('\\', "\\\\") +} + +fn write_demo_external_linter(test: &CliTest) -> Result { + test.write_file( + "lint/external/demo.toml", + r#" +name = "Demo External Linter" +description = "Shows how to configure external AST linters" + +[[rule]] +code = "EXT001" +name = "ExampleRule" +summary = "Provides illustrative coverage" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" + +[[rule]] +code = "EXT002" +name = "AnotherRule" +summary = "Provides illustrative coverage" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" +"#, + )?; + test.write_file( + "lint/external/rules/example.py", + r#" +def check_stmt(node, ctx): + if node["_kind"] == "FunctionDef": + ctx.report("external lint fired") +"#, + )?; + Ok(test.root().join("lint/external/demo.toml")) +} + #[test] fn top_level_options() -> Result<()> { let test = CliTest::new()?; @@ -168,6 +206,767 @@ inline-quotes = "single" Ok(()) } +#[test] +fn external_ast_linter_listing() -> Result<()> { + let test = CliTest::new()?; + test.write_file( + "lint/external/demo.toml", + r#" +name = "Demo External Linter" +description = "Shows how to configure external AST linters" + +[[rule]] +code = "EXT001" +name = "ExampleRule" +summary = "Provides illustrative coverage" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" + +[[rule]] +code = "EXT002" +name = "AnotherRule" +summary = "Demonstrates code-based selection" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" +"#, + )?; + test.write_file( + "lint/external/rules/example.py", + r#" +def check(): + # placeholder script body + pass +"#, + )?; + let linter_path = test.root().join("lint/external/demo.toml"); + let config = format!( + r#" +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + + let output = test + .command() + .args(["check", "--config", "ruff.toml", "--list-external-linters"]) + .output()?; + + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = std::str::from_utf8(&output.stdout)?; + assert!(stdout.contains("demo")); + assert!(stdout.contains("Demo External Linter")); + assert!(stdout.contains("EXT001")); + assert!(stdout.contains("ExampleRule")); + + Ok(()) +} + +#[test] +fn external_ast_linter_listing_filtered_by_code() -> Result<()> { + let test = CliTest::new()?; + test.write_file( + "lint/external/demo.toml", + r#" +name = "Demo External Linter" +description = "Shows how to configure external AST linters" + +[[rule]] +code = "EXT001" +name = "ExampleRule" +summary = "Provides illustrative coverage" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" + +[[rule]] +code = "EXT002" +name = "AnotherRule" +summary = "Demonstrates code-based selection" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" +"#, + )?; + test.write_file( + "lint/external/rules/example.py", + r#" +def check(): + # placeholder script body + pass +"#, + )?; + let linter_path = test.root().join("lint/external/demo.toml"); + let config = format!( + r#" +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + + let output = test + .command() + .args([ + "check", + "--config", + "ruff.toml", + "--list-external-linters", + "--select-external", + "EXT002", + ]) + .output()?; + + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = std::str::from_utf8(&output.stdout)?; + assert!(stdout.contains("EXT002")); + assert!(!stdout.contains("EXT001")); + + Ok(()) +} + +#[test] +fn external_ast_requires_explicit_selection() -> Result<()> { + let test = CliTest::new()?; + test.write_file( + "lint/external/demo.toml", + r#" +name = "Demo External Linter" +description = "Shows how to configure external AST linters" + +[[rule]] +code = "EXT001" +name = "ExampleRule" +summary = "Provides illustrative coverage" +targets = ["stmt:FunctionDef"] +script = "rules/example.py" +"#, + )?; + test.write_file( + "lint/external/rules/example.py", + r#" +def check(): + # placeholder script body + pass +"#, + )?; + let linter_path = test.root().join("lint/external/demo.toml"); + let config = format!( + r#" +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + + let output = test + .command() + .args(["check", "--config", "ruff.toml", "--show-settings"]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = std::str::from_utf8(&output.stdout)?; + assert!( + !stdout.contains("external-linter (RUF300)"), + "Expected external-linter to be disabled without an explicit selection" + ); + + let config_with_select = format!( + r#" +[lint] +extend-select = ["RUF300"] +select-external = ["EXT001"] + +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config_with_select)?; + let output = test + .command() + .args(["check", "--config", "ruff.toml", "--show-settings"]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = std::str::from_utf8(&output.stdout)?; + assert!( + stdout.contains("external-linter (RUF300)"), + "Expected external-linter to be enabled when configured via lint.select-external" + ); + + Ok(()) +} + +#[test] +fn external_ast_select_external_disabled_rule_errors() -> Result<()> { + let test = CliTest::new()?; + let linter_path = { + test.write_file( + "lint/external/disabled.toml", + r#" +enabled = false +name = "Disabled External Linter" + +[[rule]] +code = "EXTDIS001" +name = "DisabledRule" +summary = "Rule intentionally disabled" +targets = ["stmt:FunctionDef"] +script = "rules/disabled.py" +"#, + )?; + test.write_file( + "lint/external/rules/disabled.py", + r#" +def check_stmt(node, ctx): + ctx.report("should not fire") +"#, + )?; + test.root().join("lint/external/disabled.toml") + }; + let config = format!( + r#" +[lint.external-ast.disabled] +path = "{}" + +[lint] +select = ["RUF300"] +select-external = ["EXTDIS001"] +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + test.write_file("example.py", "def foo():\n pass\n")?; + + let output = test + .command() + .args(["check", "--config", "ruff.toml", "example.py"]) + .output()?; + + assert!( + !output.status.success(), + "command unexpectedly succeeded: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("Unknown external linter or rule selector(s): EXTDIS001"), + "stderr missing warning about disabled external selection: {stderr}" + ); + + Ok(()) +} + +#[test] +fn external_ast_ignore_external_cli() -> Result<()> { + let test = CliTest::new()?; + let linter_path = write_demo_external_linter(&test)?; + let config = format!( + r#" +[lint] +select-external = ["EXT001"] + +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + test.write_file( + "example.py", + r#" +def demo(): + return 1 +"#, + )?; + + let output = test + .command() + .args([ + "check", + "--config", + "ruff.toml", + "--select", + "RUF300", + "--extend-select-external", + "EXT002", + "--ignore-external", + "EXT002", + "--list-external-linters", + "example.py", + ]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("EXT001"), "stdout missing EXT001: {stdout}"); + assert!( + !stdout.contains("EXT002"), + "stdout unexpectedly included EXT002: {stdout}" + ); + Ok(()) +} + +#[test] +fn external_ast_select_external_overrides_config() -> Result<()> { + let test = CliTest::new()?; + let linter_path = write_demo_external_linter(&test)?; + let config = format!( + r#" +[lint] +select-external = ["EXT001"] + +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + test.write_file("example.py", "def demo():\n return 1\n")?; + + let output = test + .command() + .args([ + "check", + "--config", + "ruff.toml", + "--select", + "RUF300", + "--select-external", + "EXT002", + "--list-external-linters", + "example.py", + ]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("EXT002"), "stdout missing EXT002: {stdout}"); + assert!( + !stdout.contains("EXT001"), + "stdout unexpectedly included EXT001: {stdout}" + ); + Ok(()) +} + +#[test] +fn cli_select_disables_configured_external_linters() -> Result<()> { + let test = CliTest::new()?; + let linter_path = write_demo_external_linter(&test)?; + let config = format!( + r#" +[lint] +select = ["RUF300"] +select-external = ["EXT001"] + +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + test.write_file("example.py", "def demo():\n return 1\n")?; + + let output = test + .command() + .args(["check", "--config", "ruff.toml", "--show-settings"]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("external-linter (RUF300)"), + "Expected external linters to be enabled via configuration" + ); + + let output = test + .command() + .args([ + "check", + "--config", + "ruff.toml", + "--select", + "G004", + "--show-settings", + ]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + !stdout.contains("external-linter (RUF300)"), + "Expected `--select` to clear configured external linters" + ); + + let output = test + .command() + .args([ + "check", + "--config", + "ruff.toml", + "--select", + "G004", + "example.py", + ]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + !stdout.contains("EXT001"), + "Expected external linter diagnostics to be suppressed: {stdout}" + ); + + Ok(()) +} + +#[test] +fn external_ast_select_external_overrides_nested_config() -> Result<()> { + let test = CliTest::new()?; + let linter_path = write_demo_external_linter(&test)?; + let config = format!( + r#" +[lint] +select-external = ["EXT001", "EXT002"] + +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + test.write_file("nested/ruff.toml", r#"extend = "../ruff.toml""#)?; + test.write_file( + "nested/example.py", + r#" +def demo(): + return 1 +"#, + )?; + + let output = test + .command() + .args([ + "check", + "--select", + "RUF300", + "--select-external", + "EXT002", + "--list-external-linters", + "nested/example.py", + ]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("EXT002"), "stdout missing EXT002: {stdout}"); + assert!( + !stdout.contains("EXT001"), + "stdout unexpectedly included EXT001: {stdout}" + ); + + Ok(()) +} + +#[test] +fn select_external_cli_allows_nested_registry() -> Result<()> { + let test = CliTest::new()?; + let linter_path = write_demo_external_linter(&test)?; + + let nested_config = format!( + r#" +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("nested/ruff.toml", &nested_config)?; + test.write_file("example.py", "def root():\n return 1\n")?; + test.write_file("nested/example.py", "def nested():\n return 1\n")?; + + let output = test + .command() + .args([ + "check", + "--select", + "RUF300", + "--select-external", + "EXT001", + "example.py", + "nested/example.py", + ]) + .output()?; + assert!( + output.status.success(), + "command failed unexpectedly: {}", + String::from_utf8_lossy(&output.stderr) + ); + + Ok(()) +} + +#[test] +fn select_external_cli_errors_when_no_registries() -> Result<()> { + let test = CliTest::new()?; + test.write_file("example.py", "def root():\n return 1\n")?; + + let output = test + .command() + .args([ + "check", + "--select", + "RUF300", + "--select-external", + "EXT001", + "example.py", + ]) + .output()?; + assert!( + !output.status.success(), + "command unexpectedly succeeded: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("No external AST linters are configured in this workspace."), + "stderr missing missing-registry error: {stderr}" + ); + + Ok(()) +} + +#[test] +fn external_ast_ignore_external_config() -> Result<()> { + let test = CliTest::new()?; + let linter_path = write_demo_external_linter(&test)?; + let config = format!( + r#" +[lint] +select-external = ["EXT001"] +extend-select-external = ["EXT002"] +ignore-external = ["EXT002"] + +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + test.write_file( + "example.py", + r#" +def demo(): + return 1 +"#, + )?; + + let output = test + .command() + .args([ + "check", + "--config", + "ruff.toml", + "--select", + "RUF300", + "--list-external-linters", + "example.py", + ]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("EXT001"), "stdout missing EXT001: {stdout}"); + assert!( + !stdout.contains("EXT002"), + "stdout unexpectedly included EXT002: {stdout}" + ); + Ok(()) +} + +#[test] +fn external_ast_ignore_external_nested_config() -> Result<()> { + let test = CliTest::new()?; + let linter_path = write_demo_external_linter(&test)?; + let config = format!( + r#" +[lint] +select-external = ["EXT001", "EXT002"] +ignore-external = ["EXT002"] + +[lint.external-ast.demo] +path = "{}" +"#, + toml_path(&linter_path) + ); + test.write_file("ruff.toml", &config)?; + test.write_file( + "pkg/pyproject.toml", + r#" +[tool.ruff] +line-length = 88 +"#, + )?; + test.write_file( + "pkg/example.py", + r#" +def demo(): + return 1 +"#, + )?; + + let output = test + .command() + .args([ + "check", + "--config", + "ruff.toml", + "--select", + "RUF300", + "--list-external-linters", + "pkg/example.py", + ]) + .output()?; + assert!( + output.status.success(), + "command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("EXT001"), + "stdout missing surviving EXT001 listing: {stdout}" + ); + assert!( + !stdout.contains("EXT002"), + "stdout unexpectedly included ignored EXT002 listing: {stdout}" + ); + + Ok(()) +} + +#[test] +fn select_rejects_external_rules() -> Result<()> { + let test = CliTest::new()?; + + let output = test + .command() + .args(["check", "--isolated", "--select", "EXT001"]) + .output()?; + assert!(!output.status.success(), "command unexpectedly succeeded"); + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("cannot be enabled with `--select`"), + "expected failure when selecting external rule via --select" + ); + + Ok(()) +} + +#[test] +fn select_external_rejects_internal_rules() -> Result<()> { + let test = CliTest::new()?; + + let output = test + .command() + .args(["check", "--isolated", "--select-external", "F401"]) + .output()?; + assert!(!output.status.success(), "command unexpectedly succeeded"); + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("cannot be enabled with `--select-external`"), + "expected failure when selecting internal rules via --select-external" + ); + + Ok(()) +} + +#[test] +fn config_select_rejects_external_rules() -> Result<()> { + let test = CliTest::new()?; + test.write_file( + "ruff.toml", + r#" +[lint] +select = ["EXT001"] +"#, + )?; + + let output = test + .command() + .args(["check", "--config", "ruff.toml"]) + .output()?; + assert!(!output.status.success(), "command unexpectedly succeeded"); + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("lint.select-external"), + "expected parse failure when selecting external rule via configuration" + ); + + Ok(()) +} + +#[test] +fn config_select_external_rejects_internal_rules() -> Result<()> { + let test = CliTest::new()?; + test.write_file( + "ruff.toml", + r#" +[lint] +select-external = ["F401"] +"#, + )?; + + let output = test + .command() + .args(["check", "--config", "ruff.toml"]) + .output()?; + assert!(!output.status.success(), "command unexpectedly succeeded"); + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("cannot be enabled via `lint.select-external`"), + "expected lint.select-external failure when selecting internal rule" + ); + + Ok(()) +} + #[test] fn exclude() -> Result<()> { let case = CliTest::new()?; @@ -775,7 +1574,7 @@ fn valid_toml_but_nonexistent_option_provided_via_config_argument() { Could not parse the supplied argument as a `ruff.toml` configuration option: - Unknown rule selector: `F481`. External rule selectors are not supported yet. + Unknown rule selector: `F481`. External rule selectors must be provided via `lint.select-external`. For more information, try '--help'. "); @@ -907,6 +1706,10 @@ fn value_given_to_table_key_is_not_inline_table_2() { - `lint.dummy-variable-rgx` - `lint.extend-ignore` - `lint.extend-select` + - `lint.select-external` + - `lint.extend-select-external` + - `lint.ignore-external` + - `lint.extend-ignore-external` - `lint.extend-fixable` - `lint.external` - `lint.fixable` diff --git a/crates/ruff_linter/src/external/ast/registry.rs b/crates/ruff_linter/src/external/ast/registry.rs index ee0435f68c..791e4f672d 100644 --- a/crates/ruff_linter/src/external/ast/registry.rs +++ b/crates/ruff_linter/src/external/ast/registry.rs @@ -120,6 +120,43 @@ impl ExternalLintRegistry { let rule = linter.rules.get(locator.rule_index)?; Some((rule, linter)) } + + pub fn iter_enabled_rules(&self) -> impl Iterator { + self.linters + .iter() + .filter(|linter| linter.enabled) + .flat_map(|linter| linter.rules.iter()) + } + + pub fn iter_enabled_linter_rules( + &self, + ) -> impl Iterator { + self.linters + .iter() + .filter(|linter| linter.enabled) + .flat_map(|linter| linter.rules.iter().map(move |rule| (linter, rule))) + } + + pub fn iter_enabled_rule_locators(&self) -> impl Iterator + '_ { + self.linters + .iter() + .enumerate() + .filter(|(_, linter)| linter.enabled) + .flat_map(|(linter_index, linter)| { + linter + .rules + .iter() + .enumerate() + .map(move |(rule_index, _)| RuleLocator::new(linter_index, rule_index)) + }) + } + + pub fn expect_entry(&self, locator: RuleLocator) -> (&ExternalAstLinter, &ExternalAstRule) { + let (rule, linter) = self + .rule_entry(locator) + .expect("rule locator does not reference a valid entry"); + (linter, rule) + } } impl ruff_cache::CacheKey for ExternalLintRegistry { diff --git a/crates/ruff_linter/src/external/error.rs b/crates/ruff_linter/src/external/error.rs index dd4ec23f5a..95b2ee5d36 100644 --- a/crates/ruff_linter/src/external/error.rs +++ b/crates/ruff_linter/src/external/error.rs @@ -75,4 +75,25 @@ pub enum ExternalLinterError { "external rule `{rule}` in linter `{linter}` declares `call-callee-regex` but does not target `expr:Call` nodes" )] CallCalleeRegexWithoutCallTarget { linter: String, rule: String }, + + #[error("{message}")] + ScriptCompile { message: String }, +} + +impl ExternalLinterError { + #[allow(dead_code)] + pub(crate) fn format_script_compile_message( + linter: &str, + rule: &str, + path: Option, + message: impl Into, + ) -> String { + let message = message.into(); + let location = path + .map(|p| format!(" at {}", p.display())) + .unwrap_or_default(); + format!( + "failed to compile script for external rule `{rule}` in linter `{linter}`{location}: {message}" + ) + } } diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index 6203ca39f8..ecb7d68197 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -67,6 +67,11 @@ impl FromStr for RuleSelector { fn from_str(s: &str) -> Result { // **Changes should be reflected in `parse_no_redirect` as well** + // External AST rules reserve the `EXT` prefix; short-circuit before we + // attempt to interpret the selector as a built-in linter code. + if s.starts_with("EXT") && ExternalRuleCode::new(s).is_ok() { + return Ok(Self::External { code: s.into() }); + } match s { "ALL" => Ok(Self::All), "C" => Ok(Self::C), @@ -134,7 +139,9 @@ pub enum ParseError { // TODO(martin): tell the user how to discover rule codes via the CLI once such a command is // implemented (but that should of course be done only in ruff and not here) Unknown(String), - #[error("External rule selector `{0}` is not supported yet.")] + #[error( + "External rule selector `{0}` must be provided via `--select-external` or `lint.select-external`." + )] External(String), } @@ -158,6 +165,9 @@ impl Serialize for RuleSelector { where S: serde::Serializer, { + if let RuleSelector::External { code } = self { + return serializer.serialize_str(code); + } let (prefix, code) = self.prefix_and_code(); serializer.serialize_str(&format!("{prefix}{code}")) } @@ -196,7 +206,7 @@ impl Visitor<'_> for SelectorVisitor { Ok(value) => Ok(value), Err(err @ ParseError::External(_)) => Err(de::Error::custom(err.to_string())), Err(err) if looks_like_external_rule_code(v) => Err(de::Error::custom(format!( - "{err}. External rule selectors are not supported yet." + "{err}. External rule selectors must be provided via `lint.select-external`." ))), Err(err) => Err(de::Error::custom(err)), } @@ -384,6 +394,11 @@ impl RuleSelector { /// Parse [`RuleSelector`] from a string; but do not follow redirects. pub fn parse_no_redirect(s: &str) -> Result { // **Changes should be reflected in `from_str` as well** + // External AST rules reserve the `EXT` prefix; short-circuit before we + // attempt to interpret the selector as a built-in linter code. + if s.starts_with("EXT") && ExternalRuleCode::new(s).is_ok() { + return Ok(Self::External { code: s.into() }); + } match s { "ALL" => Ok(Self::All), "C" => Ok(Self::C), @@ -482,7 +497,9 @@ pub mod clap_completion { value.parse().map_err(|err| match err { ParseError::External(code) => clap::Error::raw( clap::error::ErrorKind::ValueValidation, - format!("External rule selector `{code}` is not supported yet."), + format!( + "External rule selector `{code}` must be provided via `--select-external`." + ), ), ParseError::Unknown(_) => { let mut error = diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index b94e4edafb..d7180f3ec8 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -10,6 +10,7 @@ use std::sync::LazyLock; use types::CompiledPerFileTargetVersionList; use crate::codes::RuleCodePrefix; +use crate::external::ExternalLintRegistry; use ruff_macros::CacheKey; use ruff_python_ast::PythonVersion; @@ -243,6 +244,9 @@ pub struct LinterSettings { pub builtins: Vec, pub dummy_variable_rgx: Regex, pub external: Vec, + pub external_ast: Option, + pub selected_external: Vec, + pub ignored_external: Vec, pub ignore_init_module_imports: bool, pub logger_objects: Vec, pub namespace_packages: Vec, @@ -319,6 +323,9 @@ impl Display for LinterSettings { self.typing_extensions, ] } + if let Some(registry) = &self.external_ast { + writeln!(f, "linter.external-ast = {registry:#?}")?; + } writeln!(f, "\n# Linter Plugins")?; display_settings! { formatter = f, @@ -410,6 +417,9 @@ impl LinterSettings { dummy_variable_rgx: DUMMY_VARIABLE_RGX.clone(), external: vec![], + external_ast: None, + selected_external: Vec::new(), + ignored_external: Vec::new(), ignore_init_module_imports: true, logger_objects: vec![], namespace_packages: vec![], diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index bf50749a45..2184240dea 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -21,6 +21,9 @@ use strum::IntoEnumIterator; use ruff_cache::cache_dir; use ruff_formatter::IndentStyle; use ruff_graph::{AnalyzeSettings, Direction, StringImports}; +use ruff_linter::external::{ + ExternalLintRegistry, PyprojectExternalLinterEntry, load_linter_into_registry, +}; use ruff_linter::line_width::{IndentWidth, LineLength}; use ruff_linter::registry::{INCOMPATIBLE_CODES, Rule, RuleNamespace, RuleSet}; use ruff_linter::rule_selector::{PreviewOptions, Specificity}; @@ -69,6 +72,14 @@ pub struct RuleSelection { pub extend_fixable: Vec, } +#[derive(Clone, Debug, Default)] +pub struct ExternalRuleSelection { + pub select: Option>, + pub extend_select: Vec, + pub ignore: Vec, + pub extend_ignore: Vec, +} + #[derive(Debug, Eq, PartialEq, is_macro::Is)] pub enum RuleSelectorKind { /// Enables the selected rules @@ -112,6 +123,65 @@ impl RuleSelection { .map(|selector| (RuleSelectorKind::Modify, selector)), ) } + + pub fn selectors(&self) -> impl Iterator { + self.selectors_by_kind().map(|(_, selector)| selector) + } +} + +fn build_external_registry( + entries: &BTreeMap, +) -> Result> { + let mut registry = ExternalLintRegistry::new(); + for (id, entry) in entries { + if !entry.enabled { + continue; + } + let py_entry = PyprojectExternalLinterEntry { + toml_path: entry.path.clone(), + enabled: entry.enabled, + }; + load_linter_into_registry(&mut registry, id, &py_entry)?; + } + if registry.is_empty() { + Ok(None) + } else { + Ok(Some(registry)) + } +} + +fn resolve_external_rule_selections( + selections: &[ExternalRuleSelection], +) -> (Vec, Vec) { + let mut selected: FxHashSet = FxHashSet::default(); + let mut ignored: FxHashSet = FxHashSet::default(); + + for selection in selections { + if let Some(select) = &selection.select { + selected = select.iter().cloned().collect(); + } + for code in &selection.extend_select { + selected.insert(code.clone()); + } + for code in &selection.ignore { + ignored.insert(code.clone()); + } + for code in &selection.extend_ignore { + ignored.insert(code.clone()); + } + } + + let mut selected_vec: Vec = selected.into_iter().collect(); + let mut ignored_vec: Vec = ignored.into_iter().collect(); + selected_vec.sort_unstable(); + ignored_vec.sort_unstable(); + (selected_vec, ignored_vec) +} + +#[derive(Debug, Clone)] +pub struct ExternalLinterEntry { + pub path: PathBuf, + pub enabled: bool, } #[derive(Debug, Default, Clone)] @@ -242,7 +312,8 @@ impl Configuration { let line_length = self.line_length.unwrap_or_default(); - let rules = lint.as_rule_table(lint_preview)?; + #[allow(unused_mut)] + let mut rules = lint.as_rule_table(lint_preview)?; // LinterSettings validation let isort = lint @@ -260,6 +331,75 @@ impl Configuration { let future_annotations = lint.future_annotations.unwrap_or_default(); + let (configured_selected_external_vec, configured_ignored_external_vec) = + resolve_external_rule_selections(&lint.external_rule_selections); + + let mut external_codes = lint.external.unwrap_or_default(); + let mut seen_external_codes = external_codes.iter().cloned().collect::>(); + + let external_ast_registry = match lint.external_ast.as_ref() { + Some(entries) => build_external_registry(entries)?, + None => None, + }; + + if let Some(registry) = external_ast_registry.as_ref() { + for rule in registry.iter_enabled_rules() { + let code = rule.code.as_str().to_owned(); + if seen_external_codes.insert(code.clone()) { + external_codes.push(code); + } + } + } + for code in &configured_selected_external_vec { + if seen_external_codes.insert(code.clone()) { + external_codes.push(code.clone()); + } + } + + let available_external_codes = external_ast_registry.as_ref().map(|registry| { + registry + .iter_enabled_rules() + .map(|rule| rule.code.as_str().to_string()) + .collect::>() + }); + + let selectors = lint + .rule_selections + .iter() + .flat_map(RuleSelection::selectors) + .chain(lint.extend_safe_fixes.iter()) + .chain(lint.extend_unsafe_fixes.iter()); + + let mut missing_external = FxHashSet::default(); + for selector in selectors { + if let RuleSelector::External { code } = selector { + let is_known = available_external_codes + .as_ref() + .is_some_and(|available| available.contains(code.as_ref())); + if !is_known { + missing_external.insert(code.as_ref().to_string()); + } + } + } + + if !missing_external.is_empty() { + let mut missing: Vec<_> = missing_external.into_iter().collect(); + missing.sort_unstable(); + let formatted = missing + .into_iter() + .map(|code| format!("`{code}`")) + .collect::>(); + let message = if formatted.len() == 1 { + format!("Unknown rule selector: {}", formatted[0]) + } else { + format!("Unknown rule selectors: {}", formatted.join(", ")) + }; + return Err(anyhow!(message)); + } + if external_ast_registry.is_none() { + rules.disable(Rule::ExternalLinter); + } + Ok(Settings { cache_dir: self .cache_dir @@ -306,7 +446,10 @@ impl Configuration { dummy_variable_rgx: lint .dummy_variable_rgx .unwrap_or_else(|| DUMMY_VARIABLE_RGX.clone()), - external: lint.external.unwrap_or_default(), + external: external_codes, + external_ast: external_ast_registry, + selected_external: configured_selected_external_vec, + ignored_external: configured_ignored_external_vec, ignore_init_module_imports: lint.ignore_init_module_imports.unwrap_or(true), line_length, tab_size: self.indent_width.unwrap_or_default(), @@ -638,6 +781,7 @@ pub struct LintConfiguration { pub per_file_ignores: Option>, pub rule_selections: Vec, pub explicit_preview_rules: Option, + pub external_rule_selections: Vec, // Fix configuration pub extend_unsafe_fixes: Vec, @@ -647,6 +791,7 @@ pub struct LintConfiguration { pub allowed_confusables: Option>, pub dummy_variable_rgx: Option, pub external: Option>, + pub external_ast: Option>, pub ignore_init_module_imports: Option, pub logger_objects: Option>, pub task_tags: Option>, @@ -713,6 +858,118 @@ impl LintConfiguration { options.common.ignore_init_module_imports }; + for (label, selectors) in [ + ("lint.select", options.common.select.as_ref()), + ("lint.extend-select", options.common.extend_select.as_ref()), + ] { + if let Some(selectors) = selectors { + if let Some(code) = selectors.iter().find_map(|selector| { + if let RuleSelector::External { code } = selector { + Some(code.as_ref().to_string()) + } else { + None + } + }) { + return Err(anyhow::anyhow!( + "External rule `{code}` cannot be enabled via `{label}`; use `lint.select-external` instead." + )); + } + } + } + + if let Some(internal) = options.common.select_external.as_ref().and_then(|values| { + values.iter().find(|value| { + matches!( + RuleSelector::from_str(value), + Ok(RuleSelector::Linter(_) + | RuleSelector::Prefix { .. } + | RuleSelector::Rule { .. }) + ) + }) + }) { + return Err(anyhow::anyhow!( + "Internal rule `{internal}` cannot be enabled via `lint.select-external`; use `lint.select` instead." + )); + } + if let Some(internal) = options + .common + .extend_select_external + .as_ref() + .and_then(|values| { + values.iter().find(|value| { + matches!( + RuleSelector::from_str(value), + Ok(RuleSelector::Linter(_) + | RuleSelector::Prefix { .. } + | RuleSelector::Rule { .. }) + ) + }) + }) + { + return Err(anyhow::anyhow!( + "Internal rule `{internal}` cannot be enabled via `lint.extend-select-external`; use `lint.extend-select` instead." + )); + } + if let Some(internal) = options.common.ignore_external.as_ref().and_then(|values| { + values.iter().find(|value| { + matches!( + RuleSelector::from_str(value), + Ok(RuleSelector::Linter(_) + | RuleSelector::Prefix { .. } + | RuleSelector::Rule { .. }) + ) + }) + }) { + return Err(anyhow::anyhow!( + "Internal rule `{internal}` cannot be disabled via `lint.ignore-external`; use `lint.ignore` instead." + )); + } + if let Some(internal) = options + .common + .extend_ignore_external + .as_ref() + .and_then(|values| { + values.iter().find(|value| { + matches!( + RuleSelector::from_str(value), + Ok(RuleSelector::Linter(_) + | RuleSelector::Prefix { .. } + | RuleSelector::Rule { .. }) + ) + }) + }) + { + return Err(anyhow::anyhow!( + "Internal rule `{internal}` cannot be disabled via `lint.extend-ignore-external`; use `lint.extend-ignore` instead." + )); + } + + let external_ast_entries = options + .external_ast + .map(|entries| { + entries + .into_iter() + .map(|(id, entry)| { + let raw_path = entry + .path + .ok_or_else(|| anyhow!("external linter `{id}` must define `path`"))?; + let path = if raw_path.is_absolute() { + raw_path + } else { + project_root.join(raw_path) + }; + Ok(( + id, + ExternalLinterEntry { + path, + enabled: entry.enabled.unwrap_or(true), + }, + )) + }) + .collect::>>() + }) + .transpose()?; + Ok(LintConfiguration { exclude: options.exclude.map(|paths| { paths @@ -733,6 +990,12 @@ impl LintConfiguration { unfixable, extend_fixable: options.common.extend_fixable.unwrap_or_default(), }], + external_rule_selections: vec![ExternalRuleSelection { + select: options.common.select_external, + extend_select: options.common.extend_select_external.unwrap_or_default(), + ignore: options.common.ignore_external.unwrap_or_default(), + extend_ignore: options.common.extend_ignore_external.unwrap_or_default(), + }], extend_safe_fixes: options.common.extend_safe_fixes.unwrap_or_default(), extend_unsafe_fixes: options.common.extend_unsafe_fixes.unwrap_or_default(), allowed_confusables: options.common.allowed_confusables, @@ -755,6 +1018,7 @@ impl LintConfiguration { }) .unwrap_or_default(), external: options.common.external, + external_ast: external_ast_entries, ignore_init_module_imports, explicit_preview_rules: options.common.explicit_preview_rules, per_file_ignores: options.common.per_file_ignores.map(|per_file_ignores| { @@ -1136,6 +1400,8 @@ impl LintConfiguration { let mut extend_per_file_ignores = config.extend_per_file_ignores; extend_per_file_ignores.extend(self.extend_per_file_ignores); + let mut external_rule_selections = config.external_rule_selections; + external_rule_selections.extend(self.external_rule_selections); Self { exclude: self.exclude.or(config.exclude), @@ -1143,10 +1409,12 @@ impl LintConfiguration { rule_selections, extend_safe_fixes, extend_unsafe_fixes, + external_rule_selections, allowed_confusables: self.allowed_confusables.or(config.allowed_confusables), dummy_variable_rgx: self.dummy_variable_rgx.or(config.dummy_variable_rgx), extend_per_file_ignores, external: self.external.or(config.external), + external_ast: self.external_ast.or(config.external_ast), ignore_init_module_imports: self .ignore_init_module_imports .or(config.ignore_init_module_imports), @@ -1414,6 +1682,8 @@ fn warn_about_deprecated_top_level_lint_options( pyupgrade, per_file_ignores, extend_per_file_ignores, + select_external, + .. } = top_level_options; let mut used_options = Vec::new(); @@ -1472,6 +1742,9 @@ fn warn_about_deprecated_top_level_lint_options( if select.is_some() { used_options.push("select"); } + if select_external.is_some() { + used_options.push("select-external"); + } if explicit_preview_rules.is_some() { used_options.push("explicit-preview-rules"); diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 472b0e66f4..11e38ef0ba 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -554,6 +554,8 @@ pub struct LintOptions { "# )] pub future_annotations: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub external_ast: Option>, } /// Newtype wrapper for [`LintCommonOptions`] that allows customizing the JSON schema and omitting the fields from the [`OptionsMetadata`]. @@ -569,6 +571,23 @@ impl OptionsMetadata for DeprecatedTopLevelLintOptions { } } +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[derive(Clone, Debug, PartialEq, Eq, OptionsMetadata, CombineOptions, Serialize, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +pub struct ExternalAstLinterOptions { + /// Path to the TOML file containing external lint rule definitions. + #[option( + default = "null", + value_type = "path", + example = r#"path = "lint/custom_rules.toml""# + )] + pub path: Option, + /// Whether this external linter should be considered during lint runs. + #[option(default = "true", value_type = "bool", example = "enabled = false")] + #[serde(default)] + pub enabled: Option, +} + #[cfg(feature = "schemars")] impl schemars::JsonSchema for DeprecatedTopLevelLintOptions { fn schema_name() -> std::borrow::Cow<'static, str> { @@ -666,6 +685,45 @@ pub struct LintCommonOptions { "# )] pub extend_select: Option>, + /// A list of external linter IDs or rule codes to enable. When omitted, external linters remain disabled. + #[option( + default = "[]", + value_type = "list[str]", + example = r#" + # Enable the `logging_interpolation` external linter by default. + select-external = ["logging_interpolation"] + "# + )] + pub select_external: Option>, + /// A list of external linter IDs or rule codes to enable, in addition to those specified by [`select-external`](#lint_select-external). + #[option( + default = "[]", + value_type = "list[str]", + example = r#" + # Enable the `logging_interpolation` external linter alongside those specified by `select-external`. + extend-select-external = ["logging_interpolation"] + "# + )] + pub extend_select_external: Option>, + /// A list of external linter IDs or rule codes to ignore. Prefixes are not supported. + #[option( + default = "[]", + value_type = "list[str]", + example = r#" + # Suppress the `logging_interpolation.dangerous` rule, even if the linter is otherwise enabled. + ignore-external = ["logging_interpolation.dangerous"] + "# + )] + pub ignore_external: Option>, + /// A list of external linter IDs or rule codes to ignore, in addition to those specified by [`ignore-external`](#lint_ignore-external). + #[option( + default = "[]", + value_type = "list[str]", + example = r#" + extend-ignore-external = ["logging_interpolation.dangerous"] + "# + )] + pub extend_ignore_external: Option>, /// A list of rule codes or prefixes to consider fixable, in addition to those /// specified by [`fixable`](#lint_fixable). @@ -3916,9 +3974,14 @@ pub struct LintOptionsWire { dummy_variable_rgx: Option, extend_ignore: Option>, extend_select: Option>, + select_external: Option>, + extend_select_external: Option>, + ignore_external: Option>, + extend_ignore_external: Option>, extend_fixable: Option>, extend_unfixable: Option>, external: Option>, + external_ast: Option>, fixable: Option>, ignore: Option>, extend_safe_fixes: Option>, @@ -3973,9 +4036,14 @@ impl From for LintOptions { dummy_variable_rgx, extend_ignore, extend_select, + select_external, + extend_select_external, + ignore_external, + extend_ignore_external, extend_fixable, extend_unfixable, external, + external_ast, fixable, ignore, extend_safe_fixes, @@ -4029,6 +4097,10 @@ impl From for LintOptions { dummy_variable_rgx, extend_ignore, extend_select, + select_external, + extend_select_external, + ignore_external, + extend_ignore_external, extend_fixable, extend_unfixable, external, @@ -4076,6 +4148,7 @@ impl From for LintOptions { ruff, preview, typing_extensions, + external_ast, future_annotations, } } diff --git a/crates/ruff_workspace/src/resolver.rs b/crates/ruff_workspace/src/resolver.rs index 1740cc184a..b184894548 100644 --- a/crates/ruff_workspace/src/resolver.rs +++ b/crates/ruff_workspace/src/resolver.rs @@ -257,6 +257,23 @@ impl<'a> Resolver<'a> { pub fn settings(&self) -> impl Iterator { std::iter::once(&self.pyproject_config.settings).chain(&self.settings) } + + /// Return a mutable iterator over resolved [`Settings`] excluding the base configuration. + pub fn settings_mut(&mut self) -> impl Iterator { + self.settings.iter_mut() + } + + /// Apply a transformation to each resolved [`Settings`] (excluding the base configuration) + /// and return the [`Resolver`] for further use. + pub fn transform_settings(mut self, mut f: F) -> Result + where + F: FnMut(&mut Settings) -> Result<()>, + { + for settings in &mut self.settings { + f(settings)?; + } + Ok(self) + } } /// A wrapper around `detect_package_root` to cache filesystem lookups. diff --git a/docs/configuration.md b/docs/configuration.md index 3420611e18..7796bcf505 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -679,6 +679,22 @@ Miscellaneous: Exit with a non-zero status code if any files were modified via fix, even if no lint violations remain +External linter options: + --list-external-linters + List configured external AST linters and exit + --select-external + Restrict linting to the given external linter IDs + --extend-select-external + Enable additional external linter IDs or rule codes without replacing + existing selections + --ignore-external + Disable the given external linter IDs or rule codes + --extend-ignore-external + Disable additional external linter IDs or rule codes without + replacing existing ignores + --verify-external-linters + Validate external linter definitions without running lint checks + Log levels: -v, --verbose Enable verbose logging -q, --quiet Print diagnostics, but nothing else diff --git a/ruff.schema.json b/ruff.schema.json index d8f6b34df1..32d756a67e 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -109,6 +109,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-ignore-external": { + "description": "A list of external linter IDs or rule codes to ignore, in addition to those specified by [`ignore-external`](#lint_ignore-external).", + "type": [ + "array", + "null" + ], + "deprecated": true, + "items": { + "type": "string" + } + }, "extend-include": { "description": "A list of file patterns to include when linting, in addition to those\nspecified by [`include`](#include).\n\nInclusion are based on globs, and should be single-path patterns, like\n`*.pyw`, to include any file with the `.pyw` extension.\n\nFor more information on the glob syntax, refer to the [`globset` documentation](https://docs.rs/globset/latest/globset/#syntax).", "type": [ @@ -155,6 +166,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-select-external": { + "description": "A list of external linter IDs or rule codes to enable, in addition to those specified by [`select-external`](#lint_select-external).", + "type": [ + "array", + "null" + ], + "deprecated": true, + "items": { + "type": "string" + } + }, "extend-unfixable": { "description": "A list of rule codes or prefixes to consider non-auto-fixable, in addition to those\nspecified by [`unfixable`](#lint_unfixable).", "type": [ @@ -446,6 +468,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "ignore-external": { + "description": "A list of external linter IDs or rule codes to ignore. Prefixes are not supported.", + "type": [ + "array", + "null" + ], + "deprecated": true, + "items": { + "type": "string" + } + }, "ignore-init-module-imports": { "description": "Avoid automatically removing unused imports in `__init__.py` files. Such\nimports will still be flagged, but with a dedicated message suggesting\nthat the import is either added to the module's `__all__` symbol, or\nre-exported with a redundant alias (e.g., `import os as os`).\n\nThis option is enabled by default, but you can opt-in to removal of imports\nvia an unsafe fix.", "type": [ @@ -684,6 +717,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "select-external": { + "description": "A list of external linter IDs or rule codes to enable. When omitted, external linters remain disabled.", + "type": [ + "array", + "null" + ], + "deprecated": true, + "items": { + "type": "string" + } + }, "show-fixes": { "description": "Whether to show an enumeration of all fixed lint violations\n(overridden by the `--show-fixes` command-line flag).", "type": [ @@ -907,6 +951,27 @@ } ] }, + "ExternalAstLinterOptions": { + "type": "object", + "properties": { + "enabled": { + "description": "Whether this external linter should be considered during lint runs.", + "type": [ + "boolean", + "null" + ], + "default": null + }, + "path": { + "description": "Path to the TOML file containing external lint rule definitions.", + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": false + }, "Flake8AnnotationsOptions": { "description": "Options for the `flake8-annotations` plugin.", "type": "object", @@ -2016,6 +2081,16 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-ignore-external": { + "description": "A list of external linter IDs or rule codes to ignore, in addition to those specified by [`ignore-external`](#lint_ignore-external).", + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + }, "extend-per-file-ignores": { "description": "A list of mappings from file pattern to rule codes or prefixes to\nexclude, in addition to any rules excluded by [`per-file-ignores`](#lint_per-file-ignores).", "type": [ @@ -2049,6 +2124,16 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-select-external": { + "description": "A list of external linter IDs or rule codes to enable, in addition to those specified by [`select-external`](#lint_select-external).", + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + }, "extend-unfixable": { "description": "A list of rule codes or prefixes to consider non-auto-fixable, in addition to those\nspecified by [`unfixable`](#lint_unfixable).", "type": [ @@ -2080,6 +2165,15 @@ "type": "string" } }, + "external-ast": { + "type": [ + "object", + "null" + ], + "additionalProperties": { + "$ref": "#/definitions/ExternalAstLinterOptions" + } + }, "fixable": { "description": "A list of rule codes or prefixes to consider fixable. By default,\nall rules are considered fixable.", "type": [ @@ -2294,6 +2388,16 @@ "$ref": "#/definitions/RuleSelector" } }, + "ignore-external": { + "description": "A list of external linter IDs or rule codes to ignore. Prefixes are not supported.", + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + }, "ignore-init-module-imports": { "description": "Avoid automatically removing unused imports in `__init__.py` files. Such\nimports will still be flagged, but with a dedicated message suggesting\nthat the import is either added to the module's `__all__` symbol, or\nre-exported with a redundant alias (e.g., `import os as os`).\n\nThis option is enabled by default, but you can opt-in to removal of imports\nvia an unsafe fix.", "type": [ @@ -2452,6 +2556,16 @@ "$ref": "#/definitions/RuleSelector" } }, + "select-external": { + "description": "A list of external linter IDs or rule codes to enable. When omitted, external linters remain disabled.", + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + }, "task-tags": { "description": "A list of task tags to recognize (e.g., \"TODO\", \"FIXME\", \"XXX\").\n\nComments starting with these tags will be ignored by commented-out code\ndetection (`ERA`), and skipped by line-length rules (`E501`) if\n[`ignore-overlong-task-comments`](#lint_pycodestyle_ignore-overlong-task-comments) is set to `true`.", "type": [