diff --git a/ruff_dev/src/generate_rules_table.rs b/ruff_dev/src/generate_rules_table.rs index e24d4f3273373f..a1cfa001787ad8 100644 --- a/ruff_dev/src/generate_rules_table.rs +++ b/ruff_dev/src/generate_rules_table.rs @@ -25,13 +25,13 @@ fn generate_table(table_out: &mut String, prefix: &RuleCodePrefix) { table_out.push('\n'); table_out.push_str("| ---- | ---- | ------- | --- |"); table_out.push('\n'); - for rule_code in prefix.codes() { - let kind = rule_code.kind(); + for rule in prefix.codes() { + let kind = rule.kind(); let fix_token = if kind.fixable() { "🛠" } else { "" }; table_out.push_str(&format!( "| {} | {} | {} | {} |", - kind.rule().code(), - kind.as_ref(), + rule.code(), + rule.as_ref(), kind.summary().replace('|', r"\|"), fix_token )); diff --git a/ruff_macros/src/define_rule_mapping.rs b/ruff_macros/src/define_rule_mapping.rs index b6537e6f4fe2cd..006360b103611c 100644 --- a/ruff_macros/src/define_rule_mapping.rs +++ b/ruff_macros/src/define_rule_mapping.rs @@ -70,6 +70,7 @@ pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream { Hash, PartialOrd, Ord, + AsRefStr, )] pub enum Rule { #rule_variants } diff --git a/src/registry.rs b/src/registry.rs index 8ca76132ecc83a..10aecb6512f8d1 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -725,17 +725,4 @@ mod tests { ); } } - - #[test] - fn fixable_codes() { - for rule in Rule::iter() { - let kind = rule.kind(); - if kind.fixable() { - assert!( - kind.commit().is_some(), - "{rule:?} is fixable but has no commit message." - ); - } - } - } } diff --git a/src/rules/pandas_vet/rules.rs b/src/rules/pandas_vet/rules.rs index 8244bf676f9131..9fab1cdf86f4a0 100644 --- a/src/rules/pandas_vet/rules.rs +++ b/src/rules/pandas_vet/rules.rs @@ -2,7 +2,7 @@ use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Located}; use crate::ast::types::{BindingKind, Range}; use crate::checkers::ast::Checker; -use crate::registry::{Diagnostic, Rule}; +use crate::registry::{Diagnostic, DiagnosticKind, Rule}; use crate::violations; /// PD002 @@ -68,96 +68,98 @@ pub fn check_attr( value: &Located, attr_expr: &Located, ) { - for (code, name) in vec![ - (Rule::UseOfDotIx, "ix"), - (Rule::UseOfDotAt, "at"), - (Rule::UseOfDotIat, "iat"), - (Rule::UseOfDotValues, "values"), - ] { - if checker.settings.rules.enabled(&code) { - if attr == name { - // Avoid flagging on function calls (e.g., `df.values()`). - if let Some(parent) = checker.current_expr_parent() { - if matches!(parent.node, ExprKind::Call { .. }) { - continue; - } - } - // Avoid flagging on non-DataFrames (e.g., `{"a": 1}.values`). - if super::helpers::is_dataframe_candidate(value) { - // If the target is a named variable, avoid triggering on - // irrelevant bindings (like imports). - if let ExprKind::Name { id, .. } = &value.node { - if checker.find_binding(id).map_or(true, |binding| { - matches!( - binding.kind, - BindingKind::Builtin - | BindingKind::ClassDefinition - | BindingKind::FunctionDefinition - | BindingKind::Export(..) - | BindingKind::FutureImportation - | BindingKind::StarImportation(..) - | BindingKind::Importation(..) - | BindingKind::FromImportation(..) - | BindingKind::SubmoduleImportation(..) - ) - }) { - continue; - } - } + let rules = &checker.settings.rules; + let violation: DiagnosticKind = match attr { + "ix" if rules.enabled(&Rule::UseOfDotIx) => violations::UseOfDotIx.into(), + "at" if rules.enabled(&Rule::UseOfDotAt) => violations::UseOfDotAt.into(), + "iat" if rules.enabled(&Rule::UseOfDotIat) => violations::UseOfDotIat.into(), + "values" if rules.enabled(&Rule::UseOfDotValues) => violations::UseOfDotValues.into(), + _ => return, + }; - checker - .diagnostics - .push(Diagnostic::new(code.kind(), Range::from_located(attr_expr))); - } - }; + // Avoid flagging on function calls (e.g., `df.values()`). + if let Some(parent) = checker.current_expr_parent() { + if matches!(parent.node, ExprKind::Call { .. }) { + return; } } + // Avoid flagging on non-DataFrames (e.g., `{"a": 1}.values`). + if !super::helpers::is_dataframe_candidate(value) { + return; + } + + // If the target is a named variable, avoid triggering on + // irrelevant bindings (like imports). + if let ExprKind::Name { id, .. } = &value.node { + if checker.find_binding(id).map_or(true, |binding| { + matches!( + binding.kind, + BindingKind::Builtin + | BindingKind::ClassDefinition + | BindingKind::FunctionDefinition + | BindingKind::Export(..) + | BindingKind::FutureImportation + | BindingKind::StarImportation(..) + | BindingKind::Importation(..) + | BindingKind::FromImportation(..) + | BindingKind::SubmoduleImportation(..) + ) + }) { + return; + } + } + + checker + .diagnostics + .push(Diagnostic::new(violation, Range::from_located(attr_expr))); } pub fn check_call(checker: &mut Checker, func: &Located) { - for (code, name) in vec![ - (Rule::UseOfDotIsNull, "isnull"), - (Rule::UseOfDotNotNull, "notnull"), - (Rule::UseOfDotPivotOrUnstack, "pivot"), - (Rule::UseOfDotPivotOrUnstack, "unstack"), - (Rule::UseOfDotReadTable, "read_table"), - (Rule::UseOfDotStack, "stack"), - ] { - if checker.settings.rules.enabled(&code) { - if let ExprKind::Attribute { value, attr, .. } = &func.node { - if attr == name { - if super::helpers::is_dataframe_candidate(value) { - // If the target is a named variable, avoid triggering on - // irrelevant bindings (like non-Pandas imports). - if let ExprKind::Name { id, .. } = &value.node { - if checker.find_binding(id).map_or(true, |binding| { - if let BindingKind::Importation(.., module) = &binding.kind { - module != &"pandas" - } else { - matches!( - binding.kind, - BindingKind::Builtin - | BindingKind::ClassDefinition - | BindingKind::FunctionDefinition - | BindingKind::Export(..) - | BindingKind::FutureImportation - | BindingKind::StarImportation(..) - | BindingKind::Importation(..) - | BindingKind::FromImportation(..) - | BindingKind::SubmoduleImportation(..) - ) - } - }) { - continue; - } - } + let rules = &checker.settings.rules; + let ExprKind::Attribute { value, attr, .. } = &func.node else {return}; + let violation: DiagnosticKind = match attr.as_str() { + "isnull" if rules.enabled(&Rule::UseOfDotIsNull) => violations::UseOfDotIsNull.into(), + "notnull" if rules.enabled(&Rule::UseOfDotNotNull) => violations::UseOfDotNotNull.into(), + "pivot" | "unstack" if rules.enabled(&Rule::UseOfDotPivotOrUnstack) => { + violations::UseOfDotPivotOrUnstack.into() + } + "read_table" if rules.enabled(&Rule::UseOfDotReadTable) => { + violations::UseOfDotReadTable.into() + } + "stack" if rules.enabled(&Rule::UseOfDotStack) => violations::UseOfDotStack.into(), + _ => return, + }; - checker - .diagnostics - .push(Diagnostic::new(code.kind(), Range::from_located(func))); - } - }; + if !super::helpers::is_dataframe_candidate(value) { + return; + } + + // If the target is a named variable, avoid triggering on + // irrelevant bindings (like non-Pandas imports). + if let ExprKind::Name { id, .. } = &value.node { + if checker.find_binding(id).map_or(true, |binding| { + if let BindingKind::Importation(.., module) = &binding.kind { + module != &"pandas" + } else { + matches!( + binding.kind, + BindingKind::Builtin + | BindingKind::ClassDefinition + | BindingKind::FunctionDefinition + | BindingKind::Export(..) + | BindingKind::FutureImportation + | BindingKind::StarImportation(..) + | BindingKind::Importation(..) + | BindingKind::FromImportation(..) + | BindingKind::SubmoduleImportation(..) + ) } + }) { + return; } } + + checker + .diagnostics + .push(Diagnostic::new(violation, Range::from_located(func))); }