From 35b8af8f5a6e864fc954b8a7fb3dcae03bd473a9 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 09:14:03 +0200 Subject: [PATCH 01/11] implement configured struct matching and rules --- .sphinx/.gitignore | 1 + .../src/struct_lint/matcher.rs | 13 +- .../src/struct_lint/tests.rs | 44 +- .../src/struct_lint/types.rs | 7 +- .../src/lints/struct_lint/lint.rs | 469 ++++++++++++------ test_app/expected_output | 95 +++- test_app/pup.ron | 35 ++ test_app/src/configured_struct_rules.rs | 35 ++ test_app/src/main.rs | 1 + test_app/tests/pup_ron_test.rs | 63 ++- .../attributes_and_configured_rules.rs | 46 ++ .../attributes_and_configured_rules.stderr | 68 +++ tests/ui/struct_lint_new/pup.ron | 63 ++- 13 files changed, 771 insertions(+), 169 deletions(-) create mode 100644 .sphinx/.gitignore create mode 100644 test_app/src/configured_struct_rules.rs create mode 100644 tests/ui/struct_lint_new/attributes_and_configured_rules.rs create mode 100644 tests/ui/struct_lint_new/attributes_and_configured_rules.stderr diff --git a/.sphinx/.gitignore b/.sphinx/.gitignore new file mode 100644 index 0000000..a00c7d8 --- /dev/null +++ b/.sphinx/.gitignore @@ -0,0 +1 @@ +kg.db diff --git a/cargo_pup_lint_config/src/struct_lint/matcher.rs b/cargo_pup_lint_config/src/struct_lint/matcher.rs index e120dcb..f3ff52f 100644 --- a/cargo_pup_lint_config/src/struct_lint/matcher.rs +++ b/cargo_pup_lint_config/src/struct_lint/matcher.rs @@ -12,9 +12,16 @@ impl StructMatcher { StructMatchNode::Leaf(StructMatch::Name(name.into())) } - /// Matches structs by attribute. The attribute name is - /// given by a regular expression. - /// e.g., "derive\\(.*Debug.*\\)" + /// Matches structs by an attribute name that remains attached after macro expansion. + /// + /// The regular expression must match the complete, compiler-normalised name. For example, + /// `"repr"` matches `#[repr(C)]`, but attribute arguments such as `C` are not available. + /// + /// Supported built-in names are `deprecated`, `doc`, `must_use`, `non_exhaustive`, and `repr`. + /// Custom and tool attributes can also match if they remain attached after expansion. This + /// does not work for attributes consumed while expanding code, including `derive` and + /// attribute procedural macros. The original `cfg_attr` expression is not available, but a + /// supported attribute produced by an active `cfg_attr` can match by its name. /// pub fn has_attribute(&self, attr: impl Into) -> StructMatchNode { StructMatchNode::Leaf(StructMatch::HasAttribute(attr.into())) diff --git a/cargo_pup_lint_config/src/struct_lint/tests.rs b/cargo_pup_lint_config/src/struct_lint/tests.rs index eda7a8b..222e506 100644 --- a/cargo_pup_lint_config/src/struct_lint/tests.rs +++ b/cargo_pup_lint_config/src/struct_lint/tests.rs @@ -19,10 +19,7 @@ mod tests { builder .struct_lint() .lint_named("struct_lint") - .matching(|m| { - m.name("^[A-Z][a-z]+Model$") - .and(m.has_attribute("derive\\\\(.*Debug.*\\\\)")) - }) + .matching(|m| m.name("^[A-Z][a-z]+Model$").and(m.has_attribute("repr"))) .with_severity(Severity::Error) .must_be_named("EntityModel".into()) .build(); @@ -38,7 +35,7 @@ mod tests { } if let StructMatch::HasAttribute(pattern) = &**right { - assert_eq!(pattern, "derive\\\\(.*Debug.*\\\\)"); + assert_eq!(pattern, "repr"); } else { panic!("Expected HasAttribute"); } @@ -115,7 +112,7 @@ mod tests { .matching(|m| { m.name("User") .or(m.name("Account")) - .and(m.has_attribute("derive(Debug)").not()) + .and(m.has_attribute("repr").not()) }) .must_be_named("Entity".into()) .build(); @@ -131,6 +128,41 @@ mod tests { } } + #[test] + fn test_trait_and_logical_rule_builders() { + let mut builder = LintBuilder::new(); + + builder + .struct_lint() + .lint_named("trait_rules") + .matching(|m| m.name("Service")) + .with_severity(Severity::Error) + .must_implement_trait("crate::Required") + .must_not_implement_trait("crate::Forbidden") + .add_rule(StructRule::Or( + Box::new(StructRule::MustBePrivate(Severity::Warn)), + Box::new(StructRule::MustBeNamed( + "PublicService".into(), + Severity::Warn, + )), + )) + .build(); + + let ConfiguredLint::Struct(lint) = &builder.lints[0] else { + panic!("Expected struct lint"); + }; + assert!(matches!( + &lint.rules[0], + StructRule::ImplementsTrait(path, Severity::Error) if path == "crate::Required" + )); + assert!(matches!( + &lint.rules[1], + StructRule::Not(inner) + if matches!(inner.as_ref(), StructRule::ImplementsTrait(path, Severity::Error) if path == "crate::Forbidden") + )); + assert!(matches!(&lint.rules[2], StructRule::Or(_, _))); + } + #[test] fn test_struct_visibility_rules() { let mut builder = LintBuilder::new(); diff --git a/cargo_pup_lint_config/src/struct_lint/types.rs b/cargo_pup_lint_config/src/struct_lint/types.rs index 601157b..dff75a8 100644 --- a/cargo_pup_lint_config/src/struct_lint/types.rs +++ b/cargo_pup_lint_config/src/struct_lint/types.rs @@ -8,7 +8,12 @@ use serde::{Deserialize, Serialize}; pub enum StructMatch { /// Match structs by name (exact name or regex pattern) Name(String), - /// Match structs that have a specific attribute (e.g., #[derive(Debug)]) + /// Match the name of an attribute that remains after macro expansion. + /// + /// Supported built-ins are `deprecated`, `doc`, `must_use`, `non_exhaustive`, and `repr`; + /// arguments are unavailable. Custom and tool attributes can match if they survive expansion. + /// Consumed attributes such as `derive` and attribute procedural macros cannot match. An active + /// `cfg_attr` may match a supported resulting attribute, but not the original expression. HasAttribute(String), /// Match structs that implement a specific trait ImplementsTrait(String), diff --git a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs index f760ed2..2bff99a 100644 --- a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs @@ -16,6 +16,78 @@ pub struct StructLint { struct_rules: Vec, } +fn attribute_matches(regex: &Regex, attribute: &str) -> bool { + regex + .find(attribute) + .is_some_and(|matched| matched.start() == 0 && matched.end() == attribute.len()) +} + +fn hir_attribute_name(attribute: &rustc_hir::Attribute) -> Option { + use rustc_hir::attrs::AttributeKind; + + match attribute { + rustc_hir::Attribute::Unparsed(attribute) => Some(attribute.path.to_string()), + rustc_hir::Attribute::Parsed(kind) => match kind { + AttributeKind::Deprecation { .. } => Some("deprecated".into()), + AttributeKind::Doc(_) | AttributeKind::DocComment { .. } => Some("doc".into()), + AttributeKind::MustUse { .. } => Some("must_use".into()), + AttributeKind::NonExhaustive(_) => Some("non_exhaustive".into()), + AttributeKind::Repr { .. } => Some("repr".into()), + _ => None, + }, + } +} + +fn evaluate_rule_with(rule: &StructRule, evaluate_leaf: &impl Fn(&StructRule) -> bool) -> bool { + match rule { + StructRule::And(left, right) => { + evaluate_rule_with(left, evaluate_leaf) && evaluate_rule_with(right, evaluate_leaf) + } + StructRule::Or(left, right) => { + evaluate_rule_with(left, evaluate_leaf) || evaluate_rule_with(right, evaluate_leaf) + } + StructRule::Not(inner) => !evaluate_rule_with(inner, evaluate_leaf), + leaf => evaluate_leaf(leaf), + } +} + +fn collect_rule_violations<'a>( + rule: &'a StructRule, + expected: bool, + evaluate_leaf: &impl Fn(&StructRule) -> bool, + violations: &mut Vec<(&'a StructRule, bool)>, +) { + if evaluate_rule_with(rule, evaluate_leaf) == expected { + return; + } + + match rule { + StructRule::And(left, right) if expected => { + collect_rule_violations(left, true, evaluate_leaf, violations); + collect_rule_violations(right, true, evaluate_leaf, violations); + } + StructRule::And(left, right) => { + // Both children are true when !(left && right) fails. Reporting both + // alternatives makes the available ways to satisfy the rule visible. + collect_rule_violations(left, false, evaluate_leaf, violations); + collect_rule_violations(right, false, evaluate_leaf, violations); + } + StructRule::Or(left, right) if expected => { + // Both alternatives failed, so report both leaf requirements. + collect_rule_violations(left, true, evaluate_leaf, violations); + collect_rule_violations(right, true, evaluate_leaf, violations); + } + StructRule::Or(left, right) => { + collect_rule_violations(left, false, evaluate_leaf, violations); + collect_rule_violations(right, false, evaluate_leaf, violations); + } + StructRule::Not(inner) => { + collect_rule_violations(inner, !expected, evaluate_leaf, violations); + } + leaf => violations.push((leaf, expected)), + } +} + impl StructLint { #[allow(clippy::new_ret_no_self)] pub fn new(config: &ConfiguredLint) -> Box { @@ -30,49 +102,47 @@ impl StructLint { } } - // Helper method to check if a struct in a given crate should be linted - fn matches_struct(&self, crate_name: &str, struct_name: &str) -> bool { - self.evaluate_struct_match(&self.matches, crate_name, struct_name) + fn matches_struct<'tcx>( + &self, + ctx: &LateContext<'tcx>, + item: &'tcx Item<'tcx>, + crate_name: &str, + struct_name: &str, + ) -> bool { + self.evaluate_struct_match(&self.matches, ctx, item, crate_name, struct_name) } - // Evaluates the complex matcher structure to determine if a struct matches - fn evaluate_struct_match( + fn evaluate_struct_match<'tcx>( &self, matcher: &StructMatch, + ctx: &LateContext<'tcx>, + item: &'tcx Item<'tcx>, crate_name: &str, struct_name: &str, ) -> bool { match matcher { StructMatch::Name(pattern) => { - // Try to match both the crate name and the struct name - // If it's a crate name starting with "test_", prefer that match + // For compatibility, Name patterns beginning with `test_` match the crate name. if pattern.starts_with("test_") { - // This is likely a crate name pattern self.string_matches_pattern(crate_name, pattern) } else { - // This is likely a struct name pattern self.string_matches_pattern(struct_name, pattern) } } - StructMatch::HasAttribute(_) => { - // Attribute matching not yet implemented - false - } - StructMatch::ImplementsTrait(_) => { - // Implementation will be handled in check_item - // Always return true here and do the filtering there - true + StructMatch::HasAttribute(pattern) => self.has_matching_attribute(ctx, item, pattern), + StructMatch::ImplementsTrait(pattern) => { + self.implements_matching_trait(ctx, item.owner_id.to_def_id(), pattern) } StructMatch::AndMatches(left, right) => { - self.evaluate_struct_match(left, crate_name, struct_name) - && self.evaluate_struct_match(right, crate_name, struct_name) + self.evaluate_struct_match(left, ctx, item, crate_name, struct_name) + && self.evaluate_struct_match(right, ctx, item, crate_name, struct_name) } StructMatch::OrMatches(left, right) => { - self.evaluate_struct_match(left, crate_name, struct_name) - || self.evaluate_struct_match(right, crate_name, struct_name) + self.evaluate_struct_match(left, ctx, item, crate_name, struct_name) + || self.evaluate_struct_match(right, ctx, item, crate_name, struct_name) } StructMatch::NotMatch(inner) => { - !self.evaluate_struct_match(inner, crate_name, struct_name) + !self.evaluate_struct_match(inner, ctx, item, crate_name, struct_name) } } } @@ -86,71 +156,47 @@ impl StructLint { } fn describe_pattern(&self, pattern: &str) -> &'static str { - if pattern.contains(|c: char| { - c == '*' || c == '.' || c == '+' || c == '[' || c == '(' || c == '|' - }) { + if pattern.contains(['*', '.', '+', '[', '(', '|', '^', '$', '\\']) { "pattern" } else { "name" } } - // Check if this struct has any trait implementations that match our patterns - fn has_matching_trait_impl(&self, ctx: &LateContext<'_>, def_id: DefId) -> bool { - use crate::helpers::queries; - - // Extract trait pattern if one exists in the matcher - fn needs_trait_check(matcher: &StructMatch) -> Option { - match matcher { - StructMatch::ImplementsTrait(pattern) => Some(pattern.clone()), - StructMatch::AndMatches(left, right) => { - needs_trait_check(left).or_else(|| needs_trait_check(right)) - } - StructMatch::OrMatches(left, right) => { - needs_trait_check(left).or_else(|| needs_trait_check(right)) - } - StructMatch::NotMatch(inner) => needs_trait_check(inner), - _ => None, - } - } - - // Check if we have any trait matchers - if let Some(trait_pattern) = needs_trait_check(&self.matches) { - // Create a regex from the trait pattern - let trait_regex = match Regex::new(&trait_pattern) { - Ok(regex) => regex, - Err(_) => return false, // If regex is invalid, consider no match - }; + fn has_matching_attribute( + &self, + ctx: &LateContext<'_>, + item: &Item<'_>, + pattern: &str, + ) -> bool { + let Ok(regex) = Regex::new(pattern) else { + return false; + }; + ctx.tcx.hir_attrs(item.hir_id()).iter().any(|attribute| { + hir_attribute_name(attribute) + .is_some_and(|attribute| attribute_matches(®ex, &attribute)) + }) + } - // Get the type for the struct - let ty = ctx.tcx.type_of(def_id).skip_binder(); - - // Get parameter environment for the struct - let param_env = ctx.param_env; - - // For each trait in all crates, check if: - // 1. The trait name matches our pattern - // 2. The struct implements the trait - for trait_def_id in ctx.tcx.all_traits_including_private() { - // Get the full canonical trait name - let full_trait_name = - queries::get_full_canonical_trait_name_from_def_id(&ctx.tcx, trait_def_id); - - // Check if the trait name matches our pattern - if trait_regex.is_match(&full_trait_name) { - // If the trait name matches, check if our type implements this trait - if queries::implements_trait(ctx.tcx, param_env, ty, trait_def_id) { - return true; - } - } - } + fn implements_matching_trait( + &self, + ctx: &LateContext<'_>, + def_id: DefId, + trait_pattern: &str, + ) -> bool { + use crate::helpers::queries; - // If we get here, no matching trait implementations were found + let Ok(trait_regex) = Regex::new(trait_pattern) else { return false; - } - - // If no trait patterns found, no trait constraints to enforce - true + }; + let ty = ctx.tcx.type_of(def_id).skip_binder(); + + ctx.tcx.all_traits_including_private().any(|trait_def_id| { + let full_trait_name = + queries::get_full_canonical_trait_name_from_def_id(&ctx.tcx, trait_def_id); + trait_regex.is_match(&full_trait_name) + && queries::implements_trait(ctx.tcx, ctx.param_env, ty, trait_def_id) + }) } } @@ -194,6 +240,14 @@ declare_variable_severity_lint!( "Struct must have pub(crate) visibility" ); +declare_variable_severity_lint!( + pub, + STRUCT_LINT_IMPLEMENTS_TRAIT, + STRUCT_LINT_IMPLEMENTS_TRAIT_DENY, + STRUCT_LINT_IMPLEMENTS_TRAIT_WARN, + "Struct trait implementation rules" +); + impl_lint_pass!(StructLint => [ STRUCT_LINT_MUST_BE_NAMED_DENY, STRUCT_LINT_MUST_BE_NAMED_WARN, @@ -204,7 +258,9 @@ impl_lint_pass!(StructLint => [ STRUCT_LINT_MUST_BE_PUBLIC_DENY, STRUCT_LINT_MUST_BE_PUBLIC_WARN, STRUCT_LINT_MUST_BE_PUB_CRATE_DENY, - STRUCT_LINT_MUST_BE_PUB_CRATE_WARN + STRUCT_LINT_MUST_BE_PUB_CRATE_WARN, + STRUCT_LINT_IMPLEMENTS_TRAIT_DENY, + STRUCT_LINT_IMPLEMENTS_TRAIT_WARN ]); impl ArchitectureLintRule for StructLint { @@ -248,16 +304,11 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { .crate_name(rustc_hir::def_id::LOCAL_CRATE) .to_string(); - // Check if this struct matches our patterns - if !self.matches_struct(&crate_name, &item_name) { + if !self.matches_struct(ctx, item, &crate_name, &item_name) { return; } - // Check trait implementations if needed let def_id = item.owner_id.def_id.to_def_id(); - if !self.has_matching_trait_impl(ctx, def_id) { - return; - } // Create a span that only covers the struct definition line // This includes "pub struct Name {" but not the struct fields or closing brace @@ -292,48 +343,58 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { // Truly private means no visibility keyword at all (inherited visibility) let is_private = !has_visibility_keyword; - // Apply rules - for rule in &self.struct_rules { - match rule { - StructRule::MustBeNamed(pattern, severity) => { - if !self.string_matches_pattern(&item_name, pattern) { - let pattern_type = self.describe_pattern(pattern); - let message = format!( - "Struct must match {pattern_type} '{pattern}', found '{item_name}'" - ); + let evaluate_leaf = |rule: &StructRule| match rule { + StructRule::MustBeNamed(pattern, _) => { + self.string_matches_pattern(&item_name, pattern) + } + StructRule::MustNotBeNamed(pattern, _) => { + !self.string_matches_pattern(&item_name, pattern) + } + StructRule::MustBePrivate(_) => is_private, + StructRule::MustBePublic(_) => is_public, + StructRule::MustBePubCrate(_) => is_pub_crate, + StructRule::ImplementsTrait(pattern, _) => { + self.implements_matching_trait(ctx, def_id, pattern) + } + StructRule::And(_, _) | StructRule::Or(_, _) | StructRule::Not(_) => { + unreachable!("composite rules are evaluated recursively") + } + }; - let help = if pattern_type == "pattern" { - format!("Rename this struct to match the pattern '{pattern}'") - } else { - format!("Rename this struct to '{pattern}'") - }; + for configured_rule in &self.struct_rules { + let mut violations = Vec::new(); + collect_rule_violations(configured_rule, true, &evaluate_leaf, &mut violations); - span_lint_and_help( - ctx, - STRUCT_LINT_MUST_BE_NAMED::get_by_severity(*severity), - self.name().as_str(), - definition_span, - message, - None, - help, - ); - } - } - StructRule::MustNotBeNamed(pattern, severity) => { - if self.string_matches_pattern(&item_name, pattern) { + for (rule, expected) in violations { + match rule { + StructRule::MustBeNamed(pattern, severity) + | StructRule::MustNotBeNamed(pattern, severity) => { + let must_match = + matches!(rule, StructRule::MustBeNamed(..)) == expected; let pattern_type = self.describe_pattern(pattern); - let message = - format!("Struct must not match {pattern_type} '{pattern}'"); - - let help = if pattern_type == "pattern" { - "Choose a name that doesn't match this pattern" + let (lint, message, help) = if must_match { + let help = if pattern_type == "pattern" { + format!("Rename this struct to match the pattern '{pattern}'") + } else { + format!("Rename this struct to '{pattern}'") + }; + ( + STRUCT_LINT_MUST_BE_NAMED::get_by_severity(*severity), + format!( + "Struct must match {pattern_type} '{pattern}', found '{item_name}'" + ), + help, + ) } else { - "Choose a different name for this struct" + ( + STRUCT_LINT_MUST_NOT_BE_NAMED::get_by_severity(*severity), + format!("Struct must not match {pattern_type} '{pattern}'"), + "Choose a name that doesn't match this pattern".to_string(), + ) }; - span_lint_and_help( ctx, - STRUCT_LINT_MUST_NOT_BE_NAMED::get_by_severity(*severity), + lint, self.name().as_str(), definition_span, message, @@ -341,68 +402,190 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { help, ); } - } - StructRule::MustBePrivate(severity) => { - if !is_private { + StructRule::MustBePrivate(severity) => { let visibility_desc = if is_public { "pub" } else if is_pub_crate { "pub(crate)" } else { - "restricted" // pub(super) or pub(in path) + "restricted" + }; + let (message, help) = if expected { + ( + format!( + "Struct '{item_name}' has {visibility_desc} visibility, but must be private" + ), + "Remove the visibility modifier", + ) + } else { + ( + format!("Struct '{item_name}' must not be private"), + "Add an explicit visibility modifier", + ) }; span_lint_and_help( ctx, STRUCT_LINT_MUST_BE_PRIVATE::get_by_severity(*severity), self.name().as_str(), definition_span, - format!( - "Struct '{item_name}' has {visibility_desc} visibility, but must be private" - ), + message, None, - "Remove the visibility modifier", + help, ); } - } - StructRule::MustBePublic(severity) => { - if !is_public { + StructRule::MustBePublic(severity) => { let visibility_desc = if is_pub_crate { "pub(crate)" } else { "private" }; + let (message, help) = if expected { + ( + format!( + "Struct '{item_name}' has {visibility_desc} visibility, but must be pub" + ), + "Change the visibility to 'pub'", + ) + } else { + ( + format!("Struct '{item_name}' must not be pub"), + "Restrict this struct's visibility", + ) + }; span_lint_and_help( ctx, STRUCT_LINT_MUST_BE_PUBLIC::get_by_severity(*severity), self.name().as_str(), definition_span, - format!( - "Struct '{item_name}' has {visibility_desc} visibility, but must be pub" - ), + message, None, - "Change the visibility to 'pub'", + help, ); } - } - StructRule::MustBePubCrate(severity) => { - if !is_pub_crate { + StructRule::MustBePubCrate(severity) => { let visibility_desc = if is_public { "pub" } else { "private" }; + let (message, help) = if expected { + ( + format!( + "Struct '{item_name}' has {visibility_desc} visibility, but must be pub(crate)" + ), + "Change the visibility to 'pub(crate)'", + ) + } else { + ( + format!("Struct '{item_name}' must not be pub(crate)"), + "Change this struct's visibility", + ) + }; span_lint_and_help( ctx, STRUCT_LINT_MUST_BE_PUB_CRATE::get_by_severity(*severity), self.name().as_str(), definition_span, - format!( - "Struct '{item_name}' has {visibility_desc} visibility, but must be pub(crate)" - ), + message, + None, + help, + ); + } + StructRule::ImplementsTrait(pattern, severity) => { + let (message, help) = if expected { + ( + format!( + "Struct '{item_name}' must implement trait matching '{pattern}'" + ), + "Implement the required trait for this struct", + ) + } else { + ( + format!( + "Struct '{item_name}' must not implement trait matching '{pattern}'" + ), + "Remove the forbidden trait implementation", + ) + }; + span_lint_and_help( + ctx, + STRUCT_LINT_IMPLEMENTS_TRAIT::get_by_severity(*severity), + self.name().as_str(), + definition_span, + message, None, - "Change the visibility to 'pub(crate)'", + help, ); } + StructRule::And(_, _) | StructRule::Or(_, _) | StructRule::Not(_) => { + unreachable!("only leaf rule violations are reported") + } } - _ => {} // Ignore other rule types for now } } } } } + +#[cfg(test)] +mod tests { + use super::{attribute_matches, collect_rule_violations, evaluate_rule_with}; + use cargo_pup_lint_config::{Severity, StructRule}; + use regex::Regex; + + fn named(pattern: &str) -> StructRule { + StructRule::MustBeNamed(pattern.to_string(), Severity::Warn) + } + + fn leaf_value(rule: &StructRule) -> bool { + match rule { + StructRule::MustBeNamed(pattern, _) => pattern == "passes", + _ => unreachable!(), + } + } + + #[test] + fn attribute_patterns_match_the_complete_name() { + let regex = Regex::new("repr").unwrap(); + + assert!(attribute_matches(®ex, "repr")); + assert!(!attribute_matches(®ex, "other_repr")); + } + + #[test] + fn evaluates_logical_rule_combinations() { + let pass = named("passes"); + let fail = named("fails"); + + assert!(evaluate_rule_with( + &StructRule::And(Box::new(pass.clone()), Box::new(pass.clone())), + &leaf_value, + )); + assert!(evaluate_rule_with( + &StructRule::Or(Box::new(fail.clone()), Box::new(pass.clone())), + &leaf_value, + )); + assert!(evaluate_rule_with( + &StructRule::Not(Box::new(fail)), + &leaf_value, + )); + } + + #[test] + fn reports_both_failed_or_alternatives() { + let rule = StructRule::Or(Box::new(named("first")), Box::new(named("second"))); + let mut violations = Vec::new(); + + collect_rule_violations(&rule, true, &leaf_value, &mut violations); + + assert_eq!(violations.len(), 2); + assert!(violations.iter().all(|(_, expected)| *expected)); + } + + #[test] + fn inverts_not_rule_expectation() { + let rule = StructRule::Not(Box::new(named("passes"))); + let mut violations = Vec::new(); + + collect_rule_violations(&rule, true, &leaf_value, &mut violations); + + assert_eq!(violations.len(), 1); + assert!(!violations[0].1); + } +} diff --git a/test_app/expected_output b/test_app/expected_output index b01990a..abc14f7 100644 --- a/test_app/expected_output +++ b/test_app/expected_output @@ -70,6 +70,79 @@ note: an unsafe function restricts its caller, but its body is safe by default 68 | pub unsafe fn mutate_unsafe(&mut self, ptr: *const i32) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +warning: Struct must match pattern '^AppReprAllowed', found 'AppReprWrongName' + --> src/configured_struct_rules.rs:7:1 + | +7 | / pub struct AppReprWrongName; +8 | | + | |_^ + | + = help: Rename this struct to match the pattern '^AppReprAllowed' + = note: Applied by cargo-pup rule 'attribute_matcher_check'. + = note: `#[warn(struct_lint_must_be_named)]` on by default + +warning: Struct must match pattern '^AppReprAllowed', found 'AppReprActiveWrongName' + --> src/configured_struct_rules.rs:16:1 + | +16 | / pub struct AppReprActiveWrongName; +17 | | + | |_^ + | + = help: Rename this struct to match the pattern '^AppReprAllowed' + = note: Applied by cargo-pup rule 'attribute_matcher_check'. + +warning: Struct 'AppNeedsTraitMissing' must implement trait matching 'test_app::configured_struct_rules::RequiredTrait' + --> src/configured_struct_rules.rs:18:1 + | +18 | / pub struct AppNeedsTraitMissing; +19 | | + | |_^ + | + = help: Implement the required trait for this struct + = note: Applied by cargo-pup rule 'required_trait_rule_check'. + = note: `#[warn(struct_lint_implements_trait)]` on by default + +warning: Struct 'AppMustAvoidTraitBad' must not implement trait matching 'test_app::configured_struct_rules::ForbiddenTrait' + --> src/configured_struct_rules.rs:23:1 + | +23 | / pub struct AppMustAvoidTraitBad; +24 | | impl ForbiddenTrait for AppMustAvoidTraitBad {} + | |_^ + | + = help: Remove the forbidden trait implementation + = note: Applied by cargo-pup rule 'forbidden_trait_rule_check'. + +warning: Struct must match pattern '^AppAllowedComposite', found 'AppCompositeOrBad' + --> src/configured_struct_rules.rs:28:1 + | +28 | / pub struct AppCompositeOrBad; +29 | | struct AppCompositeOrPrivate; + | |_^ + | + = help: Rename this struct to match the pattern '^AppAllowedComposite' + = note: Applied by cargo-pup rule 'or_struct_rule_check'. + +warning: Struct 'AppCompositeOrBad' has pub visibility, but must be private + --> src/configured_struct_rules.rs:28:1 + | +28 | / pub struct AppCompositeOrBad; +29 | | struct AppCompositeOrPrivate; + | |_^ + | + = help: Remove the visibility modifier + = note: Applied by cargo-pup rule 'or_struct_rule_check'. + = note: `#[warn(struct_lint_must_be_private)]` on by default + +warning: Struct must match pattern '^AppMatcherNegationAllowed', found 'AppMatcherNegationBad' + --> src/configured_struct_rules.rs:32:1 + | +32 | / pub struct AppMatcherNegationBad; +33 | | + | |_^ + | + = help: Rename this struct to match the pattern '^AppMatcherNegationAllowed' + = note: Applied by cargo-pup rule 'negated_trait_matcher_check'. + warning: Item 'DisallowedInModRs' disallowed in mod.rs due to empty-mod-file policy --> src/empty_mod_file/mod.rs:10:1 | @@ -114,14 +187,14 @@ warning: Function exceeds maximum length of 5 lines with 8 lines = note: Applied by cargo-pup rule 'function_length_check'. warning: module 'item_type' is not allowed in this module - --> src/main.rs:9:1 - | -9 | mod item_type; - | ^^^^^^^^^^^^^^ - | - = help: Consider moving this item to a different module - = note: Applied by cargo-pup rule 'item_type_restrictions'. - = note: `#[warn(module_denied_items)]` on by default + --> src/main.rs:10:1 + | +10 | mod item_type; + | ^^^^^^^^^^^^^^ + | + = help: Consider moving this item to a different module + = note: Applied by cargo-pup rule 'item_type_restrictions'. + = note: `#[warn(module_denied_items)]` on by default warning: struct 'DeniedStruct' is not allowed in this module --> src/item_type/mod.rs:4:1 @@ -257,7 +330,6 @@ warning: Struct must match pattern '.*MyTraitImpl$', found 'MyBadlyNamedThing' | = help: Rename this struct to match the pattern '.*MyTraitImpl$' = note: Applied by cargo-pup rule 'trait_restrictions'. - = note: `#[warn(struct_lint_must_be_named)]` on by default warning: Struct 'MyBadlyNamedThing' has pub visibility, but must be private --> src/trait_impl/mod.rs:17:1 @@ -267,7 +339,6 @@ warning: Struct 'MyBadlyNamedThing' has pub visibility, but must be private | = help: Remove the visibility modifier = note: Applied by cargo-pup rule 'trait_restrictions'. - = note: `#[warn(struct_lint_must_be_private)]` on by default error: Function 'with_width' is forbidden by lint rule --> src/builder_style/mod.rs:8:5 @@ -622,5 +693,5 @@ error: Function 'process_unsafe' is forbidden by lint rule = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. For more information about this error, try `rustc --explain E0133`. -warning: `test_app` (bin "test_app") generated 49 warnings -error: could not compile `test_app` (bin "test_app") due to 12 previous errors; 49 warnings emitted +warning: `test_app` (bin "test_app") generated 56 warnings +error: could not compile `test_app` (bin "test_app") due to 12 previous errors; 56 warnings emitted diff --git a/test_app/pup.ron b/test_app/pup.ron index 7dce826..7bb6431 100644 --- a/test_app/pup.ron +++ b/test_app/pup.ron @@ -73,6 +73,41 @@ ImplementsTrait("test_app::trait_impl::MyTrait", Warn), ], )), + Struct(( + name: "attribute_matcher_check", + matches: AndMatches(Name("^AppRepr"), HasAttribute("repr")), + rules: [ + MustBeNamed("^AppReprAllowed", Warn), + ], + )), + Struct(( + name: "required_trait_rule_check", + matches: Name("^AppNeedsTrait"), + rules: [ + ImplementsTrait("test_app::configured_struct_rules::RequiredTrait", Warn), + ], + )), + Struct(( + name: "forbidden_trait_rule_check", + matches: Name("^AppMustAvoidTrait"), + rules: [ + Not(ImplementsTrait("test_app::configured_struct_rules::ForbiddenTrait", Warn)), + ], + )), + Struct(( + name: "or_struct_rule_check", + matches: Name("AppCompositeOr"), + rules: [ + Or(MustBeNamed("^AppAllowedComposite", Warn), MustBePrivate(Warn)), + ], + )), + Struct(( + name: "negated_trait_matcher_check", + matches: AndMatches(Name("^AppMatcherNegation"), NotMatch(ImplementsTrait("test_app::configured_struct_rules::RequiredTrait"))), + rules: [ + MustBeNamed("^AppMatcherNegationAllowed", Warn), + ], + )), Function(( name: "result_type_check", matches: InModule("^test_app::result_error$"), diff --git a/test_app/src/configured_struct_rules.rs b/test_app/src/configured_struct_rules.rs new file mode 100644 index 0000000..28cd70d --- /dev/null +++ b/test_app/src/configured_struct_rules.rs @@ -0,0 +1,35 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +pub trait RequiredTrait {} +pub trait ForbiddenTrait {} + +#[repr(C)] +pub struct AppReprWrongName; + +#[derive(Debug)] +pub struct AppReprDerivedOnlyWrongName; + +#[cfg_attr(any(), repr(C))] +pub struct AppReprInactiveWrongName; + +#[cfg_attr(all(), repr(C))] +pub struct AppReprActiveWrongName; + +pub struct AppNeedsTraitMissing; + +pub struct AppNeedsTraitPresent; +impl RequiredTrait for AppNeedsTraitPresent {} + +pub struct AppMustAvoidTraitBad; +impl ForbiddenTrait for AppMustAvoidTraitBad {} + +pub struct AppMustAvoidTraitGood; + +pub struct AppCompositeOrBad; +struct AppCompositeOrPrivate; +pub struct AppAllowedCompositeOr; + +pub struct AppMatcherNegationBad; + +pub struct AppMatcherNegationImplemented; +impl RequiredTrait for AppMatcherNegationImplemented {} diff --git a/test_app/src/main.rs b/test_app/src/main.rs index 28caf77..8bbc730 100644 --- a/test_app/src/main.rs +++ b/test_app/src/main.rs @@ -3,6 +3,7 @@ #![allow(dead_code)] +mod configured_struct_rules; mod empty_mod; mod empty_mod_file; mod function_length; diff --git a/test_app/tests/pup_ron_test.rs b/test_app/tests/pup_ron_test.rs index cd9bfb1..29cca34 100644 --- a/test_app/tests/pup_ron_test.rs +++ b/test_app/tests/pup_ron_test.rs @@ -4,7 +4,9 @@ //! //! This test verifies that we can build a configuration with pup.ron -use cargo_pup_lint_config::{FunctionLintExt, LintBuilder, ModuleLintExt, Severity, StructLintExt}; +use cargo_pup_lint_config::{ + FunctionLintExt, LintBuilder, ModuleLintExt, Severity, StructLintExt, StructRule, +}; #[test] fn test_lint_config() { @@ -80,7 +82,8 @@ fn test_lint_config() { .build(); // Trait restrictions - builder.struct_lint() + builder + .struct_lint() .lint_named("trait_restrictions") .matching(|m| m.implements_trait("^test_app::trait_impl::MyTrait$")) .with_severity(Severity::Warn) @@ -88,7 +91,61 @@ fn test_lint_config() { .must_be_private() .must_implement_trait("test_app::trait_impl::MyTrait") .build(); - + + // Attribute matching, required and forbidden traits, and logical struct rules. + builder + .struct_lint() + .lint_named("attribute_matcher_check") + .matching(|m| { + m.name("^AppRepr") + .and(m.has_attribute("repr")) + }) + .with_severity(Severity::Warn) + .must_be_named("^AppReprAllowed".into()) + .build(); + + builder + .struct_lint() + .lint_named("required_trait_rule_check") + .matching(|m| m.name("^AppNeedsTrait")) + .with_severity(Severity::Warn) + .must_implement_trait("test_app::configured_struct_rules::RequiredTrait") + .build(); + + builder + .struct_lint() + .lint_named("forbidden_trait_rule_check") + .matching(|m| m.name("^AppMustAvoidTrait")) + .with_severity(Severity::Warn) + .must_not_implement_trait("test_app::configured_struct_rules::ForbiddenTrait") + .build(); + + builder + .struct_lint() + .lint_named("or_struct_rule_check") + .matching(|m| m.name("AppCompositeOr")) + .add_rule(StructRule::Or( + Box::new(StructRule::MustBeNamed( + "^AppAllowedComposite".into(), + Severity::Warn, + )), + Box::new(StructRule::MustBePrivate(Severity::Warn)), + )) + .build(); + + builder + .struct_lint() + .lint_named("negated_trait_matcher_check") + .matching(|m| { + m.name("^AppMatcherNegation").and( + m.implements_trait("test_app::configured_struct_rules::RequiredTrait") + .not(), + ) + }) + .with_severity(Severity::Warn) + .must_be_named("^AppMatcherNegationAllowed".into()) + .build(); + // Result error implementation check builder .function_lint() diff --git a/tests/ui/struct_lint_new/attributes_and_configured_rules.rs b/tests/ui/struct_lint_new/attributes_and_configured_rules.rs new file mode 100644 index 0000000..0ba0f88 --- /dev/null +++ b/tests/ui/struct_lint_new/attributes_and_configured_rules.rs @@ -0,0 +1,46 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +//@compile-flags: --crate-name test_attributes_and_configured_rules +//@compile-flags: --crate-type lib + +pub trait RequiredTrait {} +pub trait ForbiddenTrait {} + +// Matches inert attributes retained after expansion. +#[repr(C)] +pub struct ReprWrongName; //~ ERROR: Struct must match pattern '^ReprAllowed', found 'ReprWrongName' + +// Expansion consumes derive attributes, so they cannot satisfy HasAttribute. +#[derive(Debug)] +pub struct ReprDerivedOnlyWrongName; + +#[cfg_attr(any(), repr(C))] +pub struct ReprInactiveWrongName; + +#[cfg_attr(all(), repr(C))] +pub struct ReprActiveWrongName; //~ ERROR: Struct must match pattern '^ReprAllowed', found 'ReprActiveWrongName' + +// Requires a trait. +pub struct NeedsTraitMissing; //~ ERROR: Struct 'NeedsTraitMissing' must implement trait matching 'test_attributes_and_configured_rules::RequiredTrait' + +pub struct NeedsTraitPresent; +impl RequiredTrait for NeedsTraitPresent {} + +// Forbids a trait. +pub struct MustAvoidTraitBad; //~ ERROR: Struct 'MustAvoidTraitBad' must not implement trait matching 'test_attributes_and_configured_rules::ForbiddenTrait' +impl ForbiddenTrait for MustAvoidTraitBad {} + +pub struct MustAvoidTraitGood; + +// Tests Or. +pub struct CompositeOrBad; //~ ERROR: Struct must match pattern '^AllowedComposite', found 'CompositeOrBad' +//~^ ERROR: Struct 'CompositeOrBad' has pub visibility, but must be private + +struct CompositeOrPrivate; +pub struct AllowedCompositeOr; + +// Tests a negated trait matcher. +pub struct MatcherNegationBad; //~ ERROR: Struct must match pattern '^MatcherNegationAllowed', found 'MatcherNegationBad' + +pub struct MatcherNegationImplemented; +impl RequiredTrait for MatcherNegationImplemented {} diff --git a/tests/ui/struct_lint_new/attributes_and_configured_rules.stderr b/tests/ui/struct_lint_new/attributes_and_configured_rules.stderr new file mode 100644 index 0000000..e446155 --- /dev/null +++ b/tests/ui/struct_lint_new/attributes_and_configured_rules.stderr @@ -0,0 +1,68 @@ +error: Struct must match pattern '^ReprAllowed', found 'ReprWrongName' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:11:1 + | +LL | pub struct ReprWrongName; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern '^ReprAllowed' + = note: Applied by cargo-pup rule 'attribute_matcher_test'. + = note: `#[deny(struct_lint_must_be_named)]` on by default + +error: Struct must match pattern '^ReprAllowed', found 'ReprActiveWrongName' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:21:1 + | +LL | pub struct ReprActiveWrongName; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern '^ReprAllowed' + = note: Applied by cargo-pup rule 'attribute_matcher_test'. + +error: Struct 'NeedsTraitMissing' must implement trait matching 'test_attributes_and_configured_rules::RequiredTrait' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:24:1 + | +LL | pub struct NeedsTraitMissing; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Implement the required trait for this struct + = note: Applied by cargo-pup rule 'required_trait_rule_test'. + = note: `#[deny(struct_lint_implements_trait)]` on by default + +error: Struct 'MustAvoidTraitBad' must not implement trait matching 'test_attributes_and_configured_rules::ForbiddenTrait' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:30:1 + | +LL | pub struct MustAvoidTraitBad; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove the forbidden trait implementation + = note: Applied by cargo-pup rule 'forbidden_trait_rule_test'. + +error: Struct must match pattern '^AllowedComposite', found 'CompositeOrBad' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:36:1 + | +LL | pub struct CompositeOrBad; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern '^AllowedComposite' + = note: Applied by cargo-pup rule 'or_rule_test'. + +error: Struct 'CompositeOrBad' has pub visibility, but must be private + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:36:1 + | +LL | pub struct CompositeOrBad; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove the visibility modifier + = note: Applied by cargo-pup rule 'or_rule_test'. + = note: `#[deny(struct_lint_must_be_private)]` on by default + +error: Struct must match pattern '^MatcherNegationAllowed', found 'MatcherNegationBad' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:43:1 + | +LL | pub struct MatcherNegationBad; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern '^MatcherNegationAllowed' + = note: Applied by cargo-pup rule 'negated_trait_matcher_test'. + +error: aborting due to 7 previous errors + diff --git a/tests/ui/struct_lint_new/pup.ron b/tests/ui/struct_lint_new/pup.ron index 59e0827..23004c8 100644 --- a/tests/ui/struct_lint_new/pup.ron +++ b/tests/ui/struct_lint_new/pup.ron @@ -121,6 +121,67 @@ MustBePubCrate(Error), // Struct must be pub(crate) ] ) - ) + ), + + Struct(( + name: "attribute_matcher_test", + matches: AndMatches( + Name("test_attributes_and_configured_rules"), + HasAttribute("repr"), + ), + rules: [ + MustBeNamed("^ReprAllowed", Error), + ], + )), + + Struct(( + name: "required_trait_rule_test", + matches: AndMatches( + Name("test_attributes_and_configured_rules"), + Name("^NeedsTrait"), + ), + rules: [ + ImplementsTrait("test_attributes_and_configured_rules::RequiredTrait", Error), + ], + )), + + Struct(( + name: "forbidden_trait_rule_test", + matches: AndMatches( + Name("test_attributes_and_configured_rules"), + Name("^MustAvoidTrait"), + ), + rules: [ + Not(ImplementsTrait("test_attributes_and_configured_rules::ForbiddenTrait", Error)), + ], + )), + + Struct(( + name: "or_rule_test", + matches: AndMatches( + Name("test_attributes_and_configured_rules"), + Name("CompositeOr"), + ), + rules: [ + Or( + MustBeNamed("^AllowedComposite", Error), + MustBePrivate(Error), + ), + ], + )), + + Struct(( + name: "negated_trait_matcher_test", + matches: AndMatches( + Name("test_attributes_and_configured_rules"), + AndMatches( + Name("^MatcherNegation"), + NotMatch(ImplementsTrait("test_attributes_and_configured_rules::RequiredTrait")), + ), + ), + rules: [ + MustBeNamed("^MatcherNegationAllowed", Error), + ], + )) ] ) \ No newline at end of file From 0961dbae84ae9f54da02c19d518d3aa220cf4d8d Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 09:25:07 +0200 Subject: [PATCH 02/11] implement logical module rules --- .../src/module_lint/tests.rs | 25 ++ .../src/lints/module_lint/lint.rs | 264 +++++++++++++++++- test_app/expected_output | 34 ++- test_app/pup.ron | 21 ++ test_app/src/logical_module_rules.rs | 21 ++ test_app/src/main.rs | 1 + test_app/tests/pup_ron_test.rs | 40 ++- tests/ui/module_lint_new/logical_rules.rs | 24 ++ tests/ui/module_lint_new/logical_rules.stderr | 32 +++ tests/ui/module_lint_new/pup.ron | 31 ++ 10 files changed, 487 insertions(+), 6 deletions(-) create mode 100644 test_app/src/logical_module_rules.rs create mode 100644 tests/ui/module_lint_new/logical_rules.rs create mode 100644 tests/ui/module_lint_new/logical_rules.stderr diff --git a/cargo_pup_lint_config/src/module_lint/tests.rs b/cargo_pup_lint_config/src/module_lint/tests.rs index f34aeec..0b3da41 100644 --- a/cargo_pup_lint_config/src/module_lint/tests.rs +++ b/cargo_pup_lint_config/src/module_lint/tests.rs @@ -141,6 +141,31 @@ mod builder_tests { } } + #[test] + fn test_logical_module_rules_are_stored() { + let mut builder = LintBuilder::new(); + + builder + .module_lint() + .lint_named("logical_rules") + .matching(|m| m.module("app::core")) + .add_rule(ModuleRule::And( + Box::new(ModuleRule::MustBeNamed("core".into(), Severity::Warn)), + Box::new(ModuleRule::Or( + Box::new(ModuleRule::MustNotBeEmpty(Severity::Warn)), + Box::new(ModuleRule::Not(Box::new( + ModuleRule::NoWildcardImports(Severity::Error), + ))), + )), + )) + .build(); + + let ConfiguredLint::Module(lint) = &builder.lints[0] else { + panic!("Expected module lint"); + }; + assert!(matches!(&lint.rules[0], ModuleRule::And(_, _))); + } + #[test] fn test_complex_module_matcher() { let mut builder = LintBuilder::new(); diff --git a/cargo_pup_lint_impl/src/lints/module_lint/lint.rs b/cargo_pup_lint_impl/src/lints/module_lint/lint.rs index c031542..ccefaaf 100644 --- a/cargo_pup_lint_impl/src/lints/module_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/module_lint/lint.rs @@ -16,6 +16,61 @@ pub struct ModuleLint { config: ConfigModuleLint, } +fn evaluate_module_rule_with( + rule: &ModuleRule, + evaluate_leaf: &impl Fn(&ModuleRule) -> Option, +) -> Option { + let combine = + |left: Option, right: Option, operation: fn(bool, bool) -> bool| match ( + left, right, + ) { + (Some(left), Some(right)) => Some(operation(left, right)), + (Some(result), None) | (None, Some(result)) => Some(result), + (None, None) => None, + }; + + match rule { + ModuleRule::And(left, right) => combine( + evaluate_module_rule_with(left, evaluate_leaf), + evaluate_module_rule_with(right, evaluate_leaf), + |left, right| left && right, + ), + ModuleRule::Or(left, right) => combine( + evaluate_module_rule_with(left, evaluate_leaf), + evaluate_module_rule_with(right, evaluate_leaf), + |left, right| left || right, + ), + ModuleRule::Not(inner) => { + evaluate_module_rule_with(inner, evaluate_leaf).map(|value| !value) + } + leaf => evaluate_leaf(leaf), + } +} + +fn module_rule_severity(rule: &ModuleRule) -> Severity { + match rule { + ModuleRule::MustBeNamed(_, severity) + | ModuleRule::MustNotBeNamed(_, severity) + | ModuleRule::MustNotBeEmpty(severity) + | ModuleRule::MustBeEmpty(severity) + | ModuleRule::MustHaveEmptyModFile(severity) + | ModuleRule::NoWildcardImports(severity) => *severity, + ModuleRule::RestrictImports { severity, .. } | ModuleRule::DeniedItems { severity, .. } => { + *severity + } + ModuleRule::And(left, right) | ModuleRule::Or(left, right) => { + if module_rule_severity(left) == Severity::Error + || module_rule_severity(right) == Severity::Error + { + Severity::Error + } else { + Severity::Warn + } + } + ModuleRule::Not(inner) => module_rule_severity(inner), + } +} + impl ModuleLint { #[allow(clippy::new_ret_no_self)] pub fn new(config: &ConfiguredLint) -> Box { @@ -159,6 +214,126 @@ impl ModuleLint { None } + fn item_type<'a>(&self, ctx: &LateContext<'_>, item: &'a Item<'_>) -> &'a str { + match &item.kind { + ItemKind::Enum(..) => "enum", + ItemKind::Struct(..) => "struct", + ItemKind::Trait(..) => "trait", + ItemKind::Impl(..) => "impl", + ItemKind::Fn { .. } => self.get_proc_macro_type(ctx, item).unwrap_or("function"), + ItemKind::Mod(..) => "module", + ItemKind::Static(..) => "static", + ItemKind::Const(..) => "const", + ItemKind::Union(..) => "union", + ItemKind::Macro(..) => "declarative_macro", + _ => "", + } + } + + fn evaluate_module_rule( + &self, + ctx: &LateContext<'_>, + item: &Item<'_>, + rule: &ModuleRule, + ) -> Option { + evaluate_module_rule_with(rule, &|leaf| { + self.evaluate_module_rule_leaf(ctx, item, leaf) + }) + } + + fn evaluate_module_rule_leaf( + &self, + ctx: &LateContext<'_>, + item: &Item<'_>, + rule: &ModuleRule, + ) -> Option { + match rule { + ModuleRule::MustBeNamed(pattern, _) => match item.kind { + ItemKind::Mod(..) => Some( + self.string_matches_pattern( + &ctx.tcx + .item_name(item.owner_id.def_id.to_def_id()) + .to_string(), + pattern, + ), + ), + _ => None, + }, + ModuleRule::MustNotBeNamed(pattern, _) => match item.kind { + ItemKind::Mod(..) => Some( + !self.string_matches_pattern( + &ctx.tcx + .item_name(item.owner_id.def_id.to_def_id()) + .to_string(), + pattern, + ), + ), + _ => None, + }, + ModuleRule::MustNotBeEmpty(_) => match item.kind { + ItemKind::Mod(_, module) => Some(!module.item_ids.is_empty()), + _ => None, + }, + ModuleRule::MustBeEmpty(_) => match item.kind { + ItemKind::Mod(_, module) => Some(module.item_ids.iter().all(|item_id| { + !self.is_disallowed_in_empty_module(&ctx.tcx.hir_item(*item_id).kind) + })), + _ => None, + }, + ModuleRule::MustHaveEmptyModFile(_) => match item.kind { + ItemKind::Mod(_, module) => Some(module.item_ids.iter().all(|item_id| { + let nested_item = ctx.tcx.hir_item(*item_id); + !self.is_disallowed_in_empty_module(&nested_item.kind) + || !self.is_mod_rs_file(ctx, &nested_item.span) + })), + _ => None, + }, + ModuleRule::RestrictImports { + allowed_only, + denied, + .. + } => match &item.kind { + ItemKind::Use(path, _) => { + let import_module = path + .segments + .iter() + .map(|segment| segment.ident.as_str().to_string()) + .collect::>() + .join("::"); + let allowed = allowed_only.as_ref().is_none_or(|patterns| { + patterns + .iter() + .any(|pattern| self.string_matches_pattern(&import_module, pattern)) + }); + let denied = denied.as_ref().is_some_and(|patterns| { + patterns + .iter() + .any(|pattern| self.string_matches_pattern(&import_module, pattern)) + }); + Some(allowed && !denied) + } + _ => None, + }, + ModuleRule::NoWildcardImports(_) => match &item.kind { + ItemKind::Use(_, use_kind) => Some(!matches!(use_kind, UseKind::Glob)), + ItemKind::Mod(_, module) => Some(module.item_ids.iter().all(|item_id| { + !matches!( + ctx.tcx.hir_item(*item_id).kind, + ItemKind::Use(_, UseKind::Glob) + ) + })), + _ => None, + }, + ModuleRule::DeniedItems { items, .. } => { + let item_type = self.item_type(ctx, item); + (!item_type.is_empty()).then(|| !items.iter().any(|denied| denied == item_type)) + } + ModuleRule::And(_, _) | ModuleRule::Or(_, _) | ModuleRule::Not(_) => { + unreachable!("composite rules are evaluated recursively") + } + } + } + // Helper function to check for disallowed items in a module and call the callback when found fn check_for_disallowed_items( &self, @@ -260,6 +435,14 @@ declare_variable_severity_lint!( "Module's mod.rs file must be empty (only allowed to re-export other modules)" ); +declare_variable_severity_lint!( + pub, + MODULE_LOGICAL_RULE, + MODULE_LOGICAL_RULE_LINT_DENY, + MODULE_LOGICAL_RULE_LINT_WARN, + "Module must satisfy its configured logical rule" +); + impl_lint_pass!(ModuleLint => [ MODULE_MUST_BE_NAMED_LINT_DENY, MODULE_MUST_BE_NAMED_LINT_WARN, MODULE_MUST_NOT_BE_NAMED_LINT_DENY, MODULE_MUST_NOT_BE_NAMED_LINT_WARN, @@ -268,7 +451,8 @@ impl_lint_pass!(ModuleLint => [ MODULE_MUST_HAVE_EMPTY_MOD_FILE_LINT_DENY, MODULE_MUST_HAVE_EMPTY_MOD_FILE_LINT_WARN, MODULE_RESTRICT_IMPORTS_LINT_DENY, MODULE_RESTRICT_IMPORTS_LINT_WARN, MODULE_WILDCARD_IMPORT_LINT_DENY, MODULE_WILDCARD_IMPORT_LINT_WARN, - MODULE_DENIED_ITEMS_LINT_DENY, MODULE_DENIED_ITEMS_LINT_WARN + MODULE_DENIED_ITEMS_LINT_DENY, MODULE_DENIED_ITEMS_LINT_WARN, + MODULE_LOGICAL_RULE_LINT_DENY, MODULE_LOGICAL_RULE_LINT_WARN ]); impl ArchitectureLintRule for ModuleLint { @@ -545,9 +729,83 @@ impl<'tcx> LateLintPass<'tcx> for ModuleLint { ); } } - // Skip logical combinations for now (And, Or, Not) - ModuleRule::And(_, _) | ModuleRule::Or(_, _) | ModuleRule::Not(_) => {} + ModuleRule::And(_, _) | ModuleRule::Or(_, _) | ModuleRule::Not(_) => { + if self.evaluate_module_rule(ctx, item, rule) == Some(false) { + span_lint_and_help( + ctx, + MODULE_LOGICAL_RULE::get_by_severity(module_rule_severity(rule)), + self.name().as_str(), + item.span, + "Item does not satisfy the configured logical module rule", + None, + "Satisfy at least the required combination of module constraints", + ); + } + } } } } } + +#[cfg(test)] +mod tests { + use super::{evaluate_module_rule_with, module_rule_severity}; + use cargo_pup_lint_config::{ModuleRule, Severity}; + + fn named(name: &str, severity: Severity) -> ModuleRule { + ModuleRule::MustBeNamed(name.to_string(), severity) + } + + fn leaf_value(rule: &ModuleRule) -> Option { + match rule { + ModuleRule::MustBeNamed(name, _) => Some(name == "passes"), + ModuleRule::MustNotBeEmpty(_) => None, + _ => unreachable!(), + } + } + + #[test] + fn evaluates_and_or_and_not_rules() { + let pass = named("passes", Severity::Warn); + let fail = named("fails", Severity::Warn); + + assert_eq!( + evaluate_module_rule_with( + &ModuleRule::And(Box::new(pass.clone()), Box::new(fail.clone())), + &leaf_value, + ), + Some(false) + ); + assert_eq!( + evaluate_module_rule_with( + &ModuleRule::Or(Box::new(pass.clone()), Box::new(fail.clone())), + &leaf_value, + ), + Some(true) + ); + assert_eq!( + evaluate_module_rule_with(&ModuleRule::Not(Box::new(pass)), &leaf_value), + Some(false) + ); + } + + #[test] + fn ignores_inapplicable_leaves_without_hiding_applicable_results() { + let rule = ModuleRule::And( + Box::new(named("passes", Severity::Warn)), + Box::new(ModuleRule::MustNotBeEmpty(Severity::Warn)), + ); + + assert_eq!(evaluate_module_rule_with(&rule, &leaf_value), Some(true)); + } + + #[test] + fn logical_rule_uses_strongest_nested_severity() { + let rule = ModuleRule::Or( + Box::new(named("passes", Severity::Warn)), + Box::new(named("fails", Severity::Error)), + ); + + assert_eq!(module_rule_severity(&rule), Severity::Error); + } +} diff --git a/test_app/expected_output b/test_app/expected_output index abc14f7..4f3c424 100644 --- a/test_app/expected_output +++ b/test_app/expected_output @@ -241,6 +241,36 @@ warning: module 'nested' is not allowed in this module = help: Consider moving this item to a different module = note: Applied by cargo-pup rule 'item_type_restrictions'. +warning: Item does not satisfy the configured logical module rule + --> src/logical_module_rules.rs:3:1 + | +3 | pub mod and_bad {} + | ^^^^^^^^^^^^^^^^^^ + | + = help: Satisfy at least the required combination of module constraints + = note: Applied by cargo-pup rule 'logical_module_and_check'. + = note: `#[warn(module_logical_rule)]` on by default + +warning: Item does not satisfy the configured logical module rule + --> src/logical_module_rules.rs:15:1 + | +15 | pub mod or_bad {} + | ^^^^^^^^^^^^^^^^^ + | + = help: Satisfy at least the required combination of module constraints + = note: Applied by cargo-pup rule 'logical_module_or_check'. + +warning: Item does not satisfy the configured logical module rule + --> src/logical_module_rules.rs:19:1 + | +19 | / pub mod not_bad { +20 | | pub const VALUE: usize = 1; +21 | | } + | |_^ + | + = help: Satisfy at least the required combination of module constraints + = note: Applied by cargo-pup rule 'logical_module_not_check'. + warning: declarative macro 'forbidden_macro' is not allowed in this module --> src/macro_restriction/mod.rs:4:1 | @@ -693,5 +723,5 @@ error: Function 'process_unsafe' is forbidden by lint rule = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. For more information about this error, try `rustc --explain E0133`. -warning: `test_app` (bin "test_app") generated 56 warnings -error: could not compile `test_app` (bin "test_app") due to 12 previous errors; 56 warnings emitted +warning: `test_app` (bin "test_app") generated 59 warnings +error: could not compile `test_app` (bin "test_app") due to 12 previous errors; 59 warnings emitted diff --git a/test_app/pup.ron b/test_app/pup.ron index 7bb6431..7cc44a9 100644 --- a/test_app/pup.ron +++ b/test_app/pup.ron @@ -64,6 +64,27 @@ ), ], )), + Module(( + name: "logical_module_and_check", + matches: Module("^test_app::logical_module_rules::and_"), + rules: [ + And(MustBeNamed("^and_good$", Warn), MustNotBeEmpty(Warn)), + ], + )), + Module(( + name: "logical_module_or_check", + matches: Module("^test_app::logical_module_rules::or_"), + rules: [ + Or(MustBeNamed("^or_named$", Warn), MustNotBeEmpty(Warn)), + ], + )), + Module(( + name: "logical_module_not_check", + matches: Module("^test_app::logical_module_rules::(must_be_empty|not_bad)$"), + rules: [ + Not(MustNotBeEmpty(Warn)), + ], + )), Struct(( name: "trait_restrictions", matches: ImplementsTrait("^test_app::trait_impl::MyTrait$"), diff --git a/test_app/src/logical_module_rules.rs b/test_app/src/logical_module_rules.rs new file mode 100644 index 0000000..6741961 --- /dev/null +++ b/test_app/src/logical_module_rules.rs @@ -0,0 +1,21 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +pub mod and_bad {} + +pub mod and_good { + pub const VALUE: usize = 1; +} + +pub mod or_named {} + +pub mod or_content { + pub const VALUE: usize = 1; +} + +pub mod or_bad {} + +pub mod must_be_empty {} + +pub mod not_bad { + pub const VALUE: usize = 1; +} diff --git a/test_app/src/main.rs b/test_app/src/main.rs index 8bbc730..3cd0d24 100644 --- a/test_app/src/main.rs +++ b/test_app/src/main.rs @@ -8,6 +8,7 @@ mod empty_mod; mod empty_mod_file; mod function_length; mod item_type; +mod logical_module_rules; mod macro_restriction; mod module_usage; mod must_be_empty; diff --git a/test_app/tests/pup_ron_test.rs b/test_app/tests/pup_ron_test.rs index 29cca34..395007f 100644 --- a/test_app/tests/pup_ron_test.rs +++ b/test_app/tests/pup_ron_test.rs @@ -5,7 +5,7 @@ //! This test verifies that we can build a configuration with pup.ron use cargo_pup_lint_config::{ - FunctionLintExt, LintBuilder, ModuleLintExt, Severity, StructLintExt, StructRule, + FunctionLintExt, LintBuilder, ModuleLintExt, ModuleRule, Severity, StructLintExt, StructRule, }; #[test] @@ -81,6 +81,44 @@ fn test_lint_config() { ]) .build(); + // Logical module rule combinations. + builder + .module_lint() + .lint_named("logical_module_and_check") + .matching(|m| m.module("^test_app::logical_module_rules::and_")) + .add_rule(ModuleRule::And( + Box::new(ModuleRule::MustBeNamed( + "^and_good$".into(), + Severity::Warn, + )), + Box::new(ModuleRule::MustNotBeEmpty(Severity::Warn)), + )) + .build(); + + builder + .module_lint() + .lint_named("logical_module_or_check") + .matching(|m| m.module("^test_app::logical_module_rules::or_")) + .add_rule(ModuleRule::Or( + Box::new(ModuleRule::MustBeNamed( + "^or_named$".into(), + Severity::Warn, + )), + Box::new(ModuleRule::MustNotBeEmpty(Severity::Warn)), + )) + .build(); + + builder + .module_lint() + .lint_named("logical_module_not_check") + .matching(|m| { + m.module("^test_app::logical_module_rules::(must_be_empty|not_bad)$") + }) + .add_rule(ModuleRule::Not(Box::new( + ModuleRule::MustNotBeEmpty(Severity::Warn), + ))) + .build(); + // Trait restrictions builder .struct_lint() diff --git a/tests/ui/module_lint_new/logical_rules.rs b/tests/ui/module_lint_new/logical_rules.rs new file mode 100644 index 0000000..2d89148 --- /dev/null +++ b/tests/ui/module_lint_new/logical_rules.rs @@ -0,0 +1,24 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +//@compile-flags: --crate-name test_logical_module_rules +//@compile-flags: --crate-type lib + +pub mod and_bad {} //~ ERROR: Item does not satisfy the configured logical module rule + +pub mod and_good { + pub const VALUE: usize = 1; +} + +pub mod or_named {} + +pub mod or_content { + pub const VALUE: usize = 1; +} + +pub mod or_bad {} //~ ERROR: Item does not satisfy the configured logical module rule + +pub mod must_be_empty {} + +pub mod not_bad { //~ ERROR: Item does not satisfy the configured logical module rule + pub const VALUE: usize = 1; +} diff --git a/tests/ui/module_lint_new/logical_rules.stderr b/tests/ui/module_lint_new/logical_rules.stderr new file mode 100644 index 0000000..f3fbecf --- /dev/null +++ b/tests/ui/module_lint_new/logical_rules.stderr @@ -0,0 +1,32 @@ +error: Item does not satisfy the configured logical module rule + --> tests/ui/module_lint_new/logical_rules.rs:6:1 + | +LL | pub mod and_bad {} + | ^^^^^^^^^^^^^^^^^^ + | + = help: Satisfy at least the required combination of module constraints + = note: Applied by cargo-pup rule 'logical_and_test'. + = note: `#[deny(module_logical_rule)]` on by default + +error: Item does not satisfy the configured logical module rule + --> tests/ui/module_lint_new/logical_rules.rs:19:1 + | +LL | pub mod or_bad {} + | ^^^^^^^^^^^^^^^^^ + | + = help: Satisfy at least the required combination of module constraints + = note: Applied by cargo-pup rule 'logical_or_test'. + +error: Item does not satisfy the configured logical module rule + --> tests/ui/module_lint_new/logical_rules.rs:23:1 + | +LL | / pub mod not_bad { +LL | | pub const VALUE: usize = 1; +LL | | } + | |_^ + | + = help: Satisfy at least the required combination of module constraints + = note: Applied by cargo-pup rule 'logical_not_test'. + +error: aborting due to 3 previous errors + diff --git a/tests/ui/module_lint_new/pup.ron b/tests/ui/module_lint_new/pup.ron index a5a6c61..c94e22b 100644 --- a/tests/ui/module_lint_new/pup.ron +++ b/tests/ui/module_lint_new/pup.ron @@ -144,6 +144,37 @@ ) ), + // Logical module rule combinations + Module(( + name: "logical_and_test", + matches: Module("^test_logical_module_rules::and_"), + rules: [ + And( + MustBeNamed("^and_good$", Error), + MustNotBeEmpty(Error), + ), + ], + )), + + Module(( + name: "logical_or_test", + matches: Module("^test_logical_module_rules::or_"), + rules: [ + Or( + MustBeNamed("^or_named$", Error), + MustNotBeEmpty(Error), + ), + ], + )), + + Module(( + name: "logical_not_test", + matches: Module("^test_logical_module_rules::(must_be_empty|not_bad)$"), + rules: [ + Not(MustNotBeEmpty(Error)), + ], + )), + // Replace RestrictFunctions with Function lint Function( ( From 309a25a93152e7f2ed70c157bdc19777e9741f29 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 10:16:22 +0200 Subject: [PATCH 03/11] remove unused panicking config generator --- cargo_pup_lint_impl/src/lints/configuration_factory.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cargo_pup_lint_impl/src/lints/configuration_factory.rs b/cargo_pup_lint_impl/src/lints/configuration_factory.rs index e46bbff..b55078b 100644 --- a/cargo_pup_lint_impl/src/lints/configuration_factory.rs +++ b/cargo_pup_lint_impl/src/lints/configuration_factory.rs @@ -5,7 +5,6 @@ use crate::lints::function_lint::FunctionLint; use crate::lints::module_lint::ModuleLint; use crate::lints::struct_lint::StructLint; use anyhow::Result; -use cargo_pup_common::project_context::ProjectContext; use cargo_pup_lint_config::ConfiguredLint; use cargo_pup_lint_config::lint_builder::LintBuilder; use ron; @@ -66,8 +65,4 @@ impl LintConfigurationFactory { }) .collect()) } - - pub fn generate_file(_context: &ProjectContext) -> Result { - panic!("Not implemented!"); - } } From e70213afa315157f880aa796dd5cf10234e118df Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 10:17:27 +0200 Subject: [PATCH 04/11] deduplicate aggregated project context --- cargo_pup_common/src/project_context.rs | 91 ++++++++++++++++++-- test_app/Cargo.lock | 1 + test_app/Cargo.toml | 1 + test_app/tests/project_context_dedup_test.rs | 61 +++++++++++++ 4 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 test_app/tests/project_context_dedup_test.rs diff --git a/cargo_pup_common/src/project_context.rs b/cargo_pup_common/src/project_context.rs index 16eff56..f7aa083 100644 --- a/cargo_pup_common/src/project_context.rs +++ b/cargo_pup_common/src/project_context.rs @@ -2,6 +2,7 @@ use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; use std::fs::{self, OpenOptions}; use std::path::{Path, PathBuf}; @@ -284,13 +285,52 @@ impl ProjectContext { self.traits.extend(other.traits.clone()); } - /// Sorts modules and traits for consistent ordering + /// Merge duplicate modules and traits while preserving all discovered metadata. fn deduplicate(&mut self) { - // Sort modules by name - self.modules.sort_by(|a, b| a.name.cmp(&b.name)); + let mut modules = BTreeMap::::new(); + for module in self.modules.drain(..) { + modules + .entry(module.name.clone()) + .and_modify(|existing| { + existing + .applicable_lints + .extend(module.applicable_lints.clone()); + }) + .or_insert(module); + } + self.modules = modules + .into_values() + .map(|mut module| { + module.applicable_lints.sort(); + module.applicable_lints.dedup(); + module + }) + .collect(); - // Sort traits by name - self.traits.sort_by(|a, b| a.name.cmp(&b.name)); + let mut traits = BTreeMap::::new(); + for trait_info in self.traits.drain(..) { + traits + .entry(trait_info.name.clone()) + .and_modify(|existing| { + existing + .implementors + .extend(trait_info.implementors.clone()); + existing + .applicable_lints + .extend(trait_info.applicable_lints.clone()); + }) + .or_insert(trait_info); + } + self.traits = traits + .into_values() + .map(|mut trait_info| { + trait_info.implementors.sort(); + trait_info.implementors.dedup(); + trait_info.applicable_lints.sort(); + trait_info.applicable_lints.dedup(); + trait_info + }) + .collect(); } } @@ -393,6 +433,47 @@ mod tests { assert!(context.modules[1].applicable_lints.is_empty()); } + #[test] + fn deduplicate_merges_context_metadata() { + let mut context = ProjectContext::new(); + context.modules = vec![ + ModuleInfo { + name: "crate::module".into(), + applicable_lints: vec!["lint_b".into(), "lint_a".into()], + }, + ModuleInfo { + name: "crate::module".into(), + applicable_lints: vec!["lint_a".into(), "lint_c".into()], + }, + ]; + context.traits = vec![ + TraitInfo { + name: "crate::Trait".into(), + implementors: vec!["TypeB".into(), "TypeA".into()], + applicable_lints: vec!["lint_b".into()], + }, + TraitInfo { + name: "crate::Trait".into(), + implementors: vec!["TypeA".into(), "TypeC".into()], + applicable_lints: vec!["lint_a".into(), "lint_b".into()], + }, + ]; + + context.deduplicate(); + + assert_eq!(context.modules.len(), 1); + assert_eq!( + context.modules[0].applicable_lints, + vec!["lint_a", "lint_b", "lint_c"] + ); + assert_eq!(context.traits.len(), 1); + assert_eq!( + context.traits[0].implementors, + vec!["TypeA", "TypeB", "TypeC"] + ); + assert_eq!(context.traits[0].applicable_lints, vec!["lint_a", "lint_b"]); + } + #[test] fn roundtrip_through_files() { use tempfile::TempDir; diff --git a/test_app/Cargo.lock b/test_app/Cargo.lock index 8b0f8f1..152878e 100644 --- a/test_app/Cargo.lock +++ b/test_app/Cargo.lock @@ -327,6 +327,7 @@ name = "test_app" version = "0.1.0" dependencies = [ "anyhow", + "cargo_pup_common", "cargo_pup_lint_config", "proc_macro_test", "rand", diff --git a/test_app/Cargo.toml b/test_app/Cargo.toml index 92c3696..c872cae 100644 --- a/test_app/Cargo.toml +++ b/test_app/Cargo.toml @@ -12,4 +12,5 @@ rand = "0.8.5" proc_macro_test = { path = "proc_macro_test" } [dev-dependencies] +cargo_pup_common = { path = "../cargo_pup_common" } cargo_pup_lint_config = { path = "../cargo_pup_lint_config" } diff --git a/test_app/tests/project_context_dedup_test.rs b/test_app/tests/project_context_dedup_test.rs new file mode 100644 index 0000000..5863159 --- /dev/null +++ b/test_app/tests/project_context_dedup_test.rs @@ -0,0 +1,61 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +use cargo_pup_common::project_context::{ModuleInfo, ProjectContext, TraitInfo}; +use std::time::{SystemTime, UNIX_EPOCH}; + +#[test] +fn merged_workspace_context_deduplicates_modules_and_traits() { + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let context_dir = std::env::temp_dir().join(format!("cargo-pup-context-{unique}")); + + let mut first = ProjectContext::with_base_dir(&context_dir); + first.module_root = "first_crate".into(); + first.modules = vec![ModuleInfo { + name: "shared::module".into(), + applicable_lints: vec!["first_lint".into()], + }]; + first.traits = vec![TraitInfo { + name: "shared::Trait".into(), + implementors: vec!["FirstType".into()], + applicable_lints: vec!["first_lint".into()], + }]; + + let mut second = ProjectContext::with_base_dir(&context_dir); + second.module_root = "second_crate".into(); + second.modules = vec![ModuleInfo { + name: "shared::module".into(), + applicable_lints: vec!["second_lint".into()], + }]; + second.traits = vec![TraitInfo { + name: "shared::Trait".into(), + implementors: vec!["SecondType".into()], + applicable_lints: vec!["second_lint".into()], + }]; + + first.serialize_to_file().unwrap(); + second.serialize_to_file().unwrap(); + + let (merged, crate_names) = + ProjectContext::load_all_contexts_from_dir(&context_dir).unwrap(); + + assert_eq!(crate_names.len(), 2); + assert_eq!(merged.modules.len(), 1); + assert_eq!( + merged.modules[0].applicable_lints, + vec!["first_lint", "second_lint"] + ); + assert_eq!(merged.traits.len(), 1); + assert_eq!( + merged.traits[0].implementors, + vec!["FirstType", "SecondType"] + ); + assert_eq!( + merged.traits[0].applicable_lints, + vec!["first_lint", "second_lint"] + ); + + std::fs::remove_dir_all(context_dir).unwrap(); +} From 943a8ed6b3ecf54a8b3a1ec4d38fe46c229c017e Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 10:24:05 +0200 Subject: [PATCH 05/11] run CI tests with nextest --- .github/workflows/pr.yaml | 6 ++++-- README.md | 15 +++++++++++++-- scripts/test.sh | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100755 scripts/test.sh diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 2a22c96..b3fe9f2 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -17,10 +17,12 @@ jobs: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - run: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y - run: . $HOME/.cargo/env && rustup update + - name: Install cargo-nextest + run: . $HOME/.cargo/env && cargo install cargo-nextest --version 0.9.145 --locked - run: . $HOME/.cargo/env && cargo build --release --all - - run: . $HOME/.cargo/env && cargo test + - name: Run tests + run: . $HOME/.cargo/env && scripts/test.sh - name: Check formatting run: . $HOME/.cargo/env && cargo fmt --all -- --check - name: Check clippy lints run: . $HOME/.cargo/env && cargo clippy --all-targets --all-features -- -D warnings - - run: test_app/scripts/validate-against-expected.sh diff --git a/README.md b/README.md index 0833ab2..221cf35 100644 --- a/README.md +++ b/README.md @@ -171,11 +171,22 @@ To see this in action, check out [test_app](test_app) which uses this style of c ## How It Works cargo_pup uses `rustc`'s interface to bolt custom, dynamically defined lints into the compilation lifecycle. To do this, much like clippy and other tools that extend the compiler in this fashion, it has to compile your code using rust nightly. The output of this build is discrete from your regular build, and gets hidden in `.pup` within the project directory. +### Tests + +The complete local test suite uses [cargo-nextest](https://nexte.st/) for standard tests and Cargo's test runner for doctests and the custom UI test harness. Install the pinned Nextest version, then run the same script used by CI: + +```bash +cargo install cargo-nextest --version 0.9.145 --locked +scripts/test.sh +``` + +The script builds the local `cargo-pup` and `pup-driver` binaries, runs all workspace tests, and validates `test_app` against its expected compiler output. It also clears `test_app/.pup` before end-to-end validation so stale compiler output is not reused. + ### UI Tests -Cargo Pup includes UI tests to validate lint behavior. These tests follow the pattern used by Clippy and other Rust compiler components. +Cargo Pup includes UI tests to validate lint behaviour. These tests follow the pattern used by Clippy and other Rust compiler components. The UI suite uses a custom test harness, so it is run separately from Nextest. -To run the UI tests: +To run only the UI tests: ```bash cargo test --test ui-test diff --git a/scripts/test.sh b/scripts/test.sh new file mode 100755 index 0000000..7574da8 --- /dev/null +++ b/scripts/test.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +# This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO_ROOT" + +if ! cargo nextest --version >/dev/null 2>&1; then + echo "cargo-nextest is required. Install it with:" >&2 + echo " cargo install cargo-nextest --version 0.9.145 --locked" >&2 + exit 1 +fi + +# Build the binaries explicitly because integration tests and test_app resolve +# cargo-pup and pup-driver from target/debug rather than Cargo's test artefacts. +cargo build --bins + +# Exclude only the custom UI harness so future standard tests remain included. +cargo nextest run --workspace -E 'not binary(ui-test)' + +# Nextest does not execute doctests or custom test harnesses. +cargo test --workspace --doc +cargo test --test ui-test + +# test_app is a separate workspace with its own toolchain and ordinary tests. +( + cd test_app + cargo nextest run +) + +# Force the end-to-end validation to compile with the binaries built above. +# Its first run warms this cache, and its second run compares stable output. +rm -rf test_app/.pup +test_app/scripts/validate-against-expected.sh From cc53181f565ff29b32ec6e656ccc390787a54231 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 10:30:34 +0200 Subject: [PATCH 06/11] upgrade dependencies --- Cargo.lock | 294 +++++++++++++++------------- Cargo.toml | 16 +- test_app/Cargo.lock | 162 +++++++-------- test_app/Cargo.toml | 4 +- test_app/proc_macro_test/Cargo.toml | 2 +- 5 files changed, 243 insertions(+), 235 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 42ef800..5873837 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19,9 +19,9 @@ checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" [[package]] name = "aho-corasick" -version = "1.1.4" +version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +checksum = "c982642fa9e8606056828ee9a8505737230110bb1099153c79efe865c59d12ba" dependencies = [ "memchr", ] @@ -53,9 +53,9 @@ checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" [[package]] name = "anyhow" -version = "1.0.102" +version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" [[package]] name = "backtrace" @@ -74,72 +74,48 @@ dependencies = [ [[package]] name = "bitflags" -version = "2.11.0" +version = "2.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "843867be96c8daad0d758b57df9392b6d8d271134fce549de6ce169ff98a92af" +checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06" dependencies = [ "serde_core", ] [[package]] name = "bstr" -version = "1.12.1" +version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63044e1ae8e69f3b5a92c736ca6269b8d12fa7efe39bf34ddb06d102cf0e2cab" +checksum = "1f7dc094d718f2e1c1559ad110e27eeaae14a5465d3d56dd6dbd793079fbd530" dependencies = [ "memchr", "regex-automata", - "serde", + "serde_core", ] [[package]] name = "bumpalo" -version = "3.20.2" +version = "3.20.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" +checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" [[package]] name = "camino" -version = "1.2.2" +version = "1.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e629a66d692cb9ff1a1c664e41771b3dcaf961985a9774c0eb0bd1b51cf60a48" +checksum = "bb1307f12aa967b5a58416e87b3653360e0fd614a016b6e970db08fecbb1b80d" dependencies = [ "serde_core", ] [[package]] name = "cargo-platform" -version = "0.1.9" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e35af189006b9c0f00a064685c727031e3ed2d8020f7ba284d78cc2671bd36ea" +checksum = "122ec45a44b270afd1402f351b782c676b173e3c3fb28d86ff7ebfb4d86a4ee4" dependencies = [ "serde", ] -[[package]] -name = "cargo-platform" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87a0c0e6148f11f01f32650a2ea02d532b2ad4e81d8bd41e6e565b5adc5e6082" -dependencies = [ - "serde", - "serde_core", -] - -[[package]] -name = "cargo_metadata" -version = "0.18.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d886547e41f740c616ae73108f6eb70afe6d940c7bc697cb30f13daec073037" -dependencies = [ - "camino", - "cargo-platform 0.1.9", - "semver", - "serde", - "serde_json", - "thiserror 1.0.69", -] - [[package]] name = "cargo_metadata" version = "0.23.1" @@ -147,11 +123,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef987d17b0a113becdd19d3d0022d04d7ef41f9efe4f3fb63ac44ba61df3ade9" dependencies = [ "camino", - "cargo-platform 0.3.2", + "cargo-platform", "semver", "serde", "serde_json", - "thiserror 2.0.18", + "thiserror 2.0.20", ] [[package]] @@ -160,7 +136,7 @@ version = "0.1.8" dependencies = [ "ansi_term", "anyhow", - "cargo_metadata 0.23.1", + "cargo_metadata", "cargo_pup_common", "cargo_pup_lint_config", "cargo_pup_lint_impl", @@ -178,7 +154,7 @@ name = "cargo_pup_common" version = "0.1.8" dependencies = [ "anyhow", - "cargo_metadata 0.23.1", + "cargo_metadata", "serde", "serde_json", "tempfile", @@ -209,9 +185,9 @@ dependencies = [ [[package]] name = "cfg-if" -version = "1.0.4" +version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +checksum = "4e7648175b45a9a48536d676f68d918270699102aa8dab5496df06904c914600" [[package]] name = "color-eyre" @@ -257,9 +233,9 @@ checksum = "55b672471b4e9f9e95499ea597ff64941a309b2cdbffcc46f2cc5e2d971fd335" [[package]] name = "console" -version = "0.16.3" +version = "0.16.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d64e8af5551369d19cf50138de61f1c42074ab970f74e99be916646777f8fc87" +checksum = "4fe5f465a4f6fee88fad41b85d990f84c835335e85b5d9e6e63e0d06d28cba7c" dependencies = [ "encode_unicode", "libc", @@ -269,18 +245,18 @@ dependencies = [ [[package]] name = "crossbeam-channel" -version = "0.5.15" +version = "0.5.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82b8f8f868b36967f9606790d1903570de9ceaf870a7bf9fbbd3016d636a2cb2" +checksum = "d85363c37faeca707aef026efa9f3b34d077bce547e48f770770625c6013679e" dependencies = [ "crossbeam-utils", ] [[package]] name = "crossbeam-utils" -version = "0.8.21" +version = "0.8.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" +checksum = "a31eee39dddec8330830986fcd7625edb5a24ec90ea038215273bbc3adb08ac6" [[package]] name = "encode_unicode" @@ -326,6 +302,30 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" +[[package]] +name = "futures-core" +version = "0.3.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92d699e522242e69e3003b94ecc1f960f3a5e015aa7c5d7486e65ad01dd94f5e" + +[[package]] +name = "futures-task" +version = "0.3.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd417de3d1d015fc3bfd2b1ea46dfc7bab72ef86f1cc7cc9c78e728b34a6d1fd" + +[[package]] +name = "futures-util" +version = "0.3.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d50a92467f8ba5dd6e3ee5d4bd04d73ab2e4e1c44474a0674821dfce14b79bc" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "slab", +] + [[package]] name = "getrandom" version = "0.4.2" @@ -356,9 +356,9 @@ dependencies = [ [[package]] name = "hashbrown" -version = "0.16.1" +version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" +checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" [[package]] name = "heck" @@ -380,21 +380,21 @@ checksum = "964de6e86d545b246d84badc0fef527924ace5134f30641c203ef52ba83f58d5" [[package]] name = "indexmap" -version = "2.13.0" +version = "2.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7714e70437a7dc3ac8eb7e6f8df75fd8eb422675fc7678aff7364301092b1017" +checksum = "07aa2048142242915a31d35844fb311e0e53fcca590c3a0a40dcf1b841fa09eb" dependencies = [ "equivalent", - "hashbrown 0.16.1", + "hashbrown 0.17.1", "serde", "serde_core", ] [[package]] name = "indicatif" -version = "0.18.4" +version = "0.18.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25470f23803092da7d239834776d653104d551bc4d7eacaf31e6837854b8e9eb" +checksum = "9433806cd6b4ec1aba79c021c7e4c58fb4c3b9977c085062e611ac929998fb0c" dependencies = [ "console", "portable-atomic", @@ -411,11 +411,12 @@ checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" [[package]] name = "js-sys" -version = "0.3.94" +version = "0.3.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2e04e2ef80ce82e13552136fabeef8a5ed1f985a96805761cbb9a2c34e7664d9" +checksum = "53b44bfcdb3f8d5837a46dae1ca9660a837176eee74a28b229bc626816589102" dependencies = [ - "once_cell", + "cfg-if", + "futures-util", "wasm-bindgen", ] @@ -439,9 +440,9 @@ checksum = "db13adb97ab515a3691f56e4dbab09283d0b86cb45abd991d8634a9d6f501760" [[package]] name = "libc" -version = "0.2.183" +version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5b646652bf6661599e1da8901b3b9522896f01e736bad5f723fe7a3a27f899d" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "linux-raw-sys" @@ -457,9 +458,9 @@ checksum = "5e5032e24019045c762d3c0f28f5b6b8bbf38563a65908389bf7978758920897" [[package]] name = "memchr" -version = "2.8.0" +version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" [[package]] name = "miniz_oxide" @@ -487,9 +488,9 @@ checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" [[package]] name = "owo-colors" -version = "4.3.0" +version = "4.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d211803b9b6b570f68772237e415a029d5a50c65d382910b879fb19d3271f94d" +checksum = "13c45bb4a6ae1280ec0803b1ef9d3455eb50f01efbbe1447ab020f1d54fba9d8" [[package]] name = "pin-project-lite" @@ -499,9 +500,9 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "portable-atomic" -version = "1.13.1" +version = "1.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c33a9471896f1c69cecef8d20cbe2f7accd12527ce60845ff44c153bb2a21b49" +checksum = "05c8b63e8d9609db387f0324918f81d68fe27748f084ef092fb35954d0539a85" [[package]] name = "prettydiff" @@ -519,23 +520,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "479ca8adacdd7ce8f1fb39ce9ecccbfe93a3f1344b3d0d97f20bc0196208f62b" dependencies = [ "proc-macro2", - "syn", + "syn 2.0.119", ] [[package]] name = "proc-macro2" -version = "1.0.106" +version = "1.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" dependencies = [ "unicode-ident", ] [[package]] name = "quote" -version = "1.0.45" +version = "1.0.47" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" dependencies = [ "proc-macro2", ] @@ -548,9 +549,9 @@ checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] name = "regex" -version = "1.12.3" +version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e10754a14b9137dd7b1e3e5b0493cc9171fdd105e0ab477f51b72e7f3ac0e276" +checksum = "f020237b6c8eed93db2e2cb53c00c60a8e1bc73da7d073199a1180401450218d" dependencies = [ "aho-corasick", "memchr", @@ -560,9 +561,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.14" +version = "0.4.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" +checksum = "ad8553b9b26413251cbf30e620595c7a41b3887f03da04579c0e6b0d6a06b4b2" dependencies = [ "aho-corasick", "memchr", @@ -571,15 +572,15 @@ dependencies = [ [[package]] name = "regex-syntax" -version = "0.8.10" +version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" +checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" [[package]] name = "ron" -version = "0.12.1" +version = "0.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4147b952f3f819eca0e99527022f7d6a8d05f111aeb0a62960c74eb283bec8fc" +checksum = "81116b9531d61eabc41aeb228e4b6b2435bcca3233b98cf3b3077d4e6e9debb3" dependencies = [ "bitflags", "once_cell", @@ -591,9 +592,9 @@ dependencies = [ [[package]] name = "rustc-demangle" -version = "0.1.27" +version = "0.1.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b50b8869d9fc858ce7266cce0194bd74df58b9d0e3f6df3a9fc8eb470d95c09d" +checksum = "b74b56ffa8bb2830709a538c2cbcae9aa062db0d2a42563bfb09bdaae44020eb" [[package]] name = "rustc_version" @@ -635,20 +636,20 @@ version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d5715001c3f29532641421aaddb1302cbcbfd7507ed5f3a5dd0ddb5b808a7e0" dependencies = [ - "thiserror 2.0.18", + "thiserror 2.0.20", ] [[package]] name = "rustversion" -version = "1.0.22" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" +checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" [[package]] name = "semver" -version = "1.0.27" +version = "1.0.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d767eb0aabc880b29956c35734170f26ed551a859dbd361d140cdbeca61ab1e2" +checksum = "8a7852d02fc848982e0c167ef163aaff9cd91dc640ba85e263cb1ce46fae51cd" dependencies = [ "serde", "serde_core", @@ -656,9 +657,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" +checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" dependencies = [ "serde_core", "serde_derive", @@ -666,29 +667,29 @@ dependencies = [ [[package]] name = "serde_core" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" +checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" +checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.6", ] [[package]] name = "serde_json" -version = "1.0.149" +version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83fc039473c5595ace860d8c4fafa220ff474b3fc6bfdb4293327f1a37e94d86" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ "itoa", "memchr", @@ -715,6 +716,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" + [[package]] name = "spanned" version = "0.4.1" @@ -728,9 +735,20 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.117" +version = "2.0.119" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "syn" +version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" +checksum = "8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee" dependencies = [ "proc-macro2", "quote", @@ -761,11 +779,11 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.18" +version = "2.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" +checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" dependencies = [ - "thiserror-impl 2.0.18", + "thiserror-impl 2.0.20", ] [[package]] @@ -776,34 +794,34 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.119", ] [[package]] name = "thiserror-impl" -version = "2.0.18" +version = "2.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" +checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.6", ] [[package]] name = "thread_local" -version = "1.1.9" +version = "1.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f60246a4944f24f6e018aa17cdeffb7818b76356965d03b07d6a9886e8962185" +checksum = "1ad99c4c6d32803332c548b1af0540b357b3f5fc0be8f6c6bfe8b2e6ae784070" dependencies = [ "cfg-if", ] [[package]] name = "toml" -version = "1.1.1+spec-1.1.0" +version = "1.1.6+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "994b95d9e7bae62b34bab0e2a4510b801fa466066a6a8b2b57361fa1eba068ee" +checksum = "920602543f0911ab71da12c50d59701da54c196d1a2bf5cb4b75667f137a406a" dependencies = [ "indexmap", "serde_core", @@ -825,18 +843,18 @@ dependencies = [ [[package]] name = "toml_parser" -version = "1.1.1+spec-1.1.0" +version = "1.1.3+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "39ca317ebc49f06bd748bfba29533eac9485569dc9bf80b849024b025e814fb9" +checksum = "1d38ac1cf9b95face32296c0a3ede1fdc270627c9d9c02a7274dd6d960dc4d56" dependencies = [ "winnow", ] [[package]] name = "toml_writer" -version = "1.1.1+spec-1.1.0" +version = "1.1.2+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" [[package]] name = "tracing" @@ -887,15 +905,15 @@ checksum = "bc7d623258602320d5c55d1bc22793b57daff0ec7efc270ea7d55ce1d5f5471c" [[package]] name = "ui_test" -version = "0.30.4" +version = "0.30.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ada249620d81f010b9a1472b63a5077ac7c722dd0f4bacf6528b313d0b8c15d8" +checksum = "8c8811281d587a786747c0c49245925016c07767bc996305bdd34d5ce076786a" dependencies = [ "annotate-snippets", "anyhow", "bstr", - "cargo-platform 0.1.9", - "cargo_metadata 0.18.1", + "cargo-platform", + "cargo_metadata", "color-eyre", "colored", "comma", @@ -913,9 +931,9 @@ dependencies = [ [[package]] name = "unicode-ident" -version = "1.0.24" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +checksum = "d245f478577f809a851594d02313b640fb437e0bb33866753cff937863096954" [[package]] name = "unicode-width" @@ -961,9 +979,9 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.117" +version = "0.2.126" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0551fc1bb415591e3372d0bc4780db7e587d84e2a7e79da121051c5c4b89d0b0" +checksum = "4b067c0c11094aef6b7a801c1e34a26affafdf3d051dba08456b868789aaf9a4" dependencies = [ "cfg-if", "once_cell", @@ -974,9 +992,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.117" +version = "0.2.126" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fbdf9a35adf44786aecd5ff89b4563a90325f9da0923236f6104e603c7e86be" +checksum = "167ce5e579f6bcf889c4f7175a8a5a585de84e8ff93976ce393efa5f2837aab1" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -984,22 +1002,22 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.117" +version = "0.2.126" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dca9693ef2bab6d4e6707234500350d8dad079eb508dca05530c85dc3a529ff2" +checksum = "f3997c7839262f4ef12cf90b818d6340c18e80f263f1a94bf157d0ec4420380e" dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn", + "syn 2.0.119", "wasm-bindgen-shared", ] [[package]] name = "wasm-bindgen-shared" -version = "0.2.117" +version = "0.2.126" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "39129a682a6d2d841b6c429d0c51e5cb0ed1a03829d8b3d1e69a011e62cb3d3b" +checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24" dependencies = [ "unicode-ident", ] @@ -1050,9 +1068,9 @@ dependencies = [ [[package]] name = "which" -version = "8.0.2" +version = "8.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81995fafaaaf6ae47a7d0cc83c67caf92aeb7e5331650ae6ff856f7c0c60c459" +checksum = "bae2f2b2b816647a1cab1acc91f5bd20812d53cb344382635ec2181940c8034f" dependencies = [ "libc", ] @@ -1096,9 +1114,9 @@ dependencies = [ [[package]] name = "winnow" -version = "1.0.1" +version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09dac053f1cd375980747450bfc7250c264eaae0583872e845c0c7cd578872b5" +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" [[package]] name = "wit-bindgen" @@ -1130,7 +1148,7 @@ dependencies = [ "heck", "indexmap", "prettyplease", - "syn", + "syn 2.0.119", "wasm-metadata", "wit-bindgen-core", "wit-component", @@ -1146,7 +1164,7 @@ dependencies = [ "prettyplease", "proc-macro2", "quote", - "syn", + "syn 2.0.119", "wit-bindgen-core", "wit-bindgen-rust", ] @@ -1190,6 +1208,6 @@ dependencies = [ [[package]] name = "zmij" -version = "1.0.21" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa" +checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" diff --git a/Cargo.toml b/Cargo.toml index 0d908a7..22afd8d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,12 +20,12 @@ categories = ["development-tools"] readme = "README.md" [workspace.dependencies] -serde = { version = "1.0.228", features = ["derive"] } -serde_json = "1.0.149" -anyhow = "1.0.102" -regex = "1.12.3" +serde = { version = "1.0.229", features = ["derive"] } +serde_json = "1.0.151" +anyhow = "1.0.104" +regex = "1.13.1" tempfile = "3.27.0" -ron = "0.12.1" +ron = "0.12.2" cargo_metadata = "0.23" [dependencies] @@ -51,13 +51,13 @@ cargo_pup_lint_config = { path = "cargo_pup_lint_config", version = "=0.1.8" } # rustc_lint = {path = "/Users/scott.gerring/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/rustc-src/rust/compiler/rustc_lint", optional = true} # rustc_trait_selection = {path = "/Users/scott.gerring/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/rustc-src/rust/compiler/rustc_trait_selection", optional = true} # rustc_errors = {path = "/Users/scott.gerring/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/rustc-src/rust/compiler/rustc_errors", optional = true} -toml = "1.1.1" -which = "8.0.2" +toml = "1.1.6" +which = "8.0.6" rustup-toolchain = "0.1.10" [dev-dependencies] rustc_version = "0.4" -ui_test = "0.30.4" +ui_test = "0.30.7" ron = { workspace = true } [[test]] diff --git a/test_app/Cargo.lock b/test_app/Cargo.lock index 152878e..f912048 100644 --- a/test_app/Cargo.lock +++ b/test_app/Cargo.lock @@ -4,9 +4,9 @@ version = 4 [[package]] name = "anyhow" -version = "1.0.102" +version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" [[package]] name = "bitflags" @@ -74,9 +74,29 @@ dependencies = [ [[package]] name = "cfg-if" -version = "1.0.0" +version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +checksum = "4e7648175b45a9a48536d676f68d918270699102aa8dab5496df06904c914600" + +[[package]] +name = "chacha20" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" +dependencies = [ + "cfg-if", + "cpufeatures", + "rand_core", +] + +[[package]] +name = "cpufeatures" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ca28b0ae3115b884660db4118d803791fd6756b6e88f39c0f3f7859060d7566" +dependencies = [ + "libc", +] [[package]] name = "errno" @@ -96,25 +116,26 @@ checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" [[package]] name = "getrandom" -version = "0.2.16" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "335ff9f135e4384c8150d6f27c6daed433577f86b4750418338c01a1a2528592" +checksum = "26145e563e54f2cadc477553f1ec5ee650b00862f0a58bcd12cbdc5f0ea2d2f4" dependencies = [ "cfg-if", "libc", - "wasi 0.11.0+wasi-snapshot-preview1", + "r-efi 5.2.0", + "wasi", ] [[package]] name = "getrandom" -version = "0.3.3" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26145e563e54f2cadc477553f1ec5ee650b00862f0a58bcd12cbdc5f0ea2d2f4" +checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" dependencies = [ "cfg-if", "libc", - "r-efi", - "wasi 0.14.2+wasi-0.2.4", + "r-efi 6.0.0", + "rand_core", ] [[package]] @@ -125,9 +146,9 @@ checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" [[package]] name = "libc" -version = "0.2.183" +version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5b646652bf6661599e1da8901b3b9522896f01e736bad5f723fe7a3a27f899d" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "linux-raw-sys" @@ -147,20 +168,11 @@ version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" -[[package]] -name = "ppv-lite86" -version = "0.2.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9" -dependencies = [ - "zerocopy", -] - [[package]] name = "proc-macro2" -version = "1.0.95" +version = "1.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02b3e5e68a3a1a02aad3ec490a98007cbc13c37cbe84a3cd7b8e406d76e7f778" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" dependencies = [ "unicode-ident", ] @@ -171,14 +183,14 @@ version = "0.1.0" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.6", ] [[package]] name = "quote" -version = "1.0.40" +version = "1.0.47" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1885c039570dc00dcb4ff087a89e185fd56bae234ddc7f056a945bf36467248d" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" dependencies = [ "proc-macro2", ] @@ -190,40 +202,33 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74765f6d916ee2faa39bc8e68e4f3ed8949b48cccdac59983d287a7cb71ce9c5" [[package]] -name = "rand" -version = "0.8.5" +name = "r-efi" +version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" -dependencies = [ - "libc", - "rand_chacha", - "rand_core", -] +checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] -name = "rand_chacha" -version = "0.3.1" +name = "rand" +version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +checksum = "c7f5fa3a058cd35567ef9bfa5e75732bee0f9e4c55fa90477bef2dfcdbc4be80" dependencies = [ - "ppv-lite86", + "chacha20", + "getrandom 0.4.3", "rand_core", ] [[package]] name = "rand_core" -version = "0.6.4" +version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" -dependencies = [ - "getrandom 0.2.16", -] +checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" [[package]] name = "ron" -version = "0.12.1" +version = "0.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4147b952f3f819eca0e99527022f7d6a8d05f111aeb0a62960c74eb283bec8fc" +checksum = "81116b9531d61eabc41aeb228e4b6b2435bcca3233b98cf3b3077d4e6e9debb3" dependencies = [ "bitflags", "once_cell", @@ -257,9 +262,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" +checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" dependencies = [ "serde_core", "serde_derive", @@ -267,29 +272,29 @@ dependencies = [ [[package]] name = "serde_core" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" +checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" +checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.6", ] [[package]] name = "serde_json" -version = "1.0.149" +version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83fc039473c5595ace860d8c4fafa220ff474b3fc6bfdb4293327f1a37e94d86" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ "itoa", "memchr", @@ -300,9 +305,20 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.100" +version = "2.0.119" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b09a44accad81e1ba1cd74a32461ba89dee89095ba17b32f5d03683b1b1fc2a0" +checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "syn" +version = "3.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee" dependencies = [ "proc-macro2", "quote", @@ -350,7 +366,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.119", ] [[package]] @@ -361,15 +377,9 @@ checksum = "bc7d623258602320d5c55d1bc22793b57daff0ec7efc270ea7d55ce1d5f5471c" [[package]] name = "unicode-ident" -version = "1.0.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a5f39404a5da50712a4c1eecf25e90dd62b613502b7e925fd4e4d19b5c96512" - -[[package]] -name = "wasi" -version = "0.11.0+wasi-snapshot-preview1" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +checksum = "d245f478577f809a851594d02313b640fb437e0bb33866753cff937863096954" [[package]] name = "wasi" @@ -462,26 +472,6 @@ dependencies = [ "bitflags", ] -[[package]] -name = "zerocopy" -version = "0.8.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2586fea28e186957ef732a5f8b3be2da217d65c5969d4b1e17f973ebbe876879" -dependencies = [ - "zerocopy-derive", -] - -[[package]] -name = "zerocopy-derive" -version = "0.8.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a996a8f63c5c4448cd959ac1bab0aaa3306ccfd060472f85943ee0750f0169be" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "zmij" version = "1.0.21" diff --git a/test_app/Cargo.toml b/test_app/Cargo.toml index c872cae..492cf04 100644 --- a/test_app/Cargo.toml +++ b/test_app/Cargo.toml @@ -7,8 +7,8 @@ version = "0.1.0" edition = "2024" [dependencies] -anyhow = "1.0.80" -rand = "0.8.5" +anyhow = "1.0.104" +rand = "0.10.2" proc_macro_test = { path = "proc_macro_test" } [dev-dependencies] diff --git a/test_app/proc_macro_test/Cargo.toml b/test_app/proc_macro_test/Cargo.toml index e2d7b10..2137538 100644 --- a/test_app/proc_macro_test/Cargo.toml +++ b/test_app/proc_macro_test/Cargo.toml @@ -9,4 +9,4 @@ proc-macro = true [dependencies] proc-macro2 = "1.0" quote = "1.0" -syn = "2.0" \ No newline at end of file +syn = "3.0" From f24d5c798fdaad88147eb2353aa959f1b4ed0ef8 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 10:51:54 +0200 Subject: [PATCH 07/11] update cargo-pup self-lint configuration --- pup.ron | 44 +++++--------------------------------------- tests/ron_test.rs | 46 +++++++++------------------------------------- 2 files changed, 14 insertions(+), 76 deletions(-) diff --git a/pup.ron b/pup.ron index b708b47..17e04fe 100644 --- a/pup.ron +++ b/pup.ron @@ -2,18 +2,10 @@ lints: [ Struct(( name: "architecture_lint_rule_checker", - matches: ImplementsTrait("^pup_driver::lints::architecture_lint_rule::ArchitectureLintRule"), + matches: ImplementsTrait("^cargo_pup_lint_impl::architecture_lint_rule::ArchitectureLintRule$"), rules: [ - MustBeNamed(".*LintProcessor$", Error), - MustBePrivate(Error), - ], - )), - Struct(( - name: "lint_factory_checker", - matches: ImplementsTrait("^pup_driver::lints::configuration_factory.rs::LintFactory"), - rules: [ - MustBeNamed(".*LintFactory$", Error), - MustBePrivate(Error), + MustBeNamed("^(Function|Module|Struct)Lint$", Error), + MustBePublic(Error), ], )), Module(( @@ -23,32 +15,6 @@ MustHaveEmptyModFile(Warn), ], )), - Module(( - name: "helpers_no_structs_or_traits", - matches: Module("^pup_driver::lints::helpers$"), - rules: [ - DeniedItems( - items: [ - "struct", - "trait", - ], - severity: Error, - ), - ], - )), - Module(( - name: "utils_no_structs_or_traits", - matches: Module("^pup_driver::utils$"), - rules: [ - DeniedItems( - items: [ - "struct", - "trait", - ], - severity: Error, - ), - ], - )), Function(( name: "result_error_impl_rule", matches: InModule(".*"), @@ -58,12 +24,12 @@ )), Module(( name: "cargo_pup_no_lints_usage", - matches: Module("^cargo_pup::"), + matches: Module("^cargo_pup$"), rules: [ RestrictImports( allowed_only: None, denied: Some([ - "::lints", + "^cargo_pup_lint_impl::lints", ]), severity: Error, ), diff --git a/tests/ron_test.rs b/tests/ron_test.rs index c8d56ef..8f0521c 100644 --- a/tests/ron_test.rs +++ b/tests/ron_test.rs @@ -16,28 +16,18 @@ fn validate_cargo_pup_structure() { // Create a new LintBuilder let mut builder = LintBuilder::new(); - // Add TraitImpl rule for architecture_lint_rules + // Validate ArchitectureLintRule implementations. builder .struct_lint() .lint_named("architecture_lint_rule_checker") .matching(|m| { - m.implements_trait("^pup_driver::lints::architecture_lint_rule::ArchitectureLintRule") + m.implements_trait( + "^cargo_pup_lint_impl::architecture_lint_rule::ArchitectureLintRule$", + ) }) .with_severity(Severity::Error) - .must_be_named(".*LintProcessor$".into()) - .must_be_private() - .build(); - - // Add TraitImpl rule for configuration_factories - builder - .struct_lint() - .lint_named("lint_factory_checker") - .matching(|m| { - m.implements_trait("^pup_driver::lints::configuration_factory.rs::LintFactory") - }) - .with_severity(Severity::Error) - .must_be_named(".*LintFactory$".into()) - .must_be_private() + .must_be_named("^(Function|Module|Struct)Lint$".into()) + .must_be_public() .build(); // Add EmptyMod rule for modules following the mod.rs structure @@ -49,24 +39,6 @@ fn validate_cargo_pup_structure() { .must_have_empty_mod_file() .build(); - // Add ItemType rule for helpers_no_structs_or_traits - builder - .module_lint() - .lint_named("helpers_no_structs_or_traits") - .matching(|m| m.module("^pup_driver::lints::helpers$")) - .with_severity(Severity::Error) - .denied_items(vec!["struct".to_string(), "trait".to_string()]) - .build(); - - // Utils shouldn't contain structs or traits - builder - .module_lint() - .lint_named("utils_no_structs_or_traits") - .matching(|m| m.module("^pup_driver::utils$")) - .with_severity(Severity::Error) - .denied_items(vec!["struct".to_string(), "trait".to_string()]) - .build(); - // All Result<_,T> must return something implementing the error trait builder .function_lint() @@ -76,13 +48,13 @@ fn validate_cargo_pup_structure() { .enforce_error_trait_implementation() .build(); - // cargo_pup shouldn't use the lints subsystem + // Keep the cargo-pup binary on cargo_pup_lint_impl's public API. builder .module_lint() .lint_named("cargo_pup_no_lints_usage") - .matching(|m| m.module("^cargo_pup::")) + .matching(|m| m.module("^cargo_pup$")) .with_severity(Severity::Error) - .restrict_imports(None, Some(vec!["::lints".to_string()])) + .restrict_imports(None, Some(vec!["^cargo_pup_lint_impl::lints".to_string()])) .build(); // Write the configuration to pup.ron using the write_to_file method From dadba2b034a22c4ddef86872680e5d778e12eaa6 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 11:03:32 +0200 Subject: [PATCH 08/11] show trait lint applicability --- .gitignore | 1 + .../src/lints/struct_lint/lint.rs | 37 +++++++++++++++++-- scripts/test.sh | 1 + test_app/scripts/validate-print-traits.sh | 20 ++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100755 test_app/scripts/validate-print-traits.sh diff --git a/.gitignore b/.gitignore index ce203a6..c4d7363 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ mod CLAUDE.md .claude .idea +.sphinx diff --git a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs index 2bff99a..12025d8 100644 --- a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs @@ -38,6 +38,20 @@ fn hir_attribute_name(attribute: &rustc_hir::Attribute) -> Option { } } +fn matcher_references_trait(matcher: &StructMatch, trait_path: &str) -> bool { + match matcher { + StructMatch::ImplementsTrait(pattern) => Regex::new(pattern) + .map(|regex| regex.is_match(trait_path)) + .unwrap_or(false), + StructMatch::AndMatches(left, right) | StructMatch::OrMatches(left, right) => { + matcher_references_trait(left, trait_path) + || matcher_references_trait(right, trait_path) + } + StructMatch::NotMatch(inner) => matcher_references_trait(inner, trait_path), + StructMatch::Name(_) | StructMatch::HasAttribute(_) => false, + } +} + fn evaluate_rule_with(rule: &StructRule, evaluate_leaf: &impl Fn(&StructRule) -> bool) -> bool { match rule { StructRule::And(left, right) => { @@ -272,8 +286,8 @@ impl ArchitectureLintRule for StructLint { false } - fn applies_to_trait(&self, _trait_path: &str) -> bool { - false + fn applies_to_trait(&self, trait_path: &str) -> bool { + matcher_references_trait(&self.matches, trait_path) } fn register_late_pass(&self, lint_store: &mut LintStore) { @@ -525,8 +539,10 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { #[cfg(test)] mod tests { - use super::{attribute_matches, collect_rule_violations, evaluate_rule_with}; - use cargo_pup_lint_config::{Severity, StructRule}; + use super::{ + attribute_matches, collect_rule_violations, evaluate_rule_with, matcher_references_trait, + }; + use cargo_pup_lint_config::{Severity, StructMatch, StructRule}; use regex::Regex; fn named(pattern: &str) -> StructRule { @@ -548,6 +564,19 @@ mod tests { assert!(!attribute_matches(®ex, "other_repr")); } + #[test] + fn identifies_traits_referenced_by_complex_matchers() { + let matcher = StructMatch::AndMatches( + Box::new(StructMatch::Name("Service".into())), + Box::new(StructMatch::NotMatch(Box::new( + StructMatch::ImplementsTrait("^crate::RequiredTrait$".into()), + ))), + ); + + assert!(matcher_references_trait(&matcher, "crate::RequiredTrait")); + assert!(!matcher_references_trait(&matcher, "crate::OtherTrait")); + } + #[test] fn evaluates_logical_rule_combinations() { let pass = named("passes"); diff --git a/scripts/test.sh b/scripts/test.sh index 7574da8..248b152 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -33,3 +33,4 @@ cargo test --test ui-test # Its first run warms this cache, and its second run compares stable output. rm -rf test_app/.pup test_app/scripts/validate-against-expected.sh +test_app/scripts/validate-print-traits.sh diff --git a/test_app/scripts/validate-print-traits.sh b/test_app/scripts/validate-print-traits.sh new file mode 100755 index 0000000..fe7838a --- /dev/null +++ b/test_app/scripts/validate-print-traits.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +set -euo pipefail + +PATHS="" +[ -d "$(dirname "$0")/../../target/release" ] && PATHS="$(cd "$(dirname "$0")/../../target/release" && pwd):$PATHS" +[ -d "$(dirname "$0")/../../target/debug" ] && PATHS="$(cd "$(dirname "$0")/../../target/debug" && pwd):$PATHS" +export PATH="${PATHS}$PATH" + +pushd "$(dirname "$0")/../" >/dev/null +output="$(cargo pup print-traits 2>&1 | perl -pe 's/\e\[[0-9;]*m//g')" +popd >/dev/null + +expected="::trait_impl::MyTrait [trait_restrictions]" +if ! grep -Fq "$expected" <<<"$output"; then + echo "Expected print-traits output to contain: $expected" >&2 + echo "$output" >&2 + exit 1 +fi From 1bf86deb8e68873c9f28daff52e5189e5c2887f9 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 13:16:13 +0200 Subject: [PATCH 09/11] address review comments --- .github/workflows/pr.yaml | 4 +- cargo_pup_lint_impl/src/lib.rs | 1 + .../src/lints/module_lint/lint.rs | 227 ++++++++++++++++-- .../src/lints/struct_lint/lint.rs | 163 +++++++++++-- pup.ron | 2 +- scripts/test.sh | 10 +- src/pup_driver.rs | 5 +- test_app/Cargo.lock | 6 +- test_app/Cargo.toml | 1 + test_app/expected_output | 43 +++- test_app/proc_macro_test/Cargo.toml | 5 - test_app/pup.ron | 4 +- test_app/scripts/validate-print-traits.sh | 8 +- test_app/src/configured_struct_rules.rs | 2 +- test_app/tests/project_context_dedup_test.rs | 17 +- test_app/tests/pup_ron_test.rs | 4 +- tests/ron_test.rs | 4 +- tests/ui/module_lint_new/logical_rules.rs | 8 +- tests/ui/module_lint_new/logical_rules.stderr | 38 ++- tests/ui/module_lint_new/pup.ron | 2 +- 20 files changed, 448 insertions(+), 106 deletions(-) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index b3fe9f2..0aeb713 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -18,7 +18,9 @@ jobs: - run: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y - run: . $HOME/.cargo/env && rustup update - name: Install cargo-nextest - run: . $HOME/.cargo/env && cargo install cargo-nextest --version 0.9.145 --locked + run: | + . $HOME/.cargo/env + curl -LsSf https://get.nexte.st/0.9.145/linux | tar zxf - -C "${CARGO_HOME:-$HOME/.cargo}/bin" - run: . $HOME/.cargo/env && cargo build --release --all - name: Run tests run: . $HOME/.cargo/env && scripts/test.sh diff --git a/cargo_pup_lint_impl/src/lib.rs b/cargo_pup_lint_impl/src/lib.rs index dc20106..94ed85f 100644 --- a/cargo_pup_lint_impl/src/lib.rs +++ b/cargo_pup_lint_impl/src/lib.rs @@ -24,3 +24,4 @@ pub use architecture_lint_rule::ArchitectureLintRule; pub use helpers::architecture_lint_collection::ArchitectureLintCollection; pub use helpers::architecture_lint_runner::ArchitectureLintRunner; pub use helpers::architecture_lint_runner::Mode; +pub use lints::configuration_factory::LintConfigurationFactory; diff --git a/cargo_pup_lint_impl/src/lints/module_lint/lint.rs b/cargo_pup_lint_impl/src/lints/module_lint/lint.rs index ccefaaf..196fc25 100644 --- a/cargo_pup_lint_impl/src/lints/module_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/module_lint/lint.rs @@ -20,13 +20,13 @@ fn evaluate_module_rule_with( rule: &ModuleRule, evaluate_leaf: &impl Fn(&ModuleRule) -> Option, ) -> Option { + // A composite is inapplicable if either operand is inapplicable. let combine = |left: Option, right: Option, operation: fn(bool, bool) -> bool| match ( left, right, ) { (Some(left), Some(right)) => Some(operation(left, right)), - (Some(result), None) | (None, Some(result)) => Some(result), - (None, None) => None, + _ => None, }; match rule { @@ -47,6 +47,44 @@ fn evaluate_module_rule_with( } } +// Collect the failing leaves of an applicable composite rule. +fn collect_module_rule_violations<'a>( + rule: &'a ModuleRule, + expected: bool, + evaluate_leaf: &impl Fn(&ModuleRule) -> Option, + violations: &mut Vec<(&'a ModuleRule, bool)>, +) { + let actual = evaluate_module_rule_with(rule, evaluate_leaf).expect( + "collect_module_rule_violations is only invoked once the root rule evaluates to Some", + ); + if actual == expected { + return; + } + + match rule { + ModuleRule::And(left, right) if expected => { + collect_module_rule_violations(left, true, evaluate_leaf, violations); + collect_module_rule_violations(right, true, evaluate_leaf, violations); + } + ModuleRule::And(left, _right) => { + // Negating A && B requires only one operand to become false. + collect_module_rule_violations(left, false, evaluate_leaf, violations); + } + ModuleRule::Or(left, right) if expected => { + collect_module_rule_violations(left, true, evaluate_leaf, violations); + collect_module_rule_violations(right, true, evaluate_leaf, violations); + } + ModuleRule::Or(left, right) => { + collect_module_rule_violations(left, false, evaluate_leaf, violations); + collect_module_rule_violations(right, false, evaluate_leaf, violations); + } + ModuleRule::Not(inner) => { + collect_module_rule_violations(inner, !expected, evaluate_leaf, violations); + } + leaf => violations.push((leaf, expected)), + } +} + fn module_rule_severity(rule: &ModuleRule) -> Severity { match rule { ModuleRule::MustBeNamed(_, severity) @@ -334,6 +372,159 @@ impl ModuleLint { } } + fn render_module_rule_violation( + &self, + ctx: &LateContext<'_>, + item: &Item<'_>, + rule: &ModuleRule, + expected: bool, + severity: Severity, + ) { + match rule { + ModuleRule::MustBeNamed(pattern, _) | ModuleRule::MustNotBeNamed(pattern, _) => { + let must_match = matches!(rule, ModuleRule::MustBeNamed(..)) == expected; + let item_name = ctx + .tcx + .item_name(item.owner_id.def_id.to_def_id()) + .to_string(); + if must_match { + span_lint_and_help( + ctx, + MODULE_MUST_BE_NAMED::get_by_severity(severity), + self.name().as_str(), + item.span, + format!("Module must match pattern '{pattern}', found '{item_name}'"), + None, + format!("Rename this module to match the pattern '{pattern}'"), + ); + } else { + span_lint_and_help( + ctx, + MODULE_MUST_NOT_BE_NAMED::get_by_severity(severity), + self.name().as_str(), + item.span, + format!("Module must not match pattern '{pattern}'"), + None, + "Choose a name that doesn't match this pattern", + ); + } + } + ModuleRule::MustNotBeEmpty(_) => { + if expected { + span_lint_and_help( + ctx, + MODULE_MUST_NOT_BE_EMPTY::get_by_severity(severity), + self.name().as_str(), + item.span, + "Module must not be empty", + None, + "Add content to this module or remove it", + ); + } else { + span_lint_and_help( + ctx, + MODULE_MUST_BE_EMPTY::get_by_severity(severity), + self.name().as_str(), + item.span, + "Module must be empty", + None, + "Remove content from this module", + ); + } + } + ModuleRule::MustBeEmpty(_) => { + if expected { + span_lint_and_help( + ctx, + MODULE_MUST_BE_EMPTY::get_by_severity(severity), + self.name().as_str(), + item.span, + "Module must be empty", + None, + "Remove disallowed items from this module", + ); + } else { + span_lint_and_help( + ctx, + MODULE_MUST_NOT_BE_EMPTY::get_by_severity(severity), + self.name().as_str(), + item.span, + "Module must not be empty", + None, + "Add content to this module", + ); + } + } + ModuleRule::MustHaveEmptyModFile(_) => { + let message = if expected { + "Module's mod.rs file must be empty (only allowed to re-export other modules)" + } else { + "Module's mod.rs file must not be empty" + }; + span_lint_and_help( + ctx, + MODULE_MUST_HAVE_EMPTY_MOD_FILE::get_by_severity(severity), + self.name().as_str(), + item.span, + message, + None, + "Remove disallowed items from the mod.rs file or move them to a submodule", + ); + } + ModuleRule::RestrictImports { .. } => { + let message = if expected { + "Use of this module is not permitted by the configured import restrictions" + } else { + "Use of this module is required to be denied by the configured import restrictions" + }; + span_lint_and_help( + ctx, + MODULE_RESTRICT_IMPORTS::get_by_severity(severity), + self.name().as_str(), + item.span, + message, + None, + "Adjust this import to satisfy the configured restrictions", + ); + } + ModuleRule::NoWildcardImports(_) => { + let message = if expected { + "Wildcard imports are not allowed" + } else { + "Wildcard imports are required here" + }; + span_lint_and_help( + ctx, + MODULE_WILDCARD_IMPORT::get_by_severity(severity), + self.name().as_str(), + item.span, + message, + None, + "Import specific items instead of using a wildcard", + ); + } + ModuleRule::DeniedItems { .. } => { + let message = if expected { + "This item type is not allowed in this module" + } else { + "This item type is required to be denied in this module" + }; + span_lint_and_help( + ctx, + MODULE_DENIED_ITEMS::get_by_severity(severity), + self.name().as_str(), + item.span, + message, + None, + "Consider moving this item to a different module", + ); + } + ModuleRule::And(_, _) | ModuleRule::Or(_, _) | ModuleRule::Not(_) => { + unreachable!("composite rules are not leaves") + } + } + } + // Helper function to check for disallowed items in a module and call the callback when found fn check_for_disallowed_items( &self, @@ -435,14 +626,6 @@ declare_variable_severity_lint!( "Module's mod.rs file must be empty (only allowed to re-export other modules)" ); -declare_variable_severity_lint!( - pub, - MODULE_LOGICAL_RULE, - MODULE_LOGICAL_RULE_LINT_DENY, - MODULE_LOGICAL_RULE_LINT_WARN, - "Module must satisfy its configured logical rule" -); - impl_lint_pass!(ModuleLint => [ MODULE_MUST_BE_NAMED_LINT_DENY, MODULE_MUST_BE_NAMED_LINT_WARN, MODULE_MUST_NOT_BE_NAMED_LINT_DENY, MODULE_MUST_NOT_BE_NAMED_LINT_WARN, @@ -451,8 +634,7 @@ impl_lint_pass!(ModuleLint => [ MODULE_MUST_HAVE_EMPTY_MOD_FILE_LINT_DENY, MODULE_MUST_HAVE_EMPTY_MOD_FILE_LINT_WARN, MODULE_RESTRICT_IMPORTS_LINT_DENY, MODULE_RESTRICT_IMPORTS_LINT_WARN, MODULE_WILDCARD_IMPORT_LINT_DENY, MODULE_WILDCARD_IMPORT_LINT_WARN, - MODULE_DENIED_ITEMS_LINT_DENY, MODULE_DENIED_ITEMS_LINT_WARN, - MODULE_LOGICAL_RULE_LINT_DENY, MODULE_LOGICAL_RULE_LINT_WARN + MODULE_DENIED_ITEMS_LINT_DENY, MODULE_DENIED_ITEMS_LINT_WARN ]); impl ArchitectureLintRule for ModuleLint { @@ -730,16 +912,15 @@ impl<'tcx> LateLintPass<'tcx> for ModuleLint { } } ModuleRule::And(_, _) | ModuleRule::Or(_, _) | ModuleRule::Not(_) => { + let evaluate_leaf = + |leaf: &ModuleRule| self.evaluate_module_rule_leaf(ctx, item, leaf); if self.evaluate_module_rule(ctx, item, rule) == Some(false) { - span_lint_and_help( - ctx, - MODULE_LOGICAL_RULE::get_by_severity(module_rule_severity(rule)), - self.name().as_str(), - item.span, - "Item does not satisfy the configured logical module rule", - None, - "Satisfy at least the required combination of module constraints", - ); + let severity = module_rule_severity(rule); + let mut violations = Vec::new(); + collect_module_rule_violations(rule, true, &evaluate_leaf, &mut violations); + for (leaf, expected) in violations { + self.render_module_rule_violation(ctx, item, leaf, expected, severity); + } } } } @@ -790,13 +971,13 @@ mod tests { } #[test] - fn ignores_inapplicable_leaves_without_hiding_applicable_results() { + fn inapplicable_leaf_makes_composite_inapplicable() { let rule = ModuleRule::And( Box::new(named("passes", Severity::Warn)), Box::new(ModuleRule::MustNotBeEmpty(Severity::Warn)), ); - assert_eq!(evaluate_module_rule_with(&rule, &leaf_value), Some(true)); + assert_eq!(evaluate_module_rule_with(&rule, &leaf_value), None); } #[test] diff --git a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs index 12025d8..0c6975c 100644 --- a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs @@ -9,11 +9,16 @@ use rustc_hir::{Item, ItemKind, def_id::DefId}; use rustc_lint::{LateContext, LateLintPass, LintStore}; use rustc_session::impl_lint_pass; use rustc_span::BytePos; +use std::collections::HashMap; +use std::sync::{Mutex, OnceLock}; pub struct StructLint { name: String, matches: StructMatch, struct_rules: Vec, + // Cache trait metadata and compiled regexes for this lint pass. + trait_cache: OnceLock>, + regex_cache: Mutex>, } fn attribute_matches(regex: &Regex, attribute: &str) -> bool { @@ -38,6 +43,7 @@ fn hir_attribute_name(attribute: &rustc_hir::Attribute) -> Option { } } +// Report referenced traits in print-traits regardless of matcher polarity. fn matcher_references_trait(matcher: &StructMatch, trait_path: &str) -> bool { match matcher { StructMatch::ImplementsTrait(pattern) => Regex::new(pattern) @@ -52,6 +58,7 @@ fn matcher_references_trait(matcher: &StructMatch, trait_path: &str) -> bool { } } +#[cfg(test)] fn evaluate_rule_with(rule: &StructRule, evaluate_leaf: &impl Fn(&StructRule) -> bool) -> bool { match rule { StructRule::And(left, right) => { @@ -65,38 +72,65 @@ fn evaluate_rule_with(rule: &StructRule, evaluate_leaf: &impl Fn(&StructRule) -> } } +// Cache subtree results because violation collection revisits them. +fn evaluate_rule_cached( + rule: &StructRule, + evaluate_leaf: &impl Fn(&StructRule) -> bool, + cache: &mut HashMap<*const StructRule, bool>, +) -> bool { + let key = rule as *const StructRule; + if let Some(&value) = cache.get(&key) { + return value; + } + + let value = match rule { + StructRule::And(left, right) => { + evaluate_rule_cached(left, evaluate_leaf, cache) + && evaluate_rule_cached(right, evaluate_leaf, cache) + } + StructRule::Or(left, right) => { + evaluate_rule_cached(left, evaluate_leaf, cache) + || evaluate_rule_cached(right, evaluate_leaf, cache) + } + StructRule::Not(inner) => !evaluate_rule_cached(inner, evaluate_leaf, cache), + leaf => evaluate_leaf(leaf), + }; + + cache.insert(key, value); + value +} + fn collect_rule_violations<'a>( rule: &'a StructRule, expected: bool, evaluate_leaf: &impl Fn(&StructRule) -> bool, + cache: &mut HashMap<*const StructRule, bool>, violations: &mut Vec<(&'a StructRule, bool)>, ) { - if evaluate_rule_with(rule, evaluate_leaf) == expected { + if evaluate_rule_cached(rule, evaluate_leaf, cache) == expected { return; } match rule { StructRule::And(left, right) if expected => { - collect_rule_violations(left, true, evaluate_leaf, violations); - collect_rule_violations(right, true, evaluate_leaf, violations); + collect_rule_violations(left, true, evaluate_leaf, cache, violations); + collect_rule_violations(right, true, evaluate_leaf, cache, violations); } - StructRule::And(left, right) => { - // Both children are true when !(left && right) fails. Reporting both - // alternatives makes the available ways to satisfy the rule visible. - collect_rule_violations(left, false, evaluate_leaf, violations); - collect_rule_violations(right, false, evaluate_leaf, violations); + StructRule::And(left, _right) => { + // Negating A && B requires only one operand to become false. + collect_rule_violations(left, false, evaluate_leaf, cache, violations); } StructRule::Or(left, right) if expected => { // Both alternatives failed, so report both leaf requirements. - collect_rule_violations(left, true, evaluate_leaf, violations); - collect_rule_violations(right, true, evaluate_leaf, violations); + collect_rule_violations(left, true, evaluate_leaf, cache, violations); + collect_rule_violations(right, true, evaluate_leaf, cache, violations); } StructRule::Or(left, right) => { - collect_rule_violations(left, false, evaluate_leaf, violations); - collect_rule_violations(right, false, evaluate_leaf, violations); + collect_rule_violations(left, false, evaluate_leaf, cache, violations); + collect_rule_violations(right, false, evaluate_leaf, cache, violations); } StructRule::Not(inner) => { - collect_rule_violations(inner, !expected, evaluate_leaf, violations); + collect_rule_violations(inner, !expected, evaluate_leaf, cache, violations); } leaf => violations.push((leaf, expected)), } @@ -110,6 +144,8 @@ impl StructLint { name: s.name.clone(), matches: s.matches.clone(), struct_rules: s.rules.to_vec(), + trait_cache: OnceLock::new(), + regex_cache: Mutex::new(HashMap::new()), }) } else { panic!("Expected a Struct lint configuration") @@ -192,6 +228,18 @@ impl StructLint { }) } + fn cached_regex(&self, pattern: &str) -> Option { + if let Some(regex) = self.regex_cache.lock().unwrap().get(pattern) { + return Some(regex.clone()); + } + let regex = Regex::new(pattern).ok()?; + self.regex_cache + .lock() + .unwrap() + .insert(pattern.to_string(), regex.clone()); + Some(regex) + } + fn implements_matching_trait( &self, ctx: &LateContext<'_>, @@ -200,16 +248,30 @@ impl StructLint { ) -> bool { use crate::helpers::queries; - let Ok(trait_regex) = Regex::new(trait_pattern) else { + let Some(trait_regex) = self.cached_regex(trait_pattern) else { return false; }; let ty = ctx.tcx.type_of(def_id).skip_binder(); - ctx.tcx.all_traits_including_private().any(|trait_def_id| { - let full_trait_name = - queries::get_full_canonical_trait_name_from_def_id(&ctx.tcx, trait_def_id); - trait_regex.is_match(&full_trait_name) - && queries::implements_trait(ctx.tcx, ctx.param_env, ty, trait_def_id) + let candidates = self.trait_cache.get_or_init(|| { + ctx.tcx + .all_traits_including_private() + .filter(|&trait_def_id| { + // implements_trait currently supports traits parameterised only by Self. + ctx.tcx.generics_of(trait_def_id).own_params.len() <= 1 + }) + .map(|trait_def_id| { + ( + trait_def_id, + queries::get_full_canonical_trait_name_from_def_id(&ctx.tcx, trait_def_id), + ) + }) + .collect() + }); + + candidates.iter().any(|(trait_def_id, full_trait_name)| { + trait_regex.is_match(full_trait_name) + && queries::implements_trait(ctx.tcx, ctx.param_env, ty, *trait_def_id) }) } } @@ -300,6 +362,8 @@ impl ArchitectureLintRule for StructLint { name: name.clone(), matches: matches.clone(), struct_rules: struct_rules.clone(), + trait_cache: OnceLock::new(), + regex_cache: Mutex::new(HashMap::new()), }) }); } @@ -377,7 +441,14 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { for configured_rule in &self.struct_rules { let mut violations = Vec::new(); - collect_rule_violations(configured_rule, true, &evaluate_leaf, &mut violations); + let mut cache = HashMap::new(); + collect_rule_violations( + configured_rule, + true, + &evaluate_leaf, + &mut cache, + &mut violations, + ); for (rule, expected) in violations { match rule { @@ -403,7 +474,11 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { ( STRUCT_LINT_MUST_NOT_BE_NAMED::get_by_severity(*severity), format!("Struct must not match {pattern_type} '{pattern}'"), - "Choose a name that doesn't match this pattern".to_string(), + if pattern_type == "name" { + "Choose a different name for this struct".to_string() + } else { + "Choose a name that doesn't match this pattern".to_string() + }, ) }; span_lint_and_help( @@ -544,6 +619,7 @@ mod tests { }; use cargo_pup_lint_config::{Severity, StructMatch, StructRule}; use regex::Regex; + use std::collections::HashMap; fn named(pattern: &str) -> StructRule { StructRule::MustBeNamed(pattern.to_string(), Severity::Warn) @@ -596,23 +672,64 @@ mod tests { )); } + fn pattern_of(rule: &StructRule) -> &str { + match rule { + StructRule::MustBeNamed(pattern, _) => pattern, + _ => unreachable!(), + } + } + #[test] fn reports_both_failed_or_alternatives() { let rule = StructRule::Or(Box::new(named("first")), Box::new(named("second"))); let mut violations = Vec::new(); + let mut cache = HashMap::new(); - collect_rule_violations(&rule, true, &leaf_value, &mut violations); + collect_rule_violations(&rule, true, &leaf_value, &mut cache, &mut violations); assert_eq!(violations.len(), 2); assert!(violations.iter().all(|(_, expected)| *expected)); + let mut reported: Vec<&str> = violations + .iter() + .map(|(rule, _)| pattern_of(rule)) + .collect(); + reported.sort(); + assert_eq!(reported, vec!["first", "second"]); + } + + #[test] + fn reports_single_operand_for_negated_and() { + let rule = StructRule::And(Box::new(named("passes")), Box::new(named("passes"))); + let mut violations = Vec::new(); + let mut cache = HashMap::new(); + + collect_rule_violations(&rule, false, &leaf_value, &mut cache, &mut violations); + + assert_eq!(violations.len(), 1); + assert_eq!(pattern_of(violations[0].0), "passes"); + assert!(!violations[0].1); + } + + #[test] + fn reports_only_failing_operand_of_required_and() { + let rule = StructRule::And(Box::new(named("passes")), Box::new(named("second"))); + let mut violations = Vec::new(); + let mut cache = HashMap::new(); + + collect_rule_violations(&rule, true, &leaf_value, &mut cache, &mut violations); + + assert_eq!(violations.len(), 1); + assert_eq!(pattern_of(violations[0].0), "second"); + assert!(violations[0].1); } #[test] fn inverts_not_rule_expectation() { let rule = StructRule::Not(Box::new(named("passes"))); let mut violations = Vec::new(); + let mut cache = HashMap::new(); - collect_rule_violations(&rule, true, &leaf_value, &mut violations); + collect_rule_violations(&rule, true, &leaf_value, &mut cache, &mut violations); assert_eq!(violations.len(), 1); assert!(!violations[0].1); diff --git a/pup.ron b/pup.ron index 17e04fe..cb6dcf3 100644 --- a/pup.ron +++ b/pup.ron @@ -24,7 +24,7 @@ )), Module(( name: "cargo_pup_no_lints_usage", - matches: Module("^cargo_pup$"), + matches: Module("^(cargo_pup|pup_driver)(::.*)?$"), rules: [ RestrictImports( allowed_only: None, diff --git a/scripts/test.sh b/scripts/test.sh index 248b152..3ce761b 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -6,12 +6,18 @@ set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" cd "$REPO_ROOT" -if ! cargo nextest --version >/dev/null 2>&1; then +NEXTEST_PINNED_VERSION="0.9.145" + +if ! NEXTEST_VERSION_OUTPUT="$(cargo nextest --version 2>&1)"; then echo "cargo-nextest is required. Install it with:" >&2 - echo " cargo install cargo-nextest --version 0.9.145 --locked" >&2 + echo " cargo install cargo-nextest --version ${NEXTEST_PINNED_VERSION} --locked" >&2 exit 1 fi +if ! grep -q "$NEXTEST_PINNED_VERSION" <<<"$NEXTEST_VERSION_OUTPUT"; then + echo "Warning: installed cargo-nextest ($NEXTEST_VERSION_OUTPUT) does not match the CI-pinned version ${NEXTEST_PINNED_VERSION}; local results may diverge from CI." >&2 +fi + # Build the binaries explicitly because integration tests and test_app resolve # cargo-pup and pup-driver from target/debug rather than Cargo's test artefacts. cargo build --bins diff --git a/src/pup_driver.rs b/src/pup_driver.rs index 673c24b..6591b18 100644 --- a/src/pup_driver.rs +++ b/src/pup_driver.rs @@ -17,8 +17,9 @@ use anyhow::Result; use cargo_pup_common::cli::{PupCli, PupCommand}; use cargo_pup_common::workspace::find_workspace_pup_ron; -use cargo_pup_lint_impl::lints::configuration_factory::LintConfigurationFactory; -use cargo_pup_lint_impl::{ArchitectureLintCollection, ArchitectureLintRunner, Mode}; +use cargo_pup_lint_impl::{ + ArchitectureLintCollection, ArchitectureLintRunner, LintConfigurationFactory, Mode, +}; use rustc_session::{EarlyDiagCtxt, config::ErrorOutputType}; use std::{ env, diff --git a/test_app/Cargo.lock b/test_app/Cargo.lock index f912048..1782b91 100644 --- a/test_app/Cargo.lock +++ b/test_app/Cargo.lock @@ -180,11 +180,6 @@ dependencies = [ [[package]] name = "proc_macro_test" version = "0.1.0" -dependencies = [ - "proc-macro2", - "quote", - "syn 3.0.6", -] [[package]] name = "quote" @@ -347,6 +342,7 @@ dependencies = [ "cargo_pup_lint_config", "proc_macro_test", "rand", + "tempfile", ] [[package]] diff --git a/test_app/Cargo.toml b/test_app/Cargo.toml index 492cf04..345b575 100644 --- a/test_app/Cargo.toml +++ b/test_app/Cargo.toml @@ -14,3 +14,4 @@ proc_macro_test = { path = "proc_macro_test" } [dev-dependencies] cargo_pup_common = { path = "../cargo_pup_common" } cargo_pup_lint_config = { path = "../cargo_pup_lint_config" } +tempfile = "3.27.0" diff --git a/test_app/expected_output b/test_app/expected_output index 4f3c424..55a6561 100644 --- a/test_app/expected_output +++ b/test_app/expected_output @@ -112,14 +112,14 @@ warning: Struct 'AppMustAvoidTraitBad' must not implement trait matching 'test_a = help: Remove the forbidden trait implementation = note: Applied by cargo-pup rule 'forbidden_trait_rule_check'. -warning: Struct must match pattern '^AppAllowedComposite', found 'AppCompositeOrBad' +warning: Struct must match pattern '^AppCompositeOrAllowed', found 'AppCompositeOrBad' --> src/configured_struct_rules.rs:28:1 | 28 | / pub struct AppCompositeOrBad; 29 | | struct AppCompositeOrPrivate; | |_^ | - = help: Rename this struct to match the pattern '^AppAllowedComposite' + = help: Rename this struct to match the pattern '^AppCompositeOrAllowed' = note: Applied by cargo-pup rule 'or_struct_rule_check'. warning: Struct 'AppCompositeOrBad' has pub visibility, but must be private @@ -241,26 +241,47 @@ warning: module 'nested' is not allowed in this module = help: Consider moving this item to a different module = note: Applied by cargo-pup rule 'item_type_restrictions'. -warning: Item does not satisfy the configured logical module rule +error: Module must match pattern '^and_good$', found 'and_bad' --> src/logical_module_rules.rs:3:1 | 3 | pub mod and_bad {} | ^^^^^^^^^^^^^^^^^^ | - = help: Satisfy at least the required combination of module constraints + = help: Rename this module to match the pattern '^and_good$' = note: Applied by cargo-pup rule 'logical_module_and_check'. - = note: `#[warn(module_logical_rule)]` on by default + = note: `#[deny(module_must_be_named)]` on by default -warning: Item does not satisfy the configured logical module rule +error: Module must not be empty + --> src/logical_module_rules.rs:3:1 + | +3 | pub mod and_bad {} + | ^^^^^^^^^^^^^^^^^^ + | + = help: Add content to this module or remove it + = note: Applied by cargo-pup rule 'logical_module_and_check'. + = note: `#[deny(module_must_not_be_empty)]` on by default + +warning: Module must match pattern '^or_named$', found 'or_bad' + --> src/logical_module_rules.rs:15:1 + | +15 | pub mod or_bad {} + | ^^^^^^^^^^^^^^^^^ + | + = help: Rename this module to match the pattern '^or_named$' + = note: Applied by cargo-pup rule 'logical_module_or_check'. + = note: `#[warn(module_must_be_named)]` on by default + +warning: Module must not be empty --> src/logical_module_rules.rs:15:1 | 15 | pub mod or_bad {} | ^^^^^^^^^^^^^^^^^ | - = help: Satisfy at least the required combination of module constraints + = help: Add content to this module or remove it = note: Applied by cargo-pup rule 'logical_module_or_check'. + = note: `#[warn(module_must_not_be_empty)]` on by default -warning: Item does not satisfy the configured logical module rule +warning: Module must be empty --> src/logical_module_rules.rs:19:1 | 19 | / pub mod not_bad { @@ -268,8 +289,9 @@ warning: Item does not satisfy the configured logical module rule 21 | | } | |_^ | - = help: Satisfy at least the required combination of module constraints + = help: Remove content from this module = note: Applied by cargo-pup rule 'logical_module_not_check'. + = note: `#[warn(module_must_be_empty)]` on by default warning: declarative macro 'forbidden_macro' is not allowed in this module --> src/macro_restriction/mod.rs:4:1 @@ -314,7 +336,6 @@ warning: Item 'this_should_not_be_here' not allowed in empty module | = help: Remove this item from the module, which must be empty = note: Applied by cargo-pup rule 'must_be_empty_module'. - = note: `#[warn(module_must_be_empty)]` on by default warning: Item 'ALSO_NOT_ALLOWED' not allowed in empty module --> src/must_be_empty.rs:10:1 @@ -724,4 +745,4 @@ error: Function 'process_unsafe' is forbidden by lint rule For more information about this error, try `rustc --explain E0133`. warning: `test_app` (bin "test_app") generated 59 warnings -error: could not compile `test_app` (bin "test_app") due to 12 previous errors; 59 warnings emitted +error: could not compile `test_app` (bin "test_app") due to 14 previous errors; 59 warnings emitted diff --git a/test_app/proc_macro_test/Cargo.toml b/test_app/proc_macro_test/Cargo.toml index 2137538..e7e4433 100644 --- a/test_app/proc_macro_test/Cargo.toml +++ b/test_app/proc_macro_test/Cargo.toml @@ -5,8 +5,3 @@ edition = "2021" [lib] proc-macro = true - -[dependencies] -proc-macro2 = "1.0" -quote = "1.0" -syn = "3.0" diff --git a/test_app/pup.ron b/test_app/pup.ron index 7cc44a9..6537cf5 100644 --- a/test_app/pup.ron +++ b/test_app/pup.ron @@ -68,7 +68,7 @@ name: "logical_module_and_check", matches: Module("^test_app::logical_module_rules::and_"), rules: [ - And(MustBeNamed("^and_good$", Warn), MustNotBeEmpty(Warn)), + And(MustBeNamed("^and_good$", Warn), MustNotBeEmpty(Error)), ], )), Module(( @@ -119,7 +119,7 @@ name: "or_struct_rule_check", matches: Name("AppCompositeOr"), rules: [ - Or(MustBeNamed("^AppAllowedComposite", Warn), MustBePrivate(Warn)), + Or(MustBeNamed("^AppCompositeOrAllowed", Warn), MustBePrivate(Warn)), ], )), Struct(( diff --git a/test_app/scripts/validate-print-traits.sh b/test_app/scripts/validate-print-traits.sh index fe7838a..8d12437 100755 --- a/test_app/scripts/validate-print-traits.sh +++ b/test_app/scripts/validate-print-traits.sh @@ -9,9 +9,15 @@ PATHS="" export PATH="${PATHS}$PATH" pushd "$(dirname "$0")/../" >/dev/null -output="$(cargo pup print-traits 2>&1 | perl -pe 's/\e\[[0-9;]*m//g')" +if ! output="$(cargo pup print-traits 2>&1)"; then + echo "cargo pup print-traits failed:" >&2 + echo "$output" >&2 + exit 1 +fi popd >/dev/null +output="$(sed $'s/\e\[[0-9;]*m//g' <<<"$output")" + expected="::trait_impl::MyTrait [trait_restrictions]" if ! grep -Fq "$expected" <<<"$output"; then echo "Expected print-traits output to contain: $expected" >&2 diff --git a/test_app/src/configured_struct_rules.rs b/test_app/src/configured_struct_rules.rs index 28cd70d..1fbb99f 100644 --- a/test_app/src/configured_struct_rules.rs +++ b/test_app/src/configured_struct_rules.rs @@ -27,7 +27,7 @@ pub struct AppMustAvoidTraitGood; pub struct AppCompositeOrBad; struct AppCompositeOrPrivate; -pub struct AppAllowedCompositeOr; +pub struct AppCompositeOrAllowed; pub struct AppMatcherNegationBad; diff --git a/test_app/tests/project_context_dedup_test.rs b/test_app/tests/project_context_dedup_test.rs index 5863159..7c5526f 100644 --- a/test_app/tests/project_context_dedup_test.rs +++ b/test_app/tests/project_context_dedup_test.rs @@ -1,17 +1,13 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. use cargo_pup_common::project_context::{ModuleInfo, ProjectContext, TraitInfo}; -use std::time::{SystemTime, UNIX_EPOCH}; #[test] fn merged_workspace_context_deduplicates_modules_and_traits() { - let unique = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos(); - let context_dir = std::env::temp_dir().join(format!("cargo-pup-context-{unique}")); + let context_temp_dir = tempfile::TempDir::new().unwrap(); + let context_dir = context_temp_dir.path(); - let mut first = ProjectContext::with_base_dir(&context_dir); + let mut first = ProjectContext::with_base_dir(context_dir); first.module_root = "first_crate".into(); first.modules = vec![ModuleInfo { name: "shared::module".into(), @@ -23,7 +19,7 @@ fn merged_workspace_context_deduplicates_modules_and_traits() { applicable_lints: vec!["first_lint".into()], }]; - let mut second = ProjectContext::with_base_dir(&context_dir); + let mut second = ProjectContext::with_base_dir(context_dir); second.module_root = "second_crate".into(); second.modules = vec![ModuleInfo { name: "shared::module".into(), @@ -38,8 +34,7 @@ fn merged_workspace_context_deduplicates_modules_and_traits() { first.serialize_to_file().unwrap(); second.serialize_to_file().unwrap(); - let (merged, crate_names) = - ProjectContext::load_all_contexts_from_dir(&context_dir).unwrap(); + let (merged, crate_names) = ProjectContext::load_all_contexts_from_dir(context_dir).unwrap(); assert_eq!(crate_names.len(), 2); assert_eq!(merged.modules.len(), 1); @@ -56,6 +51,4 @@ fn merged_workspace_context_deduplicates_modules_and_traits() { merged.traits[0].applicable_lints, vec!["first_lint", "second_lint"] ); - - std::fs::remove_dir_all(context_dir).unwrap(); } diff --git a/test_app/tests/pup_ron_test.rs b/test_app/tests/pup_ron_test.rs index 395007f..e8a5beb 100644 --- a/test_app/tests/pup_ron_test.rs +++ b/test_app/tests/pup_ron_test.rs @@ -91,7 +91,7 @@ fn test_lint_config() { "^and_good$".into(), Severity::Warn, )), - Box::new(ModuleRule::MustNotBeEmpty(Severity::Warn)), + Box::new(ModuleRule::MustNotBeEmpty(Severity::Error)), )) .build(); @@ -164,7 +164,7 @@ fn test_lint_config() { .matching(|m| m.name("AppCompositeOr")) .add_rule(StructRule::Or( Box::new(StructRule::MustBeNamed( - "^AppAllowedComposite".into(), + "^AppCompositeOrAllowed".into(), Severity::Warn, )), Box::new(StructRule::MustBePrivate(Severity::Warn)), diff --git a/tests/ron_test.rs b/tests/ron_test.rs index 8f0521c..bb88646 100644 --- a/tests/ron_test.rs +++ b/tests/ron_test.rs @@ -48,11 +48,11 @@ fn validate_cargo_pup_structure() { .enforce_error_trait_implementation() .build(); - // Keep the cargo-pup binary on cargo_pup_lint_impl's public API. + // Keep both binaries on cargo_pup_lint_impl's public API. builder .module_lint() .lint_named("cargo_pup_no_lints_usage") - .matching(|m| m.module("^cargo_pup$")) + .matching(|m| m.module("^(cargo_pup|pup_driver)(::.*)?$")) .with_severity(Severity::Error) .restrict_imports(None, Some(vec!["^cargo_pup_lint_impl::lints".to_string()])) .build(); diff --git a/tests/ui/module_lint_new/logical_rules.rs b/tests/ui/module_lint_new/logical_rules.rs index 2d89148..6d4d86c 100644 --- a/tests/ui/module_lint_new/logical_rules.rs +++ b/tests/ui/module_lint_new/logical_rules.rs @@ -3,7 +3,8 @@ //@compile-flags: --crate-name test_logical_module_rules //@compile-flags: --crate-type lib -pub mod and_bad {} //~ ERROR: Item does not satisfy the configured logical module rule +pub mod and_bad {} //~ ERROR: Module must match pattern '^and_good$', found 'and_bad' +//~^ ERROR: Module must not be empty pub mod and_good { pub const VALUE: usize = 1; @@ -15,10 +16,11 @@ pub mod or_content { pub const VALUE: usize = 1; } -pub mod or_bad {} //~ ERROR: Item does not satisfy the configured logical module rule +pub mod or_bad {} //~ ERROR: Module must match pattern '^or_named$', found 'or_bad' +//~^ ERROR: Module must not be empty pub mod must_be_empty {} -pub mod not_bad { //~ ERROR: Item does not satisfy the configured logical module rule +pub mod not_bad { //~ ERROR: Module must be empty pub const VALUE: usize = 1; } diff --git a/tests/ui/module_lint_new/logical_rules.stderr b/tests/ui/module_lint_new/logical_rules.stderr index f3fbecf..8ab50f6 100644 --- a/tests/ui/module_lint_new/logical_rules.stderr +++ b/tests/ui/module_lint_new/logical_rules.stderr @@ -1,32 +1,52 @@ -error: Item does not satisfy the configured logical module rule +error: Module must match pattern '^and_good$', found 'and_bad' --> tests/ui/module_lint_new/logical_rules.rs:6:1 | LL | pub mod and_bad {} | ^^^^^^^^^^^^^^^^^^ | - = help: Satisfy at least the required combination of module constraints + = help: Rename this module to match the pattern '^and_good$' = note: Applied by cargo-pup rule 'logical_and_test'. - = note: `#[deny(module_logical_rule)]` on by default + = note: `#[deny(module_must_be_named)]` on by default -error: Item does not satisfy the configured logical module rule +error: Module must not be empty + --> tests/ui/module_lint_new/logical_rules.rs:6:1 + | +LL | pub mod and_bad {} + | ^^^^^^^^^^^^^^^^^^ + | + = help: Add content to this module or remove it + = note: Applied by cargo-pup rule 'logical_and_test'. + = note: `#[deny(module_must_not_be_empty)]` on by default + +error: Module must match pattern '^or_named$', found 'or_bad' + --> tests/ui/module_lint_new/logical_rules.rs:19:1 + | +LL | pub mod or_bad {} + | ^^^^^^^^^^^^^^^^^ + | + = help: Rename this module to match the pattern '^or_named$' + = note: Applied by cargo-pup rule 'logical_or_test'. + +error: Module must not be empty --> tests/ui/module_lint_new/logical_rules.rs:19:1 | LL | pub mod or_bad {} | ^^^^^^^^^^^^^^^^^ | - = help: Satisfy at least the required combination of module constraints + = help: Add content to this module or remove it = note: Applied by cargo-pup rule 'logical_or_test'. -error: Item does not satisfy the configured logical module rule - --> tests/ui/module_lint_new/logical_rules.rs:23:1 +error: Module must be empty + --> tests/ui/module_lint_new/logical_rules.rs:24:1 | LL | / pub mod not_bad { LL | | pub const VALUE: usize = 1; LL | | } | |_^ | - = help: Satisfy at least the required combination of module constraints + = help: Remove content from this module = note: Applied by cargo-pup rule 'logical_not_test'. + = note: `#[deny(module_must_be_empty)]` on by default -error: aborting due to 3 previous errors +error: aborting due to 5 previous errors diff --git a/tests/ui/module_lint_new/pup.ron b/tests/ui/module_lint_new/pup.ron index c94e22b..846dacb 100644 --- a/tests/ui/module_lint_new/pup.ron +++ b/tests/ui/module_lint_new/pup.ron @@ -150,7 +150,7 @@ matches: Module("^test_logical_module_rules::and_"), rules: [ And( - MustBeNamed("^and_good$", Error), + MustBeNamed("^and_good$", Warn), MustNotBeEmpty(Error), ), ], From e85991affb37441467506e3dd97ef6866c1f0e4b Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 14:52:47 +0200 Subject: [PATCH 10/11] explain why forbidden functions matched --- .../src/lints/function_lint/lint.rs | 170 ++++++++++++++++-- test_app/expected_output | 48 ++--- tests/ui/function_lint/builder_style.rs | 4 +- tests/ui/function_lint/builder_style.stderr | 8 +- tests/ui/function_lint/is_async.rs | 8 +- tests/ui/function_lint/is_async.stderr | 16 +- tests/ui/function_lint/is_unsafe_match.rs | 8 +- tests/ui/function_lint/is_unsafe_match.stderr | 16 +- 8 files changed, 210 insertions(+), 68 deletions(-) diff --git a/cargo_pup_lint_impl/src/lints/function_lint/lint.rs b/cargo_pup_lint_impl/src/lints/function_lint/lint.rs index f02550a..83aa466 100644 --- a/cargo_pup_lint_impl/src/lints/function_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/function_lint/lint.rs @@ -17,6 +17,57 @@ use std::sync::Mutex; use super::no_allocation::detect_allocation_in_mir; use super::no_panic::{PanicCategory, detect_panic_in_mir}; +#[derive(Debug, PartialEq, Eq)] +enum FunctionMatchEvidence { + Reason(String), + And(Vec), + Or(Vec), + Not(String), +} + +impl FunctionMatchEvidence { + fn describe(&self) -> String { + match self { + Self::Reason(reason) => reason.clone(), + Self::And(evidence) => evidence + .iter() + .map(Self::describe) + .collect::>() + .join(" and "), + Self::Or(evidence) => evidence + .iter() + .map(Self::describe) + .collect::>() + .join(" or "), + Self::Not(requirement) => format!("it does not satisfy {requirement}"), + } + } +} + +fn describe_matcher_requirement(matcher: &FunctionMatch) -> String { + match matcher { + FunctionMatch::NameEquals(name) => format!("the name `{name}`"), + FunctionMatch::NameRegex(pattern) => format!("the name pattern `{pattern}`"), + FunctionMatch::InModule(pattern) => format!("the module pattern `{pattern}`"), + FunctionMatch::ReturnsType(pattern) => format!("the return type constraint `{pattern:?}`"), + FunctionMatch::IsAsync => "the async constraint".to_string(), + FunctionMatch::IsUnsafe => "the unsafe constraint".to_string(), + FunctionMatch::AndMatches(left, right) => format!( + "both {} and {}", + describe_matcher_requirement(left), + describe_matcher_requirement(right) + ), + FunctionMatch::OrMatches(left, right) => format!( + "either {} or {}", + describe_matcher_requirement(left), + describe_matcher_requirement(right) + ), + FunctionMatch::NotMatch(inner) => { + format!("the negation of {}", describe_matcher_requirement(inner)) + } + } +} + // Helper: retrieve the concrete Self type of the impl the method belongs to, if any fn get_self_type<'tcx>( ctx: &LateContext<'tcx>, @@ -50,15 +101,16 @@ impl FunctionLint { } } - // Helper method to check if a function in a given module with a given name should be linted - fn matches_function( + fn matching_evidence( &self, ctx: &LateContext<'_>, module_path: &str, function_name: &str, fn_def_id: rustc_hir::def_id::DefId, - ) -> bool { - evaluate_function_match(&self.matches, ctx, module_path, function_name, fn_def_id) + ) -> Option { + evaluate_function_match(&self.matches, ctx, module_path, function_name, fn_def_id).then( + || explain_function_match(&self.matches, ctx, module_path, function_name, fn_def_id), + ) } /// Helper method to check a single panic category and emit a lint if found @@ -279,6 +331,56 @@ fn evaluate_function_match( } } +fn explain_function_match( + matcher: &FunctionMatch, + ctx: &LateContext<'_>, + module_path: &str, + function_name: &str, + fn_def_id: rustc_hir::def_id::DefId, +) -> FunctionMatchEvidence { + match matcher { + FunctionMatch::NameEquals(name) => { + FunctionMatchEvidence::Reason(format!("its name is `{name}`")) + } + FunctionMatch::NameRegex(pattern) => { + FunctionMatchEvidence::Reason(format!("its name `{function_name}` matches `{pattern}`")) + } + FunctionMatch::InModule(_) => { + FunctionMatchEvidence::Reason(format!("it is in module `{module_path}`")) + } + FunctionMatch::ReturnsType(_) => { + let return_type = ctx + .tcx + .fn_sig(fn_def_id) + .skip_binder() + .output() + .skip_binder(); + FunctionMatchEvidence::Reason(format!("its return type is `{return_type}`")) + } + FunctionMatch::IsAsync => FunctionMatchEvidence::Reason("it is async".to_string()), + FunctionMatch::IsUnsafe => FunctionMatchEvidence::Reason("it is unsafe".to_string()), + FunctionMatch::AndMatches(left, right) => FunctionMatchEvidence::And(vec![ + explain_function_match(left, ctx, module_path, function_name, fn_def_id), + explain_function_match(right, ctx, module_path, function_name, fn_def_id), + ]), + FunctionMatch::OrMatches(left, right) => { + let evidence = [left.as_ref(), right.as_ref()] + .into_iter() + .filter(|branch| { + evaluate_function_match(branch, ctx, module_path, function_name, fn_def_id) + }) + .map(|branch| { + explain_function_match(branch, ctx, module_path, function_name, fn_def_id) + }) + .collect(); + FunctionMatchEvidence::Or(evidence) + } + FunctionMatch::NotMatch(inner) => { + FunctionMatchEvidence::Not(describe_matcher_requirement(inner)) + } + } +} + // Declare the function_lint lint with variable severity declare_variable_severity_lint!( pub, @@ -332,10 +434,11 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint { let module_path = get_full_module_name(&ctx.tcx, &parent_item); let fn_def_id = item.owner_id.to_def_id(); - // Check if this function matches our patterns - if !self.matches_function(ctx, &module_path, &item_name, fn_def_id) { + let Some(match_evidence) = + self.matching_evidence(ctx, &module_path, &item_name, fn_def_id) + else { return; - } + }; // Apply rules for rule in &self.function_rules { @@ -415,9 +518,12 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint { FUNCTION_LINT::get_by_severity(*severity), self.name().as_str(), sig_span, - format!("Function '{item_name}' is forbidden by lint rule"), + format!( + "Function '{item_name}' is forbidden because {}", + match_evidence.describe() + ), None, - "Remove this function to satisfy the architectural rule", + "Change the matched function properties or remove the function", ); } FunctionRule::NoAllocation(severity) => { @@ -484,10 +590,11 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint { let module_path = get_full_module_name(&ctx.tcx, &module); let fn_def_id = impl_item.owner_id.to_def_id(); - // Check if this method matches our patterns - if !self.matches_function(ctx, &module_path, &item_name, fn_def_id) { + let Some(match_evidence) = + self.matching_evidence(ctx, &module_path, &item_name, fn_def_id) + else { return; - } + }; // Apply rules for rule in &self.function_rules { @@ -567,9 +674,12 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint { FUNCTION_LINT::get_by_severity(*severity), self.name().as_str(), sig_span, - format!("Function '{item_name}' is forbidden by lint rule"), + format!( + "Function '{item_name}' is forbidden because {}", + match_evidence.describe() + ), None, - "Remove this function to satisfy the architectural rule", + "Change the matched function properties or remove the function", ); } FunctionRule::NoAllocation(severity) => { @@ -626,3 +736,35 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint { } } } + +#[cfg(test)] +mod tests { + use super::{FunctionMatchEvidence, describe_matcher_requirement}; + use cargo_pup_lint_config::FunctionMatch; + + #[test] + fn describes_combined_match_evidence() { + let evidence = FunctionMatchEvidence::And(vec![ + FunctionMatchEvidence::Reason( + "it is in module `test_app::unsafe_functions`".to_string(), + ), + FunctionMatchEvidence::Reason("it is unsafe".to_string()), + ]); + + assert_eq!( + evidence.describe(), + "it is in module `test_app::unsafe_functions` and it is unsafe" + ); + } + + #[test] + fn describes_negated_matcher_requirement() { + let matcher = + FunctionMatch::NotMatch(Box::new(FunctionMatch::NameRegex("^allowed_".to_string()))); + + assert_eq!( + describe_matcher_requirement(&matcher), + "the negation of the name pattern `^allowed_`" + ); + } +} diff --git a/test_app/expected_output b/test_app/expected_output index 55a6561..cca3d71 100644 --- a/test_app/expected_output +++ b/test_app/expected_output @@ -391,59 +391,59 @@ warning: Struct 'MyBadlyNamedThing' has pub visibility, but must be private = help: Remove the visibility modifier = note: Applied by cargo-pup rule 'trait_restrictions'. -error: Function 'with_width' is forbidden by lint rule +error: Function 'with_width' is forbidden because its name `with_width` matches `^with_.*` and its return type is `builder_style::WidgetBuilder` --> src/builder_style/mod.rs:8:5 | 8 | pub fn with_width(self, width: u32) -> Self { | ^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'builder_style_with_consuming_forbidden'. = note: `#[deny(function_lint)]` on by default -error: Function 'set_height' is forbidden by lint rule +error: Function 'set_height' is forbidden because its name `set_height` matches `^set_.*` and its return type is `builder_style::WidgetBuilder` --> src/builder_style/mod.rs:13:5 | 13 | pub fn set_height(self, height: u32) -> Self { | ^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'builder_style_set_consuming_forbidden'. -error: Function 'forbidden_async_function' is forbidden by lint rule +error: Function 'forbidden_async_function' is forbidden because it is in module `test_app::async_functions` and it is async --> src/async_functions/mod.rs:8:1 | 8 | pub async fn forbidden_async_function() -> String { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_functions_forbidden'. -error: Function 'async_result_function' is forbidden by lint rule +error: Function 'async_result_function' is forbidden because it is in module `test_app::async_functions` and it is async --> src/async_functions/mod.rs:13:1 | 13 | pub async fn async_result_function() -> Result { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_functions_forbidden'. -error: Function 'process_async' is forbidden by lint rule +error: Function 'process_async' is forbidden because it is in module `test_app::async_functions` and it is async --> src/async_functions/mod.rs:33:5 | 33 | pub async fn process_async(&self) -> Result { | ^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_functions_forbidden'. -error: Function 'process_item' is forbidden by lint rule +error: Function 'process_item' is forbidden because it is in module `test_app::async_functions` and it is async --> src/async_functions/mod.rs:55:5 | 55 | async fn process_item(&self, item: String) -> Result { | ^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_functions_forbidden'. warning: Function allocates heap memory: calls allocating function: std::boxed::Box::::new @@ -689,58 +689,58 @@ warning: Function may panic: index bounds check may panic = help: Remove panic paths to satisfy the NoIndexPanic rule = note: Applied by cargo-pup rule 'no_panic_check'. -error: Function 'forbidden_unsafe_function' is forbidden by lint rule +error: Function 'forbidden_unsafe_function' is forbidden because it is in module `test_app::unsafe_functions` and it is unsafe --> src/unsafe_functions.rs:29:1 | 29 | pub unsafe fn forbidden_unsafe_function() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. -error: Function 'forbidden_unsafe_with_return' is forbidden by lint rule +error: Function 'forbidden_unsafe_with_return' is forbidden because it is in module `test_app::unsafe_functions` and it is unsafe --> src/unsafe_functions.rs:34:1 | 34 | pub unsafe fn forbidden_unsafe_with_return() -> i32 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. -error: Function 'forbidden_unsafe_with_params' is forbidden by lint rule +error: Function 'forbidden_unsafe_with_params' is forbidden because it is in module `test_app::unsafe_functions` and it is unsafe --> src/unsafe_functions.rs:39:1 | 39 | pub unsafe fn forbidden_unsafe_with_params(ptr: *const i32) -> i32 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. -error: Function 'get_data_unsafe' is forbidden by lint rule +error: Function 'get_data_unsafe' is forbidden because it is in module `test_app::unsafe_functions` and it is unsafe --> src/unsafe_functions.rs:63:5 | 63 | pub unsafe fn get_data_unsafe(&self) -> i32 { | ^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. -error: Function 'mutate_unsafe' is forbidden by lint rule +error: Function 'mutate_unsafe' is forbidden because it is in module `test_app::unsafe_functions` and it is unsafe --> src/unsafe_functions.rs:68:5 | 68 | pub unsafe fn mutate_unsafe(&mut self, ptr: *const i32) { | ^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. -error: Function 'process_unsafe' is forbidden by lint rule +error: Function 'process_unsafe' is forbidden because it is in module `test_app::unsafe_functions` and it is unsafe --> src/unsafe_functions.rs:91:5 | 91 | unsafe fn process_unsafe(&self) -> i32 { | ^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. For more information about this error, try `rustc --explain E0133`. diff --git a/tests/ui/function_lint/builder_style.rs b/tests/ui/function_lint/builder_style.rs index 4612925..631e688 100644 --- a/tests/ui/function_lint/builder_style.rs +++ b/tests/ui/function_lint/builder_style.rs @@ -10,7 +10,7 @@ pub struct WidgetBuilder { impl WidgetBuilder { // This should trigger the MustNotExist rule - pub fn with_val(mut self, val: i32) -> Self { //~ ERROR: Function 'with_val' is forbidden by lint rule + pub fn with_val(mut self, val: i32) -> Self { //~ ERROR: Function 'with_val' is forbidden because self.val = val; self } @@ -28,7 +28,7 @@ impl WidgetBuilder { } // Opposite rule: name starts with "set_" but returns Self – should trigger error - pub fn set_val_value(self, val: i32) -> Self { //~ ERROR: Function 'set_val_value' is forbidden by lint rule + pub fn set_val_value(self, val: i32) -> Self { //~ ERROR: Function 'set_val_value' is forbidden because Self { val } } diff --git a/tests/ui/function_lint/builder_style.stderr b/tests/ui/function_lint/builder_style.stderr index a768583..0830af3 100644 --- a/tests/ui/function_lint/builder_style.stderr +++ b/tests/ui/function_lint/builder_style.stderr @@ -1,20 +1,20 @@ -error: Function 'with_val' is forbidden by lint rule +error: Function 'with_val' is forbidden because its name `with_val` matches `^with_.*` and its return type is `WidgetBuilder` --> tests/ui/function_lint/builder_style.rs:13:5 | LL | pub fn with_val(mut self, val: i32) -> Self { | ^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'builder_style_with_methods_forbidden'. = note: `#[deny(function_lint)]` on by default -error: Function 'set_val_value' is forbidden by lint rule +error: Function 'set_val_value' is forbidden because its name `set_val_value` matches `^set_.*` and its return type is `WidgetBuilder` --> tests/ui/function_lint/builder_style.rs:31:5 | LL | pub fn set_val_value(self, val: i32) -> Self { | ^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'builder_style_set_methods_forbid_self_value'. error: aborting due to 2 previous errors diff --git a/tests/ui/function_lint/is_async.rs b/tests/ui/function_lint/is_async.rs index e25d8a2..fe90d06 100644 --- a/tests/ui/function_lint/is_async.rs +++ b/tests/ui/function_lint/is_async.rs @@ -6,11 +6,11 @@ // This test verifies that the FunctionLint's IsAsync matcher works correctly // Async function that should trigger the lint -async fn async_function() { //~ ERROR: Function 'async_function' is forbidden by lint rule +async fn async_function() { //~ ERROR: Function 'async_function' is forbidden because } // Async function with return type -async fn async_with_return() -> String { //~ ERROR: Function 'async_with_return' is forbidden by lint rule +async fn async_with_return() -> String { //~ ERROR: Function 'async_with_return' is forbidden because "hello".to_string() } @@ -27,7 +27,7 @@ fn sync_with_return() -> String { struct TestStruct; impl TestStruct { - async fn async_method(&self) { //~ ERROR: Function 'async_method' is forbidden by lint rule + async fn async_method(&self) { //~ ERROR: Function 'async_method' is forbidden because } fn sync_method(&self) { @@ -42,7 +42,7 @@ trait AsyncTrait { } impl AsyncTrait for TestStruct { - async fn trait_async_method(&self) { //~ ERROR: Function 'trait_async_method' is forbidden by lint rule + async fn trait_async_method(&self) { //~ ERROR: Function 'trait_async_method' is forbidden because } fn trait_sync_method(&self) { diff --git a/tests/ui/function_lint/is_async.stderr b/tests/ui/function_lint/is_async.stderr index 91547b2..862e431 100644 --- a/tests/ui/function_lint/is_async.stderr +++ b/tests/ui/function_lint/is_async.stderr @@ -1,38 +1,38 @@ -error: Function 'async_function' is forbidden by lint rule +error: Function 'async_function' is forbidden because it is in module `test_is_async` and it is async --> tests/ui/function_lint/is_async.rs:9:1 | LL | async fn async_function() { | ^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_function_test'. = note: `#[deny(function_lint)]` on by default -error: Function 'async_with_return' is forbidden by lint rule +error: Function 'async_with_return' is forbidden because it is in module `test_is_async` and it is async --> tests/ui/function_lint/is_async.rs:13:1 | LL | async fn async_with_return() -> String { | ^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_function_test'. -error: Function 'async_method' is forbidden by lint rule +error: Function 'async_method' is forbidden because it is in module `test_is_async` and it is async --> tests/ui/function_lint/is_async.rs:30:5 | LL | async fn async_method(&self) { | ^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_function_test'. -error: Function 'trait_async_method' is forbidden by lint rule +error: Function 'trait_async_method' is forbidden because it is in module `test_is_async` and it is async --> tests/ui/function_lint/is_async.rs:45:5 | LL | async fn trait_async_method(&self) { | ^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'async_function_test'. error: aborting due to 4 previous errors diff --git a/tests/ui/function_lint/is_unsafe_match.rs b/tests/ui/function_lint/is_unsafe_match.rs index 9634b44..22b3f75 100644 --- a/tests/ui/function_lint/is_unsafe_match.rs +++ b/tests/ui/function_lint/is_unsafe_match.rs @@ -6,11 +6,11 @@ // This test verifies that the FunctionLint's IsUnsafe matcher works correctly // Unsafe function that should trigger the lint (unsafe functions are forbidden) -unsafe fn forbidden_unsafe_function() { //~ ERROR: Function 'forbidden_unsafe_function' is forbidden by lint rule +unsafe fn forbidden_unsafe_function() { //~ ERROR: Function 'forbidden_unsafe_function' is forbidden because } // Unsafe function with return type -unsafe fn forbidden_unsafe_with_return() -> i32 { //~ ERROR: Function 'forbidden_unsafe_with_return' is forbidden by lint rule +unsafe fn forbidden_unsafe_with_return() -> i32 { //~ ERROR: Function 'forbidden_unsafe_with_return' is forbidden because 42 } @@ -27,7 +27,7 @@ fn allowed_safe_with_return() -> i32 { struct TestStruct; impl TestStruct { - unsafe fn unsafe_method(&self) { //~ ERROR: Function 'unsafe_method' is forbidden by lint rule + unsafe fn unsafe_method(&self) { //~ ERROR: Function 'unsafe_method' is forbidden because } fn safe_method(&self) { @@ -42,7 +42,7 @@ trait UnsafeTrait { } impl UnsafeTrait for TestStruct { - unsafe fn trait_unsafe_method(&self) { //~ ERROR: Function 'trait_unsafe_method' is forbidden by lint rule + unsafe fn trait_unsafe_method(&self) { //~ ERROR: Function 'trait_unsafe_method' is forbidden because } fn trait_safe_method(&self) { diff --git a/tests/ui/function_lint/is_unsafe_match.stderr b/tests/ui/function_lint/is_unsafe_match.stderr index 5550714..c3d6504 100644 --- a/tests/ui/function_lint/is_unsafe_match.stderr +++ b/tests/ui/function_lint/is_unsafe_match.stderr @@ -1,38 +1,38 @@ -error: Function 'forbidden_unsafe_function' is forbidden by lint rule +error: Function 'forbidden_unsafe_function' is forbidden because it is unsafe --> tests/ui/function_lint/is_unsafe_match.rs:9:1 | LL | unsafe fn forbidden_unsafe_function() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_forbidden_test'. = note: `#[deny(function_lint)]` on by default -error: Function 'forbidden_unsafe_with_return' is forbidden by lint rule +error: Function 'forbidden_unsafe_with_return' is forbidden because it is unsafe --> tests/ui/function_lint/is_unsafe_match.rs:13:1 | LL | unsafe fn forbidden_unsafe_with_return() -> i32 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_forbidden_test'. -error: Function 'unsafe_method' is forbidden by lint rule +error: Function 'unsafe_method' is forbidden because it is unsafe --> tests/ui/function_lint/is_unsafe_match.rs:30:5 | LL | unsafe fn unsafe_method(&self) { | ^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_forbidden_test'. -error: Function 'trait_unsafe_method' is forbidden by lint rule +error: Function 'trait_unsafe_method' is forbidden because it is unsafe --> tests/ui/function_lint/is_unsafe_match.rs:45:5 | LL | unsafe fn trait_unsafe_method(&self) { | ^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove this function to satisfy the architectural rule + = help: Change the matched function properties or remove the function = note: Applied by cargo-pup rule 'unsafe_forbidden_test'. error: aborting due to 4 previous errors From e033b1fb68aa97f301b0432a28f305f0b00c9af9 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Thu, 17 Sep 2026 17:12:18 +0200 Subject: [PATCH 11/11] support generic trait rules and applicability --- .../src/struct_lint/builder.rs | 10 ++- .../src/struct_lint/matcher.rs | 8 ++- .../src/struct_lint/types.rs | 10 ++- cargo_pup_lint_impl/src/helpers/queries.rs | 21 +++--- .../src/lints/struct_lint/lint.rs | 70 ++++++++++++++++--- test_app/expected_output | 66 +++++++++++------ test_app/pup.ron | 14 ++++ test_app/scripts/validate-print-traits.sh | 19 +++-- test_app/src/configured_struct_rules.rs | 9 +++ test_app/tests/pup_ron_test.rs | 20 ++++++ .../attributes_and_configured_rules.rs | 13 ++++ .../attributes_and_configured_rules.stderr | 34 ++++++--- tests/ui/struct_lint_new/implements_trait.rs | 32 ++++++++- .../struct_lint_new/implements_trait.stderr | 42 ++++++++++- tests/ui/struct_lint_new/pup.ron | 69 ++++++++++++++++++ 15 files changed, 373 insertions(+), 64 deletions(-) diff --git a/cargo_pup_lint_config/src/struct_lint/builder.rs b/cargo_pup_lint_config/src/struct_lint/builder.rs index 82529ca..af557db 100644 --- a/cargo_pup_lint_config/src/struct_lint/builder.rs +++ b/cargo_pup_lint_config/src/struct_lint/builder.rs @@ -127,7 +127,10 @@ impl<'a> StructConstraintBuilder<'a> { self } - /// Add a rule requiring the struct to implement a given trait + /// Add a rule requiring the struct to implement a given trait. + /// + /// For a generic trait, any valid set of generic arguments satisfies the rule. Associated + /// type values cannot be constrained by this rule. pub fn must_implement_trait(mut self, trait_path: impl Into) -> Self { self.add_rule_internal(StructRule::ImplementsTrait( trait_path.into(), @@ -136,7 +139,10 @@ impl<'a> StructConstraintBuilder<'a> { self } - /// Add a rule requiring the struct NOT to implement a given trait + /// Add a rule requiring the struct not to implement a given trait. + /// + /// For a generic trait, an implementation for any generic arguments violates the rule. + /// Associated type values cannot be constrained by this rule. pub fn must_not_implement_trait(mut self, trait_path: impl Into) -> Self { let inner = StructRule::ImplementsTrait(trait_path.into(), self.current_severity); self.add_rule_internal(StructRule::Not(Box::new(inner))); diff --git a/cargo_pup_lint_config/src/struct_lint/matcher.rs b/cargo_pup_lint_config/src/struct_lint/matcher.rs index f3ff52f..f8ac9ac 100644 --- a/cargo_pup_lint_config/src/struct_lint/matcher.rs +++ b/cargo_pup_lint_config/src/struct_lint/matcher.rs @@ -27,9 +27,11 @@ impl StructMatcher { StructMatchNode::Leaf(StructMatch::HasAttribute(attr.into())) } - /// Matches structs that implement a specific trait. The trait name - /// is given as a regular expression, - /// e.g. "^(Read|Write)$" + /// Matches structs that implement a specific trait. The trait name is given as a regular + /// expression, e.g. `"^(Read|Write)$"`. + /// + /// Generic arguments are not included in the name. A generic trait matches when the struct + /// implements it for any arguments; associated type values are not constrained. /// pub fn implements_trait(&self, trait_name: impl Into) -> StructMatchNode { StructMatchNode::Leaf(StructMatch::ImplementsTrait(trait_name.into())) diff --git a/cargo_pup_lint_config/src/struct_lint/types.rs b/cargo_pup_lint_config/src/struct_lint/types.rs index dff75a8..6c3a808 100644 --- a/cargo_pup_lint_config/src/struct_lint/types.rs +++ b/cargo_pup_lint_config/src/struct_lint/types.rs @@ -15,7 +15,10 @@ pub enum StructMatch { /// Consumed attributes such as `derive` and attribute procedural macros cannot match. An active /// `cfg_attr` may match a supported resulting attribute, but not the original expression. HasAttribute(String), - /// Match structs that implement a specific trait + /// Match structs that implement a specific trait. + /// + /// Generic arguments are not part of the trait path. A generic trait matches when the + /// struct implements it for any arguments; associated type values are not constrained. ImplementsTrait(String), /// Logical AND - both patterns must match AndMatches(Box, Box), @@ -46,7 +49,10 @@ pub enum StructRule { MustBePublic(Severity), /// Enforces that the struct has pub(crate) visibility MustBePubCrate(Severity), - /// Enforces that the struct implements a specific trait + /// Enforces that the struct implements a specific trait. + /// + /// Generic arguments are not part of the trait path. A generic trait is implemented when + /// any valid arguments exist; associated type values are not constrained. ImplementsTrait(String, Severity), /// Logical AND - both rules must pass And(Box, Box), diff --git a/cargo_pup_lint_impl/src/helpers/queries.rs b/cargo_pup_lint_impl/src/helpers/queries.rs index a1f788c..953e04b 100644 --- a/cargo_pup_lint_impl/src/helpers/queries.rs +++ b/cargo_pup_lint_impl/src/helpers/queries.rs @@ -4,7 +4,7 @@ use rustc_hir::OwnerId; use rustc_hir::def_id::DefId; use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, TypingMode}; -use rustc_span::symbol::sym; +use rustc_span::{DUMMY_SP, symbol::sym}; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; use rustc_trait_selection::traits::{Obligation, ObligationCause}; use rustc_type_ir::TypeVisitableExt; @@ -34,14 +34,11 @@ pub fn implements_trait<'tcx>( ty: Ty<'tcx>, trait_def_id: DefId, ) -> bool { - let cause = ObligationCause::dummy(); - let trait_ref = ty::TraitRef::new(tcx, trait_def_id, [ty]); - let obligation = Obligation::new(tcx, cause, param_env, trait_ref); - - // If we have certain complex types, we can't use TypingMode::Coherence - // at this point, so fall back to TypingMode::Analysis. + // Coherence mode cannot handle complex self types or inferred trait arguments here. // The ui-test test projection_type_reproduce.rs covers this. - let is_complex = ty.has_infer_types() + let has_generic_trait_arguments = tcx.generics_of(trait_def_id).own_params.len() > 1; + let is_complex = has_generic_trait_arguments + || ty.has_infer_types() || ty.has_opaque_types() || ty.walk().any(|t| { if let Some(ty) = t.as_type() { @@ -63,6 +60,14 @@ pub fn implements_trait<'tcx>( tcx.infer_ctxt().build(TypingMode::Coherence) }; + // Arguments after Self are inferred, so any valid generic instantiation counts. + // Associated type bindings are intentionally unconstrained. + let fresh_args = infcx.fresh_args_for_item(DUMMY_SP, trait_def_id); + let trait_args = + tcx.mk_args_from_iter(std::iter::once(ty.into()).chain(fresh_args.iter().skip(1))); + let trait_ref = ty::TraitRef::new(tcx, trait_def_id, trait_args); + let obligation = Obligation::new(tcx, ObligationCause::dummy(), param_env, trait_ref); + infcx.predicate_may_hold(&obligation) } diff --git a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs index 0c6975c..fdadc68 100644 --- a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs @@ -44,11 +44,15 @@ fn hir_attribute_name(attribute: &rustc_hir::Attribute) -> Option { } // Report referenced traits in print-traits regardless of matcher polarity. +fn trait_pattern_matches(pattern: &str, trait_path: &str) -> bool { + Regex::new(pattern) + .map(|regex| regex.is_match(trait_path)) + .unwrap_or(false) +} + fn matcher_references_trait(matcher: &StructMatch, trait_path: &str) -> bool { match matcher { - StructMatch::ImplementsTrait(pattern) => Regex::new(pattern) - .map(|regex| regex.is_match(trait_path)) - .unwrap_or(false), + StructMatch::ImplementsTrait(pattern) => trait_pattern_matches(pattern, trait_path), StructMatch::AndMatches(left, right) | StructMatch::OrMatches(left, right) => { matcher_references_trait(left, trait_path) || matcher_references_trait(right, trait_path) @@ -58,6 +62,21 @@ fn matcher_references_trait(matcher: &StructMatch, trait_path: &str) -> bool { } } +fn rule_references_trait(rule: &StructRule, trait_path: &str) -> bool { + match rule { + StructRule::ImplementsTrait(pattern, _) => trait_pattern_matches(pattern, trait_path), + StructRule::And(left, right) | StructRule::Or(left, right) => { + rule_references_trait(left, trait_path) || rule_references_trait(right, trait_path) + } + StructRule::Not(inner) => rule_references_trait(inner, trait_path), + StructRule::MustBeNamed(_, _) + | StructRule::MustNotBeNamed(_, _) + | StructRule::MustBePrivate(_) + | StructRule::MustBePublic(_) + | StructRule::MustBePubCrate(_) => false, + } +} + #[cfg(test)] fn evaluate_rule_with(rule: &StructRule, evaluate_leaf: &impl Fn(&StructRule) -> bool) -> bool { match rule { @@ -256,10 +275,6 @@ impl StructLint { let candidates = self.trait_cache.get_or_init(|| { ctx.tcx .all_traits_including_private() - .filter(|&trait_def_id| { - // implements_trait currently supports traits parameterised only by Self. - ctx.tcx.generics_of(trait_def_id).own_params.len() <= 1 - }) .map(|trait_def_id| { ( trait_def_id, @@ -350,6 +365,10 @@ impl ArchitectureLintRule for StructLint { fn applies_to_trait(&self, trait_path: &str) -> bool { matcher_references_trait(&self.matches, trait_path) + || self + .struct_rules + .iter() + .any(|rule| rule_references_trait(rule, trait_path)) } fn register_late_pass(&self, lint_store: &mut LintStore) { @@ -616,8 +635,9 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { mod tests { use super::{ attribute_matches, collect_rule_violations, evaluate_rule_with, matcher_references_trait, + rule_references_trait, }; - use cargo_pup_lint_config::{Severity, StructMatch, StructRule}; + use cargo_pup_lint_config::{ConfiguredLint, Severity, StructMatch, StructRule}; use regex::Regex; use std::collections::HashMap; @@ -653,6 +673,40 @@ mod tests { assert!(!matcher_references_trait(&matcher, "crate::OtherTrait")); } + #[test] + fn identifies_traits_referenced_by_complex_rules() { + let rule = StructRule::And( + Box::new(named("Service")), + Box::new(StructRule::Not(Box::new(StructRule::Or( + Box::new(StructRule::MustBePrivate(Severity::Warn)), + Box::new(StructRule::ImplementsTrait( + "^crate::RequiredTrait$".into(), + Severity::Warn, + )), + )))), + ); + + assert!(rule_references_trait(&rule, "crate::RequiredTrait")); + assert!(!rule_references_trait(&rule, "crate::OtherTrait")); + } + + #[test] + fn trait_applicability_includes_matchers_and_rules() { + let config = ConfiguredLint::Struct(cargo_pup_lint_config::struct_lint::StructLint { + name: "trait_check".into(), + matches: StructMatch::ImplementsTrait("^crate::MatchedTrait$".into()), + rules: vec![StructRule::Not(Box::new(StructRule::ImplementsTrait( + "^crate::RuledTrait$".into(), + Severity::Warn, + )))], + }); + let lint = super::StructLint::new(&config); + + assert!(lint.applies_to_trait("crate::MatchedTrait")); + assert!(lint.applies_to_trait("crate::RuledTrait")); + assert!(!lint.applies_to_trait("crate::OtherTrait")); + } + #[test] fn evaluates_logical_rule_combinations() { let pass = named("passes"); diff --git a/test_app/expected_output b/test_app/expected_output index cca3d71..99de2ad 100644 --- a/test_app/expected_output +++ b/test_app/expected_output @@ -71,10 +71,10 @@ note: an unsafe function restricts its caller, but its body is safe by default | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: Struct must match pattern '^AppReprAllowed', found 'AppReprWrongName' - --> src/configured_struct_rules.rs:7:1 + --> src/configured_struct_rules.rs:8:1 | -7 | / pub struct AppReprWrongName; -8 | | +8 | / pub struct AppReprWrongName; +9 | | | |_^ | = help: Rename this struct to match the pattern '^AppReprAllowed' @@ -82,51 +82,71 @@ warning: Struct must match pattern '^AppReprAllowed', found 'AppReprWrongName' = note: `#[warn(struct_lint_must_be_named)]` on by default warning: Struct must match pattern '^AppReprAllowed', found 'AppReprActiveWrongName' - --> src/configured_struct_rules.rs:16:1 + --> src/configured_struct_rules.rs:17:1 | -16 | / pub struct AppReprActiveWrongName; -17 | | +17 | / pub struct AppReprActiveWrongName; +18 | | | |_^ | = help: Rename this struct to match the pattern '^AppReprAllowed' = note: Applied by cargo-pup rule 'attribute_matcher_check'. warning: Struct 'AppNeedsTraitMissing' must implement trait matching 'test_app::configured_struct_rules::RequiredTrait' - --> src/configured_struct_rules.rs:18:1 + --> src/configured_struct_rules.rs:19:1 | -18 | / pub struct AppNeedsTraitMissing; -19 | | +19 | / pub struct AppNeedsTraitMissing; +20 | | | |_^ | = help: Implement the required trait for this struct = note: Applied by cargo-pup rule 'required_trait_rule_check'. = note: `#[warn(struct_lint_implements_trait)]` on by default +warning: Struct 'AppNeedsGenericTraitMissing' must implement trait matching 'test_app::configured_struct_rules::GenericRequiredTrait' + --> src/configured_struct_rules.rs:24:1 + | +24 | / pub struct AppNeedsGenericTraitMissing; +25 | | + | |_^ + | + = help: Implement the required trait for this struct + = note: Applied by cargo-pup rule 'required_generic_trait_rule_check'. + +warning: Struct must match pattern '^AppGenericMatcherAllowed', found 'AppGenericMatcherBad' + --> src/configured_struct_rules.rs:29:1 + | +29 | / pub struct AppGenericMatcherBad; +30 | | impl GenericRequiredTrait for AppGenericMatcherBad {} + | |_^ + | + = help: Rename this struct to match the pattern '^AppGenericMatcherAllowed' + = note: Applied by cargo-pup rule 'generic_trait_matcher_check'. + warning: Struct 'AppMustAvoidTraitBad' must not implement trait matching 'test_app::configured_struct_rules::ForbiddenTrait' - --> src/configured_struct_rules.rs:23:1 + --> src/configured_struct_rules.rs:32:1 | -23 | / pub struct AppMustAvoidTraitBad; -24 | | impl ForbiddenTrait for AppMustAvoidTraitBad {} +32 | / pub struct AppMustAvoidTraitBad; +33 | | impl ForbiddenTrait for AppMustAvoidTraitBad {} | |_^ | = help: Remove the forbidden trait implementation = note: Applied by cargo-pup rule 'forbidden_trait_rule_check'. warning: Struct must match pattern '^AppCompositeOrAllowed', found 'AppCompositeOrBad' - --> src/configured_struct_rules.rs:28:1 + --> src/configured_struct_rules.rs:37:1 | -28 | / pub struct AppCompositeOrBad; -29 | | struct AppCompositeOrPrivate; +37 | / pub struct AppCompositeOrBad; +38 | | struct AppCompositeOrPrivate; | |_^ | = help: Rename this struct to match the pattern '^AppCompositeOrAllowed' = note: Applied by cargo-pup rule 'or_struct_rule_check'. warning: Struct 'AppCompositeOrBad' has pub visibility, but must be private - --> src/configured_struct_rules.rs:28:1 + --> src/configured_struct_rules.rs:37:1 | -28 | / pub struct AppCompositeOrBad; -29 | | struct AppCompositeOrPrivate; +37 | / pub struct AppCompositeOrBad; +38 | | struct AppCompositeOrPrivate; | |_^ | = help: Remove the visibility modifier @@ -134,10 +154,10 @@ warning: Struct 'AppCompositeOrBad' has pub visibility, but must be private = note: `#[warn(struct_lint_must_be_private)]` on by default warning: Struct must match pattern '^AppMatcherNegationAllowed', found 'AppMatcherNegationBad' - --> src/configured_struct_rules.rs:32:1 + --> src/configured_struct_rules.rs:41:1 | -32 | / pub struct AppMatcherNegationBad; -33 | | +41 | / pub struct AppMatcherNegationBad; +42 | | | |_^ | = help: Rename this struct to match the pattern '^AppMatcherNegationAllowed' @@ -744,5 +764,5 @@ error: Function 'process_unsafe' is forbidden because it is in module `test_app: = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. For more information about this error, try `rustc --explain E0133`. -warning: `test_app` (bin "test_app") generated 59 warnings -error: could not compile `test_app` (bin "test_app") due to 14 previous errors; 59 warnings emitted +warning: `test_app` (bin "test_app") generated 61 warnings +error: could not compile `test_app` (bin "test_app") due to 14 previous errors; 61 warnings emitted diff --git a/test_app/pup.ron b/test_app/pup.ron index 6537cf5..c97ad44 100644 --- a/test_app/pup.ron +++ b/test_app/pup.ron @@ -108,6 +108,20 @@ ImplementsTrait("test_app::configured_struct_rules::RequiredTrait", Warn), ], )), + Struct(( + name: "required_generic_trait_rule_check", + matches: Name("^AppNeedsGenericTrait"), + rules: [ + ImplementsTrait("test_app::configured_struct_rules::GenericRequiredTrait", Warn), + ], + )), + Struct(( + name: "generic_trait_matcher_check", + matches: AndMatches(Name("^AppGenericMatcher"), ImplementsTrait("test_app::configured_struct_rules::GenericRequiredTrait")), + rules: [ + MustBeNamed("^AppGenericMatcherAllowed", Warn), + ], + )), Struct(( name: "forbidden_trait_rule_check", matches: Name("^AppMustAvoidTrait"), diff --git a/test_app/scripts/validate-print-traits.sh b/test_app/scripts/validate-print-traits.sh index 8d12437..313c00b 100755 --- a/test_app/scripts/validate-print-traits.sh +++ b/test_app/scripts/validate-print-traits.sh @@ -18,9 +18,16 @@ popd >/dev/null output="$(sed $'s/\e\[[0-9;]*m//g' <<<"$output")" -expected="::trait_impl::MyTrait [trait_restrictions]" -if ! grep -Fq "$expected" <<<"$output"; then - echo "Expected print-traits output to contain: $expected" >&2 - echo "$output" >&2 - exit 1 -fi +expected_lines=( + "::trait_impl::MyTrait [trait_restrictions]" + "::configured_struct_rules::RequiredTrait [negated_trait_matcher_check, required_trait_rule_check]" + "::configured_struct_rules::GenericRequiredTrait [generic_trait_matcher_check, required_generic_trait_rule_check]" +) + +for expected in "${expected_lines[@]}"; do + if ! grep -Fq "$expected" <<<"$output"; then + echo "Expected print-traits output to contain: $expected" >&2 + echo "$output" >&2 + exit 1 + fi +done diff --git a/test_app/src/configured_struct_rules.rs b/test_app/src/configured_struct_rules.rs index 1fbb99f..df10a89 100644 --- a/test_app/src/configured_struct_rules.rs +++ b/test_app/src/configured_struct_rules.rs @@ -1,6 +1,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. pub trait RequiredTrait {} +pub trait GenericRequiredTrait {} pub trait ForbiddenTrait {} #[repr(C)] @@ -20,6 +21,14 @@ pub struct AppNeedsTraitMissing; pub struct AppNeedsTraitPresent; impl RequiredTrait for AppNeedsTraitPresent {} +pub struct AppNeedsGenericTraitMissing; + +pub struct AppNeedsGenericTraitPresent; +impl GenericRequiredTrait for AppNeedsGenericTraitPresent {} + +pub struct AppGenericMatcherBad; +impl GenericRequiredTrait for AppGenericMatcherBad {} + pub struct AppMustAvoidTraitBad; impl ForbiddenTrait for AppMustAvoidTraitBad {} diff --git a/test_app/tests/pup_ron_test.rs b/test_app/tests/pup_ron_test.rs index e8a5beb..b7168e0 100644 --- a/test_app/tests/pup_ron_test.rs +++ b/test_app/tests/pup_ron_test.rs @@ -150,6 +150,26 @@ fn test_lint_config() { .must_implement_trait("test_app::configured_struct_rules::RequiredTrait") .build(); + builder + .struct_lint() + .lint_named("required_generic_trait_rule_check") + .matching(|m| m.name("^AppNeedsGenericTrait")) + .with_severity(Severity::Warn) + .must_implement_trait("test_app::configured_struct_rules::GenericRequiredTrait") + .build(); + + builder + .struct_lint() + .lint_named("generic_trait_matcher_check") + .matching(|m| { + m.name("^AppGenericMatcher").and( + m.implements_trait("test_app::configured_struct_rules::GenericRequiredTrait"), + ) + }) + .with_severity(Severity::Warn) + .must_be_named("^AppGenericMatcherAllowed".into()) + .build(); + builder .struct_lint() .lint_named("forbidden_trait_rule_check") diff --git a/tests/ui/struct_lint_new/attributes_and_configured_rules.rs b/tests/ui/struct_lint_new/attributes_and_configured_rules.rs index 0ba0f88..5e1145d 100644 --- a/tests/ui/struct_lint_new/attributes_and_configured_rules.rs +++ b/tests/ui/struct_lint_new/attributes_and_configured_rules.rs @@ -4,6 +4,7 @@ //@compile-flags: --crate-type lib pub trait RequiredTrait {} +pub trait GenericRequiredTrait {} pub trait ForbiddenTrait {} // Matches inert attributes retained after expansion. @@ -26,6 +27,18 @@ pub struct NeedsTraitMissing; //~ ERROR: Struct 'NeedsTraitMissing' must impleme pub struct NeedsTraitPresent; impl RequiredTrait for NeedsTraitPresent {} +// Requires a generic trait. +pub struct NeedsGenericTraitMissing; //~ ERROR: Struct 'NeedsGenericTraitMissing' must implement trait matching 'test_attributes_and_configured_rules::GenericRequiredTrait' + +pub struct NeedsGenericTraitPresent; +impl GenericRequiredTrait for NeedsGenericTraitPresent {} + +// Forbids a generic trait. +pub struct MustAvoidGenericTraitBad; //~ ERROR: Struct 'MustAvoidGenericTraitBad' must not implement trait matching 'test_attributes_and_configured_rules::GenericRequiredTrait' +impl GenericRequiredTrait for MustAvoidGenericTraitBad {} + +pub struct MustAvoidGenericTraitGood; + // Forbids a trait. pub struct MustAvoidTraitBad; //~ ERROR: Struct 'MustAvoidTraitBad' must not implement trait matching 'test_attributes_and_configured_rules::ForbiddenTrait' impl ForbiddenTrait for MustAvoidTraitBad {} diff --git a/tests/ui/struct_lint_new/attributes_and_configured_rules.stderr b/tests/ui/struct_lint_new/attributes_and_configured_rules.stderr index e446155..147a2ef 100644 --- a/tests/ui/struct_lint_new/attributes_and_configured_rules.stderr +++ b/tests/ui/struct_lint_new/attributes_and_configured_rules.stderr @@ -1,5 +1,5 @@ error: Struct must match pattern '^ReprAllowed', found 'ReprWrongName' - --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:11:1 + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:12:1 | LL | pub struct ReprWrongName; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | pub struct ReprWrongName; = note: `#[deny(struct_lint_must_be_named)]` on by default error: Struct must match pattern '^ReprAllowed', found 'ReprActiveWrongName' - --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:21:1 + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:22:1 | LL | pub struct ReprActiveWrongName; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -18,7 +18,7 @@ LL | pub struct ReprActiveWrongName; = note: Applied by cargo-pup rule 'attribute_matcher_test'. error: Struct 'NeedsTraitMissing' must implement trait matching 'test_attributes_and_configured_rules::RequiredTrait' - --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:24:1 + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:25:1 | LL | pub struct NeedsTraitMissing; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -27,8 +27,26 @@ LL | pub struct NeedsTraitMissing; = note: Applied by cargo-pup rule 'required_trait_rule_test'. = note: `#[deny(struct_lint_implements_trait)]` on by default +error: Struct 'NeedsGenericTraitMissing' must implement trait matching 'test_attributes_and_configured_rules::GenericRequiredTrait' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:31:1 + | +LL | pub struct NeedsGenericTraitMissing; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Implement the required trait for this struct + = note: Applied by cargo-pup rule 'required_generic_trait_rule_test'. + +error: Struct 'MustAvoidGenericTraitBad' must not implement trait matching 'test_attributes_and_configured_rules::GenericRequiredTrait' + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:37:1 + | +LL | pub struct MustAvoidGenericTraitBad; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove the forbidden trait implementation + = note: Applied by cargo-pup rule 'forbidden_generic_trait_rule_test'. + error: Struct 'MustAvoidTraitBad' must not implement trait matching 'test_attributes_and_configured_rules::ForbiddenTrait' - --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:30:1 + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:43:1 | LL | pub struct MustAvoidTraitBad; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -37,7 +55,7 @@ LL | pub struct MustAvoidTraitBad; = note: Applied by cargo-pup rule 'forbidden_trait_rule_test'. error: Struct must match pattern '^AllowedComposite', found 'CompositeOrBad' - --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:36:1 + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:49:1 | LL | pub struct CompositeOrBad; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -46,7 +64,7 @@ LL | pub struct CompositeOrBad; = note: Applied by cargo-pup rule 'or_rule_test'. error: Struct 'CompositeOrBad' has pub visibility, but must be private - --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:36:1 + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:49:1 | LL | pub struct CompositeOrBad; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -56,7 +74,7 @@ LL | pub struct CompositeOrBad; = note: `#[deny(struct_lint_must_be_private)]` on by default error: Struct must match pattern '^MatcherNegationAllowed', found 'MatcherNegationBad' - --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:43:1 + --> tests/ui/struct_lint_new/attributes_and_configured_rules.rs:56:1 | LL | pub struct MatcherNegationBad; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -64,5 +82,5 @@ LL | pub struct MatcherNegationBad; = help: Rename this struct to match the pattern '^MatcherNegationAllowed' = note: Applied by cargo-pup rule 'negated_trait_matcher_test'. -error: aborting due to 7 previous errors +error: aborting due to 9 previous errors diff --git a/tests/ui/struct_lint_new/implements_trait.rs b/tests/ui/struct_lint_new/implements_trait.rs index d1a5aa6..3d6e111 100644 --- a/tests/ui/struct_lint_new/implements_trait.rs +++ b/tests/ui/struct_lint_new/implements_trait.rs @@ -16,6 +16,13 @@ pub trait UnrelatedTrait { fn unrelated_method(&self) -> i32; } +pub trait GenericTrait { + type Output; +} + +pub trait LifetimeTrait<'a> {} +pub trait ConstTrait {} + // This struct implements TestTrait and should trigger the lint pub struct ImplementsTestTrait { //~ ERROR: Struct must match pattern 'Compliant*', found 'ImplementsTestTrait' field: i32, @@ -64,4 +71,27 @@ impl TestTrait for CompliantTraitImplementor { fn test_method(&self) -> bool { true } -} +} + +// Generic trait arguments are inferred when testing implementations. +pub struct ImplementsGenericTrait; //~ ERROR: Struct must match pattern 'GenericCompliant*', found 'ImplementsGenericTrait' + +impl GenericTrait for ImplementsGenericTrait { + type Output = usize; +} + +pub struct DoesNotImplementGenericTrait; + +pub struct ImplementsAsRefStr; //~ ERROR: Struct must match pattern 'AsRefCompliant*', found 'ImplementsAsRefStr' + +impl AsRef for ImplementsAsRefStr { + fn as_ref(&self) -> &str { + "" + } +} + +pub struct ImplementsLifetimeTrait; //~ ERROR: Struct must match pattern 'LifetimeCompliant*', found 'ImplementsLifetimeTrait' +impl<'a> LifetimeTrait<'a> for ImplementsLifetimeTrait {} + +pub struct ImplementsConstTrait; //~ ERROR: Struct must match pattern 'ConstCompliant*', found 'ImplementsConstTrait' +impl ConstTrait<8> for ImplementsConstTrait {} diff --git a/tests/ui/struct_lint_new/implements_trait.stderr b/tests/ui/struct_lint_new/implements_trait.stderr index 5175dd6..a38d5a1 100644 --- a/tests/ui/struct_lint_new/implements_trait.stderr +++ b/tests/ui/struct_lint_new/implements_trait.stderr @@ -1,5 +1,5 @@ error: Struct must match pattern 'Compliant*', found 'ImplementsTestTrait' - --> tests/ui/struct_lint_new/implements_trait.rs:20:1 + --> tests/ui/struct_lint_new/implements_trait.rs:27:1 | LL | pub struct ImplementsTestTrait { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | pub struct ImplementsTestTrait { = note: `#[deny(struct_lint_must_be_named)]` on by default error: Struct must match pattern 'Compliant*', found 'ImplementsMultipleTraits' - --> tests/ui/struct_lint_new/implements_trait.rs:31:1 + --> tests/ui/struct_lint_new/implements_trait.rs:38:1 | LL | pub struct ImplementsMultipleTraits { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,5 +17,41 @@ LL | pub struct ImplementsMultipleTraits { = help: Rename this struct to match the pattern 'Compliant*' = note: Applied by cargo-pup rule 'implements_trait_test'. -error: aborting due to 2 previous errors +error: Struct must match pattern 'GenericCompliant*', found 'ImplementsGenericTrait' + --> tests/ui/struct_lint_new/implements_trait.rs:77:1 + | +LL | pub struct ImplementsGenericTrait; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern 'GenericCompliant*' + = note: Applied by cargo-pup rule 'generic_implements_trait_test'. + +error: Struct must match pattern 'AsRefCompliant*', found 'ImplementsAsRefStr' + --> tests/ui/struct_lint_new/implements_trait.rs:85:1 + | +LL | pub struct ImplementsAsRefStr; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern 'AsRefCompliant*' + = note: Applied by cargo-pup rule 'standard_generic_implements_trait_test'. + +error: Struct must match pattern 'LifetimeCompliant*', found 'ImplementsLifetimeTrait' + --> tests/ui/struct_lint_new/implements_trait.rs:93:1 + | +LL | pub struct ImplementsLifetimeTrait; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern 'LifetimeCompliant*' + = note: Applied by cargo-pup rule 'lifetime_implements_trait_test'. + +error: Struct must match pattern 'ConstCompliant*', found 'ImplementsConstTrait' + --> tests/ui/struct_lint_new/implements_trait.rs:96:1 + | +LL | pub struct ImplementsConstTrait; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Rename this struct to match the pattern 'ConstCompliant*' + = note: Applied by cargo-pup rule 'const_implements_trait_test'. + +error: aborting due to 6 previous errors diff --git a/tests/ui/struct_lint_new/pup.ron b/tests/ui/struct_lint_new/pup.ron index 23004c8..0456127 100644 --- a/tests/ui/struct_lint_new/pup.ron +++ b/tests/ui/struct_lint_new/pup.ron @@ -66,6 +66,53 @@ ] ) ), + + Struct(( + name: "generic_implements_trait_test", + matches: AndMatches( + Name("test_implements_trait"), + ImplementsTrait("test_implements_trait::GenericTrait"), + ), + rules: [ + MustBeNamed("GenericCompliant*", Error), + ], + )), + + Struct(( + name: "standard_generic_implements_trait_test", + matches: AndMatches( + Name("test_implements_trait"), + AndMatches( + Name("^ImplementsAsRefStr$"), + ImplementsTrait("AsRef$"), + ), + ), + rules: [ + MustBeNamed("AsRefCompliant*", Error), + ], + )), + + Struct(( + name: "lifetime_implements_trait_test", + matches: AndMatches( + Name("test_implements_trait"), + ImplementsTrait("test_implements_trait::LifetimeTrait"), + ), + rules: [ + MustBeNamed("LifetimeCompliant*", Error), + ], + )), + + Struct(( + name: "const_implements_trait_test", + matches: AndMatches( + Name("test_implements_trait"), + ImplementsTrait("test_implements_trait::ConstTrait"), + ), + rules: [ + MustBeNamed("ConstCompliant*", Error), + ], + )), // Test MustBePrivate rule - only apply to test_visibility_rules crate Struct( @@ -145,6 +192,28 @@ ], )), + Struct(( + name: "required_generic_trait_rule_test", + matches: AndMatches( + Name("test_attributes_and_configured_rules"), + Name("^NeedsGenericTrait"), + ), + rules: [ + ImplementsTrait("test_attributes_and_configured_rules::GenericRequiredTrait", Error), + ], + )), + + Struct(( + name: "forbidden_generic_trait_rule_test", + matches: AndMatches( + Name("test_attributes_and_configured_rules"), + Name("^MustAvoidGenericTrait"), + ), + rules: [ + Not(ImplementsTrait("test_attributes_and_configured_rules::GenericRequiredTrait", Error)), + ], + )), + Struct(( name: "forbidden_trait_rule_test", matches: AndMatches(