proptest-rs / proptest

Hypothesis-like property testing for Rust
Apache License 2.0
1.63k stars 152 forks source link

proptest_derive triggers non-local impl nightly warning #447

Open HadrienG2 opened 2 months ago

HadrienG2 commented 2 months ago

This instance of derive(proptest_derive::Arbitrary) will result in the following warning on current nightlies:

error: non-local `impl` definition, they should be avoided as they go against expectation
Error:    --> cmakeperf/src/commands.rs:147:25
    |
147 | #[cfg_attr(test, derive(proptest_derive::Arbitrary))]
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: move this `impl` block outside the of the current constant `_IMPL_ARBITRARY_FOR_DatabaseEntry`
    = note: an `impl` definition is non-local if it is nested inside an item and may impact type checking outside of that item. This can be the case if neither the trait or the self type are at the same nesting level as the `impl`
    = note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type
    = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
    = note: the derive macro `proptest_derive::Arbitrary` may come from an old version of the `proptest_derive` crate, try updating your dependency with `cargo update -p proptest_derive`
    = note: `-D non-local-definitions` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(non_local_definitions)]`
    = note: this error originates in the derive macro `proptest_derive::Arbitrary` (in Nightly builds, run with -Z macro-backtrace for more info)

Since the struct is declared at the top-level scope, and given the suspicious _IMPL_ARBITRARY_FOR_DatabaseEntry name cited, I think this is caused by something that proptest_derive is doing, not something that I am doing.

As the warning message says, it is currently proposed to strengthen this warning into deny-by-default in edition 2024, which would make the issue (slightly) more pressing if accepted.

matthew-russo commented 3 weeks ago

Sorry for the delay -- had to take some time off to focus on other things. I'm triaging issues this weekend and this will likely be near the top since its polluting everyone's build logs.

Thanks for the report