diff --git a/Cargo.lock b/Cargo.lock index 181d848..0412906 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -138,7 +138,7 @@ dependencies = [ [[package]] name = "cargo_pup" -version = "0.1.4" +version = "0.1.5" dependencies = [ "ansi_term", "anyhow", @@ -157,7 +157,7 @@ dependencies = [ [[package]] name = "cargo_pup_common" -version = "0.1.4" +version = "0.1.5" dependencies = [ "anyhow", "cargo_metadata", @@ -168,7 +168,7 @@ dependencies = [ [[package]] name = "cargo_pup_lint_config" -version = "0.1.4" +version = "0.1.5" dependencies = [ "anyhow", "cargo_pup_common", @@ -179,7 +179,7 @@ dependencies = [ [[package]] name = "cargo_pup_lint_impl" -version = "0.1.4" +version = "0.1.5" dependencies = [ "anyhow", "cargo_pup_common", diff --git a/Cargo.toml b/Cargo.toml index 053c63f..bf3f6d4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,7 @@ exclude = ["test_app"] [package] name = "cargo_pup" -version = "0.1.4" +version = "0.1.5" edition = "2024" description = "A Rust architectural linting tool that integrates with rustc to enforce architectural patterns and boundaries" license = "Apache-2.0" @@ -34,9 +34,9 @@ anyhow = { workspace = true } tempfile = { workspace = true } ron = { workspace = true } cargo_metadata = { workspace = true } -cargo_pup_common = { path = "cargo_pup_common", version = "=0.1.4" } -cargo_pup_lint_impl = { path = "cargo_pup_lint_impl", version = "=0.1.4" } -cargo_pup_lint_config = { path = "cargo_pup_lint_config", version = "=0.1.4" } +cargo_pup_common = { path = "cargo_pup_common", version = "=0.1.5" } +cargo_pup_lint_impl = { path = "cargo_pup_lint_impl", version = "=0.1.5" } +cargo_pup_lint_config = { path = "cargo_pup_lint_config", version = "=0.1.5" } # # These bits are just to keep rust rover happy. diff --git a/README.md b/README.md index a080ad3..f395f3a 100644 --- a/README.md +++ b/README.md @@ -21,8 +21,8 @@ First, make sure to install [rustup](https://rustup.rs/) to manage your local ru Then install pup; **you must use this nightly toolchain, as pup depends on compiler internals that are otherwise unavailable!** ```bash -rustup component add --toolchain nightly-2025-07-25 rust-src rustc-dev llvm-tools-preview -cargo +nightly-2025-07-25 install cargo_pup +rustup component add --toolchain nightly-2026-01-22 rust-src rustc-dev llvm-tools-preview +cargo +nightly-2026-01-22 install cargo_pup ``` ## Getting Started @@ -121,7 +121,7 @@ First, add the following to your `Cargo.toml`: ```toml [dev-dependencies] -cargo_pup_lint_config = "0.1.4" +cargo_pup_lint_config = "0.1.5" ``` ## Examples diff --git a/cargo_pup_common/Cargo.toml b/cargo_pup_common/Cargo.toml index 709957c..a04babf 100644 --- a/cargo_pup_common/Cargo.toml +++ b/cargo_pup_common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo_pup_common" -version = "0.1.4" +version = "0.1.5" edition = "2024" description = "Common utilities and shared components for cargo-pup architectural linting tool" license = "Apache-2.0" diff --git a/cargo_pup_lint_config/Cargo.toml b/cargo_pup_lint_config/Cargo.toml index fd74d27..3a83b8e 100644 --- a/cargo_pup_lint_config/Cargo.toml +++ b/cargo_pup_lint_config/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo_pup_lint_config" -version = "0.1.4" +version = "0.1.5" edition = "2024" description = "Configuration and rule builder utilities for cargo-pup architectural linting" license = "Apache-2.0" @@ -13,4 +13,4 @@ serde.workspace = true ron.workspace = true tempfile.workspace = true anyhow.workspace = true -cargo_pup_common = { path = "../cargo_pup_common", version = "=0.1.4" } +cargo_pup_common = { path = "../cargo_pup_common", version = "=0.1.5" } diff --git a/cargo_pup_lint_config/README.md b/cargo_pup_lint_config/README.md index 7ce883d..7ce11be 100644 --- a/cargo_pup_lint_config/README.md +++ b/cargo_pup_lint_config/README.md @@ -24,7 +24,7 @@ Add this to your `Cargo.toml`: ```toml [dev-dependencies] -cargo_pup_lint_config = "0.1.4" +cargo_pup_lint_config = "0.1.5" ``` ## Example diff --git a/cargo_pup_lint_config/src/function_lint/builder.rs b/cargo_pup_lint_config/src/function_lint/builder.rs index 5f75b9f..02505f7 100644 --- a/cargo_pup_lint_config/src/function_lint/builder.rs +++ b/cargo_pup_lint_config/src/function_lint/builder.rs @@ -138,6 +138,37 @@ impl<'a> FunctionConstraintBuilder<'a> { self } + /// Forbid unwrap/expect calls on Option and Result + /// This detects: Option::unwrap, Option::expect, Result::unwrap, Result::expect, + /// Result::unwrap_err, Result::expect_err + pub fn no_unwrap(mut self) -> Self { + self.add_rule_internal(FunctionRule::NoUnwrap(self.current_severity)); + self + } + + /// Forbid all panic-family macros: panic!(), unreachable!(), unimplemented!(), todo!(), assert!() + /// Note: MIR-level analysis cannot distinguish between these macros as they all + /// compile to similar underlying panic functions. + pub fn no_panic(mut self) -> Self { + self.add_rule_internal(FunctionRule::NoPanic(self.current_severity)); + self + } + + /// Forbid index bounds panics + pub fn no_index_panic(mut self) -> Self { + self.add_rule_internal(FunctionRule::NoIndexPanic(self.current_severity)); + self + } + + /// Convenience method: forbid ALL panic sources + /// This adds all panic-related rules: NoUnwrap, NoPanic, and NoIndexPanic + pub fn no_panics(mut self) -> Self { + self.add_rule_internal(FunctionRule::NoUnwrap(self.current_severity)); + self.add_rule_internal(FunctionRule::NoPanic(self.current_severity)); + self.add_rule_internal(FunctionRule::NoIndexPanic(self.current_severity)); + self + } + /// Create a new MaxLength rule with the current severity pub fn create_max_length_rule(&self, length: usize) -> FunctionRule { FunctionRule::MaxLength(length, self.current_severity) diff --git a/cargo_pup_lint_config/src/function_lint/matcher.rs b/cargo_pup_lint_config/src/function_lint/matcher.rs index e8f270f..27ca69a 100644 --- a/cargo_pup_lint_config/src/function_lint/matcher.rs +++ b/cargo_pup_lint_config/src/function_lint/matcher.rs @@ -80,6 +80,11 @@ impl FunctionMatcher { pub fn is_async(&self) -> FunctionMatchNode { FunctionMatchNode::Leaf(FunctionMatch::IsAsync) } + + /// Matches unsafe functions + pub fn is_unsafe(&self) -> FunctionMatchNode { + FunctionMatchNode::Leaf(FunctionMatch::IsUnsafe) + } } /// Node in the matcher expression tree diff --git a/cargo_pup_lint_config/src/function_lint/types.rs b/cargo_pup_lint_config/src/function_lint/types.rs index de47032..986b23b 100644 --- a/cargo_pup_lint_config/src/function_lint/types.rs +++ b/cargo_pup_lint_config/src/function_lint/types.rs @@ -37,6 +37,8 @@ pub enum FunctionMatch { ReturnsType(ReturnTypePattern), /// Match async functions IsAsync, + /// Match unsafe functions + IsUnsafe, /// Logical AND - both patterns must match AndMatches(Box, Box), /// Logical OR - either pattern must match @@ -64,6 +66,15 @@ pub enum FunctionRule { MustNotExist(Severity), /// Enforces that a function must not perform heap allocations NoAllocation(Severity), + /// Enforces that a function must not call unwrap/expect on Option or Result + NoUnwrap(Severity), + /// Enforces that a function must not use any panic-family macros. + /// This includes: panic!(), unreachable!(), unimplemented!(), todo!(), and assert!(). + /// Note: MIR-level analysis cannot distinguish between these macros as they all + /// compile to similar underlying panic functions. + NoPanic(Severity), + /// Enforces that a function must not trigger index bounds panics + NoIndexPanic(Severity), } // Helper methods for FunctionRule diff --git a/cargo_pup_lint_config/src/lint_builder_ext.rs b/cargo_pup_lint_config/src/lint_builder_ext.rs index dc841e2..c001ffe 100644 --- a/cargo_pup_lint_config/src/lint_builder_ext.rs +++ b/cargo_pup_lint_config/src/lint_builder_ext.rs @@ -189,23 +189,24 @@ fn find_workspace_root() -> Result { let mut check_dir = current_exe.parent(); while let Some(dir) = check_dir { + // Check if this directory has the specific cargo-pup source structure + // This is the most reliable indicator that we're in the cargo-pup workspace + if dir.join("src").join("pup_driver.rs").exists() { + return Ok(dir.to_path_buf()); + } + let cargo_toml = dir.join("Cargo.toml"); if cargo_toml.exists() && let Ok(contents) = std::fs::read_to_string(&cargo_toml) { - // Look specifically for cargo-pup project or workspace with cargo-pup - if contents.contains("name = \"cargo-pup\"") - || (contents.contains("[workspace]") && contents.contains("cargo_pup")) - { + // Look specifically for the cargo-pup package itself (not a dependency) + // We check for the exact package name pattern to avoid matching projects + // that merely depend on cargo_pup_lint_config + if contents.contains("name = \"cargo_pup\"") { return Ok(dir.to_path_buf()); } } - // Also check if this directory has the specific cargo-pup source structure - if dir.join("src").join("pup_driver.rs").exists() { - return Ok(dir.to_path_buf()); - } - check_dir = dir.parent(); } diff --git a/cargo_pup_lint_config/src/struct_lint/builder.rs b/cargo_pup_lint_config/src/struct_lint/builder.rs index 960132d..82529ca 100644 --- a/cargo_pup_lint_config/src/struct_lint/builder.rs +++ b/cargo_pup_lint_config/src/struct_lint/builder.rs @@ -148,4 +148,10 @@ impl<'a> StructConstraintBuilder<'a> { self.add_rule_internal(StructRule::MustBePublic(self.current_severity)); self } + + /// Add a rule requiring the struct to have pub(crate) visibility + pub fn must_be_pub_crate(mut self) -> Self { + self.add_rule_internal(StructRule::MustBePubCrate(self.current_severity)); + self + } } diff --git a/cargo_pup_lint_config/src/struct_lint/types.rs b/cargo_pup_lint_config/src/struct_lint/types.rs index 68974c5..601157b 100644 --- a/cargo_pup_lint_config/src/struct_lint/types.rs +++ b/cargo_pup_lint_config/src/struct_lint/types.rs @@ -35,10 +35,12 @@ pub enum StructRule { MustBeNamed(String, Severity), /// Enforces that the struct name does not match the specified pattern MustNotBeNamed(String, Severity), - /// Enforces that the struct has private visibility + /// Enforces that the struct has private visibility (not pub, not pub(crate), not pub(super)) MustBePrivate(Severity), - /// Enforces that the struct has public visibility + /// Enforces that the struct has public visibility (pub) MustBePublic(Severity), + /// Enforces that the struct has pub(crate) visibility + MustBePubCrate(Severity), /// Enforces that the struct implements a specific trait ImplementsTrait(String, Severity), /// Logical AND - both rules must pass diff --git a/cargo_pup_lint_impl/Cargo.toml b/cargo_pup_lint_impl/Cargo.toml index 5d8e383..0be47c7 100644 --- a/cargo_pup_lint_impl/Cargo.toml +++ b/cargo_pup_lint_impl/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo_pup_lint_impl" -version = "0.1.4" +version = "0.1.5" edition = "2024" description = "Core lint implementations and rustc integration for cargo-pup architectural linting" license = "Apache-2.0" @@ -9,8 +9,8 @@ homepage = "https://github.com/datadog/cargo-pup" readme = "README.md" [dependencies] -cargo_pup_lint_config = { path = "../cargo_pup_lint_config", version = "=0.1.4" } -cargo_pup_common = { path = "../cargo_pup_common", version = "=0.1.4" } +cargo_pup_lint_config = { path = "../cargo_pup_lint_config", version = "=0.1.5" } +cargo_pup_common = { path = "../cargo_pup_common", version = "=0.1.5" } anyhow.workspace = true regex.workspace = true serde_json.workspace = true diff --git a/cargo_pup_lint_impl/src/helpers/architecture_lint_runner.rs b/cargo_pup_lint_impl/src/helpers/architecture_lint_runner.rs index 71a2f91..fe7225f 100644 --- a/cargo_pup_lint_impl/src/helpers/architecture_lint_runner.rs +++ b/cargo_pup_lint_impl/src/helpers/architecture_lint_runner.rs @@ -192,7 +192,7 @@ impl ArchitectureLintRunner { { // This is a trait implementation // Get the canonical trait name using the centralized helper - let trait_def_id = trait_ref.path.res.def_id(); + let trait_def_id = trait_ref.trait_ref.path.res.def_id(); let canonical_full_name = crate::helpers::queries::get_full_canonical_trait_name_from_def_id( &tcx, @@ -261,7 +261,6 @@ impl Callbacks for ArchitectureLintRunner { config.register_lints = Some(Box::new(move |_sess, lint_store| { // If we're actually linting, recreate the lints and add them all if let Mode::Check = mode { - //let lints = setup_lints_yaml().expect("can load lints"); for lint in lint_collection.lints() { lint.register_late_pass(lint_store); } @@ -326,8 +325,6 @@ impl Callbacks for ArchitectureLintRunner { _compiler: &rustc_interface::interface::Compiler, _tcx: TyCtxt<'_>, ) -> rustc_driver::Compilation { - _compiler.sess.coverage_discard_all_spans_in_codegen(); - rustc_driver::Compilation::Continue } } diff --git a/cargo_pup_lint_impl/src/lib.rs b/cargo_pup_lint_impl/src/lib.rs index e29b661..dc20106 100644 --- a/cargo_pup_lint_impl/src/lib.rs +++ b/cargo_pup_lint_impl/src/lib.rs @@ -1,6 +1,5 @@ #![feature(rustc_private)] // This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. -#![feature(array_windows)] #![feature(try_blocks)] pub mod helpers; 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 cb33f84..f02550a 100644 --- a/cargo_pup_lint_impl/src/lints/function_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/function_lint/lint.rs @@ -11,10 +11,11 @@ use rustc_lint::{LateContext, LateLintPass, LintStore}; use rustc_middle::ty::TyKind; use rustc_session::impl_lint_pass; use rustc_span::BytePos; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::Mutex; use super::no_allocation::detect_allocation_in_mir; +use super::no_panic::{PanicCategory, detect_panic_in_mir}; // Helper: retrieve the concrete Self type of the impl the method belongs to, if any fn get_self_type<'tcx>( @@ -22,7 +23,7 @@ fn get_self_type<'tcx>( fn_def_id: rustc_hir::def_id::DefId, ) -> Option> { ctx.tcx - .impl_of_method(fn_def_id) + .impl_of_assoc(fn_def_id) .map(|impl_def_id| ctx.tcx.type_of(impl_def_id).instantiate_identity()) } @@ -60,7 +61,33 @@ impl FunctionLint { evaluate_function_match(&self.matches, ctx, module_path, function_name, fn_def_id) } - // Evaluates the complex matcher structure to determine if a function matches + /// Helper method to check a single panic category and emit a lint if found + fn check_panic_category( + &self, + ctx: &LateContext<'_>, + fn_def_id: rustc_hir::def_id::DefId, + severity: cargo_pup_lint_config::Severity, + category: PanicCategory, + rule_name: &str, + ) { + if ctx.tcx.is_mir_available(fn_def_id) { + let mir = ctx.tcx.optimized_mir(fn_def_id); + let mut categories = HashSet::new(); + categories.insert(category); + + if let Some(violation) = detect_panic_in_mir(ctx.tcx, mir, &categories) { + span_lint_and_help( + ctx, + FUNCTION_LINT::get_by_severity(severity), + self.name().as_str(), + violation.span, + format!("Function may panic: {}", violation.reason), + None, + format!("Remove panic paths to satisfy the {} rule", rule_name), + ); + } + } + } } fn evaluate_function_match( @@ -204,6 +231,40 @@ fn evaluate_function_match( } false } + FunctionMatch::IsUnsafe => { + // Check if the function is unsafe by examining the HIR + if let Some(local_def_id) = fn_def_id.as_local() { + let node = ctx.tcx.hir_node_by_def_id(local_def_id); + match node { + rustc_hir::Node::Item(item) => { + if let rustc_hir::ItemKind::Fn { sig, .. } = &item.kind { + return matches!( + sig.header.safety, + rustc_hir::HeaderSafety::Normal(rustc_hir::Safety::Unsafe) + ); + } + } + rustc_hir::Node::TraitItem(trait_item) => { + if let rustc_hir::TraitItemKind::Fn(sig, _) = &trait_item.kind { + return matches!( + sig.header.safety, + rustc_hir::HeaderSafety::Normal(rustc_hir::Safety::Unsafe) + ); + } + } + rustc_hir::Node::ImplItem(impl_item) => { + if let rustc_hir::ImplItemKind::Fn(sig, _) = &impl_item.kind { + return matches!( + sig.header.safety, + rustc_hir::HeaderSafety::Normal(rustc_hir::Safety::Unsafe) + ); + } + } + _ => {} + } + } + false + } FunctionMatch::AndMatches(left, right) => { evaluate_function_match(left, ctx, module_path, function_name, fn_def_id) && evaluate_function_match(right, ctx, module_path, function_name, fn_def_id) @@ -381,6 +442,33 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint { } } } + FunctionRule::NoUnwrap(severity) => { + self.check_panic_category( + ctx, + fn_def_id, + *severity, + PanicCategory::Unwrap, + "NoUnwrap", + ); + } + FunctionRule::NoPanic(severity) => { + self.check_panic_category( + ctx, + fn_def_id, + *severity, + PanicCategory::ExplicitPanic, + "NoPanic", + ); + } + FunctionRule::NoIndexPanic(severity) => { + self.check_panic_category( + ctx, + fn_def_id, + *severity, + PanicCategory::IndexBounds, + "NoIndexPanic", + ); + } } } } @@ -506,6 +594,33 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint { } } } + FunctionRule::NoUnwrap(severity) => { + self.check_panic_category( + ctx, + fn_def_id, + *severity, + PanicCategory::Unwrap, + "NoUnwrap", + ); + } + FunctionRule::NoPanic(severity) => { + self.check_panic_category( + ctx, + fn_def_id, + *severity, + PanicCategory::ExplicitPanic, + "NoPanic", + ); + } + FunctionRule::NoIndexPanic(severity) => { + self.check_panic_category( + ctx, + fn_def_id, + *severity, + PanicCategory::IndexBounds, + "NoIndexPanic", + ); + } } } } diff --git a/cargo_pup_lint_impl/src/lints/function_lint/mod.rs b/cargo_pup_lint_impl/src/lints/function_lint/mod.rs index 645d944..3fad3cd 100644 --- a/cargo_pup_lint_impl/src/lints/function_lint/mod.rs +++ b/cargo_pup_lint_impl/src/lints/function_lint/mod.rs @@ -2,5 +2,6 @@ mod lint; mod no_allocation; +mod no_panic; pub use lint::FunctionLint; diff --git a/cargo_pup_lint_impl/src/lints/function_lint/no_allocation.rs b/cargo_pup_lint_impl/src/lints/function_lint/no_allocation.rs index 5d0554d..fae553d 100644 --- a/cargo_pup_lint_impl/src/lints/function_lint/no_allocation.rs +++ b/cargo_pup_lint_impl/src/lints/function_lint/no_allocation.rs @@ -58,6 +58,8 @@ pub fn detect_allocation_in_mir<'tcx>( None } } + // RuntimeChecks are UB checks inserted by the compiler, not relevant for allocation detection + Operand::RuntimeChecks(_) => None, }; if let Some(closure_def_id) = closure_def_id { diff --git a/cargo_pup_lint_impl/src/lints/function_lint/no_panic.rs b/cargo_pup_lint_impl/src/lints/function_lint/no_panic.rs new file mode 100644 index 0000000..2686bb6 --- /dev/null +++ b/cargo_pup_lint_impl/src/lints/function_lint/no_panic.rs @@ -0,0 +1,244 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +use rustc_hir::def_id::DefId; +use rustc_middle::mir::{AssertKind, Body, TerminatorKind}; +use rustc_middle::ty::TyCtxt; +use rustc_span::Span; +use std::collections::{HashMap, HashSet}; + +/// Resolves a span to its original call site if it comes from a macro expansion. +/// This is important for macros like panic!(), unreachable!(), etc. where the MIR +/// span points to the macro definition, not where it was called. +fn resolve_span_to_callsite(span: Span) -> Span { + // Use source_callsite() to get the original call site, walking through macro expansions + span.source_callsite() +} + +/// Internal categorization of panic sources +/// Note: MIR-level analysis cannot distinguish between panic!(), unreachable!(), +/// unimplemented!(), todo!(), and assert!() macros - they all compile to similar +/// underlying panic functions and are grouped under ExplicitPanic. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum PanicCategory { + /// Option/Result unwrap/expect calls + Unwrap, + /// All panic-family macros: panic!(), unreachable!(), unimplemented!(), todo!(), assert!() + ExplicitPanic, + /// Index bounds checking panics + IndexBounds, +} + +/// Classifies a function path into a panic category +pub fn classify_panic_source(path: &str) -> Option { + // Option::unwrap/expect + if (path.contains("Option::<") || path.contains("::Option::")) + && (path.ends_with(">::unwrap") || path.ends_with(">::expect")) + { + return Some(PanicCategory::Unwrap); + } + + // Result::unwrap/expect/unwrap_err/expect_err + if (path.contains("Result::<") || path.contains("::Result::")) + && (path.ends_with(">::unwrap") + || path.ends_with(">::expect") + || path.ends_with(">::unwrap_err") + || path.ends_with(">::expect_err")) + { + return Some(PanicCategory::Unwrap); + } + + // Index bounds - slice/array index panics + if path.contains("slice_index") + || path.contains("index_len_fail") + || path.contains("slice_start_index") + || path.contains("slice_end_index") + { + return Some(PanicCategory::IndexBounds); + } + + // All panic-family functions: panic!(), unreachable!(), unimplemented!(), todo!(), assert!() + // At MIR level these all compile to similar underlying panic functions + if path.contains("core::panicking::") + || path.contains("std::panicking::") + || path.contains("begin_panic") + || path.contains("panic_fmt") + || path.contains("unreachable") + || path.contains("assert_failed") + { + return Some(PanicCategory::ExplicitPanic); + } + + None +} + +/// Represents a violation of the no-panic rule +#[derive(Debug)] +pub struct PanicViolation { + pub span: Span, + pub reason: String, +} + +/// Detects panic-inducing calls in a function's MIR, filtered by categories. +/// Performs transitive analysis - if function A calls B which panics, A is flagged. +pub fn detect_panic_in_mir<'tcx>( + tcx: TyCtxt<'tcx>, + mir: &Body<'tcx>, + categories: &HashSet, +) -> Option { + let mut cache = HashMap::new(); + analyze_mir(tcx, mir, &mut cache, categories) +} + +/// Core MIR analysis. Separate function to allow recursive calls with shared cache. +fn analyze_mir<'tcx>( + tcx: TyCtxt<'tcx>, + mir: &Body<'tcx>, + cache: &mut HashMap, + categories: &HashSet, +) -> Option { + for (_bb, bb_data) in mir.basic_blocks.iter_enumerated() { + let Some(terminator) = &bb_data.terminator else { + continue; + }; + + match &terminator.kind { + // Check Assert terminators - these are compiler-inserted checks (bounds, overflow, etc.) + // Note: The assert!() macro does NOT use this - it compiles to function calls + TerminatorKind::Assert { msg, .. } => { + // Only check bounds checks for NoIndexPanic + // Overflow/division checks are implicit compiler checks, not explicit user assertions + if let AssertKind::BoundsCheck { .. } = &**msg + && categories.contains(&PanicCategory::IndexBounds) + { + return Some(PanicViolation { + span: resolve_span_to_callsite(terminator.source_info.span), + reason: "index bounds check may panic".to_string(), + }); + } + } + + // Check function calls + TerminatorKind::Call { func, args, .. } => { + // Extract function DefId using const_fn_def + if let Some((callee_def_id, _generics)) = func.const_fn_def() { + let path = tcx.def_path_str(callee_def_id); + + // Check arguments for closures that might panic + for arg in args.iter() { + use rustc_middle::mir::Operand; + + // Try to extract closure DefId from the operand + let closure_def_id = match &arg.node { + Operand::Constant(constant) => { + // Check if this is a closure type + let ty = constant.const_.ty(); + if let rustc_middle::ty::TyKind::Closure(def_id, _) = ty.kind() { + Some(*def_id) + } else if let rustc_middle::ty::TyKind::FnDef(def_id, _) = ty.kind() + { + Some(*def_id) + } else { + None + } + } + Operand::Move(place) | Operand::Copy(place) => { + // For Move/Copy operands, check the type of the place + let ty = place.ty(mir, tcx).ty; + if let rustc_middle::ty::TyKind::Closure(def_id, _) = ty.kind() { + Some(*def_id) + } else if let rustc_middle::ty::TyKind::FnDef(def_id, _) = ty.kind() + { + Some(*def_id) + } else { + None + } + } + // RuntimeChecks are UB checks inserted by the compiler, not relevant + Operand::RuntimeChecks(_) => None, + }; + + if let Some(closure_def_id) = closure_def_id { + // Analyze the closure if it's local + if closure_def_id.krate == rustc_hir::def_id::LOCAL_CRATE + && tcx.is_mir_available(closure_def_id) + && function_panics_with_categories( + tcx, + closure_def_id, + cache, + categories, + ) + { + return Some(PanicViolation { + span: resolve_span_to_callsite(terminator.source_info.span), + reason: format!("passes panicking closure to {path}"), + }); + } + } + } + + // Check if it's a known panicking function in the requested categories + if let Some(category) = classify_panic_source(&path) + && categories.contains(&category) + { + return Some(PanicViolation { + span: resolve_span_to_callsite(terminator.source_info.span), + reason: format!("calls panicking function: {path}"), + }); + } + + // Check transitively (with cycle detection) + if should_analyze_transitively(tcx, callee_def_id) + && function_panics_with_categories(tcx, callee_def_id, cache, categories) + { + return Some(PanicViolation { + span: resolve_span_to_callsite(terminator.source_info.span), + reason: format!("calls function that may panic: {path}"), + }); + } + } + } + + // Other terminator kinds don't represent panics we're looking for + _ => {} + } + } + + None +} + +/// Determines if we should recursively analyze a function +fn should_analyze_transitively(tcx: TyCtxt<'_>, def_id: DefId) -> bool { + // Only analyze functions in the local crate + // External crates are harder to analyze and may not have MIR available + def_id.krate == rustc_hir::def_id::LOCAL_CRATE && tcx.is_mir_available(def_id) +} + +/// Recursively checks if a function panics with specific categories, with memoization +fn function_panics_with_categories<'tcx>( + tcx: TyCtxt<'tcx>, + def_id: DefId, + cache: &mut HashMap, + categories: &HashSet, +) -> bool { + // Check cache + if let Some(&result) = cache.get(&def_id) { + return result; + } + + // Mark as false initially (cycle detection) + cache.insert(def_id, false); + + // Try to get MIR + if !tcx.is_mir_available(def_id) { + // Conservative: assume external functions don't panic + // This prevents false positives for standard library functions + return false; + } + + let mir = tcx.optimized_mir(def_id); + let panics = analyze_mir(tcx, mir, cache, categories).is_some(); + + // Update cache with actual result + cache.insert(def_id, panics); + panics +} 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 be0f4ef..c031542 100644 --- a/cargo_pup_lint_impl/src/lints/module_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/module_lint/lint.rs @@ -122,9 +122,8 @@ impl ModuleLint { fn is_mod_rs_file(&self, ctx: &LateContext<'_>, span: &rustc_span::Span) -> bool { let filename = ctx.sess().source_map().span_to_filename(*span); if let rustc_span::FileName::Real(filename) = filename { - let filename_str = - filename.to_string_lossy(rustc_span::FileNameDisplayPreference::Local); - filename_str.ends_with("/mod.rs") + let path = filename.path(rustc_span::RemapPathScopeComponents::DIAGNOSTICS); + path.to_string_lossy().ends_with("/mod.rs") } else { false } @@ -142,11 +141,17 @@ impl ModuleLint { let attrs = ctx.tcx.hir_attrs(item.hir_id()); for attr in attrs { - if attr.has_name(rustc_span::sym::proc_macro) { + // Detect proc_macro attributes via their Debug representation + // (the AttributeKind enum is not publicly accessible) + let attr_str = format!("{:?}", attr); + if attr_str.contains("ProcMacro(") + && !attr_str.contains("ProcMacroAttribute") + && !attr_str.contains("ProcMacroDerive") + { return Some("proc_macro"); - } else if attr.has_name(rustc_span::sym::proc_macro_attribute) { + } else if attr_str.contains("ProcMacroAttribute(") { return Some("proc_macro_attribute"); - } else if attr.has_name(rustc_span::sym::proc_macro_derive) { + } else if attr_str.contains("ProcMacroDerive") { return Some("proc_macro_derive"); } } 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 2b6b5d0..f760ed2 100644 --- a/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/struct_lint/lint.rs @@ -186,6 +186,14 @@ declare_variable_severity_lint!( "Struct must have public visibility" ); +declare_variable_severity_lint!( + pub, + STRUCT_LINT_MUST_BE_PUB_CRATE, + STRUCT_LINT_MUST_BE_PUB_CRATE_DENY, + STRUCT_LINT_MUST_BE_PUB_CRATE_WARN, + "Struct must have pub(crate) visibility" +); + impl_lint_pass!(StructLint => [ STRUCT_LINT_MUST_BE_NAMED_DENY, STRUCT_LINT_MUST_BE_NAMED_WARN, @@ -194,7 +202,9 @@ impl_lint_pass!(StructLint => [ STRUCT_LINT_MUST_BE_PRIVATE_DENY, STRUCT_LINT_MUST_BE_PRIVATE_WARN, STRUCT_LINT_MUST_BE_PUBLIC_DENY, - STRUCT_LINT_MUST_BE_PUBLIC_WARN + STRUCT_LINT_MUST_BE_PUBLIC_WARN, + STRUCT_LINT_MUST_BE_PUB_CRATE_DENY, + STRUCT_LINT_MUST_BE_PUB_CRATE_WARN ]); impl ArchitectureLintRule for StructLint { @@ -266,6 +276,22 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { let struct_visibility = ctx.tcx.visibility(def_id); let is_public = struct_visibility == rustc_middle::ty::Visibility::Public; + // Check if there's a visibility keyword (pub, pub(crate), pub(super), etc.) + let has_visibility_keyword = !item.vis_span.is_empty(); + + // Check if visibility is pub(crate): + // - Must have a visibility keyword (not inherited/private) + // - Must be restricted to the crate root + let is_pub_crate = match struct_visibility { + rustc_middle::ty::Visibility::Restricted(restricted_to) => { + has_visibility_keyword && restricted_to.is_crate_root() + } + _ => false, + }; + + // 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 { @@ -317,28 +343,60 @@ impl<'tcx> LateLintPass<'tcx> for StructLint { } } StructRule::MustBePrivate(severity) => { - if is_public { + if !is_private { + let visibility_desc = if is_public { + "pub" + } else if is_pub_crate { + "pub(crate)" + } else { + "restricted" // pub(super) or pub(in path) + }; span_lint_and_help( ctx, STRUCT_LINT_MUST_BE_PRIVATE::get_by_severity(*severity), self.name().as_str(), definition_span, - format!("Struct '{item_name}' is public, but must be private"), + format!( + "Struct '{item_name}' has {visibility_desc} visibility, but must be private" + ), None, - "Remove the 'pub' visibility modifier", + "Remove the visibility modifier", ); } } StructRule::MustBePublic(severity) => { if !is_public { + let visibility_desc = if is_pub_crate { + "pub(crate)" + } else { + "private" + }; span_lint_and_help( ctx, STRUCT_LINT_MUST_BE_PUBLIC::get_by_severity(*severity), self.name().as_str(), definition_span, - format!("Struct '{item_name}' is private, but must be public"), + format!( + "Struct '{item_name}' has {visibility_desc} visibility, but must be pub" + ), + None, + "Change the visibility to 'pub'", + ); + } + } + StructRule::MustBePubCrate(severity) => { + if !is_pub_crate { + let visibility_desc = if is_public { "pub" } else { "private" }; + 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)" + ), None, - "Add the 'pub' visibility modifier", + "Change the visibility to 'pub(crate)'", ); } } diff --git a/rust-toolchain.toml b/rust-toolchain.toml index e023780..4947801 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2025-07-25" +channel = "nightly-2026-01-22" components = ["llvm-tools-preview", "rustc-dev", "rust-analyzer", "rust-src"] diff --git a/src/main.rs b/src/main.rs index 3cc2466..6f3cba1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -59,7 +59,6 @@ //! //! -#![feature(array_windows)] #![feature(try_blocks)] #![warn(rust_2018_idioms, unused_lifetimes)] diff --git a/src/pup_driver.rs b/src/pup_driver.rs index 2389582..673c24b 100644 --- a/src/pup_driver.rs +++ b/src/pup_driver.rs @@ -1,6 +1,5 @@ #![feature(rustc_private)] // This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. -#![feature(array_windows)] #![feature(try_blocks)] extern crate rustc_driver; diff --git a/test_app/Cargo.lock b/test_app/Cargo.lock index 1228c1c..d3873a2 100644 --- a/test_app/Cargo.lock +++ b/test_app/Cargo.lock @@ -57,7 +57,7 @@ dependencies = [ [[package]] name = "cargo_pup_common" -version = "0.1.3" +version = "0.1.5" dependencies = [ "anyhow", "cargo_metadata", @@ -68,7 +68,7 @@ dependencies = [ [[package]] name = "cargo_pup_lint_config" -version = "0.1.3" +version = "0.1.5" dependencies = [ "anyhow", "cargo_pup_common", diff --git a/test_app/expected_output b/test_app/expected_output index fe6daa2..b01990a 100644 --- a/test_app/expected_output +++ b/test_app/expected_output @@ -39,7 +39,36 @@ warning: unused macro definition: `forbidden_macro` 4 | macro_rules! forbidden_macro { | ^^^^^^^^^^^^^^^ | - = note: `#[warn(unused_macros)]` on by default + = note: `#[warn(unused_macros)]` (part of `#[warn(unused)]`) on by default + +warning[E0133]: dereference of raw pointer is unsafe and requires unsafe block + --> src/unsafe_functions.rs:40:5 + | +40 | *ptr + | ^^^^ dereference of raw pointer + | + = note: for more information, see + = note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior +note: an unsafe function restricts its caller, but its body is safe by default + --> src/unsafe_functions.rs:39:1 + | +39 | pub unsafe fn forbidden_unsafe_with_params(ptr: *const i32) -> i32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `#[warn(unsafe_op_in_unsafe_fn)]` (part of `#[warn(rust_2024_compatibility)]`) on by default + +warning[E0133]: dereference of raw pointer is unsafe and requires unsafe block + --> src/unsafe_functions.rs:69:21 + | +69 | self.data = *ptr; + | ^^^^ dereference of raw pointer + | + = note: for more information, see + = note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior +note: an unsafe function restricts its caller, but its body is safe by default + --> src/unsafe_functions.rs:68:5 + | +68 | pub unsafe fn mutate_unsafe(&mut self, ptr: *const i32) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: Item 'DisallowedInModRs' disallowed in mod.rs due to empty-mod-file policy --> src/empty_mod_file/mod.rs:10:1 @@ -108,8 +137,8 @@ warning: struct 'DeniedStruct' is not allowed in this module warning: enum 'DeniedEnum' is not allowed in this module --> src/item_type/mod.rs:8:1 | -8 | / pub enum DeniedEnum { -9 | | Variant1, + 8 | / pub enum DeniedEnum { + 9 | | Variant1, 10 | | Variant2, 11 | | } | |_^ @@ -230,13 +259,13 @@ warning: Struct must match pattern '.*MyTraitImpl$', found 'MyBadlyNamedThing' = note: Applied by cargo-pup rule 'trait_restrictions'. = note: `#[warn(struct_lint_must_be_named)]` on by default -warning: Struct 'MyBadlyNamedThing' is public, but must be private +warning: Struct 'MyBadlyNamedThing' has pub visibility, but must be private --> src/trait_impl/mod.rs:17:1 | 17 | pub struct MyBadlyNamedThing {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove the 'pub' visibility modifier + = help: Remove the visibility modifier = note: Applied by cargo-pup rule 'trait_restrictions'. = note: `#[warn(struct_lint_must_be_private)]` on by default @@ -358,5 +387,240 @@ warning: Function allocates heap memory: calls function that allocates: no_alloc = help: Remove heap allocations to satisfy the NoAllocation rule = note: Applied by cargo-pup rule 'no_allocation_check'. -warning: `test_app` (bin "test_app") generated 27 warnings -error: could not compile `test_app` (bin "test_app") due to 6 previous errors; 27 warnings emitted +warning: Function may panic: calls panicking function: std::option::Option::::unwrap + --> src/no_panic.rs:57:5 + | +57 | opt.unwrap() + | ^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::option::Option::::expect + --> src/no_panic.rs:62:5 + | +62 | opt.expect("expected a value") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::result::Result::::unwrap + --> src/no_panic.rs:67:5 + | +67 | res.unwrap() + | ^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::result::Result::::expect + --> src/no_panic.rs:72:5 + | +72 | res.expect("expected success") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::result::Result::::unwrap_err + --> src/no_panic.rs:77:5 + | +77 | res.unwrap_err() + | ^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::result::Result::::expect_err + --> src/no_panic.rs:82:5 + | +82 | res.expect_err("expected error") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::option::Option::::unwrap + --> src/no_panic.rs:91:5 + | +91 | opt.unwrap() + | ^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls function that may panic: no_panic::helper_that_panics + --> src/no_panic.rs:96:5 + | +96 | helper_that_panics(opt) + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::option::Option::::expect + --> src/no_panic.rs:101:5 + | +101 | opt.expect("deep panic") + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls function that may panic: no_panic::deep_panicking_helper + --> src/no_panic.rs:106:5 + | +106 | deep_panicking_helper(opt) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls function that may panic: no_panic::middle_helper + --> src/no_panic.rs:111:5 + | +111 | middle_helper(opt) + | ^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::option::Option::::unwrap + --> src/no_panic.rs:134:9 + | +134 | self.value.unwrap() + | ^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: std::rt::panic_fmt + --> src/no_panic.rs:144:5 + | +144 | panic!("explicit panic"); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: core::panicking::panic + --> src/no_panic.rs:149:5 + | +149 | assert!(x > 0); + | ^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: core::panicking::assert_failed + --> src/no_panic.rs:154:5 + | +154 | assert_eq!(x, y); + | ^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: core::panicking::panic + --> src/no_panic.rs:159:5 + | +159 | unreachable!() + | ^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: core::panicking::panic + --> src/no_panic.rs:164:5 + | +164 | unimplemented!() + | ^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: calls panicking function: core::panicking::panic + --> src/no_panic.rs:169:5 + | +169 | todo!() + | ^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: index bounds check may panic + --> src/no_panic.rs:178:5 + | +178 | arr[0] + | ^^^^^^ + | + = help: Remove panic paths to satisfy the NoIndexPanic rule + = note: Applied by cargo-pup rule 'no_panic_check'. + +warning: Function may panic: index bounds check may panic + --> src/no_panic.rs:183:5 + | +183 | arr[idx] + | ^^^^^^^^ + | + = 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 + --> src/unsafe_functions.rs:29:1 + | +29 | pub unsafe fn forbidden_unsafe_function() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove this function to satisfy the architectural rule + = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. + +error: Function 'forbidden_unsafe_with_return' is forbidden by lint rule + --> src/unsafe_functions.rs:34:1 + | +34 | pub unsafe fn forbidden_unsafe_with_return() -> i32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove this function to satisfy the architectural rule + = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. + +error: Function 'forbidden_unsafe_with_params' is forbidden by lint rule + --> 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 + = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. + +error: Function 'get_data_unsafe' is forbidden by lint rule + --> src/unsafe_functions.rs:63:5 + | +63 | pub unsafe fn get_data_unsafe(&self) -> i32 { + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove this function to satisfy the architectural rule + = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. + +error: Function 'mutate_unsafe' is forbidden by lint rule + --> 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 + = note: Applied by cargo-pup rule 'unsafe_functions_forbidden'. + +error: Function 'process_unsafe' is forbidden by lint rule + --> src/unsafe_functions.rs:91:5 + | +91 | unsafe fn process_unsafe(&self) -> i32 { + | ^^^^^^^^^^^^^^^^^^^ + | + = help: Remove this function to satisfy the architectural 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 diff --git a/test_app/pup.ron b/test_app/pup.ron index 861a48e..7dce826 100644 --- a/test_app/pup.ron +++ b/test_app/pup.ron @@ -70,6 +70,7 @@ rules: [ MustBeNamed(".*MyTraitImpl$", Warn), MustBePrivate(Warn), + ImplementsTrait("test_app::trait_impl::MyTrait", Warn), ], )), Function(( @@ -79,12 +80,28 @@ ResultErrorMustImplementError(Warn), ], )), + Function(( + name: "builder_style_with_consuming_forbidden", + matches: AndMatches(NameRegex("^with_.*"), ReturnsType(SelfValue)), + rules: [ + MustNotExist(Error), + ], + )), + Function(( + name: "builder_style_set_consuming_forbidden", + matches: AndMatches(NameRegex("^set_.*"), ReturnsType(SelfValue)), + rules: [ + MustNotExist(Error), + ], + )), Module(( name: "macro_restriction_test", matches: Module("^test_app::macro_restriction$"), rules: [ DeniedItems( - items: ["declarative_macro"], + items: [ + "declarative_macro", + ], severity: Warn, ), ], @@ -94,46 +111,43 @@ matches: Module("^proc_macro_test$"), rules: [ DeniedItems( - items: ["proc_macro", "proc_macro_attribute", "proc_macro_derive"], + items: [ + "proc_macro", + "proc_macro_attribute", + "proc_macro_derive", + ], severity: Warn, ), ], )), Function(( - name: "builder_style_with_consuming_forbidden", - matches: AndMatches( - NameRegex("^with_.*"), - ReturnsType(SelfValue) - ), + name: "async_functions_forbidden", + matches: AndMatches(InModule("^test_app::async_functions$"), IsAsync), rules: [ MustNotExist(Error), ], )), Function(( - name: "builder_style_set_consuming_forbidden", - matches: AndMatches( - NameRegex("^set_.*"), - ReturnsType(SelfValue) - ), + name: "no_allocation_check", + matches: InModule("^test_app::no_allocation$"), rules: [ - MustNotExist(Error), + NoAllocation(Warn), ], )), Function(( - name: "async_functions_forbidden", - matches: AndMatches( - InModule("^test_app::async_functions$"), - IsAsync - ), + name: "no_panic_check", + matches: InModule("^test_app::no_panic$"), rules: [ - MustNotExist(Error), + NoUnwrap(Warn), + NoPanic(Warn), + NoIndexPanic(Warn), ], )), Function(( - name: "no_allocation_check", - matches: InModule("^test_app::no_allocation$"), + name: "unsafe_functions_forbidden", + matches: AndMatches(InModule("^test_app::unsafe_functions$"), IsUnsafe), rules: [ - NoAllocation(Warn), + MustNotExist(Error), ], )), ], diff --git a/test_app/src/main.rs b/test_app/src/main.rs index af1ea5f..28caf77 100644 --- a/test_app/src/main.rs +++ b/test_app/src/main.rs @@ -15,6 +15,8 @@ mod trait_impl; mod builder_style; mod async_functions; mod no_allocation; +mod no_panic; +mod unsafe_functions; fn main() { println!("Hello, world!"); diff --git a/test_app/src/no_panic.rs b/test_app/src/no_panic.rs new file mode 100644 index 0000000..cc92ee2 --- /dev/null +++ b/test_app/src/no_panic.rs @@ -0,0 +1,194 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +//! Test module for NoPanic rule validation +//! +//! This module tests that the NoPanic lint correctly identifies: +//! - Direct unwrap/expect calls on Option +//! - Direct unwrap/expect calls on Result +//! - Transitive panic paths through function calls + +// ============================================================================ +// Functions that should PASS (no panic paths) +// ============================================================================ + +/// Pure computation - no panic possible +pub fn pure_math(x: i32, y: i32) -> i32 { + x + y +} + +/// Using unwrap_or - safe alternative to unwrap +pub fn safe_option_handling(opt: Option) -> i32 { + opt.unwrap_or(0) +} + +/// Using unwrap_or_else - safe alternative with closure +pub fn safe_option_with_closure(opt: Option) -> i32 { + opt.unwrap_or_else(|| 42) +} + +/// Using match - explicit handling of both cases +pub fn safe_result_match(res: Result) -> i32 { + match res { + Ok(v) => v, + Err(_) => -1, + } +} + +/// Using if let - safe pattern matching +pub fn safe_if_let(opt: Option) -> i32 { + if let Some(v) = opt { + v + } else { + 0 + } +} + +/// Using ok() to convert Result to Option safely +pub fn safe_result_to_option(res: Result) -> Option { + res.ok() +} + +// ============================================================================ +// Functions that should FAIL (may panic) +// ============================================================================ + +/// Direct unwrap on Option - will panic if None +pub fn panics_option_unwrap(opt: Option) -> i32 { + opt.unwrap() +} + +/// Direct expect on Option - will panic if None +pub fn panics_option_expect(opt: Option) -> i32 { + opt.expect("expected a value") +} + +/// Direct unwrap on Result - will panic if Err +pub fn panics_result_unwrap(res: Result) -> i32 { + res.unwrap() +} + +/// Direct expect on Result - will panic if Err +pub fn panics_result_expect(res: Result) -> i32 { + res.expect("expected success") +} + +/// Direct unwrap_err on Result - will panic if Ok +pub fn panics_result_unwrap_err(res: Result) -> &str { + res.unwrap_err() +} + +/// Direct expect_err on Result - will panic if Ok +pub fn panics_result_expect_err(res: Result) -> &str { + res.expect_err("expected error") +} + +// ============================================================================ +// Transitive panic tests +// ============================================================================ + +/// Helper function that panics +fn helper_that_panics(opt: Option) -> i32 { + opt.unwrap() +} + +/// Calls a function that may panic - should be flagged +pub fn calls_panicking_helper(opt: Option) -> i32 { + helper_that_panics(opt) +} + +/// Deeply nested helper that panics +fn deep_panicking_helper(opt: Option) -> i32 { + opt.expect("deep panic") +} + +/// Middle helper in call chain +fn middle_helper(opt: Option) -> i32 { + deep_panicking_helper(opt) +} + +/// Top-level function with deeply nested panic +pub fn deeply_nested_panic(opt: Option) -> i32 { + middle_helper(opt) +} + +// ============================================================================ +// Method tests +// ============================================================================ + +pub struct PanicService { + value: Option, +} + +impl PanicService { + pub fn new(value: Option) -> Self { + Self { value } + } + + /// Safe method using unwrap_or + pub fn get_safe(&self) -> i32 { + self.value.unwrap_or(0) + } + + /// Panicking method using unwrap + pub fn get_panicking(&self) -> i32 { + self.value.unwrap() + } +} + +// ============================================================================ +// NoPanic rule tests (panic-family macros) +// ============================================================================ + +/// Explicit panic - forbidden by NoPanic +pub fn uses_panic() { + panic!("explicit panic"); +} + +/// Assert macro - forbidden by NoPanic +pub fn uses_assert(x: i32) { + assert!(x > 0); +} + +/// Assert_eq macro - forbidden by NoPanic +pub fn uses_assert_eq(x: i32, y: i32) { + assert_eq!(x, y); +} + +/// Unreachable macro - forbidden by NoPanic +pub fn uses_unreachable() -> i32 { + unreachable!() +} + +/// Unimplemented macro - forbidden by NoPanic +pub fn uses_unimplemented() -> i32 { + unimplemented!() +} + +/// Todo macro - forbidden by NoPanic +pub fn uses_todo() -> i32 { + todo!() +} + +// ============================================================================ +// NoIndexPanic rule tests (bounds checking) +// ============================================================================ + +/// Direct slice indexing - forbidden by NoIndexPanic +pub fn slice_index(arr: &[i32]) -> i32 { + arr[0] +} + +/// Array indexing with variable - forbidden by NoIndexPanic +pub fn array_index(arr: [i32; 3], idx: usize) -> i32 { + arr[idx] +} + +/// Safe alternative using get() +pub fn safe_slice_get(arr: &[i32]) -> Option<&i32> { + arr.get(0) +} + +/// Safe alternative using first() +pub fn safe_slice_first(arr: &[i32]) -> Option<&i32> { + arr.first() +} diff --git a/test_app/src/unsafe_functions.rs b/test_app/src/unsafe_functions.rs new file mode 100644 index 0000000..18665ad --- /dev/null +++ b/test_app/src/unsafe_functions.rs @@ -0,0 +1,94 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +//! Test module for IsUnsafe matcher validation +//! +//! This module tests that the IsUnsafe function matcher correctly identifies: +//! - Unsafe free functions +//! - Unsafe methods +//! - Unsafe trait implementations + +// ============================================================================ +// Functions that should PASS (safe functions - not matched by IsUnsafe) +// ============================================================================ + +/// Regular safe function +pub fn allowed_safe_function() -> i32 { + 42 +} + +/// Safe function with complex logic +pub fn allowed_safe_with_logic(x: i32) -> i32 { + if x > 0 { x * 2 } else { x } +} + +// ============================================================================ +// Functions that should FAIL (unsafe functions - matched by IsUnsafe) +// ============================================================================ + +/// Unsafe free function - forbidden +pub unsafe fn forbidden_unsafe_function() { + // Pretend to do something unsafe +} + +/// Unsafe function with return value - forbidden +pub unsafe fn forbidden_unsafe_with_return() -> i32 { + 42 +} + +/// Unsafe function with parameters - forbidden +pub unsafe fn forbidden_unsafe_with_params(ptr: *const i32) -> i32 { + *ptr +} + +// ============================================================================ +// Struct with mixed methods +// ============================================================================ + +pub struct UnsafeService { + data: i32, +} + +impl UnsafeService { + /// Safe constructor + pub fn new(data: i32) -> Self { + Self { data } + } + + /// Safe method + pub fn get_data(&self) -> i32 { + self.data + } + + /// Unsafe method - forbidden + pub unsafe fn get_data_unsafe(&self) -> i32 { + self.data + } + + /// Another unsafe method - forbidden + pub unsafe fn mutate_unsafe(&mut self, ptr: *const i32) { + self.data = *ptr; + } +} + +// ============================================================================ +// Trait with unsafe methods +// ============================================================================ + +pub trait UnsafeProcessor { + /// Safe trait method + fn process_safe(&self) -> i32; + + /// Unsafe trait method + unsafe fn process_unsafe(&self) -> i32; +} + +impl UnsafeProcessor for UnsafeService { + fn process_safe(&self) -> i32 { + self.data + } + + /// Unsafe trait implementation - forbidden + unsafe fn process_unsafe(&self) -> i32 { + self.data * 2 + } +} diff --git a/test_app/tests/pup_ron_test.rs b/test_app/tests/pup_ron_test.rs index a5cfc2d..cd9bfb1 100644 --- a/test_app/tests/pup_ron_test.rs +++ b/test_app/tests/pup_ron_test.rs @@ -118,6 +118,80 @@ fn test_lint_config() { .must_not_exist() .build(); + // ------------------------------------------------------------------ + // Macro restriction rules + // ------------------------------------------------------------------ + + builder + .module_lint() + .lint_named("macro_restriction_test") + .matching(|m| m.module("^test_app::macro_restriction$")) + .with_severity(Severity::Warn) + .denied_items(vec!["declarative_macro".to_string()]) + .build(); + + builder + .module_lint() + .lint_named("proc_macro_restriction_test") + .matching(|m| m.module("^proc_macro_test$")) + .with_severity(Severity::Warn) + .denied_items(vec![ + "proc_macro".to_string(), + "proc_macro_attribute".to_string(), + "proc_macro_derive".to_string(), + ]) + .build(); + + // ------------------------------------------------------------------ + // Async functions forbidden + // ------------------------------------------------------------------ + + builder + .function_lint() + .lint_named("async_functions_forbidden") + .matching(|m| m.in_module("^test_app::async_functions$").and(m.is_async())) + .with_severity(Severity::Error) + .must_not_exist() + .build(); + + // ------------------------------------------------------------------ + // NoAllocation rule + // ------------------------------------------------------------------ + + builder + .function_lint() + .lint_named("no_allocation_check") + .matching(|m| m.in_module("^test_app::no_allocation$")) + .with_severity(Severity::Warn) + .no_allocation() + .build(); + + // ------------------------------------------------------------------ + // Panic detection rules (NoUnwrap, NoPanic, NoIndexPanic) + // ------------------------------------------------------------------ + + builder + .function_lint() + .lint_named("no_panic_check") + .matching(|m| m.in_module("^test_app::no_panic$")) + .with_severity(Severity::Warn) + .no_unwrap() + .no_panic() + .no_index_panic() + .build(); + + // ------------------------------------------------------------------ + // Unsafe functions forbidden + // ------------------------------------------------------------------ + + builder + .function_lint() + .lint_named("unsafe_functions_forbidden") + .matching(|m| m.in_module("^test_app::unsafe_functions$").and(m.is_unsafe())) + .with_severity(Severity::Error) + .must_not_exist() + .build(); + // Write the configuration to pup.ron using the fixed write_to_file method builder .write_to_file("pup.ron") diff --git a/tests/ui/function_lint/is_unsafe_match.rs b/tests/ui/function_lint/is_unsafe_match.rs new file mode 100644 index 0000000..9634b44 --- /dev/null +++ b/tests/ui/function_lint/is_unsafe_match.rs @@ -0,0 +1,50 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +//@compile-flags: --crate-name test_is_unsafe_match +//@compile-flags: --crate-type lib + +// 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 function with return type +unsafe fn forbidden_unsafe_with_return() -> i32 { //~ ERROR: Function 'forbidden_unsafe_with_return' is forbidden by lint rule + 42 +} + +// Regular safe function that should NOT trigger the lint +fn allowed_safe_function() { +} + +// Safe function with return type +fn allowed_safe_with_return() -> i32 { + 42 +} + +// Unsafe method in impl block +struct TestStruct; + +impl TestStruct { + unsafe fn unsafe_method(&self) { //~ ERROR: Function 'unsafe_method' is forbidden by lint rule + } + + fn safe_method(&self) { + } +} + +// Unsafe functions in traits +trait UnsafeTrait { + unsafe fn trait_unsafe_method(&self); + + fn trait_safe_method(&self); +} + +impl UnsafeTrait for TestStruct { + unsafe fn trait_unsafe_method(&self) { //~ ERROR: Function 'trait_unsafe_method' is forbidden by lint rule + } + + 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 new file mode 100644 index 0000000..5550714 --- /dev/null +++ b/tests/ui/function_lint/is_unsafe_match.stderr @@ -0,0 +1,39 @@ +error: Function 'forbidden_unsafe_function' is forbidden by lint rule + --> 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 + = 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 + --> 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 + = note: Applied by cargo-pup rule 'unsafe_forbidden_test'. + +error: Function 'unsafe_method' is forbidden by lint rule + --> 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 + = note: Applied by cargo-pup rule 'unsafe_forbidden_test'. + +error: Function 'trait_unsafe_method' is forbidden by lint rule + --> 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 + = note: Applied by cargo-pup rule 'unsafe_forbidden_test'. + +error: aborting due to 4 previous errors + diff --git a/tests/ui/function_lint/no_panic.rs b/tests/ui/function_lint/no_panic.rs new file mode 100644 index 0000000..15eced4 --- /dev/null +++ b/tests/ui/function_lint/no_panic.rs @@ -0,0 +1,163 @@ +//@compile-flags: --crate-name=test_no_panic + +// Test functions that should pass (no panic paths) + +fn pure_computation(x: i32, y: i32) -> i32 { + x + y +} + +fn uses_unwrap_or(x: Option) -> i32 { + x.unwrap_or(0) +} + +fn uses_unwrap_or_else(x: Option) -> i32 { + x.unwrap_or_else(|| 42) +} + +fn uses_unwrap_or_default(x: Option) -> i32 { + x.unwrap_or_default() +} + +fn uses_result_ok(x: Result) -> Option { + x.ok() +} + +fn uses_result_err(x: Result) -> Option<&str> { + x.err() +} + +fn uses_if_let(x: Option) -> i32 { + if let Some(v) = x { + v + } else { + 0 + } +} + +fn uses_match(x: Result) -> i32 { + match x { + Ok(v) => v, + Err(_) => -1, + } +} + +// Test functions that should fail (may panic) + +fn uses_unwrap(x: Option) -> i32 { + x.unwrap() //~ ERROR: Function may panic +} + +fn uses_expect(x: Option) -> i32 { + x.expect("expected a value") //~ ERROR: Function may panic +} + +fn uses_result_unwrap(x: Result) -> i32 { + x.unwrap() //~ ERROR: Function may panic +} + +fn uses_result_expect(x: Result) -> i32 { + x.expect("expected ok") //~ ERROR: Function may panic +} + +fn uses_result_unwrap_err(x: Result) -> &str { + x.unwrap_err() //~ ERROR: Function may panic +} + +fn uses_result_expect_err(x: Result) -> &str { + x.expect_err("expected error") //~ ERROR: Function may panic +} + +// Note: MIR-level analysis cannot distinguish between panic!(), unreachable!(), unimplemented!(), +// todo!(), and assert!() macros - they all compile to similar underlying panic functions. +// All are detected by the NoPanic rule. + +// Transitive panic tests + +fn helper_panics(x: Option) -> i32 { + x.unwrap() //~ ERROR: Function may panic +} + +fn calls_panicking_function(x: Option) -> i32 { + helper_panics(x) //~ ERROR: Function may panic +} + +// Method tests +struct MyStruct { + value: Option, +} + +impl MyStruct { + fn safe_method(&self) -> i32 { + self.value.unwrap_or(0) + } + + fn panicking_method(&self) -> i32 { + self.value.unwrap() //~ ERROR: Function may panic + } +} + +// NoPanic tests (explicit panic!() calls) + +fn uses_panic_macro() { + panic!("explicit panic"); //~ ERROR: Function may panic +} + +fn uses_panic_with_format() { + panic!("panic with {}", "formatting"); //~ ERROR: Function may panic +} + +// Other panic macros (all caught by NoPanic rule) + +fn uses_unreachable() -> i32 { + unreachable!() //~ ERROR: Function may panic +} + +fn uses_unreachable_with_message() -> i32 { + unreachable!("should not reach here") //~ ERROR: Function may panic +} + +fn uses_unimplemented() -> i32 { + unimplemented!() //~ ERROR: Function may panic +} + +fn uses_todo() -> i32 { + todo!() //~ ERROR: Function may panic +} + +fn uses_todo_with_message() -> i32 { + todo!("implement this later") //~ ERROR: Function may panic +} + +fn uses_assert(x: i32) { + assert!(x > 0); //~ ERROR: Function may panic +} + +fn uses_assert_eq(x: i32, y: i32) { + assert_eq!(x, y); //~ ERROR: Function may panic +} + +fn uses_assert_ne(x: i32, y: i32) { + assert_ne!(x, y); //~ ERROR: Function may panic +} + +// NoIndexPanic tests + +fn uses_slice_index(arr: &[i32]) -> i32 { + arr[0] //~ ERROR: Function may panic +} + +fn uses_array_index(arr: [i32; 3], idx: usize) -> i32 { + arr[idx] //~ ERROR: Function may panic +} + +// Safe alternatives (should NOT trigger errors) + +fn safe_slice_get(arr: &[i32]) -> Option<&i32> { + arr.get(0) +} + +fn safe_slice_first(arr: &[i32]) -> Option<&i32> { + arr.first() +} + +fn main() {} diff --git a/tests/ui/function_lint/no_panic.stderr b/tests/ui/function_lint/no_panic.stderr new file mode 100644 index 0000000..2f8f6f9 --- /dev/null +++ b/tests/ui/function_lint/no_panic.stderr @@ -0,0 +1,192 @@ +error: Function may panic: calls panicking function: std::option::Option::::unwrap + --> tests/ui/function_lint/no_panic.rs:47:5 + | +LL | x.unwrap() + | ^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + = note: `#[deny(function_lint)]` on by default + +error: Function may panic: calls panicking function: std::option::Option::::expect + --> tests/ui/function_lint/no_panic.rs:51:5 + | +LL | x.expect("expected a value") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls panicking function: std::result::Result::::unwrap + --> tests/ui/function_lint/no_panic.rs:55:5 + | +LL | x.unwrap() + | ^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls panicking function: std::result::Result::::expect + --> tests/ui/function_lint/no_panic.rs:59:5 + | +LL | x.expect("expected ok") + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls panicking function: std::result::Result::::unwrap_err + --> tests/ui/function_lint/no_panic.rs:63:5 + | +LL | x.unwrap_err() + | ^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls panicking function: std::result::Result::::expect_err + --> tests/ui/function_lint/no_panic.rs:67:5 + | +LL | x.expect_err("expected error") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls panicking function: std::option::Option::::unwrap + --> tests/ui/function_lint/no_panic.rs:77:5 + | +LL | x.unwrap() + | ^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls function that may panic: helper_panics + --> tests/ui/function_lint/no_panic.rs:81:5 + | +LL | helper_panics(x) + | ^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls panicking function: std::option::Option::::unwrap + --> tests/ui/function_lint/no_panic.rs:95:9 + | +LL | self.value.unwrap() + | ^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoUnwrap rule + = note: Applied by cargo-pup rule 'no_panic_test'. + +error: Function may panic: calls panicking function: std::rt::panic_fmt + --> tests/ui/function_lint/no_panic.rs:102:5 + | +LL | panic!("explicit panic"); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: std::rt::panic_fmt + --> tests/ui/function_lint/no_panic.rs:106:5 + | +LL | panic!("panic with {}", "formatting"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: core::panicking::panic + --> tests/ui/function_lint/no_panic.rs:112:5 + | +LL | unreachable!() + | ^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: std::rt::panic_fmt + --> tests/ui/function_lint/no_panic.rs:116:5 + | +LL | unreachable!("should not reach here") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: core::panicking::panic + --> tests/ui/function_lint/no_panic.rs:120:5 + | +LL | unimplemented!() + | ^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: core::panicking::panic + --> tests/ui/function_lint/no_panic.rs:124:5 + | +LL | todo!() + | ^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: std::rt::panic_fmt + --> tests/ui/function_lint/no_panic.rs:128:5 + | +LL | todo!("implement this later") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: core::panicking::panic + --> tests/ui/function_lint/no_panic.rs:132:5 + | +LL | assert!(x > 0); + | ^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: core::panicking::assert_failed + --> tests/ui/function_lint/no_panic.rs:136:5 + | +LL | assert_eq!(x, y); + | ^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: calls panicking function: core::panicking::assert_failed + --> tests/ui/function_lint/no_panic.rs:140:5 + | +LL | assert_ne!(x, y); + | ^^^^^^^^^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoPanic rule + = note: Applied by cargo-pup rule 'no_explicit_panic_test'. + +error: Function may panic: index bounds check may panic + --> tests/ui/function_lint/no_panic.rs:146:5 + | +LL | arr[0] + | ^^^^^^ + | + = help: Remove panic paths to satisfy the NoIndexPanic rule + = note: Applied by cargo-pup rule 'no_index_panic_test'. + +error: Function may panic: index bounds check may panic + --> tests/ui/function_lint/no_panic.rs:150:5 + | +LL | arr[idx] + | ^^^^^^^^ + | + = help: Remove panic paths to satisfy the NoIndexPanic rule + = note: Applied by cargo-pup rule 'no_index_panic_test'. + +error: aborting due to 21 previous errors + diff --git a/tests/ui/function_lint/pup.ron b/tests/ui/function_lint/pup.ron index e9f4c45..25f5a2b 100644 --- a/tests/ui/function_lint/pup.ron +++ b/tests/ui/function_lint/pup.ron @@ -368,6 +368,66 @@ ) ] ) + ), + + // ====================================================================== + // SECTION: NoPanic Tests (for no_panic.rs) + // ====================================================================== + + // Test that functions must not use unwrap/expect on Option/Result + Function( + ( + name: "no_panic_test", + matches: InModule("test_no_panic"), + rules: [ + NoUnwrap( + Error, + ) + ] + ) + ), + + // Test NoPanic rule (all panic-family macros: panic!, unreachable!, unimplemented!, todo!, assert!) + Function( + ( + name: "no_explicit_panic_test", + matches: InModule("test_no_panic"), + rules: [ + NoPanic( + Error, + ) + ] + ) + ), + + // Test NoIndexPanic rule + Function( + ( + name: "no_index_panic_test", + matches: InModule("test_no_panic"), + rules: [ + NoIndexPanic( + Error, + ) + ] + ) + ), + + // ====================================================================== + // SECTION: IsUnsafe Matcher Tests (for is_unsafe_match.rs) + // ====================================================================== + + // Test that unsafe functions can be matched and forbidden + Function( + ( + name: "unsafe_forbidden_test", + matches: IsUnsafe, + rules: [ + MustNotExist( + Error, + ) + ] + ) ) ] ) \ No newline at end of file diff --git a/tests/ui/struct_lint_new/pub_crate_visibility.rs b/tests/ui/struct_lint_new/pub_crate_visibility.rs new file mode 100644 index 0000000..cf689a6 --- /dev/null +++ b/tests/ui/struct_lint_new/pub_crate_visibility.rs @@ -0,0 +1,21 @@ +// This product includes software developed at Datadog (https://www.datadoghq.com/) Copyright 2024 Datadog, Inc. + +//@compile-flags: --crate-name test_pub_crate_visibility +//@compile-flags: --crate-type lib + +// This test verifies that the StructRule::MustBePubCrate rule works correctly + +// Testing MustBePubCrate rule - struct that is pub but should be pub(crate) +pub struct CrateInternal { //~ ERROR: Struct 'CrateInternal' has pub visibility, but must be pub(crate) + field: i32, +} + +// Testing MustBePubCrate rule - struct that is private but should be pub(crate) +struct AlsoInternal { //~ ERROR: Struct 'AlsoInternal' has private visibility, but must be pub(crate) + value: String, +} + +// This should not trigger any warnings/errors (pub(crate), as required) +pub(crate) struct CorrectlyPubCrate { + data: Vec, +} diff --git a/tests/ui/struct_lint_new/pub_crate_visibility.stderr b/tests/ui/struct_lint_new/pub_crate_visibility.stderr new file mode 100644 index 0000000..59e4cb1 --- /dev/null +++ b/tests/ui/struct_lint_new/pub_crate_visibility.stderr @@ -0,0 +1,21 @@ +error: Struct 'CrateInternal' has pub visibility, but must be pub(crate) + --> tests/ui/struct_lint_new/pub_crate_visibility.rs:9:1 + | +LL | pub struct CrateInternal { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: Change the visibility to 'pub(crate)' + = note: Applied by cargo-pup rule 'must_be_pub_crate_test'. + = note: `#[deny(struct_lint_must_be_pub_crate)]` on by default + +error: Struct 'AlsoInternal' has private visibility, but must be pub(crate) + --> tests/ui/struct_lint_new/pub_crate_visibility.rs:14:1 + | +LL | struct AlsoInternal { + | ^^^^^^^^^^^^^^^^^^^^^ + | + = help: Change the visibility to 'pub(crate)' + = note: Applied by cargo-pup rule 'must_be_pub_crate_test'. + +error: aborting due to 2 previous errors + diff --git a/tests/ui/struct_lint_new/pup.ron b/tests/ui/struct_lint_new/pup.ron index 6859c23..59e0827 100644 --- a/tests/ui/struct_lint_new/pup.ron +++ b/tests/ui/struct_lint_new/pup.ron @@ -107,6 +107,20 @@ MustBePrivate(Error), // All InternalModel structs must be private ] ) + ), + + // Test MustBePubCrate rule - only apply to test_pub_crate_visibility crate + Struct( + ( + name: "must_be_pub_crate_test", + matches: AndMatches( + Name("test_pub_crate_visibility"), // Match based on crate name + Name(".*Internal"), // Match structs ending with Internal + ), + rules: [ + MustBePubCrate(Error), // Struct must be pub(crate) + ] + ) ) ] ) \ No newline at end of file diff --git a/tests/ui/struct_lint_new/visibility_rules.rs b/tests/ui/struct_lint_new/visibility_rules.rs index 40ced37..8770314 100644 --- a/tests/ui/struct_lint_new/visibility_rules.rs +++ b/tests/ui/struct_lint_new/visibility_rules.rs @@ -6,7 +6,7 @@ // This test verifies that the StructRule::MustBePrivate and StructRule::MustBePublic rules work correctly // Testing MustBePrivate rule: -pub struct InternalData { //~ ERROR: Struct 'InternalData' is public, but must be private +pub struct InternalData { //~ ERROR: Struct 'InternalData' has pub visibility, but must be private field: i32, } @@ -16,7 +16,7 @@ struct CorrectlyPrivate { } // Testing MustBePublic rule: -struct HiddenApi { //~ WARN: Struct 'HiddenApi' is private, but must be public +struct HiddenApi { //~ WARN: Struct 'HiddenApi' has private visibility, but must be pub id: u64, } @@ -26,7 +26,7 @@ pub struct CorrectlyPublic { } // Also testing name-pattern + visibility combination -pub struct InternalModel { //~ ERROR: Struct 'InternalModel' is public, but must be private +pub struct InternalModel { //~ ERROR: Struct 'InternalModel' has pub visibility, but must be private sensitive_data: String, } diff --git a/tests/ui/struct_lint_new/visibility_rules.stderr b/tests/ui/struct_lint_new/visibility_rules.stderr index 00b805b..aa0fbb7 100644 --- a/tests/ui/struct_lint_new/visibility_rules.stderr +++ b/tests/ui/struct_lint_new/visibility_rules.stderr @@ -1,30 +1,30 @@ -error: Struct 'InternalData' is public, but must be private +error: Struct 'InternalData' has pub visibility, but must be private --> tests/ui/struct_lint_new/visibility_rules.rs:9:1 | LL | pub struct InternalData { | ^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove the 'pub' visibility modifier + = help: Remove the visibility modifier = note: Applied by cargo-pup rule 'must_be_private_test'. = note: `#[deny(struct_lint_must_be_private)]` on by default -warning: Struct 'HiddenApi' is private, but must be public +warning: Struct 'HiddenApi' has private visibility, but must be pub --> tests/ui/struct_lint_new/visibility_rules.rs:19:1 | LL | struct HiddenApi { | ^^^^^^^^^^^^^^^^^^ | - = help: Add the 'pub' visibility modifier + = help: Change the visibility to 'pub' = note: Applied by cargo-pup rule 'must_be_public_test'. = note: `#[warn(struct_lint_must_be_public)]` on by default -error: Struct 'InternalModel' is public, but must be private +error: Struct 'InternalModel' has pub visibility, but must be private --> tests/ui/struct_lint_new/visibility_rules.rs:29:1 | LL | pub struct InternalModel { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: Remove the 'pub' visibility modifier + = help: Remove the visibility modifier = note: Applied by cargo-pup rule 'name_and_visibility_test'. error: aborting due to 2 previous errors; 1 warning emitted