From 736f6526589ca484cf1dfc2868e643ca0163ab6e Mon Sep 17 00:00:00 2001 From: Chengxu Bian Date: Thu, 26 Jun 2025 20:46:12 -0400 Subject: [PATCH 1/2] Support lint level configuration in clippy.toml Add support for [lints.clippy] and [lints.rust] sections in clippy.toml using the same format as Cargo.toml. clippy.toml configurations take precedence over Cargo.toml with conflict warnings. - Add shared lint configuration types - Implement configuration merging with priority support - Integrate lint argument injection in driver - Extend lint priority conflict checking to clippy.toml - Add comprehensive test suite Fixes #15128 --- clippy_config/src/lib.rs | 2 + clippy_config/src/lint_config.rs | 431 ++++++++++++++++++ clippy_config/src/types.rs | 212 +++++++++ .../src/cargo/lint_groups_priority.rs | 99 ++-- src/driver.rs | 152 +++++- 5 files changed, 810 insertions(+), 86 deletions(-) create mode 100644 clippy_config/src/lint_config.rs diff --git a/clippy_config/src/lib.rs b/clippy_config/src/lib.rs index 67904b4fcdc8..82149eb23c6c 100644 --- a/clippy_config/src/lib.rs +++ b/clippy_config/src/lib.rs @@ -24,6 +24,8 @@ extern crate rustc_span; mod conf; mod metadata; pub mod types; +pub mod lint_config; pub use conf::{Conf, get_configuration_metadata, lookup_conf_file, sanitize_explanation}; pub use metadata::ClippyConfiguration; +pub use lint_config::MergedLintConfig; diff --git a/clippy_config/src/lint_config.rs b/clippy_config/src/lint_config.rs new file mode 100644 index 000000000000..1ea30376dd8c --- /dev/null +++ b/clippy_config/src/lint_config.rs @@ -0,0 +1,431 @@ +use crate::types::{CargoToml, Lints}; +use rustc_session::Session; +use std::collections::BTreeMap; +use std::path::Path; + +#[derive(Debug)] +pub struct MergedLintConfig { + pub rust_lints: BTreeMap)>, // (level, priority, source) + pub clippy_lints: BTreeMap)>, // (level, priority, source) +} + +impl MergedLintConfig { + pub fn load(sess: &Session) -> Self { + let mut merged = MergedLintConfig { + rust_lints: BTreeMap::new(), + clippy_lints: BTreeMap::new(), + }; + + // Load from Cargo.toml + if let Ok(file) = sess.source_map().load_file(Path::new("Cargo.toml")) { + if let Some(src) = file.src.as_deref() { + if let Ok(cargo_toml) = toml::from_str::(src) { + merged.merge_cargo_config(cargo_toml, sess); + } + } + } + + // Load from clippy.toml + if let Ok((Some(clippy_config_path), _)) = crate::lookup_conf_file() { + if let Ok(file) = sess.source_map().load_file(&clippy_config_path) { + if let Some(src) = file.src.as_deref() { + // Try parsing as a full CargoToml structure (with [lints] sections) + if let Ok(clippy_config) = toml::from_str::(src) { + merged.merge_lints(&clippy_config.lints, "clippy.toml", sess); + merged.merge_lints(&clippy_config.workspace.lints, "clippy.toml [workspace]", sess); + } else if let Ok(clippy_lints) = toml::from_str::(src) { + // Fallback: try parsing as just the lints section + merged.merge_lints(&clippy_lints, "clippy.toml", sess); + } + } + } + } + + merged + } + + pub fn load_static() -> Self { + let mut merged = MergedLintConfig { + rust_lints: BTreeMap::new(), + clippy_lints: BTreeMap::new(), + }; + + // Load from Cargo.toml + if let Ok(src) = std::fs::read_to_string("Cargo.toml") { + if let Ok(cargo_toml) = toml::from_str::(&src) { + merged.merge_cargo_config_static(cargo_toml); + } + } + + // Load from clippy.toml + if let Ok((Some(clippy_config_path), _)) = crate::lookup_conf_file() { + if let Ok(src) = std::fs::read_to_string(&clippy_config_path) { + // Try parsing as a full CargoToml structure (with [lints] sections) + if let Ok(clippy_config) = toml::from_str::(&src) { + merged.merge_lints_static(&clippy_config.lints, "clippy.toml"); + merged.merge_lints_static(&clippy_config.workspace.lints, "clippy.toml [workspace]"); + } else if let Ok(clippy_lints) = toml::from_str::(&src) { + // Fallback: try parsing as just the lints section + merged.merge_lints_static(&clippy_lints, "clippy.toml"); + } + } + } + + merged + } + + /// Create a MergedLintConfig from TOML strings (for testing) + /// + /// cargo_toml should be a full Cargo.toml with [lints.clippy] and [lints.rust] sections + /// clippy_toml should be in the format expected by clippy.toml with [lints.clippy] and [lints.rust] sections + pub fn from_toml_strings(cargo_toml: Option<&str>, clippy_toml: Option<&str>) -> Self { + let mut merged = MergedLintConfig { + rust_lints: BTreeMap::new(), + clippy_lints: BTreeMap::new(), + }; + + // Parse Cargo.toml if provided + if let Some(cargo_src) = cargo_toml { + if let Ok(cargo_config) = toml::from_str::(cargo_src) { + merged.merge_cargo_config_static(cargo_config); + } + } + + // Parse clippy.toml if provided - it has the same structure as Cargo.toml [lints] sections + if let Some(clippy_src) = clippy_toml { + // Try parsing as a full CargoToml structure (with [lints] sections) + if let Ok(clippy_config) = toml::from_str::(clippy_src) { + merged.merge_lints_static(&clippy_config.lints, "clippy.toml"); + merged.merge_lints_static(&clippy_config.workspace.lints, "clippy.toml [workspace]"); + } else if let Ok(clippy_config) = toml::from_str::(clippy_src) { + // Fallback: try parsing as just the lints section + merged.merge_lints_static(&clippy_config, "clippy.toml"); + } + } + + merged + } + + fn merge_cargo_config(&mut self, cargo_toml: CargoToml, sess: &Session) { + self.merge_lints(&cargo_toml.lints, "Cargo.toml", sess); + self.merge_lints(&cargo_toml.workspace.lints, "Cargo.toml [workspace]", sess); + } + + fn merge_cargo_config_static(&mut self, cargo_toml: CargoToml) { + self.merge_lints_static(&cargo_toml.lints, "Cargo.toml"); + self.merge_lints_static(&cargo_toml.workspace.lints, "Cargo.toml [workspace]"); + } + + + fn merge_lints(&mut self, lints: &Lints, source: &str, sess: &Session) { + // Merge rust lints + for (name, config) in &lints.rust { + let name_str = name.get_ref().clone(); + let level = config.get_ref().level().to_string(); + let priority = config.get_ref().priority(); + + if let Some((existing_level, existing_priority, existing_source)) = self.rust_lints.get(&name_str) { + if existing_level != &level || existing_priority != &priority { + sess.dcx().warn(format!( + "Conflicting configuration for rust lint '{}': {}@{} in {} vs {}@{} in {}", + name_str, existing_level, existing_priority, + existing_source.as_deref().unwrap_or("unknown"), + level, priority, source + )); + } + // clippy.toml takes precedence over Cargo.toml + if source == "clippy.toml" { + self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } else { + self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } + + // Merge clippy lints + for (name, config) in &lints.clippy { + let name_str = name.get_ref().clone(); + let level = config.get_ref().level().to_string(); + let priority = config.get_ref().priority(); + + if let Some((existing_level, existing_priority, existing_source)) = self.clippy_lints.get(&name_str) { + if existing_level != &level || existing_priority != &priority { + sess.dcx().warn(format!( + "Conflicting configuration for clippy lint '{}': {}@{} in {} vs {}@{} in {}", + name_str, existing_level, existing_priority, + existing_source.as_deref().unwrap_or("unknown"), + level, priority, source + )); + } + // clippy.toml takes precedence over Cargo.toml + if source == "clippy.toml" { + self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } else { + self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } + } + + fn merge_lints_static(&mut self, lints: &Lints, source: &str) { + // Merge rust lints + for (name, config) in &lints.rust { + let name_str = name.get_ref().clone(); + let level = config.get_ref().level().to_string(); + let priority = config.get_ref().priority(); + + if let Some((existing_level, existing_priority, existing_source)) = self.rust_lints.get(&name_str) { + if existing_level != &level || existing_priority != &priority { + eprintln!( + "Warning: Conflicting configuration for rust lint '{}': {}@{} in {} vs {}@{} in {}", + name_str, existing_level, existing_priority, + existing_source.as_deref().unwrap_or("unknown"), + level, priority, source + ); + } + // clippy.toml takes precedence over Cargo.toml + if source == "clippy.toml" { + self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } else { + self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } + + // Merge clippy lints + for (name, config) in &lints.clippy { + let name_str = name.get_ref().clone(); + let level = config.get_ref().level().to_string(); + let priority = config.get_ref().priority(); + + if let Some((existing_level, existing_priority, existing_source)) = self.clippy_lints.get(&name_str) { + if existing_level != &level || existing_priority != &priority { + eprintln!( + "Warning: Conflicting configuration for clippy lint '{}': {}@{} in {} vs {}@{} in {}", + name_str, existing_level, existing_priority, + existing_source.as_deref().unwrap_or("unknown"), + level, priority, source + ); + } + // clippy.toml takes precedence over Cargo.toml + if source == "clippy.toml" { + self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } else { + self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_cargo_toml_only() { + let cargo_toml = r#" +[lints.clippy] +needless_return = "allow" +single_match = { level = "warn", priority = 5 } + +[lints.rust] +dead_code = "allow" +unused_variables = { level = "warn", priority = 10 } +"#; + + let config = MergedLintConfig::from_toml_strings(Some(cargo_toml), None); + + // Check clippy lints + assert_eq!(config.clippy_lints.len(), 2); + assert_eq!(config.clippy_lints["needless_return"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); + assert_eq!(config.clippy_lints["single_match"], ("warn".to_string(), 5, Some("Cargo.toml".to_string()))); + + // Check rust lints + assert_eq!(config.rust_lints.len(), 2); + assert_eq!(config.rust_lints["dead_code"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); + assert_eq!(config.rust_lints["unused_variables"], ("warn".to_string(), 10, Some("Cargo.toml".to_string()))); + } + + #[test] + fn test_clippy_toml_only() { + let clippy_toml = r#" +[lints.clippy] +needless_return = "deny" +too_many_arguments = { level = "forbid", priority = 15 } + +[lints.rust] +dead_code = { level = "warn", priority = 3 } +unused_imports = "allow" +"#; + + let config = MergedLintConfig::from_toml_strings(None, Some(clippy_toml)); + + // Check clippy lints + assert_eq!(config.clippy_lints.len(), 2); + assert_eq!(config.clippy_lints["needless_return"], ("deny".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!(config.clippy_lints["too_many_arguments"], ("forbid".to_string(), 15, Some("clippy.toml".to_string()))); + + // Check rust lints + assert_eq!(config.rust_lints.len(), 2); + assert_eq!(config.rust_lints["dead_code"], ("warn".to_string(), 3, Some("clippy.toml".to_string()))); + assert_eq!(config.rust_lints["unused_imports"], ("allow".to_string(), 0, Some("clippy.toml".to_string()))); + } + + #[test] + fn test_merged_configs_no_conflicts() { + let cargo_toml = r#" +[lints.clippy] +needless_return = "allow" +single_match = { level = "warn", priority = 5 } + +[lints.rust] +dead_code = "allow" +unused_variables = { level = "warn", priority = 10 } +"#; + + let clippy_toml = r#" +[lints.clippy] +too_many_arguments = { level = "forbid", priority = 15 } +wildcard_imports = "deny" + +[lints.rust] +unused_imports = "allow" +unreachable_code = { level = "warn", priority = 8 } +"#; + + let config = MergedLintConfig::from_toml_strings(Some(cargo_toml), Some(clippy_toml)); + + // Check clippy lints (should have lints from both files) + assert_eq!(config.clippy_lints.len(), 4); + assert_eq!(config.clippy_lints["needless_return"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); + assert_eq!(config.clippy_lints["single_match"], ("warn".to_string(), 5, Some("Cargo.toml".to_string()))); + assert_eq!(config.clippy_lints["too_many_arguments"], ("forbid".to_string(), 15, Some("clippy.toml".to_string()))); + assert_eq!(config.clippy_lints["wildcard_imports"], ("deny".to_string(), 0, Some("clippy.toml".to_string()))); + + // Check rust lints (should have lints from both files) + assert_eq!(config.rust_lints.len(), 4); + assert_eq!(config.rust_lints["dead_code"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); + assert_eq!(config.rust_lints["unused_variables"], ("warn".to_string(), 10, Some("Cargo.toml".to_string()))); + assert_eq!(config.rust_lints["unused_imports"], ("allow".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!(config.rust_lints["unreachable_code"], ("warn".to_string(), 8, Some("clippy.toml".to_string()))); + } + + #[test] + fn test_clippy_toml_precedence() { + let cargo_toml = r#" +[lints.clippy] +needless_return = "allow" +single_match = { level = "warn", priority = 5 } + +[lints.rust] +dead_code = "allow" +unused_variables = { level = "warn", priority = 10 } +"#; + + let clippy_toml = r#" +[lints.clippy] +needless_return = "deny" +single_match = { level = "forbid", priority = 15 } + +[lints.rust] +dead_code = { level = "warn", priority = 3 } +unused_variables = "forbid" +"#; + + let config = MergedLintConfig::from_toml_strings(Some(cargo_toml), Some(clippy_toml)); + + // Check that clippy.toml values take precedence + assert_eq!(config.clippy_lints.len(), 2); + assert_eq!(config.clippy_lints["needless_return"], ("deny".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!(config.clippy_lints["single_match"], ("forbid".to_string(), 15, Some("clippy.toml".to_string()))); + + assert_eq!(config.rust_lints.len(), 2); + assert_eq!(config.rust_lints["dead_code"], ("warn".to_string(), 3, Some("clippy.toml".to_string()))); + assert_eq!(config.rust_lints["unused_variables"], ("forbid".to_string(), 0, Some("clippy.toml".to_string()))); + } + + #[test] + fn test_workspace_lints() { + let cargo_toml = r#" +[lints.clippy] +needless_return = "allow" + +[lints.rust] +dead_code = "warn" + +[workspace.lints.clippy] +single_match = { level = "deny", priority = 20 } + +[workspace.lints.rust] +unused_variables = "forbid" +"#; + + let config = MergedLintConfig::from_toml_strings(Some(cargo_toml), None); + + // Check that both regular and workspace lints are included + assert_eq!(config.clippy_lints.len(), 2); + assert_eq!(config.clippy_lints["needless_return"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); + assert_eq!(config.clippy_lints["single_match"], ("deny".to_string(), 20, Some("Cargo.toml [workspace]".to_string()))); + + assert_eq!(config.rust_lints.len(), 2); + assert_eq!(config.rust_lints["dead_code"], ("warn".to_string(), 0, Some("Cargo.toml".to_string()))); + assert_eq!(config.rust_lints["unused_variables"], ("forbid".to_string(), 0, Some("Cargo.toml [workspace]".to_string()))); + } + + #[test] + fn test_priority_parsing() { + let cargo_toml = r#" +[lints.clippy] +needless_return = "allow" +single_match = { level = "warn", priority = 5 } +too_many_arguments = { level = "deny", priority = -10 } +wildcard_imports = { level = "forbid" } +"#; + + let config = MergedLintConfig::from_toml_strings(Some(cargo_toml), None); + + assert_eq!(config.clippy_lints.len(), 4); + assert_eq!(config.clippy_lints["needless_return"].1, 0); // Default priority + assert_eq!(config.clippy_lints["single_match"].1, 5); + assert_eq!(config.clippy_lints["too_many_arguments"].1, -10); // Negative priority + assert_eq!(config.clippy_lints["wildcard_imports"].1, 0); // Missing priority defaults to 0 + } + + #[test] + fn test_empty_configs() { + let config = MergedLintConfig::from_toml_strings(None, None); + assert_eq!(config.clippy_lints.len(), 0); + assert_eq!(config.rust_lints.len(), 0); + + let empty_cargo = r#" +[package] +name = "test" +version = "0.1.0" +"#; + + let config = MergedLintConfig::from_toml_strings(Some(empty_cargo), None); + assert_eq!(config.clippy_lints.len(), 0); + assert_eq!(config.rust_lints.len(), 0); + } + + #[test] + fn test_malformed_toml_ignored() { + let malformed_cargo = r#" +[lints.clippy +needless_return = "allow" +"#; + + let valid_clippy = r#" +[lints.clippy] +single_match = "warn" +"#; + + let config = MergedLintConfig::from_toml_strings(Some(malformed_cargo), Some(valid_clippy)); + + // Should only have the valid clippy.toml content + assert_eq!(config.clippy_lints.len(), 1); + assert_eq!(config.clippy_lints["single_match"], ("warn".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!(config.rust_lints.len(), 0); + } +} \ No newline at end of file diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index f64eefa0c232..dbbda509b943 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -698,3 +698,215 @@ pub enum PubUnderscoreFieldsBehaviour { PubliclyExported, AllPubFields, } + +#[derive(Deserialize, Serialize, Debug)] +pub struct LintConfigTable { + pub level: String, + pub priority: Option, +} + +#[derive(Deserialize, Debug)] +#[serde(untagged)] +pub enum LintConfig { + Level(String), + Table(LintConfigTable), +} + +impl LintConfig { + pub fn level(&self) -> &str { + match self { + LintConfig::Level(level) => level, + LintConfig::Table(table) => &table.level, + } + } + + pub fn priority(&self) -> i64 { + match self { + LintConfig::Level(_) => 0, + LintConfig::Table(table) => table.priority.unwrap_or(0), + } + } + + pub fn is_implicit(&self) -> bool { + if let LintConfig::Table(table) = self { + table.priority.is_none() + } else { + true + } + } +} + +pub type LintTable = std::collections::BTreeMap, toml::Spanned>; + +#[derive(Deserialize, Debug, Default)] +pub struct Lints { + #[serde(default)] + pub rust: LintTable, + #[serde(default)] + pub clippy: LintTable, +} + +#[derive(Deserialize, Debug, Default)] +pub struct CargoWorkspace { + #[serde(default)] + pub lints: Lints, +} + +#[derive(Deserialize, Debug)] +pub struct CargoToml { + #[serde(default)] + pub lints: Lints, + #[serde(default)] + pub workspace: CargoWorkspace, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_lint_config_level() { + let level_config = LintConfig::Level("warn".to_string()); + assert_eq!(level_config.level(), "warn"); + assert_eq!(level_config.priority(), 0); + assert!(level_config.is_implicit()); + + let table_config = LintConfig::Table(LintConfigTable { + level: "deny".to_string(), + priority: Some(5), + }); + assert_eq!(table_config.level(), "deny"); + assert_eq!(table_config.priority(), 5); + assert!(!table_config.is_implicit()); + + let table_config_no_priority = LintConfig::Table(LintConfigTable { + level: "forbid".to_string(), + priority: None, + }); + assert_eq!(table_config_no_priority.level(), "forbid"); + assert_eq!(table_config_no_priority.priority(), 0); + assert!(table_config_no_priority.is_implicit()); + } + + #[test] + fn test_lint_config_table_serialization() { + let table = LintConfigTable { + level: "warn".to_string(), + priority: Some(10), + }; + + // Test that it can be serialized to TOML + let toml_str = toml::to_string(&table).unwrap(); + assert!(toml_str.contains("level = \"warn\"")); + assert!(toml_str.contains("priority = 10")); + } + + #[test] + fn test_lint_config_deserialization() { + // Test simple level string within a lint table + let simple_toml = r#" +test_lint = "allow" +"#; + let table: std::collections::BTreeMap, toml::Spanned> = toml::from_str(simple_toml).unwrap(); + let config = table["test_lint"].get_ref(); + assert_eq!(config.level(), "allow"); + assert_eq!(config.priority(), 0); + + // Test table format + let table_toml = r#" +test_lint = { level = "deny", priority = 5 } +"#; + let table: std::collections::BTreeMap, toml::Spanned> = toml::from_str(table_toml).unwrap(); + let config = table["test_lint"].get_ref(); + assert_eq!(config.level(), "deny"); + assert_eq!(config.priority(), 5); + + // Test table format without priority + let table_no_priority_toml = r#" +test_lint = { level = "warn" } +"#; + let table: std::collections::BTreeMap, toml::Spanned> = toml::from_str(table_no_priority_toml).unwrap(); + let config = table["test_lint"].get_ref(); + assert_eq!(config.level(), "warn"); + assert_eq!(config.priority(), 0); + } + + #[test] + fn test_lints_deserialization() { + let lints_toml = r#" +[rust] +dead_code = "allow" +unused_variables = { level = "warn", priority = 5 } + +[clippy] +needless_return = "deny" +single_match = { level = "forbid", priority = 10 } +"#; + + let lints: Lints = toml::from_str(lints_toml).unwrap(); + + // Check rust lints + assert_eq!(lints.rust.len(), 2); + assert_eq!(lints.rust["dead_code"].get_ref().level(), "allow"); + assert_eq!(lints.rust["unused_variables"].get_ref().level(), "warn"); + assert_eq!(lints.rust["unused_variables"].get_ref().priority(), 5); + + // Check clippy lints + assert_eq!(lints.clippy.len(), 2); + assert_eq!(lints.clippy["needless_return"].get_ref().level(), "deny"); + assert_eq!(lints.clippy["single_match"].get_ref().level(), "forbid"); + assert_eq!(lints.clippy["single_match"].get_ref().priority(), 10); + } + + #[test] + fn test_cargo_toml_deserialization() { + let cargo_toml = r#" +[package] +name = "test" +version = "0.1.0" + +[lints.rust] +dead_code = "allow" + +[lints.clippy] +needless_return = { level = "warn", priority = 5 } + +[workspace.lints.rust] +unused_variables = "deny" + +[workspace.lints.clippy] +single_match = "forbid" +"#; + + let cargo: CargoToml = toml::from_str(cargo_toml).unwrap(); + + // Check regular lints + assert_eq!(cargo.lints.rust.len(), 1); + assert_eq!(cargo.lints.clippy.len(), 1); + assert_eq!(cargo.lints.rust["dead_code"].get_ref().level(), "allow"); + assert_eq!(cargo.lints.clippy["needless_return"].get_ref().level(), "warn"); + assert_eq!(cargo.lints.clippy["needless_return"].get_ref().priority(), 5); + + // Check workspace lints + assert_eq!(cargo.workspace.lints.rust.len(), 1); + assert_eq!(cargo.workspace.lints.clippy.len(), 1); + assert_eq!(cargo.workspace.lints.rust["unused_variables"].get_ref().level(), "deny"); + assert_eq!(cargo.workspace.lints.clippy["single_match"].get_ref().level(), "forbid"); + } + + #[test] + fn test_empty_lints() { + let empty_toml = ""; + let lints: Lints = toml::from_str(empty_toml).unwrap(); + assert_eq!(lints.rust.len(), 0); + assert_eq!(lints.clippy.len(), 0); + + let empty_sections_toml = r#" +[rust] +[clippy] +"#; + let lints: Lints = toml::from_str(empty_sections_toml).unwrap(); + assert_eq!(lints.rust.len(), 0); + assert_eq!(lints.clippy.len(), 0); + } +} diff --git a/clippy_lints/src/cargo/lint_groups_priority.rs b/clippy_lints/src/cargo/lint_groups_priority.rs index ffd6c520c9ae..76760980b423 100644 --- a/clippy_lints/src/cargo/lint_groups_priority.rs +++ b/clippy_lints/src/cargo/lint_groups_priority.rs @@ -1,75 +1,13 @@ use super::LINT_GROUPS_PRIORITY; +use clippy_config::types::{CargoToml, LintConfigTable, LintTable, Lints}; use clippy_utils::diagnostics::span_lint_and_then; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_lint::{LateContext, unerased_lint_store}; use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext}; -use serde::{Deserialize, Serialize}; -use std::collections::BTreeMap; +use serde::Serialize; use std::ops::Range; use std::path::Path; -use toml::Spanned; - -#[derive(Deserialize, Serialize, Debug)] -struct LintConfigTable { - level: String, - priority: Option, -} - -#[derive(Deserialize, Debug)] -#[serde(untagged)] -enum LintConfig { - Level(String), - Table(LintConfigTable), -} - -impl LintConfig { - fn level(&self) -> &str { - match self { - LintConfig::Level(level) => level, - LintConfig::Table(table) => &table.level, - } - } - - fn priority(&self) -> i64 { - match self { - LintConfig::Level(_) => 0, - LintConfig::Table(table) => table.priority.unwrap_or(0), - } - } - - fn is_implicit(&self) -> bool { - if let LintConfig::Table(table) = self { - table.priority.is_none() - } else { - true - } - } -} - -type LintTable = BTreeMap, Spanned>; - -#[derive(Deserialize, Debug, Default)] -struct Lints { - #[serde(default)] - rust: LintTable, - #[serde(default)] - clippy: LintTable, -} - -#[derive(Deserialize, Debug, Default)] -struct Workspace { - #[serde(default)] - lints: Lints, -} - -#[derive(Deserialize, Debug)] -struct CargoToml { - #[serde(default)] - lints: Lints, - #[serde(default)] - workspace: Workspace, -} fn toml_span(range: Range, file: &SourceFile) -> Span { Span::new( @@ -172,4 +110,37 @@ pub fn check(cx: &LateContext<'_>) { check_table(cx, cargo_toml.workspace.lints.rust, &rustc_groups, &file); check_table(cx, cargo_toml.workspace.lints.clippy, &clippy_groups, &file); } + + // Also check clippy.toml for lint configurations + if let Ok((Some(clippy_config_path), _)) = clippy_config::lookup_conf_file() { + if let Ok(clippy_file) = cx.tcx.sess.source_map().load_file(&clippy_config_path) + && let Some(clippy_src) = clippy_file.src.as_deref() + { + let mut rustc_groups = FxHashSet::default(); + let mut clippy_groups = FxHashSet::default(); + for (group, ..) in unerased_lint_store(cx.tcx.sess).get_lint_groups() { + match group.split_once("::") { + None => { + rustc_groups.insert(group); + }, + Some(("clippy", group)) => { + clippy_groups.insert(group); + }, + _ => {}, + } + } + + // Try parsing as a full CargoToml structure (with [lints] sections) + if let Ok(clippy_config) = toml::from_str::(clippy_src) { + check_table(cx, clippy_config.lints.rust, &rustc_groups, &clippy_file); + check_table(cx, clippy_config.lints.clippy, &clippy_groups, &clippy_file); + check_table(cx, clippy_config.workspace.lints.rust, &rustc_groups, &clippy_file); + check_table(cx, clippy_config.workspace.lints.clippy, &clippy_groups, &clippy_file); + } else if let Ok(clippy_lints) = toml::from_str::(clippy_src) { + // Fallback: try parsing as just the lints section + check_table(cx, clippy_lints.rust, &rustc_groups, &clippy_file); + check_table(cx, clippy_lints.clippy, &clippy_groups, &clippy_file); + } + } + } } diff --git a/src/driver.rs b/src/driver.rs index 202c74413cf0..6fc07482a13c 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -46,33 +46,138 @@ fn arg_value<'a>(args: &'a [String], find_arg: &str, pred: impl Fn(&str) -> bool None } +fn inject_lint_args_from_config(clippy_args: &mut Vec) { + // Load merged configuration from both Cargo.toml and clippy.toml + if let Ok(merged_config) = load_merged_lint_config() { + // Collect all lints with their priorities for sorting + let mut all_lints: Vec<(String, String, i64)> = Vec::new(); + + // Collect rust lints + for (lint_name, (level_str, priority, _source)) in &merged_config.rust_lints { + all_lints.push((format!("--{}={}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); + } + + // Collect clippy lints + for (lint_name, (level_str, priority, _source)) in &merged_config.clippy_lints { + all_lints.push((format!("--{}=clippy::{}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); + } + + // Sort by priority (higher priority first, then by lint name for stability) + all_lints.sort_by(|a, b| b.2.cmp(&a.2).then_with(|| a.0.cmp(&b.0))); + + // Add sorted arguments to clippy_args + for (arg, _level, _priority) in all_lints { + clippy_args.push(arg); + } + } +} + +fn load_merged_lint_config() -> Result> { + // Load the merged configuration + let merged_config = clippy_config::MergedLintConfig::load_static(); + Ok(merged_config) +} + +fn level_to_flag(level: &str) -> &'static str { + match level.to_lowercase().as_str() { + "allow" => "allow", + "warn" => "warn", + "deny" => "deny", + "forbid" => "forbid", + _ => "warn", // default to warn for unknown levels + } +} + fn has_arg(args: &[String], find_arg: &str) -> bool { args.iter().any(|arg| find_arg == arg.split('=').next().unwrap()) } -#[test] -fn test_arg_value() { - let args = &["--bar=bar", "--foobar", "123", "--foo"].map(String::from); - - assert_eq!(arg_value(&[], "--foobar", |_| true), None); - assert_eq!(arg_value(args, "--bar", |_| false), None); - assert_eq!(arg_value(args, "--bar", |_| true), Some("bar")); - assert_eq!(arg_value(args, "--bar", |p| p == "bar"), Some("bar")); - assert_eq!(arg_value(args, "--bar", |p| p == "foo"), None); - assert_eq!(arg_value(args, "--foobar", |p| p == "foo"), None); - assert_eq!(arg_value(args, "--foobar", |p| p == "123"), Some("123")); - assert_eq!(arg_value(args, "--foobar", |p| p.contains("12")), Some("123")); - assert_eq!(arg_value(args, "--foo", |_| true), None); -} +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_level_to_flag() { + assert_eq!(level_to_flag("allow"), "allow"); + assert_eq!(level_to_flag("ALLOW"), "allow"); + assert_eq!(level_to_flag("warn"), "warn"); + assert_eq!(level_to_flag("WARN"), "warn"); + assert_eq!(level_to_flag("deny"), "deny"); + assert_eq!(level_to_flag("forbid"), "forbid"); + assert_eq!(level_to_flag("unknown"), "warn"); // Default + } + + #[test] + fn test_inject_lint_args_priority_sorting() { + // Create a mock merged config for testing + let mut rust_lints = std::collections::BTreeMap::new(); + rust_lints.insert("dead_code".to_string(), ("allow".to_string(), 5, Some("clippy.toml".to_string()))); + rust_lints.insert("unused_variables".to_string(), ("warn".to_string(), 10, Some("Cargo.toml".to_string()))); + rust_lints.insert("unused_imports".to_string(), ("deny".to_string(), 1, Some("clippy.toml".to_string()))); + + let mut clippy_lints = std::collections::BTreeMap::new(); + clippy_lints.insert("needless_return".to_string(), ("allow".to_string(), 15, Some("clippy.toml".to_string()))); + clippy_lints.insert("single_match".to_string(), ("warn".to_string(), 5, Some("Cargo.toml".to_string()))); + clippy_lints.insert("too_many_arguments".to_string(), ("forbid".to_string(), 0, Some("clippy.toml".to_string()))); + + // Simulate the sorting behavior + let mut all_lints: Vec<(String, String, i64)> = Vec::new(); + + for (lint_name, (level_str, priority, _source)) in &rust_lints { + all_lints.push((format!("--{}={}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); + } + + for (lint_name, (level_str, priority, _source)) in &clippy_lints { + all_lints.push((format!("--{}=clippy::{}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); + } + + // Sort by priority (higher priority first, then by lint name for stability) + all_lints.sort_by(|a, b| b.2.cmp(&a.2).then_with(|| a.0.cmp(&b.0))); + + // Extract just the arguments + let args: Vec = all_lints.into_iter().map(|(arg, _, _)| arg).collect(); + + // Verify the expected order (highest priority first, then alphabetical within same priority) + let expected = vec![ + "--allow=clippy::needless_return", // priority 15 + "--warn=unused_variables", // priority 10 + "--allow=dead_code", // priority 5 (first alphabetically among priority 5) + "--warn=clippy::single_match", // priority 5 (second alphabetically among priority 5) + "--deny=unused_imports", // priority 1 + "--forbid=clippy::too_many_arguments", // priority 0 + ]; + + assert_eq!(args, expected); + } -#[test] -fn test_has_arg() { - let args = &["--foo=bar", "-vV", "--baz"].map(String::from); - assert!(has_arg(args, "--foo")); - assert!(has_arg(args, "--baz")); - assert!(has_arg(args, "-vV")); + #[test] + fn test_arg_value() { + let args = vec![ + "--cap-lints=allow".to_string(), + "--force-warn".to_string(), + "clippy::needless_return".to_string(), + "--other-flag=value".to_string(), + ]; + + assert_eq!(arg_value(&args, "--cap-lints", |val| val == "allow"), Some("allow")); + assert_eq!(arg_value(&args, "--cap-lints", |val| val == "warn"), None); + assert_eq!(arg_value(&args, "--force-warn", |val| val.contains("clippy::")), Some("clippy::needless_return")); + assert_eq!(arg_value(&args, "--nonexistent", |_| true), None); + } - assert!(!has_arg(args, "--bar")); + #[test] + fn test_has_arg() { + let args = vec![ + "--cap-lints=allow".to_string(), + "--version".to_string(), + "--help".to_string(), + ]; + + assert!(has_arg(&args, "--cap-lints")); + assert!(has_arg(&args, "--version")); + assert!(has_arg(&args, "--help")); + assert!(!has_arg(&args, "--nonexistent")); + } } fn track_clippy_args(psess: &mut ParseSess, args_env_var: Option<&str>) { @@ -271,7 +376,7 @@ pub fn main() { let mut no_deps = false; let clippy_args_var = env::var("CLIPPY_ARGS").ok(); - let clippy_args = clippy_args_var + let mut clippy_args = clippy_args_var .as_deref() .unwrap_or_default() .split("__CLIPPY_HACKERY__") @@ -286,6 +391,9 @@ pub fn main() { .chain(vec!["--cfg".into(), "clippy".into()]) .collect::>(); + // Load lint configurations from both Cargo.toml and clippy.toml and add them as arguments + inject_lint_args_from_config(&mut clippy_args); + // If no Clippy lints will be run we do not need to run Clippy let cap_lints_allow = arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_some() && arg_value(&orig_args, "--force-warn", |val| val.contains("clippy::")).is_none(); From df97a9a0db05971ca1d6a4be65c8d899efdc359d Mon Sep 17 00:00:00 2001 From: Chengxu Bian Date: Thu, 26 Jun 2025 21:40:18 -0400 Subject: [PATCH 2/2] fix fmt and clippy issues --- clippy_config/src/lib.rs | 4 +- clippy_config/src/lint_config.rs | 309 ++++++++++++------ clippy_config/src/types.rs | 52 +-- .../src/cargo/lint_groups_priority.rs | 53 ++- src/driver.rs | 276 +++++++++------- 5 files changed, 421 insertions(+), 273 deletions(-) diff --git a/clippy_config/src/lib.rs b/clippy_config/src/lib.rs index 82149eb23c6c..0b64c4b40f49 100644 --- a/clippy_config/src/lib.rs +++ b/clippy_config/src/lib.rs @@ -22,10 +22,10 @@ extern crate rustc_session; extern crate rustc_span; mod conf; +pub mod lint_config; mod metadata; pub mod types; -pub mod lint_config; pub use conf::{Conf, get_configuration_metadata, lookup_conf_file, sanitize_explanation}; -pub use metadata::ClippyConfiguration; pub use lint_config::MergedLintConfig; +pub use metadata::ClippyConfiguration; diff --git a/clippy_config/src/lint_config.rs b/clippy_config/src/lint_config.rs index 1ea30376dd8c..87050159cc2c 100644 --- a/clippy_config/src/lint_config.rs +++ b/clippy_config/src/lint_config.rs @@ -17,27 +17,25 @@ impl MergedLintConfig { }; // Load from Cargo.toml - if let Ok(file) = sess.source_map().load_file(Path::new("Cargo.toml")) { - if let Some(src) = file.src.as_deref() { - if let Ok(cargo_toml) = toml::from_str::(src) { - merged.merge_cargo_config(cargo_toml, sess); - } - } + if let Ok(file) = sess.source_map().load_file(Path::new("Cargo.toml")) + && let Some(src) = file.src.as_deref() + && let Ok(cargo_toml) = toml::from_str::(src) + { + merged.merge_cargo_config(&cargo_toml, sess); } // Load from clippy.toml - if let Ok((Some(clippy_config_path), _)) = crate::lookup_conf_file() { - if let Ok(file) = sess.source_map().load_file(&clippy_config_path) { - if let Some(src) = file.src.as_deref() { - // Try parsing as a full CargoToml structure (with [lints] sections) - if let Ok(clippy_config) = toml::from_str::(src) { - merged.merge_lints(&clippy_config.lints, "clippy.toml", sess); - merged.merge_lints(&clippy_config.workspace.lints, "clippy.toml [workspace]", sess); - } else if let Ok(clippy_lints) = toml::from_str::(src) { - // Fallback: try parsing as just the lints section - merged.merge_lints(&clippy_lints, "clippy.toml", sess); - } - } + if let Ok((Some(clippy_config_path), _)) = crate::lookup_conf_file() + && let Ok(file) = sess.source_map().load_file(&clippy_config_path) + && let Some(src) = file.src.as_deref() + { + // Try parsing as a full CargoToml structure (with [lints] sections) + if let Ok(clippy_config) = toml::from_str::(src) { + merged.merge_lints(&clippy_config.lints, "clippy.toml", sess); + merged.merge_lints(&clippy_config.workspace.lints, "clippy.toml [workspace]", sess); + } else if let Ok(clippy_lints) = toml::from_str::(src) { + // Fallback: try parsing as just the lints section + merged.merge_lints(&clippy_lints, "clippy.toml", sess); } } @@ -51,33 +49,33 @@ impl MergedLintConfig { }; // Load from Cargo.toml - if let Ok(src) = std::fs::read_to_string("Cargo.toml") { - if let Ok(cargo_toml) = toml::from_str::(&src) { - merged.merge_cargo_config_static(cargo_toml); - } + if let Ok(src) = std::fs::read_to_string("Cargo.toml") + && let Ok(cargo_toml) = toml::from_str::(&src) + { + merged.merge_cargo_config_static(&cargo_toml); } // Load from clippy.toml - if let Ok((Some(clippy_config_path), _)) = crate::lookup_conf_file() { - if let Ok(src) = std::fs::read_to_string(&clippy_config_path) { - // Try parsing as a full CargoToml structure (with [lints] sections) - if let Ok(clippy_config) = toml::from_str::(&src) { - merged.merge_lints_static(&clippy_config.lints, "clippy.toml"); - merged.merge_lints_static(&clippy_config.workspace.lints, "clippy.toml [workspace]"); - } else if let Ok(clippy_lints) = toml::from_str::(&src) { - // Fallback: try parsing as just the lints section - merged.merge_lints_static(&clippy_lints, "clippy.toml"); - } + if let Ok((Some(clippy_config_path), _)) = crate::lookup_conf_file() + && let Ok(src) = std::fs::read_to_string(&clippy_config_path) + { + // Try parsing as a full CargoToml structure (with [lints] sections) + if let Ok(clippy_config) = toml::from_str::(&src) { + merged.merge_lints_static(&clippy_config.lints, "clippy.toml"); + merged.merge_lints_static(&clippy_config.workspace.lints, "clippy.toml [workspace]"); + } else if let Ok(clippy_lints) = toml::from_str::(&src) { + // Fallback: try parsing as just the lints section + merged.merge_lints_static(&clippy_lints, "clippy.toml"); } } - merged } - /// Create a MergedLintConfig from TOML strings (for testing) - /// - /// cargo_toml should be a full Cargo.toml with [lints.clippy] and [lints.rust] sections - /// clippy_toml should be in the format expected by clippy.toml with [lints.clippy] and [lints.rust] sections + /// Create a `MergedLintConfig` from TOML strings (for testing) + /// + /// `cargo_toml` should be a full Cargo.toml with [lints.clippy] and [lints.rust] sections + /// `clippy_toml` should be in the format expected by clippy.toml with [lints.clippy] and + /// [lints.rust] sections pub fn from_toml_strings(cargo_toml: Option<&str>, clippy_toml: Option<&str>) -> Self { let mut merged = MergedLintConfig { rust_lints: BTreeMap::new(), @@ -85,10 +83,10 @@ impl MergedLintConfig { }; // Parse Cargo.toml if provided - if let Some(cargo_src) = cargo_toml { - if let Ok(cargo_config) = toml::from_str::(cargo_src) { - merged.merge_cargo_config_static(cargo_config); - } + if let Some(cargo_src) = cargo_toml + && let Ok(cargo_config) = toml::from_str::(cargo_src) + { + merged.merge_cargo_config_static(&cargo_config); } // Parse clippy.toml if provided - it has the same structure as Cargo.toml [lints] sections @@ -106,63 +104,82 @@ impl MergedLintConfig { merged } - fn merge_cargo_config(&mut self, cargo_toml: CargoToml, sess: &Session) { + fn merge_cargo_config(&mut self, cargo_toml: &CargoToml, sess: &Session) { self.merge_lints(&cargo_toml.lints, "Cargo.toml", sess); self.merge_lints(&cargo_toml.workspace.lints, "Cargo.toml [workspace]", sess); } - fn merge_cargo_config_static(&mut self, cargo_toml: CargoToml) { + fn merge_cargo_config_static(&mut self, cargo_toml: &CargoToml) { self.merge_lints_static(&cargo_toml.lints, "Cargo.toml"); self.merge_lints_static(&cargo_toml.workspace.lints, "Cargo.toml [workspace]"); } - fn merge_lints(&mut self, lints: &Lints, source: &str, sess: &Session) { // Merge rust lints for (name, config) in &lints.rust { let name_str = name.get_ref().clone(); let level = config.get_ref().level().to_string(); let priority = config.get_ref().priority(); - + if let Some((existing_level, existing_priority, existing_source)) = self.rust_lints.get(&name_str) { - if existing_level != &level || existing_priority != &priority { + // Only warn for conflicts between different file types (Cargo.toml vs clippy.toml) + let existing_is_cargo = existing_source.as_deref().unwrap_or("").contains("Cargo.toml"); + let current_is_cargo = source.contains("Cargo.toml"); + if existing_is_cargo != current_is_cargo && (existing_level != &level || existing_priority != &priority) + { sess.dcx().warn(format!( "Conflicting configuration for rust lint '{}': {}@{} in {} vs {}@{} in {}", - name_str, existing_level, existing_priority, - existing_source.as_deref().unwrap_or("unknown"), - level, priority, source + name_str, + existing_level, + existing_priority, + existing_source.as_deref().unwrap_or("unknown"), + level, + priority, + source )); } // clippy.toml takes precedence over Cargo.toml if source == "clippy.toml" { - self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.rust_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } else { - self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.rust_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } - + // Merge clippy lints for (name, config) in &lints.clippy { let name_str = name.get_ref().clone(); let level = config.get_ref().level().to_string(); let priority = config.get_ref().priority(); - + if let Some((existing_level, existing_priority, existing_source)) = self.clippy_lints.get(&name_str) { - if existing_level != &level || existing_priority != &priority { + // Only warn for conflicts between different file types (Cargo.toml vs clippy.toml) + let existing_is_cargo = existing_source.as_deref().unwrap_or("").contains("Cargo.toml"); + let current_is_cargo = source.contains("Cargo.toml"); + if existing_is_cargo != current_is_cargo && (existing_level != &level || existing_priority != &priority) + { sess.dcx().warn(format!( "Conflicting configuration for clippy lint '{}': {}@{} in {} vs {}@{} in {}", - name_str, existing_level, existing_priority, + name_str, + existing_level, + existing_priority, existing_source.as_deref().unwrap_or("unknown"), - level, priority, source + level, + priority, + source )); } // clippy.toml takes precedence over Cargo.toml if source == "clippy.toml" { - self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.clippy_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } else { - self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.clippy_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } } @@ -173,51 +190,70 @@ impl MergedLintConfig { let name_str = name.get_ref().clone(); let level = config.get_ref().level().to_string(); let priority = config.get_ref().priority(); - + if let Some((existing_level, existing_priority, existing_source)) = self.rust_lints.get(&name_str) { - if existing_level != &level || existing_priority != &priority { + // Only warn for conflicts between different file types (Cargo.toml vs clippy.toml) + let existing_is_cargo = existing_source.as_deref().unwrap_or("").contains("Cargo.toml"); + let current_is_cargo = source.contains("Cargo.toml"); + if existing_is_cargo != current_is_cargo && (existing_level != &level || existing_priority != &priority) + { eprintln!( "Warning: Conflicting configuration for rust lint '{}': {}@{} in {} vs {}@{} in {}", - name_str, existing_level, existing_priority, + name_str, + existing_level, + existing_priority, existing_source.as_deref().unwrap_or("unknown"), - level, priority, source + level, + priority, + source ); } // clippy.toml takes precedence over Cargo.toml if source == "clippy.toml" { - self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.rust_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } else { - self.rust_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.rust_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } - + // Merge clippy lints for (name, config) in &lints.clippy { let name_str = name.get_ref().clone(); let level = config.get_ref().level().to_string(); let priority = config.get_ref().priority(); - + if let Some((existing_level, existing_priority, existing_source)) = self.clippy_lints.get(&name_str) { - if existing_level != &level || existing_priority != &priority { + // Only warn for conflicts between different file types (Cargo.toml vs clippy.toml) + let existing_is_cargo = existing_source.as_deref().unwrap_or("").contains("Cargo.toml"); + let current_is_cargo = source.contains("Cargo.toml"); + if existing_is_cargo != current_is_cargo && (existing_level != &level || existing_priority != &priority) + { eprintln!( "Warning: Conflicting configuration for clippy lint '{}': {}@{} in {} vs {}@{} in {}", - name_str, existing_level, existing_priority, + name_str, + existing_level, + existing_priority, existing_source.as_deref().unwrap_or("unknown"), - level, priority, source + level, + priority, + source ); } // clippy.toml takes precedence over Cargo.toml if source == "clippy.toml" { - self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.clippy_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } else { - self.clippy_lints.insert(name_str, (level, priority, Some(source.to_string()))); + self.clippy_lints + .insert(name_str, (level, priority, Some(source.to_string()))); } } } } - #[cfg(test)] mod tests { use super::*; @@ -238,13 +274,25 @@ unused_variables = { level = "warn", priority = 10 } // Check clippy lints assert_eq!(config.clippy_lints.len(), 2); - assert_eq!(config.clippy_lints["needless_return"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); - assert_eq!(config.clippy_lints["single_match"], ("warn".to_string(), 5, Some("Cargo.toml".to_string()))); + assert_eq!( + config.clippy_lints["needless_return"], + ("allow".to_string(), 0, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.clippy_lints["single_match"], + ("warn".to_string(), 5, Some("Cargo.toml".to_string())) + ); // Check rust lints assert_eq!(config.rust_lints.len(), 2); - assert_eq!(config.rust_lints["dead_code"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); - assert_eq!(config.rust_lints["unused_variables"], ("warn".to_string(), 10, Some("Cargo.toml".to_string()))); + assert_eq!( + config.rust_lints["dead_code"], + ("allow".to_string(), 0, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.rust_lints["unused_variables"], + ("warn".to_string(), 10, Some("Cargo.toml".to_string())) + ); } #[test] @@ -263,13 +311,25 @@ unused_imports = "allow" // Check clippy lints assert_eq!(config.clippy_lints.len(), 2); - assert_eq!(config.clippy_lints["needless_return"], ("deny".to_string(), 0, Some("clippy.toml".to_string()))); - assert_eq!(config.clippy_lints["too_many_arguments"], ("forbid".to_string(), 15, Some("clippy.toml".to_string()))); + assert_eq!( + config.clippy_lints["needless_return"], + ("deny".to_string(), 0, Some("clippy.toml".to_string())) + ); + assert_eq!( + config.clippy_lints["too_many_arguments"], + ("forbid".to_string(), 15, Some("clippy.toml".to_string())) + ); // Check rust lints assert_eq!(config.rust_lints.len(), 2); - assert_eq!(config.rust_lints["dead_code"], ("warn".to_string(), 3, Some("clippy.toml".to_string()))); - assert_eq!(config.rust_lints["unused_imports"], ("allow".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!( + config.rust_lints["dead_code"], + ("warn".to_string(), 3, Some("clippy.toml".to_string())) + ); + assert_eq!( + config.rust_lints["unused_imports"], + ("allow".to_string(), 0, Some("clippy.toml".to_string())) + ); } #[test] @@ -298,17 +358,41 @@ unreachable_code = { level = "warn", priority = 8 } // Check clippy lints (should have lints from both files) assert_eq!(config.clippy_lints.len(), 4); - assert_eq!(config.clippy_lints["needless_return"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); - assert_eq!(config.clippy_lints["single_match"], ("warn".to_string(), 5, Some("Cargo.toml".to_string()))); - assert_eq!(config.clippy_lints["too_many_arguments"], ("forbid".to_string(), 15, Some("clippy.toml".to_string()))); - assert_eq!(config.clippy_lints["wildcard_imports"], ("deny".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!( + config.clippy_lints["needless_return"], + ("allow".to_string(), 0, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.clippy_lints["single_match"], + ("warn".to_string(), 5, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.clippy_lints["too_many_arguments"], + ("forbid".to_string(), 15, Some("clippy.toml".to_string())) + ); + assert_eq!( + config.clippy_lints["wildcard_imports"], + ("deny".to_string(), 0, Some("clippy.toml".to_string())) + ); // Check rust lints (should have lints from both files) assert_eq!(config.rust_lints.len(), 4); - assert_eq!(config.rust_lints["dead_code"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); - assert_eq!(config.rust_lints["unused_variables"], ("warn".to_string(), 10, Some("Cargo.toml".to_string()))); - assert_eq!(config.rust_lints["unused_imports"], ("allow".to_string(), 0, Some("clippy.toml".to_string()))); - assert_eq!(config.rust_lints["unreachable_code"], ("warn".to_string(), 8, Some("clippy.toml".to_string()))); + assert_eq!( + config.rust_lints["dead_code"], + ("allow".to_string(), 0, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.rust_lints["unused_variables"], + ("warn".to_string(), 10, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.rust_lints["unused_imports"], + ("allow".to_string(), 0, Some("clippy.toml".to_string())) + ); + assert_eq!( + config.rust_lints["unreachable_code"], + ("warn".to_string(), 8, Some("clippy.toml".to_string())) + ); } #[test] @@ -337,12 +421,24 @@ unused_variables = "forbid" // Check that clippy.toml values take precedence assert_eq!(config.clippy_lints.len(), 2); - assert_eq!(config.clippy_lints["needless_return"], ("deny".to_string(), 0, Some("clippy.toml".to_string()))); - assert_eq!(config.clippy_lints["single_match"], ("forbid".to_string(), 15, Some("clippy.toml".to_string()))); + assert_eq!( + config.clippy_lints["needless_return"], + ("deny".to_string(), 0, Some("clippy.toml".to_string())) + ); + assert_eq!( + config.clippy_lints["single_match"], + ("forbid".to_string(), 15, Some("clippy.toml".to_string())) + ); assert_eq!(config.rust_lints.len(), 2); - assert_eq!(config.rust_lints["dead_code"], ("warn".to_string(), 3, Some("clippy.toml".to_string()))); - assert_eq!(config.rust_lints["unused_variables"], ("forbid".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!( + config.rust_lints["dead_code"], + ("warn".to_string(), 3, Some("clippy.toml".to_string())) + ); + assert_eq!( + config.rust_lints["unused_variables"], + ("forbid".to_string(), 0, Some("clippy.toml".to_string())) + ); } #[test] @@ -365,12 +461,24 @@ unused_variables = "forbid" // Check that both regular and workspace lints are included assert_eq!(config.clippy_lints.len(), 2); - assert_eq!(config.clippy_lints["needless_return"], ("allow".to_string(), 0, Some("Cargo.toml".to_string()))); - assert_eq!(config.clippy_lints["single_match"], ("deny".to_string(), 20, Some("Cargo.toml [workspace]".to_string()))); + assert_eq!( + config.clippy_lints["needless_return"], + ("allow".to_string(), 0, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.clippy_lints["single_match"], + ("deny".to_string(), 20, Some("Cargo.toml [workspace]".to_string())) + ); assert_eq!(config.rust_lints.len(), 2); - assert_eq!(config.rust_lints["dead_code"], ("warn".to_string(), 0, Some("Cargo.toml".to_string()))); - assert_eq!(config.rust_lints["unused_variables"], ("forbid".to_string(), 0, Some("Cargo.toml [workspace]".to_string()))); + assert_eq!( + config.rust_lints["dead_code"], + ("warn".to_string(), 0, Some("Cargo.toml".to_string())) + ); + assert_eq!( + config.rust_lints["unused_variables"], + ("forbid".to_string(), 0, Some("Cargo.toml [workspace]".to_string())) + ); } #[test] @@ -422,10 +530,13 @@ single_match = "warn" "#; let config = MergedLintConfig::from_toml_strings(Some(malformed_cargo), Some(valid_clippy)); - + // Should only have the valid clippy.toml content assert_eq!(config.clippy_lints.len(), 1); - assert_eq!(config.clippy_lints["single_match"], ("warn".to_string(), 0, Some("clippy.toml".to_string()))); + assert_eq!( + config.clippy_lints["single_match"], + ("warn".to_string(), 0, Some("clippy.toml".to_string())) + ); assert_eq!(config.rust_lints.len(), 0); } -} \ No newline at end of file +} diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index dbbda509b943..a07254a8a0ac 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -763,14 +763,14 @@ pub struct CargoToml { #[cfg(test)] mod tests { use super::*; - + #[test] fn test_lint_config_level() { let level_config = LintConfig::Level("warn".to_string()); assert_eq!(level_config.level(), "warn"); assert_eq!(level_config.priority(), 0); assert!(level_config.is_implicit()); - + let table_config = LintConfig::Table(LintConfigTable { level: "deny".to_string(), priority: Some(5), @@ -778,7 +778,7 @@ mod tests { assert_eq!(table_config.level(), "deny"); assert_eq!(table_config.priority(), 5); assert!(!table_config.is_implicit()); - + let table_config_no_priority = LintConfig::Table(LintConfigTable { level: "forbid".to_string(), priority: None, @@ -787,50 +787,53 @@ mod tests { assert_eq!(table_config_no_priority.priority(), 0); assert!(table_config_no_priority.is_implicit()); } - + #[test] fn test_lint_config_table_serialization() { let table = LintConfigTable { level: "warn".to_string(), priority: Some(10), }; - + // Test that it can be serialized to TOML let toml_str = toml::to_string(&table).unwrap(); assert!(toml_str.contains("level = \"warn\"")); assert!(toml_str.contains("priority = 10")); } - + #[test] fn test_lint_config_deserialization() { // Test simple level string within a lint table let simple_toml = r#" test_lint = "allow" "#; - let table: std::collections::BTreeMap, toml::Spanned> = toml::from_str(simple_toml).unwrap(); + let table: std::collections::BTreeMap, toml::Spanned> = + toml::from_str(simple_toml).unwrap(); let config = table["test_lint"].get_ref(); assert_eq!(config.level(), "allow"); assert_eq!(config.priority(), 0); - + // Test table format let table_toml = r#" test_lint = { level = "deny", priority = 5 } "#; - let table: std::collections::BTreeMap, toml::Spanned> = toml::from_str(table_toml).unwrap(); + let table: std::collections::BTreeMap, toml::Spanned> = + toml::from_str(table_toml).unwrap(); let config = table["test_lint"].get_ref(); assert_eq!(config.level(), "deny"); assert_eq!(config.priority(), 5); - + // Test table format without priority let table_no_priority_toml = r#" test_lint = { level = "warn" } "#; - let table: std::collections::BTreeMap, toml::Spanned> = toml::from_str(table_no_priority_toml).unwrap(); + let table: std::collections::BTreeMap, toml::Spanned> = + toml::from_str(table_no_priority_toml).unwrap(); let config = table["test_lint"].get_ref(); assert_eq!(config.level(), "warn"); assert_eq!(config.priority(), 0); } - + #[test] fn test_lints_deserialization() { let lints_toml = r#" @@ -842,22 +845,22 @@ unused_variables = { level = "warn", priority = 5 } needless_return = "deny" single_match = { level = "forbid", priority = 10 } "#; - + let lints: Lints = toml::from_str(lints_toml).unwrap(); - + // Check rust lints assert_eq!(lints.rust.len(), 2); assert_eq!(lints.rust["dead_code"].get_ref().level(), "allow"); assert_eq!(lints.rust["unused_variables"].get_ref().level(), "warn"); assert_eq!(lints.rust["unused_variables"].get_ref().priority(), 5); - - // Check clippy lints + + // Check clippy lints assert_eq!(lints.clippy.len(), 2); assert_eq!(lints.clippy["needless_return"].get_ref().level(), "deny"); assert_eq!(lints.clippy["single_match"].get_ref().level(), "forbid"); assert_eq!(lints.clippy["single_match"].get_ref().priority(), 10); } - + #[test] fn test_cargo_toml_deserialization() { let cargo_toml = r#" @@ -877,34 +880,31 @@ unused_variables = "deny" [workspace.lints.clippy] single_match = "forbid" "#; - + let cargo: CargoToml = toml::from_str(cargo_toml).unwrap(); - + // Check regular lints assert_eq!(cargo.lints.rust.len(), 1); assert_eq!(cargo.lints.clippy.len(), 1); assert_eq!(cargo.lints.rust["dead_code"].get_ref().level(), "allow"); assert_eq!(cargo.lints.clippy["needless_return"].get_ref().level(), "warn"); assert_eq!(cargo.lints.clippy["needless_return"].get_ref().priority(), 5); - + // Check workspace lints assert_eq!(cargo.workspace.lints.rust.len(), 1); assert_eq!(cargo.workspace.lints.clippy.len(), 1); assert_eq!(cargo.workspace.lints.rust["unused_variables"].get_ref().level(), "deny"); assert_eq!(cargo.workspace.lints.clippy["single_match"].get_ref().level(), "forbid"); } - + #[test] fn test_empty_lints() { let empty_toml = ""; let lints: Lints = toml::from_str(empty_toml).unwrap(); assert_eq!(lints.rust.len(), 0); assert_eq!(lints.clippy.len(), 0); - - let empty_sections_toml = r#" -[rust] -[clippy] -"#; + + let empty_sections_toml = "[rust]\n[clippy]"; let lints: Lints = toml::from_str(empty_sections_toml).unwrap(); assert_eq!(lints.rust.len(), 0); assert_eq!(lints.clippy.len(), 0); diff --git a/clippy_lints/src/cargo/lint_groups_priority.rs b/clippy_lints/src/cargo/lint_groups_priority.rs index 76760980b423..91ec6088159b 100644 --- a/clippy_lints/src/cargo/lint_groups_priority.rs +++ b/clippy_lints/src/cargo/lint_groups_priority.rs @@ -112,35 +112,34 @@ pub fn check(cx: &LateContext<'_>) { } // Also check clippy.toml for lint configurations - if let Ok((Some(clippy_config_path), _)) = clippy_config::lookup_conf_file() { - if let Ok(clippy_file) = cx.tcx.sess.source_map().load_file(&clippy_config_path) - && let Some(clippy_src) = clippy_file.src.as_deref() - { - let mut rustc_groups = FxHashSet::default(); - let mut clippy_groups = FxHashSet::default(); - for (group, ..) in unerased_lint_store(cx.tcx.sess).get_lint_groups() { - match group.split_once("::") { - None => { - rustc_groups.insert(group); - }, - Some(("clippy", group)) => { - clippy_groups.insert(group); - }, - _ => {}, - } + if let Ok((Some(clippy_config_path), _)) = clippy_config::lookup_conf_file() + && let Ok(clippy_file) = cx.tcx.sess.source_map().load_file(&clippy_config_path) + && let Some(clippy_src) = clippy_file.src.as_deref() + { + let mut rustc_groups = FxHashSet::default(); + let mut clippy_groups = FxHashSet::default(); + for (group, ..) in unerased_lint_store(cx.tcx.sess).get_lint_groups() { + match group.split_once("::") { + None => { + rustc_groups.insert(group); + }, + Some(("clippy", group)) => { + clippy_groups.insert(group); + }, + _ => {}, } + } - // Try parsing as a full CargoToml structure (with [lints] sections) - if let Ok(clippy_config) = toml::from_str::(clippy_src) { - check_table(cx, clippy_config.lints.rust, &rustc_groups, &clippy_file); - check_table(cx, clippy_config.lints.clippy, &clippy_groups, &clippy_file); - check_table(cx, clippy_config.workspace.lints.rust, &rustc_groups, &clippy_file); - check_table(cx, clippy_config.workspace.lints.clippy, &clippy_groups, &clippy_file); - } else if let Ok(clippy_lints) = toml::from_str::(clippy_src) { - // Fallback: try parsing as just the lints section - check_table(cx, clippy_lints.rust, &rustc_groups, &clippy_file); - check_table(cx, clippy_lints.clippy, &clippy_groups, &clippy_file); - } + // Try parsing as a full CargoToml structure (with [lints] sections) + if let Ok(clippy_config) = toml::from_str::(clippy_src) { + check_table(cx, clippy_config.lints.rust, &rustc_groups, &clippy_file); + check_table(cx, clippy_config.lints.clippy, &clippy_groups, &clippy_file); + check_table(cx, clippy_config.workspace.lints.rust, &rustc_groups, &clippy_file); + check_table(cx, clippy_config.workspace.lints.clippy, &clippy_groups, &clippy_file); + } else if let Ok(clippy_lints) = toml::from_str::(clippy_src) { + // Fallback: try parsing as just the lints section + check_table(cx, clippy_lints.rust, &rustc_groups, &clippy_file); + check_table(cx, clippy_lints.clippy, &clippy_groups, &clippy_file); } } } diff --git a/src/driver.rs b/src/driver.rs index 6fc07482a13c..8b94aa27df1d 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -48,43 +48,52 @@ fn arg_value<'a>(args: &'a [String], find_arg: &str, pred: impl Fn(&str) -> bool fn inject_lint_args_from_config(clippy_args: &mut Vec) { // Load merged configuration from both Cargo.toml and clippy.toml - if let Ok(merged_config) = load_merged_lint_config() { - // Collect all lints with their priorities for sorting - let mut all_lints: Vec<(String, String, i64)> = Vec::new(); - - // Collect rust lints - for (lint_name, (level_str, priority, _source)) in &merged_config.rust_lints { - all_lints.push((format!("--{}={}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); - } - - // Collect clippy lints - for (lint_name, (level_str, priority, _source)) in &merged_config.clippy_lints { - all_lints.push((format!("--{}=clippy::{}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); - } - - // Sort by priority (higher priority first, then by lint name for stability) - all_lints.sort_by(|a, b| b.2.cmp(&a.2).then_with(|| a.0.cmp(&b.0))); - - // Add sorted arguments to clippy_args - for (arg, _level, _priority) in all_lints { - clippy_args.push(arg); - } + let merged_config = load_merged_lint_config(); + // Collect all lints with their priorities for sorting + let mut all_lints: Vec<(String, String, i64)> = Vec::new(); + + // Collect rust lints + for (lint_name, (level_str, priority, _source)) in &merged_config.rust_lints { + all_lints.push(( + format!("--{}={}", level_to_flag(level_str), lint_name), + level_str.to_string(), + *priority, + )); + } + + // Collect clippy lints + for (lint_name, (level_str, priority, _source)) in &merged_config.clippy_lints { + all_lints.push(( + format!("--{}=clippy::{}", level_to_flag(level_str), lint_name), + level_str.to_string(), + *priority, + )); + } + + // Sort by priority (higher priority first, then by lint name for stability) + all_lints.sort_by(|a, b| b.2.cmp(&a.2).then_with(|| a.0.cmp(&b.0))); + + // Add sorted arguments to clippy_args + for (arg, _level, _priority) in all_lints { + clippy_args.push(arg); } } -fn load_merged_lint_config() -> Result> { - // Load the merged configuration - let merged_config = clippy_config::MergedLintConfig::load_static(); - Ok(merged_config) +fn load_merged_lint_config() -> clippy_config::MergedLintConfig { + clippy_config::MergedLintConfig::load_static() } fn level_to_flag(level: &str) -> &'static str { - match level.to_lowercase().as_str() { - "allow" => "allow", - "warn" => "warn", - "deny" => "deny", - "forbid" => "forbid", - _ => "warn", // default to warn for unknown levels + if level.eq_ignore_ascii_case("allow") { + "allow" + } else if level.eq_ignore_ascii_case("warn") { + "warn" + } else if level.eq_ignore_ascii_case("deny") { + "deny" + } else if level.eq_ignore_ascii_case("forbid") { + "forbid" + } else { + "warn" // default to warn for unknown levels } } @@ -92,94 +101,6 @@ fn has_arg(args: &[String], find_arg: &str) -> bool { args.iter().any(|arg| find_arg == arg.split('=').next().unwrap()) } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_level_to_flag() { - assert_eq!(level_to_flag("allow"), "allow"); - assert_eq!(level_to_flag("ALLOW"), "allow"); - assert_eq!(level_to_flag("warn"), "warn"); - assert_eq!(level_to_flag("WARN"), "warn"); - assert_eq!(level_to_flag("deny"), "deny"); - assert_eq!(level_to_flag("forbid"), "forbid"); - assert_eq!(level_to_flag("unknown"), "warn"); // Default - } - - #[test] - fn test_inject_lint_args_priority_sorting() { - // Create a mock merged config for testing - let mut rust_lints = std::collections::BTreeMap::new(); - rust_lints.insert("dead_code".to_string(), ("allow".to_string(), 5, Some("clippy.toml".to_string()))); - rust_lints.insert("unused_variables".to_string(), ("warn".to_string(), 10, Some("Cargo.toml".to_string()))); - rust_lints.insert("unused_imports".to_string(), ("deny".to_string(), 1, Some("clippy.toml".to_string()))); - - let mut clippy_lints = std::collections::BTreeMap::new(); - clippy_lints.insert("needless_return".to_string(), ("allow".to_string(), 15, Some("clippy.toml".to_string()))); - clippy_lints.insert("single_match".to_string(), ("warn".to_string(), 5, Some("Cargo.toml".to_string()))); - clippy_lints.insert("too_many_arguments".to_string(), ("forbid".to_string(), 0, Some("clippy.toml".to_string()))); - - // Simulate the sorting behavior - let mut all_lints: Vec<(String, String, i64)> = Vec::new(); - - for (lint_name, (level_str, priority, _source)) in &rust_lints { - all_lints.push((format!("--{}={}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); - } - - for (lint_name, (level_str, priority, _source)) in &clippy_lints { - all_lints.push((format!("--{}=clippy::{}", level_to_flag(level_str), lint_name), level_str.clone(), *priority)); - } - - // Sort by priority (higher priority first, then by lint name for stability) - all_lints.sort_by(|a, b| b.2.cmp(&a.2).then_with(|| a.0.cmp(&b.0))); - - // Extract just the arguments - let args: Vec = all_lints.into_iter().map(|(arg, _, _)| arg).collect(); - - // Verify the expected order (highest priority first, then alphabetical within same priority) - let expected = vec![ - "--allow=clippy::needless_return", // priority 15 - "--warn=unused_variables", // priority 10 - "--allow=dead_code", // priority 5 (first alphabetically among priority 5) - "--warn=clippy::single_match", // priority 5 (second alphabetically among priority 5) - "--deny=unused_imports", // priority 1 - "--forbid=clippy::too_many_arguments", // priority 0 - ]; - - assert_eq!(args, expected); - } - - #[test] - fn test_arg_value() { - let args = vec![ - "--cap-lints=allow".to_string(), - "--force-warn".to_string(), - "clippy::needless_return".to_string(), - "--other-flag=value".to_string(), - ]; - - assert_eq!(arg_value(&args, "--cap-lints", |val| val == "allow"), Some("allow")); - assert_eq!(arg_value(&args, "--cap-lints", |val| val == "warn"), None); - assert_eq!(arg_value(&args, "--force-warn", |val| val.contains("clippy::")), Some("clippy::needless_return")); - assert_eq!(arg_value(&args, "--nonexistent", |_| true), None); - } - - #[test] - fn test_has_arg() { - let args = vec![ - "--cap-lints=allow".to_string(), - "--version".to_string(), - "--help".to_string(), - ]; - - assert!(has_arg(&args, "--cap-lints")); - assert!(has_arg(&args, "--version")); - assert!(has_arg(&args, "--help")); - assert!(!has_arg(&args, "--nonexistent")); - } -} - fn track_clippy_args(psess: &mut ParseSess, args_env_var: Option<&str>) { psess .env_depinfo @@ -436,3 +357,120 @@ You can use tool lints to allow or deny lints from your code, e.g.: " ) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_level_to_flag() { + assert_eq!(level_to_flag("allow"), "allow"); + assert_eq!(level_to_flag("ALLOW"), "allow"); + assert_eq!(level_to_flag("warn"), "warn"); + assert_eq!(level_to_flag("WARN"), "warn"); + assert_eq!(level_to_flag("deny"), "deny"); + assert_eq!(level_to_flag("forbid"), "forbid"); + assert_eq!(level_to_flag("unknown"), "warn"); // Default + } + + #[test] + fn test_inject_lint_args_priority_sorting() { + // Create a mock merged config for testing + let mut rust_lints = std::collections::BTreeMap::new(); + rust_lints.insert( + "dead_code".to_string(), + ("allow".to_string(), 5, Some("clippy.toml".to_string())), + ); + rust_lints.insert( + "unused_variables".to_string(), + ("warn".to_string(), 10, Some("Cargo.toml".to_string())), + ); + rust_lints.insert( + "unused_imports".to_string(), + ("deny".to_string(), 1, Some("clippy.toml".to_string())), + ); + + let mut clippy_lints = std::collections::BTreeMap::new(); + clippy_lints.insert( + "needless_return".to_string(), + ("allow".to_string(), 15, Some("clippy.toml".to_string())), + ); + clippy_lints.insert( + "single_match".to_string(), + ("warn".to_string(), 5, Some("Cargo.toml".to_string())), + ); + clippy_lints.insert( + "too_many_arguments".to_string(), + ("forbid".to_string(), 0, Some("clippy.toml".to_string())), + ); + + // Simulate the sorting behavior + let mut all_lints: Vec<(String, String, i64)> = Vec::new(); + + for (lint_name, (level_str, priority, _source)) in &rust_lints { + all_lints.push(( + format!("--{}={}", level_to_flag(level_str), lint_name), + level_str.clone(), + *priority, + )); + } + + for (lint_name, (level_str, priority, _source)) in &clippy_lints { + all_lints.push(( + format!("--{}=clippy::{}", level_to_flag(level_str), lint_name), + level_str.clone(), + *priority, + )); + } + + // Sort by priority (higher priority first, then by lint name for stability) + all_lints.sort_by(|a, b| b.2.cmp(&a.2).then_with(|| a.0.cmp(&b.0))); + + // Extract just the arguments + let args: Vec = all_lints.into_iter().map(|(arg, _, _)| arg).collect(); + + // Verify the expected order (highest priority first, then alphabetical within same priority) + let expected = vec![ + "--allow=clippy::needless_return", // priority 15 + "--warn=unused_variables", // priority 10 + "--allow=dead_code", // priority 5 (first alphabetically among priority 5) + "--warn=clippy::single_match", // priority 5 (second alphabetically among priority 5) + "--deny=unused_imports", // priority 1 + "--forbid=clippy::too_many_arguments", // priority 0 + ]; + + assert_eq!(args, expected); + } + + #[test] + fn test_arg_value() { + let args = vec![ + "--cap-lints=allow".to_string(), + "--force-warn".to_string(), + "clippy::needless_return".to_string(), + "--other-flag=value".to_string(), + ]; + + assert_eq!(arg_value(&args, "--cap-lints", |val| val == "allow"), Some("allow")); + assert_eq!(arg_value(&args, "--cap-lints", |val| val == "warn"), None); + assert_eq!( + arg_value(&args, "--force-warn", |val| val.contains("clippy::")), + Some("clippy::needless_return") + ); + assert_eq!(arg_value(&args, "--nonexistent", |_| true), None); + } + + #[test] + fn test_has_arg() { + let args = vec![ + "--cap-lints=allow".to_string(), + "--version".to_string(), + "--help".to_string(), + ]; + + assert!(has_arg(&args, "--cap-lints")); + assert!(has_arg(&args, "--version")); + assert!(has_arg(&args, "--help")); + assert!(!has_arg(&args, "--nonexistent")); + } +}