From 07693ec59c09c34ff13fed7602c0050f71624df6 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 9 Sep 2021 20:41:21 -0600 Subject: [PATCH 1/2] Add a CI check for reproducible builds Add a --cfg reprocheck setting. When enabled, mock! and #[automock] will evaluate every call twice and verify that the output is identical. Also add a test case that is known to currently generate non-reproducible output. --- .cirrus.yml | 2 ++ .../automock_multiple_lifetime_parameters.rs | 15 +++++++++++ mockall_derive/src/lib.rs | 27 +++++++++++++++++-- 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 mockall/tests/automock_multiple_lifetime_parameters.rs diff --git a/.cirrus.yml b/.cirrus.yml index e97dfc90..fb9d415c 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -35,6 +35,8 @@ task: lint_script: - rustup component add clippy - cargo clippy $CARGO_ARGS --all-targets --workspace -- -D warnings + reproducibility_script: + - env RUSTFLAGS="--cfg reprocheck" cargo check $CARGO_ARGS --all-targets minver_script: - cargo update -Zminimal-versions - cargo test $CARGO_ARGS diff --git a/mockall/tests/automock_multiple_lifetime_parameters.rs b/mockall/tests/automock_multiple_lifetime_parameters.rs new file mode 100644 index 00000000..e0c93882 --- /dev/null +++ b/mockall/tests/automock_multiple_lifetime_parameters.rs @@ -0,0 +1,15 @@ +// vim: tw=80 +//! Methods with multiple generic lifetime parameters should produce their +//! generated code deterministically. +//! +//! This test is designed to work with "--cfg reprocheck" + +#![deny(warnings)] +#![allow(clippy::needless_lifetimes)] + +use mockall::*; + +#[automock] +trait Foo { + fn foo<'a, 'b, 'c, 'd, 'e, 'f>(&self, x: &'a &'b &'c &'d &'e &'f i32); +} diff --git a/mockall_derive/src/lib.rs b/mockall_derive/src/lib.rs index a4841cff..73b4a136 100644 --- a/mockall_derive/src/lib.rs +++ b/mockall_derive/src/lib.rs @@ -1033,7 +1033,7 @@ fn mock_it>(inputs: M) -> TokenStream ts } -fn do_mock(input: TokenStream) -> TokenStream +fn do_mock_once(input: TokenStream) -> TokenStream { let item: MockableStruct = match syn::parse2(input) { Ok(mock) => mock, @@ -1044,6 +1044,18 @@ fn do_mock(input: TokenStream) -> TokenStream mock_it(item) } +fn do_mock(input: TokenStream) -> TokenStream +{ + cfg_if! { + if #[cfg(reprocheck)] { + let ts_a = do_mock_once(input.clone()); + let ts_b = do_mock_once(input.clone()); + assert_eq!(ts_a.to_string(), ts_b.to_string()); + } + } + do_mock_once(input) +} + #[proc_macro] pub fn mock(input: proc_macro::TokenStream) -> proc_macro::TokenStream { do_mock(input.into()).into() @@ -1058,7 +1070,7 @@ pub fn automock(attrs: proc_macro::TokenStream, input: proc_macro::TokenStream) do_automock(attrs, input).into() } -fn do_automock(attrs: TokenStream, input: TokenStream) -> TokenStream { +fn do_automock_once(attrs: TokenStream, input: TokenStream) -> TokenStream { let mut output = input.clone(); let attrs: Attrs = match parse2(attrs) { Ok(a) => a, @@ -1076,6 +1088,17 @@ fn do_automock(attrs: TokenStream, input: TokenStream) -> TokenStream { output } +fn do_automock(attrs: TokenStream, input: TokenStream) -> TokenStream { + cfg_if! { + if #[cfg(reprocheck)] { + let ts_a = do_automock_once(attrs.clone(), input.clone()); + let ts_b = do_automock_once(attrs.clone(), input.clone()); + assert_eq!(ts_a.to_string(), ts_b.to_string()); + } + } + do_automock_once(attrs, input) +} + #[cfg(test)] mod t { use super::*; From e2f5285e59091a8ce4c3c8bd2d5709ab9ef3ddff Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 9 Sep 2021 21:50:09 -0600 Subject: [PATCH 2/2] Fix nondeterministic code generation mockall_derive was internally using HashMap and HashSet and iterating over their contents. But by default those types use random internal state, resulting in nondeterministic output. Switch them to use consistent state. Reported by: David Tolnay @dtolnay --- mockall_derive/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mockall_derive/src/lib.rs b/mockall_derive/src/lib.rs index 73b4a136..3e466c40 100644 --- a/mockall_derive/src/lib.rs +++ b/mockall_derive/src/lib.rs @@ -12,8 +12,8 @@ use cfg_if::cfg_if; use proc_macro2::{Span, TokenStream}; use quote::{ToTokens, format_ident, quote}; use std::{ - collections::{HashMap, HashSet}, env, + hash::BuildHasherDefault }; use syn::{ *, @@ -34,6 +34,10 @@ use crate::mock_item::MockItem; use crate::mock_item_struct::MockItemStruct; use crate::mockable_item::MockableItem; +// Define deterministic aliases for these common types. +type HashMap = std::collections::HashMap>; +type HashSet = std::collections::HashSet>; + cfg_if! { // proc-macro2's Span::unstable method requires the nightly feature, and it // doesn't work in test mode. @@ -126,7 +130,7 @@ fn deanonymize(literal_type: &mut Type) { fn declosurefy(gen: &Generics, args: &Punctuated) -> (Generics, Vec, Vec) { - let mut hm = HashMap::new(); + let mut hm = HashMap::default(); let mut save_fn_types = |ident: &Ident, tpb: &TypeParamBound| { if let TypeParamBound::Trait(tb) = tpb {