diff --git a/clippy_config/src/lib.rs b/clippy_config/src/lib.rs index 67904b4fcdc8..0b64c4b40f49 100644 --- a/clippy_config/src/lib.rs +++ b/clippy_config/src/lib.rs @@ -22,8 +22,10 @@ extern crate rustc_session; extern crate rustc_span; mod conf; +pub mod lint_config; mod metadata; pub mod types; pub use conf::{Conf, get_configuration_metadata, lookup_conf_file, sanitize_explanation}; +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 new file mode 100644 index 000000000000..87050159cc2c --- /dev/null +++ b/clippy_config/src/lint_config.rs @@ -0,0 +1,542 @@ +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")) + && 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() + && 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); + } + } + + 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") + && 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() + && 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 + && 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) { + // 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 + )); + } + // 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) { + // 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, + 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) { + // 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, + 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) { + // 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, + 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); + } +} diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index f64eefa0c232..a07254a8a0ac 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 = "[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 ffd6c520c9ae..91ec6088159b 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,36 @@ 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() + && 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..8b94aa27df1d 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -46,33 +46,59 @@ fn arg_value<'a>(args: &'a [String], find_arg: &str, pred: impl Fn(&str) -> bool None } -fn has_arg(args: &[String], find_arg: &str) -> bool { - args.iter().any(|arg| find_arg == arg.split('=').next().unwrap()) +fn inject_lint_args_from_config(clippy_args: &mut Vec) { + // Load merged configuration from both Cargo.toml and clippy.toml + 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); + } } -#[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); +fn load_merged_lint_config() -> clippy_config::MergedLintConfig { + clippy_config::MergedLintConfig::load_static() } -#[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")); +fn level_to_flag(level: &str) -> &'static str { + 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 + } +} - assert!(!has_arg(args, "--bar")); +fn has_arg(args: &[String], find_arg: &str) -> bool { + args.iter().any(|arg| find_arg == arg.split('=').next().unwrap()) } fn track_clippy_args(psess: &mut ParseSess, args_env_var: Option<&str>) { @@ -271,7 +297,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 +312,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(); @@ -328,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")); + } +}