diff --git a/crates/sniff-test/src/annotations/mod.rs b/crates/sniff-test/src/annotations/mod.rs index 8484bff..2ee4a6e 100644 --- a/crates/sniff-test/src/annotations/mod.rs +++ b/crates/sniff-test/src/annotations/mod.rs @@ -2,12 +2,14 @@ use crate::{ ARGS, annotations::{doc::get_comment_doc_str, span::Mergeable, toml::TomlAnnotation}, + check::LocalError, properties::Property, + reachability::LocallyReachable, }; use regex::Regex; use rustc_hir::{Attribute, def_id::DefId}; use rustc_middle::ty::TyCtxt; -use rustc_span::{ErrorGuaranteed, Span, source_map::Spanned}; +use rustc_span::{Span, source_map::Spanned}; use std::{collections::HashMap, fmt::Debug, ops::Range}; mod doc; @@ -80,13 +82,14 @@ pub struct ExpressionAnnotation { } impl ExpressionAnnotation { - pub fn satisfies_obligation( + pub fn satisfies_obligation<'tcx, P: Property>( &self, obligation: &Obligation, call_to: DefId, from_span: Span, - tcx: TyCtxt<'_>, - ) -> Result<(), ErrorGuaranteed> { + in_fn: &LocallyReachable, + // tcx: TyCtxt<'_>, + ) -> Result<(), LocalError<'tcx, P>> { match obligation { Obligation::ConsiderProperty => Ok(()), Obligation::ConsiderConditions(conditions) => { @@ -103,16 +106,12 @@ impl ExpressionAnnotation { .iter() .map(|a| &a.node.name) .collect::>(); - Err(tcx - .dcx() - .struct_span_err( - from_span, - format!( - "call to {:?} w/ text {:?} didn't consider some obligations {:?}", - self.text, call_to, names - ), - ) - .emit()) + Err(LocalError::CallMissedObligations { + func: in_fn.clone(), + callsite_comment: self.text.clone(), + callsite_span: from_span, + obligations: names.into_iter().cloned().collect(), + }) } } } diff --git a/crates/sniff-test/src/bin/sniff-test-driver.rs b/crates/sniff-test/src/bin/sniff-test-driver.rs index 981d8c8..f2f9b38 100644 --- a/crates/sniff-test/src/bin/sniff-test-driver.rs +++ b/crates/sniff-test/src/bin/sniff-test-driver.rs @@ -2,5 +2,23 @@ fn main() { sniff_test::env_logger_init(true); + let args: Vec = std::env::args().collect(); + // If there are enough args that we're trying to call the real rustc, + // just pass through to calling the real rustc + if args.len() >= 2 { + let real_rustc = &args[1]; + let rest = &args[2..]; + let is_passthrough = rest + .iter() + .any(|a| a.starts_with("--print") || a == "-vV" || a == "--version" || a == "-V") + || rest.is_empty(); + + if is_passthrough { + use std::os::unix::process::CommandExt; + let err = std::process::Command::new(real_rustc).args(rest).exec(); + panic!("failed to exec rustc: {err}"); + } + } + rustc_plugin::driver_main(sniff_test::PrintAllItemsPlugin); } diff --git a/crates/sniff-test/src/check/err.rs b/crates/sniff-test/src/check/err.rs index b4077ff..465eab3 100644 --- a/crates/sniff-test/src/check/err.rs +++ b/crates/sniff-test/src/check/err.rs @@ -1,4 +1,4 @@ -use crate::properties::FoundAxiom; +use crate::{check::LocalError, properties::FoundAxiom}; use itertools::Itertools; use rustc_errors::Diag; use rustc_middle::ty::TyCtxt; @@ -11,31 +11,62 @@ use crate::{ pub fn report_errors<'tcx, P: Property>( tcx: TyCtxt<'tcx>, - func: LocallyReachable, _property: P, - unjustified_axioms: Vec>, - unjustified_calls: Vec, + errors: Vec>, ) -> ErrorGuaranteed { - let dcx = tcx.dcx(); - let def_span = tcx.def_span(func.reach); - let fn_name = tcx.def_path_str(func.reach.to_def_id()); - - let mut diag = dcx.struct_span_err( - def_span, - summary::summary_string::

(&fn_name, &unjustified_axioms, &unjustified_calls), - ); - - diag = diag.with_note(reachability_str(&fn_name, tcx, &func)); - - for axiom in unjustified_axioms { - diag = extend_diag_axiom::

(diag, axiom); - } + errors + .into_iter() + .map(|error| report_error(tcx, error)) + .last() + .expect("don't call this on empty errors") +} - for calls in unjustified_calls { - diag = extend_diag_calls(diag, tcx, calls); +fn report_error<'tcx, P: Property>( + tcx: TyCtxt<'tcx>, + error: LocalError<'tcx, P>, +) -> ErrorGuaranteed { + let dcx = tcx.dcx(); + let def_span = tcx.def_span(error.func().reach); + let fn_name = tcx.def_path_str(error.func().reach.to_def_id()); + + match error { + LocalError::Basic { tcx, func, _property, unjustified_axioms, unjustified_calls } => { + let mut diag = dcx.struct_span_err( + def_span, + summary::summary_string::

(&fn_name, &unjustified_axioms, &unjustified_calls), + ); + + diag = diag.with_note(reachability_str(&fn_name, tcx, &func)); + + for axiom in unjustified_axioms { + diag = extend_diag_axiom::

(diag, axiom); + } + + for calls in unjustified_calls { + diag = extend_diag_calls(diag, tcx, calls); + } + + diag.emit() + }, + LocalError::CallMissedObligations { callsite_comment: _, callsite_span, obligations, .. } => { + dcx.struct_span_err( + callsite_span, + format!("call to {fn_name} here fails to consider its named obligations {obligations:?}"), + ).emit() + }, + LocalError::FnDefShouldHaveKeyword { needed_keyword, .. } => { + dcx.struct_span_err( + def_span, + format!("function definition of {fn_name} here should have the {needed_keyword} keyword because of the {} property", P::property_name()), + ).emit() + }, + LocalError::Trait { inconsistent_w_trait, .. } => { + dcx.struct_span_err( + def_span, + format!("implementation {fn_name} here has {} obligations that are inconsistent with those on the definition of the {} trait", tcx.def_path_debug_str(inconsistent_w_trait), P::property_name()), + ).with_span_note(tcx.def_span(inconsistent_w_trait), "which is defined here").emit() + } } - - diag.emit() } fn extend_diag_axiom<'tcx, P: Property>( diff --git a/crates/sniff-test/src/check/mod.rs b/crates/sniff-test/src/check/mod.rs index d50b4bb..9fee28b 100644 --- a/crates/sniff-test/src/check/mod.rs +++ b/crates/sniff-test/src/check/mod.rs @@ -5,9 +5,9 @@ use crate::{ }; use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId}; use rustc_middle::ty::TyCtxt; -use rustc_span::ErrorGuaranteed; +use rustc_span::Span; -mod err; +pub mod err; mod expr; #[derive(Debug, Default, Clone)] @@ -21,9 +21,10 @@ pub struct CheckStats { /// Checks that all local functions in the crate are properly annotated. pub fn check_crate_for_property( - tcx: TyCtxt, + tcx: TyCtxt<'_>, property: P, -) -> Result { + is_dependency: bool, +) -> Result>> { // Parse TOML annotations from file let toml_path = "sniff-test.toml"; let toml_annotations = match TomlAnnotation::from_file(toml_path) { @@ -39,7 +40,7 @@ pub fn check_crate_for_property( }; let mut stats = CheckStats::default(); - let entry = reachability::analysis_entry_points::

(tcx); + let entry = reachability::analysis_entry_points::

(tcx, is_dependency); // Debug print all our entries and where they are in the src // (this isn't actually needed for analysis) @@ -61,6 +62,7 @@ pub fn check_crate_for_property( stats.entrypoints = entry.len(); let reachable = reachability::locally_reachable_from(tcx, entry); + let mut local_errors = Vec::new(); log::info!( "the {} reachable functions for {} in {} are {reachable:#?}", @@ -77,17 +79,21 @@ pub fn check_crate_for_property( match annotations::parse_fn_def(tcx, &toml_annotations, func.reach, property) { Some(annotation) if annotation.creates_obligation().is_some() => { stats.w_obligation += 1; - if let Some(trait_def) = is_impl_of_trait(tcx, func.reach) { - check_consistent_w_trait_requirements( + if let Some(trait_def) = is_impl_of_trait(tcx, func.reach) + && let Err(e) = check_consistent_w_trait_requirements( tcx, &func, &annotation, trait_def, property, &toml_annotations, - )?; + ) + { + local_errors.push(e); + } + if let Err(e) = property.additional_check(tcx, func.clone()) { + local_errors.push(e); } - property.additional_check(tcx, func.reach.to_def_id())?; // TODO: in the future, could check to make sure this annotation doesn't create unneeded obligations. log::debug!( "fn {:?} has obligations {:?}, we'll trust it...", @@ -109,30 +115,62 @@ pub fn check_crate_for_property( tcx.crate_name(LOCAL_CRATE) ); - let mut res = Ok(()); - - // For all reachable local function definitions, ensure their axioms align with their annotations. - for func in reachable_no_obligations { - // Continue checking functions, even if one fails to ensure we report as many errors as possible. - // TODO: is this actually bad? one could imagine properly documenting one function could also - // fix errors for where it is called. - if let Err(e) = - check_function_for_property(tcx, &toml_annotations, func, property, &mut stats) - { - res = Err(e); - } + local_errors.extend(reachable_no_obligations.into_iter().filter_map(|func| { + check_function_for_property(tcx, &toml_annotations, func, property, &mut stats).err() + })); + + if !local_errors.is_empty() { + return Err(local_errors); } - res.map(|()| stats) + Ok(stats) } -fn check_function_for_property( - tcx: TyCtxt, +pub enum LocalError<'tcx, P: Property> { + Basic { + tcx: TyCtxt<'tcx>, + func: LocallyReachable, + _property: P, + unjustified_axioms: Vec>, + unjustified_calls: Vec, + }, + Trait { + func_has_obligations: LocallyReachable, + inconsistent_w_trait: DefId, + }, + CallMissedObligations { + func: LocallyReachable, + callsite_comment: String, + callsite_span: Span, + obligations: Vec, + }, + FnDefShouldHaveKeyword { + fn_def: LocallyReachable, + needed_keyword: &'static str, + }, +} + +impl LocalError<'_, P> { + pub fn func(&self) -> &LocallyReachable { + match self { + Self::Basic { func, .. } + | Self::CallMissedObligations { func, .. } + | Self::FnDefShouldHaveKeyword { fn_def: func, .. } + | Self::Trait { + func_has_obligations: func, + .. + } => func, + } + } +} + +fn check_function_for_property<'tcx, P: Property>( + tcx: TyCtxt<'tcx>, toml_annotations: &TomlAnnotation, func: LocallyReachable, property: P, stats: &mut CheckStats, -) -> Result<(), ErrorGuaranteed> { +) -> Result<(), LocalError<'tcx, P>> { // Look for all axioms within this function let axioms = properties::find_axioms(tcx, &func, property).collect::>(); log::debug!("fn {:?} has raw axioms {:#?}", func.reach, axioms); @@ -152,7 +190,7 @@ fn check_function_for_property( stats.calls_checked += call_ct; log::debug!("fn {:?} has raw calls {:#?}", func.reach, calls); let mut unjustified_calls = Vec::new(); - let only_unjustified = only_unjustified_callsites(tcx, func.reach, property); + let only_unjustified = only_unjustified_callsites(tcx, func.clone(), property); for c in calls { match only_unjustified(c) { JustificationStatus::AllCallsJustified => (), @@ -179,24 +217,24 @@ fn check_function_for_property( Ok(()) } else { // Unjustified issues, report them!! - Err(err::report_errors( + Err(LocalError::Basic { tcx, func, - property, + _property: property, unjustified_axioms, unjustified_calls, - )) + }) } } -fn check_consistent_w_trait_requirements( - tcx: TyCtxt, +fn check_consistent_w_trait_requirements<'tcx, P: Property>( + tcx: TyCtxt<'tcx>, func: &LocallyReachable, annotation: &DefAnnotation, t: DefId, property: P, toml_annotations: &TomlAnnotation, -) -> Result<(), ErrorGuaranteed> { +) -> Result<(), LocalError<'tcx, P>> { let name = tcx.item_ident(func.reach); let trait_fn = tcx @@ -211,8 +249,10 @@ fn check_consistent_w_trait_requirements( if annotation.creates_obligation() == def_obligation { Ok(()) } else { - let a = tcx.dcx().struct_err(format!("function {:?} has obligations, which is inconsistent with the definition of that associated function for trait {:?}!", func.reach, t)).emit(); - Err(a) + Err(LocalError::Trait { + func_has_obligations: func.clone(), + inconsistent_w_trait: t, + }) } } @@ -241,23 +281,23 @@ fn only_unjustified_axioms<'tcx, P: Property>( } } -enum JustificationStatus { +enum JustificationStatus<'tcx, P: Property> { AllCallsJustified, SomeNotJustified(CallsWObligations), - ImproperJustification(ErrorGuaranteed), + ImproperJustification(LocalError<'tcx, P>), } /// Filter a set of calls to a function for only those which are not property justified. -fn only_unjustified_callsites( - tcx: TyCtxt, - in_fn: LocalDefId, +fn only_unjustified_callsites<'tcx, P: Property>( + tcx: TyCtxt<'tcx>, + in_fn: LocallyReachable, property: P, -) -> impl Fn(CallsWObligations) -> JustificationStatus { +) -> impl Fn(CallsWObligations) -> JustificationStatus<'tcx, P> { move |mut calls| { let mut new_spans = Vec::new(); for call_span in calls.from_spans { - let call_expr = expr::find_expr_for_call(tcx, calls.call_to, in_fn, call_span); + let call_expr = expr::find_expr_for_call(tcx, calls.call_to, in_fn.reach, call_span); let callsite_annotation = parse_expr(tcx, call_expr, property); match callsite_annotation { @@ -266,7 +306,8 @@ fn only_unjustified_callsites( &calls.obligation, calls.call_to, call_span, - tcx, + &in_fn, + // tcx, ) { return JustificationStatus::ImproperJustification(e); } diff --git a/crates/sniff-test/src/lib.rs b/crates/sniff-test/src/lib.rs index 39cc7b8..fa44687 100644 --- a/crates/sniff-test/src/lib.rs +++ b/crates/sniff-test/src/lib.rs @@ -11,6 +11,7 @@ clippy::missing_panics_doc, // TODO: should remove this, kinda ironic for us to be using it... clippy::missing_errors_doc, clippy::needless_pass_by_value, + clippy::result_large_err )] extern crate lazy_static; @@ -35,7 +36,7 @@ pub mod utils; use std::{borrow::Cow, env, process::Command, sync::Mutex}; -use clap::Parser; +use clap::{Parser, ValueEnum}; use rustc_hir::def_id::LOCAL_CRATE; use rustc_middle::ty::TyCtxt; use rustc_plugin::{CrateFilter, RustcPlugin, RustcPluginArgs, Utf8Path}; @@ -49,15 +50,15 @@ pub struct PrintAllItemsPlugin; // To parse CLI arguments, we use Clap for this example. But that // detail is up to you. -#[derive(Parser, Serialize, Deserialize, Clone, Default, Debug)] +#[derive(Parser, Serialize, Deserialize, Default, Clone, Debug)] pub struct SniffTestArgs { - // #[arg(short, long)] - // allcaps: bool, + /// How to handle this workspace's dependencies. + #[arg(short, long)] + dependencies: DependenciesPosture, - // #[arg(short, long)] - // release: bool, #[arg(short, long)] - /// Whether or not to check + /// LEGACY ARG (i'm keeping it around to be faster, will remove later): + /// whether or not dependencies have to have sniff-test formatted code comments. check_dependencies: bool, #[arg(short, long)] @@ -70,7 +71,26 @@ pub struct SniffTestArgs { cargo_args: Vec, } -const TO_FILE: bool = true; +#[derive(ValueEnum, Clone, Debug, Default, Serialize, Deserialize)] +enum DependenciesPosture { + #[default] + /// Trust that dependencies have been properly documented with regard to the desired properties. + /// + /// *"I trust them"* + Trust, + /// Analyze the **used** public functions of all transitive dependencies, flagging potential issues + /// to be fixed at the boundary of the current workspace. + /// + /// *"I don't care if their code is correct, I just want to make sure how I'm using it is fine."* + Find, + /// Analyze the public functions of all transitive dependencies, ensuring that they + /// would pass the same analysis done on this workspace. + /// + /// *"Let's make sure their code is correct too."* + Verify, +} + +const TO_FILE: bool = false; pub static ARGS: Mutex> = Mutex::new(None); @@ -125,7 +145,7 @@ impl RustcPlugin for PrintAllItemsPlugin { // If one of the CLI arguments was a specific file to analyze, then you // could provide a different filter. fn args(&self, _target_dir: &Utf8Path) -> RustcPluginArgs { - let args = SniffTestArgs::parse_from(env::args().skip(1)); + let args = SniffTestArgs::parse_from(env::args()); let filter = CrateFilter::AllCrates; RustcPluginArgs { args, filter } } @@ -145,6 +165,13 @@ impl RustcPlugin for PrintAllItemsPlugin { // Register the sniff_tool let existing = std::env::var("RUSTFLAGS").unwrap_or_default(); cargo.env("RUSTFLAGS", format!("-Zcrate-attr=feature(register_tool) -Zcrate-attr=register_tool(sniff_tool) -Aunused-doc-comments {existing} -Zcrate-attr=feature(custom_inner_attributes)")); + + // Point to the driver binary, not the cargo subcommand binary + let driver = std::env::current_exe() + .unwrap() + .with_file_name("sniff-test-driver"); // <-- driver, not cargo-sniff-test + cargo.env("RUSTC_WRAPPER", &driver); + cargo.env_remove("RUSTC_WORKSPACE_WRAPPER"); } // In the driver, we use the Rustc API to start a compiler session @@ -158,7 +185,8 @@ impl RustcPlugin for PrintAllItemsPlugin { *ARGS.lock().unwrap() = Some(plugin_args.clone()); let mut callbacks = PrintAllItemsCallbacks { - args: Some(plugin_args), + args: Some(plugin_args.clone()), + is_dependency: is_dependency(&compiler_args), }; rustc_driver::run_compiler(&compiler_args, &mut callbacks); @@ -169,6 +197,68 @@ impl RustcPlugin for PrintAllItemsPlugin { #[allow(dead_code)] struct PrintAllItemsCallbacks { args: Option, + is_dependency: bool, +} + +/// Checks if a given compiler invocation is for compiling something outside the current workspace. +// TODO: right now this uses a silly hack with the args, but there's got to be a better way... +fn is_dependency(compiler_args: &[String]) -> bool { + let typical_path_slot = &compiler_args[4]; + + if !std::path::Path::new(typical_path_slot) + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("rs")) + { + // This is very bad, but if there's not a rust file in this slot, I think you're likely not being + // ultimately invoked by cargo, so you can't be a dependency. I'm 110% sure I will be proven wrong about this + // sometime and come back here to find this issue. + return false; + } + + // And this is very hacky, but library dependencies are installed in the .cargo/registry, so we can evilly + // use whether the path is absolute to check if the crate to be compiled is from the registry and, thus, + // must be a dependency. + typical_path_slot + .chars() + .next() + .map(|first| first == '/') + .expect("shouldn't have an empty string here") +} + +// FIXME: move to check submodule +fn analyze_crate( + tcx: TyCtxt, + crate_name: rustc_span::Symbol, + is_dependency: bool, + args: &SniffTestArgs, +) -> rustc_driver::Compilation { + match (is_dependency, &args.dependencies) { + // If we're not a dependency, or we are but we're verifying them -> run full analysis + (false, _) | (true, DependenciesPosture::Verify) => { + let property = properties::SafetyProperty; + let stats = match check_crate_for_property(tcx, property, is_dependency) { + Ok(stats) => stats, + Err(local_err) => { + crate::check::err::report_errors(tcx, property, local_err); + println!("the {crate_name} crate FAILED the sniff test"); + return rustc_driver::Compilation::Stop; + } + }; + + println!( + "the {crate_name:^20} crate passes the sniff test!! \t\t(stable id {:16x?}) - {:>5}", + tcx.stable_crate_id(LOCAL_CRATE).as_u64(), + if is_dependency { "dep" } else { "local" }, + ); + log::debug!("\tstats for `{crate_name}` are {stats:?}"); + } + (true, DependenciesPosture::Find) => { + // find property 'caveats' + todo!("do check, but don't error. just write to file for later analysis"); + } + (true, DependenciesPosture::Trust) => { /* Nothing to be done! We're trusting :) */ } + } + rustc_driver::Compilation::Continue } impl rustc_driver::Callbacks for PrintAllItemsCallbacks { @@ -182,18 +272,15 @@ impl rustc_driver::Callbacks for PrintAllItemsCallbacks { ) -> rustc_driver::Compilation { let crate_name = tcx.crate_name(LOCAL_CRATE); - log::debug!("checking crate {crate_name}"); - let Ok(stats) = check_crate_for_property(tcx, properties::SafetyProperty) else { - return rustc_driver::Compilation::Stop; - }; - - println!("the `{crate_name}` crate passes the sniff test!!"); - log::debug!("\tstats for `{crate_name}` are {stats:?}"); - // Note that you should generally allow compilation to continue. If // your plugin is being invoked on a dependency, then you need to ensure // the dependency is type-checked (its .rmeta file is emitted into target/) // so that its dependents can read the compiler outputs. - rustc_driver::Compilation::Continue + analyze_crate( + tcx, + crate_name, + self.is_dependency, + self.args.as_ref().unwrap(), + ) } } diff --git a/crates/sniff-test/src/properties/mod.rs b/crates/sniff-test/src/properties/mod.rs index 42a3b08..cbdd3f5 100644 --- a/crates/sniff-test/src/properties/mod.rs +++ b/crates/sniff-test/src/properties/mod.rs @@ -1,16 +1,13 @@ //! A module for detecting axiomatic program patterns +use crate::check::LocalError; use crate::{annotations::PropertyViolation, reachability::LocallyReachable}; use regex::Regex; -use rustc_hir::{ - def_id::DefId, - intravisit::{self, Visitor}, -}; +use rustc_hir::intravisit::{self, Visitor}; use rustc_middle::{ hir::nested_filter, ty::{TyCtxt, TypeckResults}, }; -use rustc_span::ErrorGuaranteed; use std::fmt::Debug; use std::fmt::Display; @@ -40,7 +37,11 @@ pub trait Property: Debug + Copy + 'static { ) -> Vec>; /// An additional check to perform on all function defs that are annotated as having this property. - fn additional_check(&self, _tcx: TyCtxt, _fn_def: DefId) -> Result<(), ErrorGuaranteed> { + fn additional_check<'tcx>( + &self, + _tcx: TyCtxt<'tcx>, + _fn_def: LocallyReachable, + ) -> Result<(), LocalError<'tcx, Self>> { Ok(()) } } diff --git a/crates/sniff-test/src/properties/safety.rs b/crates/sniff-test/src/properties/safety.rs index b0196eb..25f5f71 100644 --- a/crates/sniff-test/src/properties/safety.rs +++ b/crates/sniff-test/src/properties/safety.rs @@ -9,7 +9,9 @@ use rustc_type_ir::TyKind; use super::Axiom; use crate::{ annotations::PropertyViolation, + check::LocalError, properties::{FoundAxiom, Property}, + reachability::LocallyReachable, }; #[derive(Debug, Clone, Copy)] @@ -52,13 +54,16 @@ impl Property for SafetyProperty { vec![] } - fn additional_check( + fn additional_check<'tcx>( &self, - tcx: TyCtxt, - fn_def: rustc_hir::def_id::DefId, // TODO: change to fn_def - ) -> Result<(), rustc_span::ErrorGuaranteed> { - match tcx.fn_sig(fn_def).skip_binder().safety() { - rustc_hir::Safety::Safe => Err(tcx.dcx().struct_span_err(tcx.def_span(fn_def), format!("function {fn_def:?} is annotated as having safety preconditions, but does not use the `unsafe` keyword!")).emit()), + tcx: TyCtxt<'tcx>, + fn_def: LocallyReachable, // TODO: change to fn_def + ) -> Result<(), LocalError<'tcx, Self>> { + match tcx.fn_sig(fn_def.reach).skip_binder().safety() { + rustc_hir::Safety::Safe => Err(LocalError::FnDefShouldHaveKeyword { + fn_def, + needed_keyword: "unsafe", + }), rustc_hir::Safety::Unsafe => Ok(()), } } diff --git a/crates/sniff-test/src/reachability/entry.rs b/crates/sniff-test/src/reachability/entry.rs index 8933a24..b6d2d29 100644 --- a/crates/sniff-test/src/reachability/entry.rs +++ b/crates/sniff-test/src/reachability/entry.rs @@ -11,11 +11,15 @@ use crate::{ reachability::attrs::{self, SniffToolAttr}, }; -pub fn analysis_entry_points(tcx: TyCtxt) -> Vec { +pub fn analysis_entry_points(tcx: TyCtxt, is_dependency: bool) -> Vec { // TODO: should use a btree rather than a hash set here so that we'll have a consistent order // but local def ids aren't ord so this will likely require an upstream changes. let mut entry_points = HashSet::new(); + if is_dependency { + return all_pub_local_fn_defs(tcx).collect::>(); + } + if let Some(global_annotation) = find_global_annotation::

(tcx) { if global_annotation.just_check_pub { // A `_pub` annotation can also be used in conjunction with other non-pub functions, diff --git a/tests/Cargo.toml b/tests/Cargo.toml index a55968a..4ff8d60 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -15,3 +15,10 @@ lazy_static = "1.5.0" serde = "1.0.228" walkdir = "2.5.0" serde_json = "1.0.145" + +# the insta docs recommend using a higher level of optimizations for faster diffing +[profile.dev.package.insta] +opt-level = 3 + +[profile.dev.package.similar] +opt-level = 3 \ No newline at end of file diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..a245261 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,3 @@ +# Testing + +To accept all new snapshots, run `INSTA_UPDATE=always cargo test` or to accept only for new tests, run `INSTA_UPDATE=unseen cargo test`. \ No newline at end of file diff --git a/tests/lib.rs b/tests/lib.rs index e68a03e..31d6cfe 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -4,7 +4,7 @@ use serde::Serialize; use std::{ ffi::OsString, - io::Write, + io::{BufRead, Write}, os::unix::fs::PermissionsExt, path::{Path, PathBuf}, process::{Command, Output}, @@ -14,7 +14,7 @@ use walkdir::WalkDir; const CARGO_SNIFF_NAME: &str = "cargo-sniff-test"; const SNIFF_DRIVER_NAME: &str = "sniff-test-driver"; -const BUILD_DIR: &str = "../target/debug"; +const BUILD_DIR: &str = "../target/release"; static CARGO_SNIFF_TEST_PATH: LazyLock = LazyLock::new(|| { let canon = Path::new(&format!("{BUILD_DIR}/{CARGO_SNIFF_NAME}")).canonicalize(); @@ -50,6 +50,18 @@ impl TryFrom for SniffTestOutput { #[test] fn snapshots() -> anyhow::Result<()> { + // rebuild sniff-test to ensure we're using an updated version + let build_res = Command::new("cargo") + .arg("build") + .arg("--release") + .current_dir("..") + .output() + .unwrap(); + assert!( + build_res.status.success(), + "rebuilding sniff-test didn't succeed" + ); + let root = Path::new(".").canonicalize()?; println!("root is {root:?}"); @@ -111,6 +123,20 @@ fn is_inside_cargo_project(path: &Path, root: &Path) -> bool { false } +fn snapshot_filters(root: &Path) -> Vec<(&str, &str)> { + vec![ + ( + r"process didn't exit successfully: `(.*?)`", + "process didn't exit successfully: [BINARY PATH ELIDED]", + ), + ( + root.to_str().expect("should be valid unicode"), + "[SNIFF_TEST_DIR]", + ), + (r"\(stable id (.*?)\)", "(stable id [CRATE ID ELIDED])"), + ] +} + fn has_rust_files(path: &Path) -> bool { std::fs::read_dir(path) .ok() @@ -135,19 +161,11 @@ fn snapshot_cargo_dir(path: &Path, root: &Path) -> anyhow::Result<()> { .unwrap_or("[unknown name]") .to_owned(); - let out_path = path.to_path_buf(); let out = cargo_sniff(path)?; - // panic!( - // "root is {}", - // root.to_str().expect("should be valid unicode") - // ); insta::with_settings!({ - snapshot_path => out_path, - filters => vec![ - (r"process didn't exit successfully: `(.*?)`", "process didn't exit successfully: [BINARY PATH ELIDED]"), - (root.to_str().expect("should be valid unicode"), "[SNIFF_TEST_DIR]") - ], + snapshot_path => path, + filters => snapshot_filters(root), prepend_module_to_snapshot => false, omit_expression => true, }, { @@ -191,10 +209,7 @@ fn snapshot_single_rust_file(file_path: &Path, root: &Path) -> anyhow::Result<() insta::with_settings!({ snapshot_path => out_path, - filters => vec![ - (r"process didn't exit successfully: `(.*?)`", "process didn't exit successfully: [BINARY PATH ELIDED]"), - (root.to_str().expect("should be valid unicode"), "[SNIFF_TEST_DIR]") - ], + filters => snapshot_filters(root), prepend_module_to_snapshot => false, omit_expression => true, }, { @@ -206,14 +221,21 @@ fn snapshot_single_rust_file(file_path: &Path, root: &Path) -> anyhow::Result<() } fn cargo_sniff(path: &Path) -> anyhow::Result { - // cargo clean first Command::new("cargo") .arg("clean") .current_dir(path) .output()?; - println!("path is {:?}", CARGO_SNIFF_TEST_PATH.clone().into_string()); + let first_line = std::io::BufReader::new( + std::fs::File::open(path.join("Cargo.toml")) + .unwrap_or_else(|_| panic!("no lib.rs in {}", path.display())), + ) + .lines() + .next() + .expect("file shouldn't be empty")?; + let mut cmd = Command::new(&*CARGO_SNIFF_TEST_PATH); + cmd.args(first_line.split(' ').skip(1)); // skip the first one, as it's the "#" or "//" to start a comment cmd.env("CARGO_TERM_COLOR", "never"); cmd.current_dir(path); diff --git a/tests/toml/fail_external/Cargo.toml b/tests/toml/fail_external/Cargo.toml index dab498a..b67fbe0 100644 --- a/tests/toml/fail_external/Cargo.toml +++ b/tests/toml/fail_external/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "toml_fail_external" version = "0.1.0" diff --git a/tests/toml/fail_external/fail_external.snap b/tests/toml/fail_external/fail_external.snap index caabbd9..28458fc 100644 --- a/tests/toml/fail_external/fail_external.snap +++ b/tests/toml/fail_external/fail_external.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 101 -stdout = '' +stdout = ''' +the toml_fail_external crate FAILED the sniff test +''' stderr = ''' error: function main directly contains 1 unjustified call to annotated unsafe functions, but is not annotated unsafe --> src/main.rs:5:1 diff --git a/tests/toml/pass_external/Cargo.toml b/tests/toml/pass_external/Cargo.toml index 6e39018..3a4c102 100644 --- a/tests/toml/pass_external/Cargo.toml +++ b/tests/toml/pass_external/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "toml_pass_external" version = "0.1.0" diff --git a/tests/toml/pass_external/pass_external.snap b/tests/toml/pass_external/pass_external.snap index b57ae0d..f5e2799 100644 --- a/tests/toml/pass_external/pass_external.snap +++ b/tests/toml/pass_external/pass_external.snap @@ -3,6 +3,6 @@ source: tests/lib.rs --- exit_code = 0 stdout = ''' -the `toml_pass_external` crate passes the sniff test!! +the toml_pass_external crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/toml/pass_local/Cargo.toml b/tests/toml/pass_local/Cargo.toml index 7ec2618..9ca9ca9 100644 --- a/tests/toml/pass_local/Cargo.toml +++ b/tests/toml/pass_local/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "toml_pass_local" version = "0.1.0" diff --git a/tests/toml/pass_local/pass_local.snap b/tests/toml/pass_local/pass_local.snap index de7aaa3..41c94f8 100644 --- a/tests/toml/pass_local/pass_local.snap +++ b/tests/toml/pass_local/pass_local.snap @@ -3,6 +3,6 @@ source: tests/lib.rs --- exit_code = 0 stdout = ''' -the `toml_pass_local` crate passes the sniff test!! +the toml_pass_local crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/annotations/global.snap b/tests/unsafe/annotations/global.snap index 38576e8..4faf729 100644 --- a/tests/unsafe/annotations/global.snap +++ b/tests/unsafe/annotations/global.snap @@ -1,9 +1,10 @@ --- source: tests/lib.rs -assertion_line: 201 --- exit_code = 1 -stdout = '' +stdout = ''' +the global crate FAILED the sniff test +''' stderr = ''' error: function bar directly contains 1 unjustified unsafe axiom, but is not annotated unsafe --> [SNIFF_TEST_DIR]/unsafe/annotations/global.rs:7:1 diff --git a/tests/unsafe/annotations/global_pub.snap b/tests/unsafe/annotations/global_pub.snap index 9879009..232d5f6 100644 --- a/tests/unsafe/annotations/global_pub.snap +++ b/tests/unsafe/annotations/global_pub.snap @@ -1,9 +1,10 @@ --- source: tests/lib.rs -assertion_line: 201 --- exit_code = 1 -stdout = '' +stdout = ''' +the global_pub crate FAILED the sniff test +''' stderr = ''' error: function foo directly contains 1 unjustified unsafe axiom, but is not annotated unsafe --> [SNIFF_TEST_DIR]/unsafe/annotations/global_pub.rs:3:1 diff --git a/tests/unsafe/axioms/raw_deref.snap b/tests/unsafe/axioms/raw_deref.snap index d165dd1..3d07197 100644 --- a/tests/unsafe/axioms/raw_deref.snap +++ b/tests/unsafe/axioms/raw_deref.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 1 -stdout = '' +stdout = ''' +the raw_deref crate FAILED the sniff test +''' stderr = ''' error: function foo directly contains 1 unjustified unsafe axiom, but is not annotated unsafe --> [SNIFF_TEST_DIR]/unsafe/axioms/raw_deref.rs:3:1 diff --git a/tests/unsafe/axioms/ref_deref.snap b/tests/unsafe/axioms/ref_deref.snap index 01ed6fb..b51d2d0 100644 --- a/tests/unsafe/axioms/ref_deref.snap +++ b/tests/unsafe/axioms/ref_deref.snap @@ -3,6 +3,6 @@ source: tests/lib.rs --- exit_code = 0 stdout = ''' -the `ref_deref` crate passes the sniff test!! +the ref_deref crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/calls/justified_call.snap b/tests/unsafe/calls/justified_call.snap index 310352e..a26bd18 100644 --- a/tests/unsafe/calls/justified_call.snap +++ b/tests/unsafe/calls/justified_call.snap @@ -3,7 +3,7 @@ source: tests/lib.rs --- exit_code = 0 stdout = ''' -the `justified_call` crate passes the sniff test!! +the justified_call crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = ''' warning: unused doc comment diff --git a/tests/unsafe/calls/unjustified_call.snap b/tests/unsafe/calls/unjustified_call.snap index 8593032..8e6817c 100644 --- a/tests/unsafe/calls/unjustified_call.snap +++ b/tests/unsafe/calls/unjustified_call.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 1 -stdout = '' +stdout = ''' +the unjustified_call crate FAILED the sniff test +''' stderr = ''' error: function main directly contains 1 unjustified call to annotated unsafe functions, but is not annotated unsafe --> [SNIFF_TEST_DIR]/unsafe/calls/unjustified_call.rs:10:1 diff --git a/tests/unsafe/example_crate/Cargo.toml b/tests/unsafe/example_crate/Cargo.toml index 133758a..3588a59 100644 --- a/tests/unsafe/example_crate/Cargo.toml +++ b/tests/unsafe/example_crate/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "example" version = "0.1.0" diff --git a/tests/unsafe/example_crate/example_crate.snap b/tests/unsafe/example_crate/example_crate.snap index b0849c8..2cf6811 100644 --- a/tests/unsafe/example_crate/example_crate.snap +++ b/tests/unsafe/example_crate/example_crate.snap @@ -3,6 +3,6 @@ source: tests/lib.rs --- exit_code = 0 stdout = ''' -the `example` crate passes the sniff test!! +the example crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/fail_nested/Cargo.toml b/tests/unsafe/fail_nested/Cargo.toml index 9d99bb7..5548118 100644 --- a/tests/unsafe/fail_nested/Cargo.toml +++ b/tests/unsafe/fail_nested/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "fail_nested" version = "0.1.0" diff --git a/tests/unsafe/fail_nested/fail_nested.snap b/tests/unsafe/fail_nested/fail_nested.snap index f7b5f69..b2f1b85 100644 --- a/tests/unsafe/fail_nested/fail_nested.snap +++ b/tests/unsafe/fail_nested/fail_nested.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 101 -stdout = '' +stdout = ''' +the fail_nested crate FAILED the sniff test +''' stderr = ''' error: function bar directly contains 1 unjustified call to annotated unsafe functions, but is not annotated unsafe --> src/main.rs:8:1 diff --git a/tests/unsafe/fail_not_annotated/Cargo.toml b/tests/unsafe/fail_not_annotated/Cargo.toml index b0fdcf7..f74665a 100644 --- a/tests/unsafe/fail_not_annotated/Cargo.toml +++ b/tests/unsafe/fail_not_annotated/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "fail_not_annotated" version = "0.1.0" diff --git a/tests/unsafe/fail_not_annotated/fail_not_annotated.snap b/tests/unsafe/fail_not_annotated/fail_not_annotated.snap index 769ac78..dfee363 100644 --- a/tests/unsafe/fail_not_annotated/fail_not_annotated.snap +++ b/tests/unsafe/fail_not_annotated/fail_not_annotated.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 101 -stdout = '' +stdout = ''' +the fail_not_annotated crate FAILED the sniff test +''' stderr = ''' error: function foo directly contains 1 unjustified unsafe axiom, but is not annotated unsafe --> src/main.rs:1:1 diff --git a/tests/unsafe/fail_simple/Cargo.toml b/tests/unsafe/fail_simple/Cargo.toml index b0775dd..595b6a0 100644 --- a/tests/unsafe/fail_simple/Cargo.toml +++ b/tests/unsafe/fail_simple/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "fail_simple" version = "0.1.0" diff --git a/tests/unsafe/fail_simple/fail_simple.snap b/tests/unsafe/fail_simple/fail_simple.snap index a96d200..5ade38f 100644 --- a/tests/unsafe/fail_simple/fail_simple.snap +++ b/tests/unsafe/fail_simple/fail_simple.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 101 -stdout = '' +stdout = ''' +the fail_simple crate FAILED the sniff test +''' stderr = ''' error: function main directly contains 1 unjustified call to annotated unsafe functions, but is not annotated unsafe --> src/main.rs:8:1 diff --git a/tests/unsafe/pass_nested/Cargo.toml b/tests/unsafe/pass_nested/Cargo.toml index 3d86a2c..3cb9b23 100644 --- a/tests/unsafe/pass_nested/Cargo.toml +++ b/tests/unsafe/pass_nested/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "pass_nested" version = "0.1.0" diff --git a/tests/unsafe/pass_nested/pass_nested.snap b/tests/unsafe/pass_nested/pass_nested.snap index 259ac08..2c7a2fa 100644 --- a/tests/unsafe/pass_nested/pass_nested.snap +++ b/tests/unsafe/pass_nested/pass_nested.snap @@ -3,6 +3,6 @@ source: tests/lib.rs --- exit_code = 0 stdout = ''' -the `pass_nested` crate passes the sniff test!! +the pass_nested crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/pass_simple/Cargo.toml b/tests/unsafe/pass_simple/Cargo.toml index 4a475b8..25ffd01 100644 --- a/tests/unsafe/pass_simple/Cargo.toml +++ b/tests/unsafe/pass_simple/Cargo.toml @@ -1,3 +1,4 @@ +# -d trust [package] name = "pass_simple" version = "0.1.0" diff --git a/tests/unsafe/pass_simple/pass_simple.snap b/tests/unsafe/pass_simple/pass_simple.snap index b2f6f50..5e35e3f 100644 --- a/tests/unsafe/pass_simple/pass_simple.snap +++ b/tests/unsafe/pass_simple/pass_simple.snap @@ -3,6 +3,6 @@ source: tests/lib.rs --- exit_code = 0 stdout = ''' -the `pass_simple` crate passes the sniff test!! +the pass_simple crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/placement/call_block.snap b/tests/unsafe/placement/call_block.snap index 6202b65..c6e6a34 100644 --- a/tests/unsafe/placement/call_block.snap +++ b/tests/unsafe/placement/call_block.snap @@ -1,9 +1,8 @@ --- source: tests/lib.rs -assertion_line: 201 --- exit_code = 0 stdout = ''' -the `call_block` crate passes the sniff test!! +the call_block crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/placement/call_block_def.snap b/tests/unsafe/placement/call_block_def.snap index 314b20c..8a6e080 100644 --- a/tests/unsafe/placement/call_block_def.snap +++ b/tests/unsafe/placement/call_block_def.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 1 -stdout = '' +stdout = ''' +the call_block_def crate FAILED the sniff test +''' stderr = ''' error: function main directly contains 1 unjustified call to annotated unsafe functions, but is not annotated unsafe --> [SNIFF_TEST_DIR]/unsafe/placement/call_block_def.rs:8:1 diff --git a/tests/unsafe/placement/call_direct.snap b/tests/unsafe/placement/call_direct.snap index a7244a8..bd95d68 100644 --- a/tests/unsafe/placement/call_direct.snap +++ b/tests/unsafe/placement/call_direct.snap @@ -1,9 +1,8 @@ --- source: tests/lib.rs -assertion_line: 201 --- exit_code = 0 stdout = ''' -the `call_direct` crate passes the sniff test!! +the call_direct crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/placement/call_let.snap b/tests/unsafe/placement/call_let.snap index 0c17016..d77b173 100644 --- a/tests/unsafe/placement/call_let.snap +++ b/tests/unsafe/placement/call_let.snap @@ -1,9 +1,8 @@ --- source: tests/lib.rs -assertion_line: 201 --- exit_code = 0 stdout = ''' -the `call_let` crate passes the sniff test!! +the call_let crate passes the sniff test!! (stable id [CRATE ID ELIDED]) - local ''' stderr = '' diff --git a/tests/unsafe/placement/call_let_def.snap b/tests/unsafe/placement/call_let_def.snap index 810769f..d4f8e79 100644 --- a/tests/unsafe/placement/call_let_def.snap +++ b/tests/unsafe/placement/call_let_def.snap @@ -2,7 +2,9 @@ source: tests/lib.rs --- exit_code = 1 -stdout = '' +stdout = ''' +the call_let_def crate FAILED the sniff test +''' stderr = ''' error: function main directly contains 1 unjustified call to annotated unsafe functions, but is not annotated unsafe --> [SNIFF_TEST_DIR]/unsafe/placement/call_let_def.rs:8:1