diff --git a/src/cargo/lints/lint.rs b/src/cargo/lints/lint.rs new file mode 100644 index 00000000000..292e67bd7df --- /dev/null +++ b/src/cargo/lints/lint.rs @@ -0,0 +1,242 @@ +use std::cmp::{Reverse, max_by_key}; +use std::fmt::Display; + +use cargo_util_schemas::manifest::RustVersion; +use cargo_util_schemas::manifest::TomlLintLevel; +use cargo_util_schemas::manifest::TomlToolLints; +use cargo_util_terminal::report::Level; + +use crate::core::{Feature, Features}; + +#[derive(Clone, Debug)] +pub struct Lint { + pub name: &'static str, + pub desc: &'static str, + pub primary_group: &'static LintGroup, + /// The minimum supported Rust version for applying this lint + /// + /// Note: If the lint is on by default and did not qualify as a hard-warning before the + /// linting system, then at earliest an MSRV of 1.78 is required as `[lints.cargo]` was a hard + /// error before then. + pub msrv: Option, + pub feature_gate: Option<&'static Feature>, + /// This is a markdown formatted string that will be used when generating + /// the lint documentation. If docs is `None`, the lint will not be + /// documented. + pub docs: Option<&'static str>, +} + +impl Lint { + pub fn level( + &self, + pkg_lints: &TomlToolLints, + pkg_rust_version: Option<&RustVersion>, + unstable_features: &Features, + ) -> (LintLevel, LintLevelSource) { + // We should return `Allow` if a lint is behind a feature, but it is + // not enabled, that way the lint does not run. + if self + .feature_gate + .is_some_and(|f| !unstable_features.is_enabled(f)) + { + return (LintLevel::Allow, LintLevelSource::Default); + } + + if let (Some(msrv), Some(pkg_rust_version)) = (&self.msrv, pkg_rust_version) { + let pkg_rust_version = pkg_rust_version.to_partial(); + if !msrv.is_compatible_with(&pkg_rust_version) { + return (LintLevel::Allow, LintLevelSource::Default); + } + } + + let lint_level_priority = + level_priority(self.name, self.primary_group.default_level, pkg_lints); + + let group_level_priority = level_priority( + self.primary_group.name, + self.primary_group.default_level, + pkg_lints, + ); + + let (_, (l, s, _)) = max_by_key( + (self.name, lint_level_priority), + (self.primary_group.name, group_level_priority), + |(n, (l, s, p))| { + ( + l == &LintLevel::Forbid, + *s != LintLevelSource::Default, + *p, + Reverse(*n), + ) + }, + ); + (l, s) + } + + pub fn emitted_source(&self, lint_level: LintLevel, source: LintLevelSource) -> String { + format!("`cargo::{}` is set to `{lint_level}` {source}", self.name,) + } +} + +#[derive(Copy, Clone, Debug, PartialEq)] +pub enum LintLevel { + Allow, + Warn, + Deny, + Forbid, +} + +impl Display for LintLevel { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + LintLevel::Allow => write!(f, "allow"), + LintLevel::Warn => write!(f, "warn"), + LintLevel::Deny => write!(f, "deny"), + LintLevel::Forbid => write!(f, "forbid"), + } + } +} + +impl LintLevel { + pub fn is_warn(&self) -> bool { + self == &LintLevel::Warn + } + + pub fn is_error(&self) -> bool { + self == &LintLevel::Forbid || self == &LintLevel::Deny + } + + pub fn to_diagnostic_level(self) -> Level<'static> { + match self { + LintLevel::Allow => unreachable!("allow does not map to a diagnostic level"), + LintLevel::Warn => Level::WARNING, + LintLevel::Deny => Level::ERROR, + LintLevel::Forbid => Level::ERROR, + } + } + + pub fn force(self) -> bool { + match self { + Self::Allow => false, + Self::Warn => true, + Self::Deny => true, + Self::Forbid => true, + } + } +} + +impl From for LintLevel { + fn from(toml_lint_level: TomlLintLevel) -> LintLevel { + match toml_lint_level { + TomlLintLevel::Allow => LintLevel::Allow, + TomlLintLevel::Warn => LintLevel::Warn, + TomlLintLevel::Deny => LintLevel::Deny, + TomlLintLevel::Forbid => LintLevel::Forbid, + } + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum LintLevelSource { + Default, + Package, +} + +impl Display for LintLevelSource { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + LintLevelSource::Default => write!(f, "by default"), + LintLevelSource::Package => write!(f, "in `[lints]`"), + } + } +} + +impl LintLevelSource { + pub(crate) fn is_user_specified(&self) -> bool { + match self { + LintLevelSource::Default => false, + LintLevelSource::Package => true, + } + } +} + +pub(crate) fn level_priority( + name: &str, + default_level: LintLevel, + pkg_lints: &TomlToolLints, +) -> (LintLevel, LintLevelSource, i8) { + if let Some(defined_level) = pkg_lints.get(name) { + ( + defined_level.level().into(), + LintLevelSource::Package, + defined_level.priority(), + ) + } else { + (default_level, LintLevelSource::Default, 0) + } +} + +#[derive(Clone, Debug)] +pub struct LintGroup { + pub name: &'static str, + pub default_level: LintLevel, + pub desc: &'static str, + pub feature_gate: Option<&'static Feature>, + pub hidden: bool, +} + +#[cfg(test)] +mod tests { + use super::*; + + const STYLE: LintGroup = LintGroup { + name: "style", + desc: "code that should be written in a more idiomatic way", + default_level: LintLevel::Warn, + feature_gate: None, + hidden: false, + }; + + fn test_lint(name: &'static str, group: &'static LintGroup) -> Lint { + Lint { + name, + desc: "test lint", + primary_group: group, + msrv: None, + feature_gate: None, + docs: None, + } + } + + #[test] + fn lint_level_prefers_user_specified_over_default() { + let lint = test_lint("unused_dependencies", &STYLE); + + let mut pkg_lints = TomlToolLints::new(); + pkg_lints.insert( + "unused_dependencies".to_string(), + cargo_util_schemas::manifest::TomlLint::Level(TomlLintLevel::Deny), + ); + let features = Features::default(); + + let (level, source) = lint.level(&pkg_lints, None, &features); + assert_eq!(level, LintLevel::Deny); + assert_eq!(source, LintLevelSource::Package); + } + + #[test] + fn lint_level_group_overrides_default() { + let lint = test_lint("non_kebab_case_bins", &STYLE); + + let mut pkg_lints = TomlToolLints::new(); + pkg_lints.insert( + "style".to_string(), + cargo_util_schemas::manifest::TomlLint::Level(TomlLintLevel::Deny), + ); + let features = Features::default(); + + let (level, source) = lint.level(&pkg_lints, None, &features); + assert_eq!(level, LintLevel::Deny); + assert_eq!(source, LintLevelSource::Package); + } +} diff --git a/src/cargo/lints/mod.rs b/src/cargo/lints/mod.rs index 5deddbacca2..920279bd249 100644 --- a/src/cargo/lints/mod.rs +++ b/src/cargo/lints/mod.rs @@ -1,36 +1,24 @@ -use std::borrow::Cow; -use std::cmp::{Reverse, max_by_key}; -use std::fmt::Display; -use std::ops::Range; use std::path::Path; use cargo_util_schemas::manifest::RustVersion; -use cargo_util_schemas::manifest::TomlLintLevel; use cargo_util_schemas::manifest::TomlToolLints; use cargo_util_terminal::report::AnnotationKind; use cargo_util_terminal::report::Group; use cargo_util_terminal::report::Level; use cargo_util_terminal::report::Snippet; -use pathdiff::diff_paths; use crate::core::Workspace; use crate::core::{Edition, Feature, Features, MaybePackage, Package}; use crate::{CargoResult, GlobalContext}; +mod lint; +mod report; + pub mod rules; -pub use rules::LINTS; -pub static LINT_GROUPS: &[LintGroup] = &[ - COMPLEXITY, - CORRECTNESS, - NURSERY, - PEDANTIC, - PERF, - RESTRICTION, - STYLE, - SUSPICIOUS, - TEST_DUMMY_UNSTABLE, -]; +pub use lint::{Lint, LintGroup, LintLevel, LintLevelSource}; +pub use report::{AsIndex, get_key_value, get_key_value_span, rel_cwd_manifest_path}; +pub use rules::{LINT_GROUPS, LINTS}; /// Scope at which a lint runs: package-level or workspace-level. pub enum ManifestFor<'a> { @@ -111,7 +99,7 @@ pub fn analyze_cargo_lints_table( continue; }; - let (_, source, _) = level_priority(name, *default_level, cargo_lints); + let (_, source, _) = lint::level_priority(name, *default_level, cargo_lints); // Only run analysis on user-specified lints if !source.is_user_specified() { @@ -209,533 +197,3 @@ fn report_feature_not_enabled( Ok(()) } - -#[derive(Clone)] -pub struct TomlSpan { - pub key: Range, - pub value: Range, -} - -#[derive(Copy, Clone)] -pub enum TomlIndex<'i> { - Key(&'i str), - Offset(usize), -} - -impl<'i> TomlIndex<'i> { - fn as_key(&self) -> Option<&'i str> { - match self { - TomlIndex::Key(key) => Some(key), - TomlIndex::Offset(_) => None, - } - } -} - -pub trait AsIndex { - fn as_index<'i>(&'i self) -> TomlIndex<'i>; -} - -impl AsIndex for TomlIndex<'_> { - fn as_index<'i>(&'i self) -> TomlIndex<'i> { - match self { - TomlIndex::Key(key) => TomlIndex::Key(key), - TomlIndex::Offset(offset) => TomlIndex::Offset(*offset), - } - } -} - -impl AsIndex for &str { - fn as_index<'i>(&'i self) -> TomlIndex<'i> { - TomlIndex::Key(self) - } -} - -impl AsIndex for String { - fn as_index<'i>(&'i self) -> TomlIndex<'i> { - TomlIndex::Key(self.as_str()) - } -} - -impl AsIndex for usize { - fn as_index<'i>(&'i self) -> TomlIndex<'i> { - TomlIndex::Offset(*self) - } -} - -pub fn get_key_value<'doc, 'i>( - document: &'doc toml::Spanned>, - path: &[impl AsIndex], -) -> Option<( - &'doc toml::Spanned>, - &'doc toml::Spanned>, -)> { - let table = document.get_ref(); - let mut iter = path.into_iter(); - let index0 = iter.next()?.as_index(); - let key0 = index0.as_key()?; - let (mut current_key, mut current_item) = table.get_key_value(key0)?; - - while let Some(index) = iter.next() { - match index.as_index() { - TomlIndex::Key(key) => { - if let Some(table) = current_item.get_ref().as_table() { - (current_key, current_item) = table.get_key_value(key)?; - } else if let Some(array) = current_item.get_ref().as_array() { - current_item = array.iter().find(|item| match item.get_ref() { - toml::de::DeValue::String(s) => s == key, - _ => false, - })?; - } else { - return None; - } - } - TomlIndex::Offset(offset) => { - let array = current_item.get_ref().as_array()?; - current_item = array.get(offset)?; - } - } - } - Some((current_key, current_item)) -} - -pub fn get_key_value_span<'i>( - document: &toml::Spanned>, - path: &[impl AsIndex], -) -> Option { - get_key_value(document, path).map(|(k, v)| TomlSpan { - key: k.span(), - value: v.span(), - }) -} - -/// Gets the relative path to a manifest from the current working directory, or -/// the absolute path of the manifest if a relative path cannot be constructed -pub fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String { - diff_paths(path, gctx.cwd()) - .unwrap_or_else(|| path.to_path_buf()) - .display() - .to_string() -} - -#[derive(Clone, Debug)] -pub struct LintGroup { - pub name: &'static str, - pub default_level: LintLevel, - pub desc: &'static str, - pub feature_gate: Option<&'static Feature>, - pub hidden: bool, -} - -const COMPLEXITY: LintGroup = LintGroup { - name: "complexity", - desc: "code that does something simple but in a complex way", - default_level: LintLevel::Warn, - feature_gate: None, - hidden: false, -}; - -const CORRECTNESS: LintGroup = LintGroup { - name: "correctness", - desc: "code that is outright wrong or useless", - default_level: LintLevel::Deny, - feature_gate: None, - hidden: false, -}; - -const NURSERY: LintGroup = LintGroup { - name: "nursery", - desc: "new lints that are still under development", - default_level: LintLevel::Allow, - feature_gate: None, - hidden: false, -}; - -const PEDANTIC: LintGroup = LintGroup { - name: "pedantic", - desc: "lints which are rather strict or have occasional false positives", - default_level: LintLevel::Allow, - feature_gate: None, - hidden: false, -}; - -const PERF: LintGroup = LintGroup { - name: "perf", - desc: "code that can be written to run faster", - default_level: LintLevel::Warn, - feature_gate: None, - hidden: false, -}; - -const RESTRICTION: LintGroup = LintGroup { - name: "restriction", - desc: "lints which prevent the use of Cargo features", - default_level: LintLevel::Allow, - feature_gate: None, - hidden: false, -}; - -const STYLE: LintGroup = LintGroup { - name: "style", - desc: "code that should be written in a more idiomatic way", - default_level: LintLevel::Warn, - feature_gate: None, - hidden: false, -}; - -const SUSPICIOUS: LintGroup = LintGroup { - name: "suspicious", - desc: "code that is most likely wrong or useless", - default_level: LintLevel::Warn, - feature_gate: None, - hidden: false, -}; - -/// This lint group is only to be used for testing purposes -const TEST_DUMMY_UNSTABLE: LintGroup = LintGroup { - name: "test_dummy_unstable", - desc: "test_dummy_unstable is meant to only be used in tests", - default_level: LintLevel::Allow, - feature_gate: Some(Feature::test_dummy_unstable()), - hidden: true, -}; - -#[derive(Clone, Debug)] -pub struct Lint { - pub name: &'static str, - pub desc: &'static str, - pub primary_group: &'static LintGroup, - /// The minimum supported Rust version for applying this lint - /// - /// Note: If the lint is on by default and did not qualify as a hard-warning before the - /// linting system, then at earliest an MSRV of 1.78 is required as `[lints.cargo]` was a hard - /// error before then. - pub msrv: Option, - pub feature_gate: Option<&'static Feature>, - /// This is a markdown formatted string that will be used when generating - /// the lint documentation. If docs is `None`, the lint will not be - /// documented. - pub docs: Option<&'static str>, -} - -impl Lint { - pub fn level( - &self, - pkg_lints: &TomlToolLints, - pkg_rust_version: Option<&RustVersion>, - unstable_features: &Features, - ) -> (LintLevel, LintLevelSource) { - // We should return `Allow` if a lint is behind a feature, but it is - // not enabled, that way the lint does not run. - if self - .feature_gate - .is_some_and(|f| !unstable_features.is_enabled(f)) - { - return (LintLevel::Allow, LintLevelSource::Default); - } - - if let (Some(msrv), Some(pkg_rust_version)) = (&self.msrv, pkg_rust_version) { - let pkg_rust_version = pkg_rust_version.to_partial(); - if !msrv.is_compatible_with(&pkg_rust_version) { - return (LintLevel::Allow, LintLevelSource::Default); - } - } - - let lint_level_priority = - level_priority(self.name, self.primary_group.default_level, pkg_lints); - - let group_level_priority = level_priority( - self.primary_group.name, - self.primary_group.default_level, - pkg_lints, - ); - - let (_, (l, s, _)) = max_by_key( - (self.name, lint_level_priority), - (self.primary_group.name, group_level_priority), - |(n, (l, s, p))| { - ( - l == &LintLevel::Forbid, - *s != LintLevelSource::Default, - *p, - Reverse(*n), - ) - }, - ); - (l, s) - } - - pub fn emitted_source(&self, lint_level: LintLevel, source: LintLevelSource) -> String { - format!("`cargo::{}` is set to `{lint_level}` {source}", self.name,) - } -} - -#[derive(Copy, Clone, Debug, PartialEq)] -pub enum LintLevel { - Allow, - Warn, - Deny, - Forbid, -} - -impl Display for LintLevel { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - LintLevel::Allow => write!(f, "allow"), - LintLevel::Warn => write!(f, "warn"), - LintLevel::Deny => write!(f, "deny"), - LintLevel::Forbid => write!(f, "forbid"), - } - } -} - -impl LintLevel { - pub fn is_warn(&self) -> bool { - self == &LintLevel::Warn - } - - pub fn is_error(&self) -> bool { - self == &LintLevel::Forbid || self == &LintLevel::Deny - } - - pub fn to_diagnostic_level(self) -> Level<'static> { - match self { - LintLevel::Allow => unreachable!("allow does not map to a diagnostic level"), - LintLevel::Warn => Level::WARNING, - LintLevel::Deny => Level::ERROR, - LintLevel::Forbid => Level::ERROR, - } - } - - pub fn force(self) -> bool { - match self { - Self::Allow => false, - Self::Warn => true, - Self::Deny => true, - Self::Forbid => true, - } - } -} - -impl From for LintLevel { - fn from(toml_lint_level: TomlLintLevel) -> LintLevel { - match toml_lint_level { - TomlLintLevel::Allow => LintLevel::Allow, - TomlLintLevel::Warn => LintLevel::Warn, - TomlLintLevel::Deny => LintLevel::Deny, - TomlLintLevel::Forbid => LintLevel::Forbid, - } - } -} - -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub enum LintLevelSource { - Default, - Package, -} - -impl Display for LintLevelSource { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - LintLevelSource::Default => write!(f, "by default"), - LintLevelSource::Package => write!(f, "in `[lints]`"), - } - } -} - -impl LintLevelSource { - fn is_user_specified(&self) -> bool { - match self { - LintLevelSource::Default => false, - LintLevelSource::Package => true, - } - } -} - -fn level_priority( - name: &str, - default_level: LintLevel, - pkg_lints: &TomlToolLints, -) -> (LintLevel, LintLevelSource, i8) { - if let Some(defined_level) = pkg_lints.get(name) { - ( - defined_level.level().into(), - LintLevelSource::Package, - defined_level.priority(), - ) - } else { - (default_level, LintLevelSource::Default, 0) - } -} - -#[cfg(test)] -mod tests { - use itertools::Itertools; - use snapbox::ToDebug; - use std::collections::HashSet; - - use super::*; - - fn test_lint(name: &'static str, group: &'static LintGroup) -> Lint { - Lint { - name, - desc: "test lint", - primary_group: group, - msrv: None, - feature_gate: None, - docs: None, - } - } - - #[test] - fn lint_level_prefers_user_specified_over_default() { - let lint = test_lint("unused_dependencies", &STYLE); - - let mut pkg_lints = TomlToolLints::new(); - pkg_lints.insert( - "unused_dependencies".to_string(), - cargo_util_schemas::manifest::TomlLint::Level(TomlLintLevel::Deny), - ); - let features = Features::default(); - - let (level, source) = lint.level(&pkg_lints, None, &features); - assert_eq!(level, LintLevel::Deny); - assert_eq!(source, LintLevelSource::Package); - } - - #[test] - fn lint_level_group_overrides_default() { - let lint = test_lint("non_kebab_case_bins", &STYLE); - - let mut pkg_lints = TomlToolLints::new(); - pkg_lints.insert( - "style".to_string(), - cargo_util_schemas::manifest::TomlLint::Level(TomlLintLevel::Deny), - ); - let features = Features::default(); - - let (level, source) = lint.level(&pkg_lints, None, &features); - assert_eq!(level, LintLevel::Deny); - assert_eq!(source, LintLevelSource::Package); - } - - #[test] - fn ensure_lint_groups_do_not_default_to_forbid() { - let forbid_groups = super::LINT_GROUPS - .iter() - .filter(|g| matches!(g.default_level, super::LintLevel::Forbid)) - .collect::>(); - - assert!( - forbid_groups.is_empty(), - "\n`LintGroup`s should never default to `forbid`, but the following do:\n\ - {}\n", - forbid_groups.iter().map(|g| g.name).join("\n") - ); - } - - #[test] - fn ensure_sorted_lints() { - // This will be printed out if the fields are not sorted. - let location = std::panic::Location::caller(); - println!("\nTo fix this test, sort `LINTS` in {}\n", location.file(),); - - let actual = super::LINTS - .iter() - .map(|l| l.name.to_uppercase()) - .collect::>(); - - let mut expected = actual.clone(); - expected.sort(); - snapbox::assert_data_eq!(actual.to_debug(), expected.to_debug()); - } - - #[test] - fn ensure_sorted_lint_groups() { - // This will be printed out if the fields are not sorted. - let location = std::panic::Location::caller(); - println!( - "\nTo fix this test, sort `LINT_GROUPS` in {}\n", - location.file(), - ); - let actual = super::LINT_GROUPS - .iter() - .map(|l| l.name.to_uppercase()) - .collect::>(); - - let mut expected = actual.clone(); - expected.sort(); - snapbox::assert_data_eq!(actual.to_debug(), expected.to_debug()); - } - - #[test] - fn ensure_updated_lints() { - let dir = snapbox::utils::current_dir!().join("rules"); - let mut expected = HashSet::new(); - for entry in std::fs::read_dir(&dir).unwrap() { - let entry = entry.unwrap(); - let path = entry.path(); - if path.ends_with("mod.rs") { - continue; - } - let lint_name = path.file_stem().unwrap().to_string_lossy(); - assert!(expected.insert(lint_name.into()), "duplicate lint found"); - } - - let actual = super::LINTS - .iter() - .map(|l| l.name.to_string()) - .collect::>(); - let diff = expected.difference(&actual).sorted().collect::>(); - - let mut need_added = String::new(); - for name in &diff { - need_added.push_str(&format!("{name}\n")); - } - assert!( - diff.is_empty(), - "\n`LINTS` did not contain all `Lint`s found in {}\n\ - Please add the following to `LINTS`:\n\ - {need_added}", - dir.display(), - ); - } - - #[test] - fn ensure_updated_lint_groups() { - let path = snapbox::utils::current_rs!(); - let expected = std::fs::read_to_string(&path).unwrap(); - let expected = expected - .lines() - .filter_map(|l| { - if l.ends_with(": LintGroup = LintGroup {") { - Some( - l.chars() - .skip(6) - .take_while(|c| *c != ':') - .collect::(), - ) - } else { - None - } - }) - .collect::>(); - let actual = super::LINT_GROUPS - .iter() - .map(|l| l.name.to_uppercase()) - .collect::>(); - let diff = expected.difference(&actual).sorted().collect::>(); - - let mut need_added = String::new(); - for name in &diff { - need_added.push_str(&format!("{}\n", name)); - } - assert!( - diff.is_empty(), - "\n`LINT_GROUPS` did not contain all `LintGroup`s found in {}\n\ - Please add the following to `LINT_GROUPS`:\n\ - {}", - path.display(), - need_added - ); - } -} diff --git a/src/cargo/lints/report.rs b/src/cargo/lints/report.rs new file mode 100644 index 00000000000..4dbc9b6d760 --- /dev/null +++ b/src/cargo/lints/report.rs @@ -0,0 +1,114 @@ +use std::borrow::Cow; +use std::ops::Range; +use std::path::Path; + +use pathdiff::diff_paths; + +use crate::GlobalContext; + +/// Gets the relative path to a manifest from the current working directory, or +/// the absolute path of the manifest if a relative path cannot be constructed +pub fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String { + diff_paths(path, gctx.cwd()) + .unwrap_or_else(|| path.to_path_buf()) + .display() + .to_string() +} + +pub fn get_key_value<'doc, 'i>( + document: &'doc toml::Spanned>, + path: &[impl AsIndex], +) -> Option<( + &'doc toml::Spanned>, + &'doc toml::Spanned>, +)> { + let table = document.get_ref(); + let mut iter = path.into_iter(); + let index0 = iter.next()?.as_index(); + let key0 = index0.as_key()?; + let (mut current_key, mut current_item) = table.get_key_value(key0)?; + + while let Some(index) = iter.next() { + match index.as_index() { + TomlIndex::Key(key) => { + if let Some(table) = current_item.get_ref().as_table() { + (current_key, current_item) = table.get_key_value(key)?; + } else if let Some(array) = current_item.get_ref().as_array() { + current_item = array.iter().find(|item| match item.get_ref() { + toml::de::DeValue::String(s) => s == key, + _ => false, + })?; + } else { + return None; + } + } + TomlIndex::Offset(offset) => { + let array = current_item.get_ref().as_array()?; + current_item = array.get(offset)?; + } + } + } + Some((current_key, current_item)) +} + +pub fn get_key_value_span<'i>( + document: &toml::Spanned>, + path: &[impl AsIndex], +) -> Option { + get_key_value(document, path).map(|(k, v)| TomlSpan { + key: k.span(), + value: v.span(), + }) +} + +#[derive(Clone)] +pub struct TomlSpan { + pub key: Range, + pub value: Range, +} + +#[derive(Copy, Clone)] +pub enum TomlIndex<'i> { + Key(&'i str), + Offset(usize), +} + +impl<'i> TomlIndex<'i> { + fn as_key(&self) -> Option<&'i str> { + match self { + TomlIndex::Key(key) => Some(key), + TomlIndex::Offset(_) => None, + } + } +} + +pub trait AsIndex { + fn as_index<'i>(&'i self) -> TomlIndex<'i>; +} + +impl AsIndex for TomlIndex<'_> { + fn as_index<'i>(&'i self) -> TomlIndex<'i> { + match self { + TomlIndex::Key(key) => TomlIndex::Key(key), + TomlIndex::Offset(offset) => TomlIndex::Offset(*offset), + } + } +} + +impl AsIndex for &str { + fn as_index<'i>(&'i self) -> TomlIndex<'i> { + TomlIndex::Key(self) + } +} + +impl AsIndex for String { + fn as_index<'i>(&'i self) -> TomlIndex<'i> { + TomlIndex::Key(self.as_str()) + } +} + +impl AsIndex for usize { + fn as_index<'i>(&'i self) -> TomlIndex<'i> { + TomlIndex::Offset(*self) + } +} diff --git a/src/cargo/lints/rules/blanket_hint_mostly_unused.rs b/src/cargo/lints/rules/blanket_hint_mostly_unused.rs index 78cd5dc8656..eaa9deaff94 100644 --- a/src/cargo/lints/rules/blanket_hint_mostly_unused.rs +++ b/src/cargo/lints/rules/blanket_hint_mostly_unused.rs @@ -10,13 +10,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::SUSPICIOUS; use crate::CargoResult; use crate::GlobalContext; use crate::core::MaybePackage; use crate::core::Workspace; use crate::lints::Lint; use crate::lints::LintLevel; -use crate::lints::SUSPICIOUS; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/im_a_teapot.rs b/src/cargo/lints/rules/im_a_teapot.rs index 0cd6fbb31da..1d84fe2d068 100644 --- a/src/cargo/lints/rules/im_a_teapot.rs +++ b/src/cargo/lints/rules/im_a_teapot.rs @@ -8,13 +8,13 @@ use cargo_util_terminal::report::Origin; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::TEST_DUMMY_UNSTABLE; use crate::CargoResult; use crate::GlobalContext; use crate::core::Feature; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; -use crate::lints::TEST_DUMMY_UNSTABLE; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/implicit_minimum_version_req.rs b/src/cargo/lints/rules/implicit_minimum_version_req.rs index 1761d051d78..b47bd903dec 100644 --- a/src/cargo/lints/rules/implicit_minimum_version_req.rs +++ b/src/cargo/lints/rules/implicit_minimum_version_req.rs @@ -13,6 +13,7 @@ use cargo_util_terminal::report::Snippet; use toml::de::DeValue; use tracing::instrument; +use super::PEDANTIC; use crate::CargoResult; use crate::GlobalContext; use crate::core::Manifest; @@ -22,7 +23,6 @@ use crate::core::Workspace; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::PEDANTIC; use crate::lints::get_key_value; use crate::lints::rel_cwd_manifest_path; use crate::util::OptVersionReq; diff --git a/src/cargo/lints/rules/missing_lints_inheritance.rs b/src/cargo/lints/rules/missing_lints_inheritance.rs index 5d0fdc095c3..8a11dd094c8 100644 --- a/src/cargo/lints/rules/missing_lints_inheritance.rs +++ b/src/cargo/lints/rules/missing_lints_inheritance.rs @@ -8,13 +8,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::SUSPICIOUS; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::core::Workspace; use crate::lints::Lint; use crate::lints::LintLevel; -use crate::lints::SUSPICIOUS; use crate::lints::rel_cwd_manifest_path; pub static LINT: &Lint = &Lint { diff --git a/src/cargo/lints/rules/mod.rs b/src/cargo/lints/rules/mod.rs index b2ee0d74f4d..cfec4b2ec54 100644 --- a/src/cargo/lints/rules/mod.rs +++ b/src/cargo/lints/rules/mod.rs @@ -35,6 +35,9 @@ pub use unused_dependencies::unused_build_dependencies_no_build_rs; pub use unused_workspace_dependencies::unused_workspace_dependencies; pub use unused_workspace_package_fields::unused_workspace_package_fields; +use super::LintGroup; +use super::LintLevel; + pub static LINTS: &[&crate::lints::Lint] = &[ blanket_hint_mostly_unused::LINT, implicit_minimum_version_req::LINT, @@ -61,3 +64,216 @@ pub static LINTS: &[&crate::lints::Lint] = &[ /// another way of disabling it. static CARGO_LINTS_MSRV: cargo_util_schemas::manifest::RustVersion = cargo_util_schemas::manifest::RustVersion::new(1, 79, 0); + +pub static LINT_GROUPS: &[LintGroup] = &[ + COMPLEXITY, + CORRECTNESS, + NURSERY, + PEDANTIC, + PERF, + RESTRICTION, + STYLE, + SUSPICIOUS, + TEST_DUMMY_UNSTABLE, +]; + +const COMPLEXITY: LintGroup = LintGroup { + name: "complexity", + desc: "code that does something simple but in a complex way", + default_level: LintLevel::Warn, + feature_gate: None, + hidden: false, +}; + +const CORRECTNESS: LintGroup = LintGroup { + name: "correctness", + desc: "code that is outright wrong or useless", + default_level: LintLevel::Deny, + feature_gate: None, + hidden: false, +}; + +const NURSERY: LintGroup = LintGroup { + name: "nursery", + desc: "new lints that are still under development", + default_level: LintLevel::Allow, + feature_gate: None, + hidden: false, +}; + +const PEDANTIC: LintGroup = LintGroup { + name: "pedantic", + desc: "lints which are rather strict or have occasional false positives", + default_level: LintLevel::Allow, + feature_gate: None, + hidden: false, +}; + +const PERF: LintGroup = LintGroup { + name: "perf", + desc: "code that can be written to run faster", + default_level: LintLevel::Warn, + feature_gate: None, + hidden: false, +}; + +const RESTRICTION: LintGroup = LintGroup { + name: "restriction", + desc: "lints which prevent the use of Cargo features", + default_level: LintLevel::Allow, + feature_gate: None, + hidden: false, +}; + +const STYLE: LintGroup = LintGroup { + name: "style", + desc: "code that should be written in a more idiomatic way", + default_level: LintLevel::Warn, + feature_gate: None, + hidden: false, +}; + +const SUSPICIOUS: LintGroup = LintGroup { + name: "suspicious", + desc: "code that is most likely wrong or useless", + default_level: LintLevel::Warn, + feature_gate: None, + hidden: false, +}; + +/// This lint group is only to be used for testing purposes +const TEST_DUMMY_UNSTABLE: LintGroup = LintGroup { + name: "test_dummy_unstable", + desc: "test_dummy_unstable is meant to only be used in tests", + default_level: LintLevel::Allow, + feature_gate: Some(crate::core::Feature::test_dummy_unstable()), + hidden: true, +}; + +#[cfg(test)] +mod tests { + use itertools::Itertools; + use snapbox::ToDebug; + use std::collections::HashSet; + + #[test] + fn ensure_lint_groups_do_not_default_to_forbid() { + let forbid_groups = super::LINT_GROUPS + .iter() + .filter(|g| matches!(g.default_level, super::LintLevel::Forbid)) + .collect::>(); + + assert!( + forbid_groups.is_empty(), + "\n`LintGroup`s should never default to `forbid`, but the following do:\n\ + {}\n", + forbid_groups.iter().map(|g| g.name).join("\n") + ); + } + + #[test] + fn ensure_sorted_lints() { + // This will be printed out if the fields are not sorted. + let location = std::panic::Location::caller(); + println!("\nTo fix this test, sort `LINTS` in {}\n", location.file(),); + + let actual = super::LINTS + .iter() + .map(|l| l.name.to_uppercase()) + .collect::>(); + + let mut expected = actual.clone(); + expected.sort(); + snapbox::assert_data_eq!(actual.to_debug(), expected.to_debug()); + } + + #[test] + fn ensure_sorted_lint_groups() { + // This will be printed out if the fields are not sorted. + let location = std::panic::Location::caller(); + println!( + "\nTo fix this test, sort `LINT_GROUPS` in {}\n", + location.file(), + ); + let actual = super::LINT_GROUPS + .iter() + .map(|l| l.name.to_uppercase()) + .collect::>(); + + let mut expected = actual.clone(); + expected.sort(); + snapbox::assert_data_eq!(actual.to_debug(), expected.to_debug()); + } + + #[test] + fn ensure_updated_lints() { + let dir = snapbox::utils::current_dir!(); + let mut expected = HashSet::new(); + for entry in std::fs::read_dir(&dir).unwrap() { + let entry = entry.unwrap(); + let path = entry.path(); + if path.ends_with("mod.rs") { + continue; + } + let lint_name = path.file_stem().unwrap().to_string_lossy(); + assert!(expected.insert(lint_name.into()), "duplicate lint found"); + } + + let actual = super::LINTS + .iter() + .map(|l| l.name.to_string()) + .collect::>(); + let diff = expected.difference(&actual).sorted().collect::>(); + + let mut need_added = String::new(); + for name in &diff { + need_added.push_str(&format!("{name}\n")); + } + assert!( + diff.is_empty(), + "\n`LINTS` did not contain all `Lint`s found in {}\n\ + Please add the following to `LINTS`:\n\ + {need_added}", + dir.display(), + ); + } + + #[test] + fn ensure_updated_lint_groups() { + let path = snapbox::utils::current_rs!(); + let expected = std::fs::read_to_string(&path).unwrap(); + let expected = expected + .lines() + .filter_map(|l| { + if l.ends_with(": LintGroup = LintGroup {") { + Some( + l.chars() + .skip(6) + .take_while(|c| *c != ':') + .collect::(), + ) + } else { + None + } + }) + .collect::>(); + let actual = super::LINT_GROUPS + .iter() + .map(|l| l.name.to_uppercase()) + .collect::>(); + let diff = expected.difference(&actual).sorted().collect::>(); + + let mut need_added = String::new(); + for name in &diff { + need_added.push_str(&format!("{}\n", name)); + } + assert!( + diff.is_empty(), + "\n`LINT_GROUPS` did not contain all `LintGroup`s found in {}\n\ + Please add the following to `LINT_GROUPS`:\n\ + {}", + path.display(), + need_added + ); + } +} diff --git a/src/cargo/lints/rules/non_kebab_case_bins.rs b/src/cargo/lints/rules/non_kebab_case_bins.rs index 4642dc3b0ad..48b0e40f8ba 100644 --- a/src/cargo/lints/rules/non_kebab_case_bins.rs +++ b/src/cargo/lints/rules/non_kebab_case_bins.rs @@ -9,6 +9,7 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::STYLE; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; @@ -17,7 +18,6 @@ use crate::lints::AsIndex; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::STYLE; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/non_kebab_case_features.rs b/src/cargo/lints/rules/non_kebab_case_features.rs index 800b0fd4d31..fac7a804a9d 100644 --- a/src/cargo/lints/rules/non_kebab_case_features.rs +++ b/src/cargo/lints/rules/non_kebab_case_features.rs @@ -9,13 +9,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::RESTRICTION; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::RESTRICTION; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/non_kebab_case_packages.rs b/src/cargo/lints/rules/non_kebab_case_packages.rs index 90b92c6a45c..9af98746a90 100644 --- a/src/cargo/lints/rules/non_kebab_case_packages.rs +++ b/src/cargo/lints/rules/non_kebab_case_packages.rs @@ -9,13 +9,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::RESTRICTION; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::RESTRICTION; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/non_snake_case_features.rs b/src/cargo/lints/rules/non_snake_case_features.rs index 5c89b64d674..b478d8f937f 100644 --- a/src/cargo/lints/rules/non_snake_case_features.rs +++ b/src/cargo/lints/rules/non_snake_case_features.rs @@ -9,13 +9,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::RESTRICTION; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::RESTRICTION; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/non_snake_case_packages.rs b/src/cargo/lints/rules/non_snake_case_packages.rs index 4a212829098..beddcc6587b 100644 --- a/src/cargo/lints/rules/non_snake_case_packages.rs +++ b/src/cargo/lints/rules/non_snake_case_packages.rs @@ -9,13 +9,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::RESTRICTION; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::RESTRICTION; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/redundant_homepage.rs b/src/cargo/lints/rules/redundant_homepage.rs index b3f0866f110..30250a59fc9 100644 --- a/src/cargo/lints/rules/redundant_homepage.rs +++ b/src/cargo/lints/rules/redundant_homepage.rs @@ -10,13 +10,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::STYLE; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::STYLE; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/redundant_readme.rs b/src/cargo/lints/rules/redundant_readme.rs index 42c9c3efdcc..14fae4d88a9 100644 --- a/src/cargo/lints/rules/redundant_readme.rs +++ b/src/cargo/lints/rules/redundant_readme.rs @@ -11,13 +11,13 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::STYLE; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::LintLevelSource; -use crate::lints::STYLE; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; use crate::util::toml::DEFAULT_README_FILES; diff --git a/src/cargo/lints/rules/text_direction_codepoint_in_comment.rs b/src/cargo/lints/rules/text_direction_codepoint_in_comment.rs index 4005d4e0073..06abb63f011 100644 --- a/src/cargo/lints/rules/text_direction_codepoint_in_comment.rs +++ b/src/cargo/lints/rules/text_direction_codepoint_in_comment.rs @@ -13,10 +13,10 @@ use toml_parser::parser::EventKind; use toml_parser::parser::EventReceiver; use tracing::instrument; +use super::CORRECTNESS; use crate::CargoResult; use crate::GlobalContext; use crate::core::MaybePackage; -use crate::lints::CORRECTNESS; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::ManifestFor; diff --git a/src/cargo/lints/rules/text_direction_codepoint_in_literal.rs b/src/cargo/lints/rules/text_direction_codepoint_in_literal.rs index 9808fe16963..02b588a55eb 100644 --- a/src/cargo/lints/rules/text_direction_codepoint_in_literal.rs +++ b/src/cargo/lints/rules/text_direction_codepoint_in_literal.rs @@ -14,10 +14,10 @@ use toml_parser::parser::EventKind; use toml_parser::parser::EventReceiver; use tracing::instrument; +use super::CORRECTNESS; use crate::CargoResult; use crate::GlobalContext; use crate::core::MaybePackage; -use crate::lints::CORRECTNESS; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::ManifestFor; diff --git a/src/cargo/lints/rules/unknown_lints.rs b/src/cargo/lints/rules/unknown_lints.rs index 4a64cc96e5e..8e0f471d482 100644 --- a/src/cargo/lints/rules/unknown_lints.rs +++ b/src/cargo/lints/rules/unknown_lints.rs @@ -6,14 +6,14 @@ use cargo_util_terminal::report::Origin; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::LINT_GROUPS; +use super::LINTS; +use super::SUSPICIOUS; use crate::CargoResult; use crate::GlobalContext; -use crate::lints::LINT_GROUPS; -use crate::lints::LINTS; use crate::lints::Lint; use crate::lints::LintLevel; use crate::lints::ManifestFor; -use crate::lints::SUSPICIOUS; use crate::lints::get_key_value_span; pub static LINT: &Lint = &Lint { diff --git a/src/cargo/lints/rules/unused_dependencies.rs b/src/cargo/lints/rules/unused_dependencies.rs index ec0b57b1f3b..31ee25bcecb 100644 --- a/src/cargo/lints/rules/unused_dependencies.rs +++ b/src/cargo/lints/rules/unused_dependencies.rs @@ -10,12 +10,12 @@ use cargo_util_terminal::report::Patch; use cargo_util_terminal::report::Snippet; use tracing::instrument; +use super::STYLE; use crate::CargoResult; use crate::GlobalContext; use crate::core::Package; use crate::lints::Lint; use crate::lints::LintLevel; -use crate::lints::STYLE; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/unused_workspace_dependencies.rs b/src/cargo/lints/rules/unused_workspace_dependencies.rs index b40ab2b7228..667cb5af979 100644 --- a/src/cargo/lints/rules/unused_workspace_dependencies.rs +++ b/src/cargo/lints/rules/unused_workspace_dependencies.rs @@ -11,13 +11,13 @@ use cargo_util_terminal::report::Snippet; use indexmap::IndexSet; use tracing::instrument; +use super::SUSPICIOUS; use crate::CargoResult; use crate::GlobalContext; use crate::core::MaybePackage; use crate::core::Workspace; use crate::lints::Lint; use crate::lints::LintLevel; -use crate::lints::SUSPICIOUS; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path; diff --git a/src/cargo/lints/rules/unused_workspace_package_fields.rs b/src/cargo/lints/rules/unused_workspace_package_fields.rs index adb8092604f..9f57accc785 100644 --- a/src/cargo/lints/rules/unused_workspace_package_fields.rs +++ b/src/cargo/lints/rules/unused_workspace_package_fields.rs @@ -10,13 +10,13 @@ use cargo_util_terminal::report::Snippet; use indexmap::IndexSet; use tracing::instrument; +use super::SUSPICIOUS; use crate::CargoResult; use crate::GlobalContext; use crate::core::MaybePackage; use crate::core::Workspace; use crate::lints::Lint; use crate::lints::LintLevel; -use crate::lints::SUSPICIOUS; use crate::lints::get_key_value_span; use crate::lints::rel_cwd_manifest_path;