Skip to content

feat(fakeable): add fakeable macros - #770

Open
martin-kolinek wants to merge 9 commits into
mainfrom
fakeable
Open

martin-kolinek wants to merge 9 commits into
mainfrom
fakeable

Conversation

@martin-kolinek

@martin-kolinek martin-kolinek commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

🤖 ## Summary

  • import the fakeable and fakeable_impl procedural macro crates at version 0.2.0
  • add a private end-to-end fixture covering real, manual fake, generic, async, and Mockall behavior
  • preserve generic implementation arguments and async method constraints in generated code
  • reject Mockall method shapes that cannot match the generated wrapper API
  • document the generated wrapper design and explicitly mark the crate and Mockall integration as experimental
  • integrate workspace dependencies, generated READMEs, packaging metadata, and spelling

Validation

  • just anvil-clippy
  • just anvil-fmt --fix
  • just anvil-readme --fix
  • just anvil-spellcheck
  • just anvil-mutants-diff
  • cargo +nightly-2026-05-30 udeps --package fakeable_impl --all-targets --all-features
  • cargo +nightly-2026-05-30 llvm-cov --package fakeable_impl --all-features --tests --summary-only --fail-under-lines 100
  • cargo +nightly-2026-05-30 test --no-run --package fakeable_test --no-default-features --locked

Import the fakeable procedural macro family at version 0.2.0, including its implementation crate, private integration fixture, generated READMEs, and design documentation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Warn that fakeable's generated API is experimental and that the optional Mockall integration requires careful compatibility review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI lite review requested due to automatic review settings September 18, 2026 15:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical generic implementation handling and additional Mockall and trait-generation issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds experimental fakeable and fakeable_impl procedural macro crates with wrapper generation, Mockall integration, documentation, snapshots, and end-to-end fixtures.

Changes:

  • Adds workspace crates, dependencies, packaging metadata, and spelling entries.
  • Implements real/fake, async, trait, derive, and Mockall support.
  • Adds generated READMEs, design documentation, integration tests, and snapshots.
File summaries
File Summary
docs/design/README.md Links fakeable design documentation.
docs/design/fakeable.md Documents wrapper and Mockall behavior.
crates/fakeable/src/lib.rs Defines the public macro API.
crates/fakeable/README.md Generated crate README.
crates/fakeable/Cargo.toml Configures the public crate.
crates/fakeable_test/tests/usage_without_fakes.rs Tests production-only usage.
crates/fakeable_test/tests/usage_with_mockall.rs Tests Mockall usage.
crates/fakeable_test/tests/usage_with_fakes.rs Tests manual fake usage.
crates/fakeable_test/src/my_service.rs Defines the main service fixture.
crates/fakeable_test/src/my_service_unused_fake.rs Tests disabled fake generation.
crates/fakeable_test/src/my_service_mockall.rs Defines the Mockall fixture.
crates/fakeable_test/src/lib.rs Exposes fixture modules.
crates/fakeable_test/Cargo.toml Configures the private test package.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_with_expect.snap Captures fakeable output.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_struct_with_expect.snap Captures struct transformation output.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_trait_impl_generated_expected_code.snap Captures trait implementation output.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_struct_generates_expected_code.snap Captures struct wrapper output.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_struct_generates_expected_code_with_derive.snap Captures derive handling.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_struct_generates_expected_code_with_accessibility.snap Captures accessibility handling.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_impl_with_ref_receiver_returning_self_generates_expected_code.snap Captures self-returning delegation.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_impl_with_mockall_generates_expected_code.snap Captures Mockall generation.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_impl_with_mockall_current_module_generates_expected_code.snap Captures current-module Mockall output.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_impl_with_mockall_and_async_unit_function_generates_expected_code.snap Captures async Mockall output.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_impl_with_async_generates_expected_code.snap Captures async delegation.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_on_impl_generates_expected_code.snap Captures impl delegation.
crates/fakeable_impl/tests/snapshots/output_verification__fakeable_impl_with_expect.snap Captures impl attribute output.
crates/fakeable_impl/tests/output_verification.rs Adds generation snapshot tests.
crates/fakeable_impl/src/lib.rs Implements parsing and code generation.
crates/fakeable_impl/src/args.rs Parses macro arguments.
crates/fakeable_impl/README.md Generated implementation README.
crates/fakeable_impl/Cargo.toml Configures implementation dependencies and tests.
Cargo.toml Registers workspace dependencies.
Cargo.lock Records workspace packages and dependencies.
.spelling Adds the new crate name.
Review details

Suppressed comments (3)

crates/fakeable_impl/src/lib.rs:627

  • The wrapper generator delegates every public or restricted inherent method, but this Mockall generator keeps only exactly pub methods and also omits every &mut self method. Thus an impl containing pub(crate)/pub(super) or mutable methods produces a wrapper that calls methods absent from the configured Mock... fake type (the test fixture's mutable method at fakeable_test/src/my_service.rs:57-59 is an example). Please make the generated mock match the wrapper's delegated method set, or reject these combinations with a targeted diagnostic instead of emitting an incomplete fake type.
        if let syn::ImplItem::Fn(method) = item {
            // Skip non-public methods
            let is_public = matches!(method.vis, syn::Visibility::Public(_));

            // Skip constructors (methods that don't take self and return Self)
            let has_self = method.sig.inputs.iter().any(|input| matches!(input, syn::FnArg::Receiver(_)));

            // Skip mutable methods as mockall doesn't support them well
            let is_mut = method
                .sig
                .inputs
                .iter()
                .any(|input| matches!(input, syn::FnArg::Receiver(receiver) if receiver.mutability.is_some()));

            if is_public && has_self && !is_mut {

crates/fakeable_impl/src/lib.rs:504

  • The generated Mockall signature for an async method is rebuilt from only ident, generics, inputs, and output; the original where_clause (and other signature-level constraints) is discarded. An async method with a where bound therefore produces a mock declaration that does not preserve the method's constraints. Carry the original where-clause and other relevant signature metadata into the generated future-returning declaration.
fn convert_async_to_impl_future(sig: &syn::Signature) -> proc_macro2::TokenStream {
    let ident = &sig.ident;
    let generics = &sig.generics;
    let inputs = &sig.inputs;

    // Extract the original return type
    let output_type = match &sig.output {
        syn::ReturnType::Default => quote! { () },
        syn::ReturnType::Type(_, ty) => quote! { #ty },
    };

    // Create the new signature without async but with impl Future return type
    quote! {
        fn #ident #generics(#inputs) -> impl std::future::Future<Output = #output_type> + Send
    }

crates/fakeable_impl/src/lib.rs:312

  • This unconditional rejection also applies to trait-associated functions with no receiver, even though the public design says trait implementations delegate all methods (docs/design/fakeable.md:33-35). A trait such as impl Trait for MyService { fn make() -> Self { ... } } reaches this branch and is not transformed; either generate the supported delegation semantics for receiver-less trait methods or explicitly document/reject this trait case before claiming all trait methods are delegated.
    } else {
        return Err(syn::Error::new_spanned(
            method_sig,
            "methods without self parameter are not supported except for constructors",
        ));
  • Files reviewed: 33/34 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/fakeable_impl/src/lib.rs Outdated
Preserve generic self-type arguments, retain async where clauses, reject unsupported Mockall method shapes, and correct feature-minimal test selection. Consolidate implementation tests to satisfy the coverage and mutation gates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI review requested due to automatic review settings September 18, 2026 17:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical associated-item handling and async Mockall contract issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

crates/fakeable/src/lib.rs:136

  • This doctest enables the crate's mockall feature and expands generate_mockall_fake = true, but the generated code refers directly to mockall::mock!. mockall is not a dependency or dev-dependency of fakeable (crates/fakeable/Cargo.toml:31-40), and the example does not show the consumer dependency required by the expansion, so the documented Mockall example is not self-contained. Add the dependency needed to compile the doctest and explicitly document that downstream users must depend on Mockall when enabling this option.
/// By specifying `generate_mockall_fake = true` in the attribute for the impl block, this macro
/// will generate a mock implementation using the `mockall` crate. The generated mock will be placed
/// in the specified module (default: "mocks"). This option requires the `mockall` Cargo feature.

crates/fakeable_impl/src/lib.rs:514

  • The Mockall-only signature unconditionally adds + Send to every async method's returned future. That changes the accepted method contract: an otherwise valid async method whose future is not Send (for example, one borrowing a non-Sync service or returning a non-Send captured value) is now rejected or cannot be represented by the generated mock. Either preserve the original future's auto-traits or explicitly reject/document non-Send async methods instead of silently strengthening the API.
    converted.output = parse_quote!(-> impl ::std::future::Future<Output = #output_type> + Send);
  • Files reviewed: 37/38 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/fakeable_impl/src/lib.rs
Keep associated types and constants when generating wrapper trait implementations, add snapshot coverage, and apply repository formatting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI review requested due to automatic review settings September 18, 2026 17:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

crates/fakeable_impl/src/args.rs:110

  • fake_constructor is accepted as an arbitrary string and is later passed to format_ident! in process_struct (crates/fakeable_impl/src/lib.rs:71-73). Values such as an empty string or "not-valid" therefore are not rejected while parsing as invalid Rust identifiers, leaving the proc macro to fail during expansion instead of producing a targeted diagnostic. Parse this option as a syn::Ident (or validate the string and return a syn::Error) before code generation; apply the same validation to mockall_fake_module.
            "fake_constructor" => {
                let value: LitStr = input.parse()?;
                Ok(Self::FakeConstructor(value.value()))

docs/design/fakeable.md:36

  • The documentation says associated types and constants are preserved on both the hidden real implementation and the wrapper, but generate_wrapper_impl only copies non-function items when is_trait_impl is true; inherent associated items are deliberately omitted from the wrapper (see crates/fakeable_impl/src/lib.rs:248-265). Please document the inherent-impl behavior accurately so users do not rely on MyService::CONSTANT/associated types after applying the macro.
  • Files reviewed: 37/38 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/fakeable_impl/src/lib.rs
Emit a targeted diagnostic for unsupported typed receivers instead of generating invalid delegation syntax, and document the supported receiver forms.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI review requested due to automatic review settings September 18, 2026 17:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Two moderate review findings and one documentation nit remain unresolved.

Review details

Suppressed comments (3)

crates/fakeable/src/lib.rs:136

  • The generated expansion at crates/fakeable_impl/src/lib.rs:709 invokes mockall::mock! in the consumer crate, but enabling this crate's mockall feature does not make mockall available to that consumer. The documentation currently says only that the feature is required, so users following it can enable the feature and still lack the direct dependency needed by the generated code. Please document the required direct mockall dependency (as the fakeable_test fixture does).
/// By specifying `generate_mockall_fake = true` in the attribute for the impl block, this macro
/// will generate a mock implementation using the `mockall` crate. The generated mock will be placed
/// in the specified module (default: "mocks"). This option requires the `mockall` Cargo feature.

crates/fakeable_impl/src/args.rs:128

  • mockall_fake_module is accepted as an arbitrary string and then passed to format_ident! at lib.rs:701, so values that are not a single Rust identifier (for example, a nested module path or a name containing spaces) are not diagnosed at argument parsing and can make macro expansion fail instead of producing a targeted error. Please validate this option as an identifier (and likewise validate fake_constructor) when parsing the attribute.
            "mockall_fake_module" => {
                let value: LitStr = input.parse()?;
                Ok(Self::MockallFakeModule(value.value()))

crates/fakeable_test/tests/usage_without_fakes.rs:15

  • This test is run with fakeable_test's default test-util feature, so it does not exercise the feature-disabled expansion that this fixture is intended to cover. The only no-default-features validation listed is cargo test --no-run, which checks compilation but not the real-only behavior; please add an executed no-default-features test invocation (or an equivalent CI test target) for this branch.
  • Files reviewed: 37/38 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (36b9e57) to head (d5ecfb4).

Additional details and impacted files
@@           Coverage Diff            @@
##             main     #770    +/-   ##
========================================
  Coverage   100.0%   100.0%            
========================================
  Files         640      641     +1     
  Lines       85528    86017   +489     
========================================
+ Hits        85528    86017   +489     
Flag Coverage Δ
linux 100.0% <100.0%> (ø)
linux-arm 100.0% <100.0%> (ø)
scheduled ?
windows 100.0% <100.0%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Run async fixture assertions with futures executor instead of constructing a Tokio runtime that requires unsupported Windows IOCP calls under Miri.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI review requested due to automatic review settings September 18, 2026 19:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical doctest configuration and dependency issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 37/38 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread crates/fakeable/src/lib.rs
Comment thread crates/fakeable/src/lib.rs
Separate consumer-feature fake usage from production doctesting and provide Mockall to feature-enabled doctests while documenting the required direct consumer dependency.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI review requested due to automatic review settings September 18, 2026 19:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Generic implementations using Mockall can generate an invalid mock API and need targeted handling.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 37/38 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/fakeable_impl/src/lib.rs
Emit a targeted diagnostic when Mockall generation is requested for a generic impl block, while retaining generic support for manual fakes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI review requested due to automatic review settings September 18, 2026 19:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical Mockall and conditional-compilation issues remain, along with moderate input-validation issues.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

crates/fakeable_impl/src/args.rs:110

  • fake_constructor accepts any string literal here, but the implementation passes it directly to format_ident! (fakeable_impl/src/lib.rs:72-73). Inputs such as an empty string or "fake-name" therefore reach identifier construction instead of producing a normal macro diagnostic. Validate the string as a Rust identifier while parsing (or return a syn::Error from expansion) before constructing the generated method name.
            "fake_constructor" => {
                let value: LitStr = input.parse()?;
                Ok(Self::FakeConstructor(value.value()))

crates/fakeable_impl/src/args.rs:128

  • mockall_fake_module is also accepted as an arbitrary string and later passed directly to format_ident! at fakeable_impl/src/lib.rs:709. A value such as "mocks::nested" or "mocks-v2" is not a single module identifier, so malformed user input is not diagnosed at the attribute boundary. Parse/validate this option as an identifier (or explicitly support a path) before generating the module.
            "mockall_fake_module" => {
                let value: LitStr = input.parse()?;
                Ok(Self::MockallFakeModule(value.value()))
  • Files reviewed: 37/38 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread crates/fakeable_impl/src/lib.rs
Comment thread crates/fakeable_impl/src/lib.rs
Propagate direct cfg attributes across generated struct support items, reject disabling cfg_attr forms, and reject unsupported Mockall generation on trait impl blocks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9c93382-7f7a-4044-b8ba-4708d22db107
Copilot AI review requested due to automatic review settings September 18, 2026 20:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Only minor documentation nits remain; no blocking issues were identified.

Review details

Suppressed comments (2)

crates/fakeable/src/lib.rs:141

  • The documented default module is inconsistent with the implementation: process_impl falls back to "fakes" at crates/fakeable_impl/src/lib.rs:228, and the end-to-end test uses fakes::MockMyService without configuring a module (crates/fakeable_test/src/my_service_mockall.rs:15-16), but this API documentation says the default is "mocks". Users following the documented default will look in the wrong module; please document fakes (or change the implementation and tests consistently).
/// will generate a mock implementation using the `mockall` crate. The generated mock will be placed
/// in the specified module (default: "mocks"). This option requires the `fakeable` `mockall` Cargo
/// feature and a direct `mockall` dependency in the consuming crate.

crates/fakeable/src/lib.rs:35

  • This documentation says associated types and constants are preserved on both the hidden and wrapper implementations, but the generator only copies non-function items when processing a trait impl (crates/fakeable_impl/src/lib.rs:309-326); inherent associated items are intentionally omitted, as the fixture test at crates/fakeable_impl/tests/output_verification.rs:1076-1091 verifies. Please narrow this statement to trait implementations (or update the generator) so consumers are not led to expect MyService::VERSION/inherent associated items on the wrapper.
/// This macro can be applied to both struct definitions and impl blocks to create
/// a wrapper structure that can switch between real and fake implementations at runtime.
  • Files reviewed: 38/39 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants