Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change std macro calls to be fully qualified #469

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

BurntSushi
Copy link
Contributor

@BurntSushi BurntSushi commented Mar 27, 2024

This changes calls like format! and vec! within insta's macros to be
std::format! and std::vec!. I ran into this issue in a crate where
std's prelude isn't enabled, even in tests. While perhaps a bit of a
niche case, I don't think this fix has any downsides.

The specific kind of error that happens is if you use
insta::assert_snapshot! (or similar) and the (for example) format!
macro isn't in scope. Here's a small example. First a Cargo.toml:

[package]
publish = false
name = "insta-no-std"
version = "0.1.0"
edition = "2021"

[dev-dependencies]
insta = "1.37.0"

[lib]
name = "insta_no_std"
path = "lib.rs"

And now lib.rs:

(EDIT: I forgot to include the #![no_std] attribute when I originally wrote this. I've included it now.)

#![no_std]

#[cfg(test)]
extern crate std;

fn itworks() {
    insta::assert_snapshot!((-128i8).saturating_abs());
}

And now here's what happens when you try to build it:

$ cargo t --no-run
   Compiling insta-no-std v0.1.0 (/home/andrew/tmp/scratch/rust/insta-no-std)
error: cannot find macro `format` in this scope
 --> lib.rs:8:5
  |
8 |     insta::assert_snapshot!((-128i8).saturating_abs());
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `insta::assert_snapshot` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this macro
  |
3 + use std::format;
  |

error: could not compile `insta-no-std` (lib test) due to 1 previous error

This can be pretty easily worked around by adding use std::format; so
that the macro can call it. But I figure it's better to just have the
macros do the right thing by default. Moreover, I suspect this is also
important in other (perhaps strange) contexts where there is a custom
format! macro defined and in scope. insta would use the custom macro
instead of the one from std. But with this change, it will always use
the one from std.

BurntSushi and others added 2 commits March 27, 2024 16:49
This changes calls like `format!` and `vec!` within insta's macros to be
`std::format!` and `std::vec!`. I ran into this issue in a crate where
std's prelude isn't enabled, even in tests. While perhaps a bit of a
niche case, I don't think this fix has any downsides.

The specific kind of error that happens is if you use
`insta::assert_snapshot!` (or similar) and the (for example) `format!`
macro isn't in scope. Here's a small example. First a `Cargo.toml`:

```toml
[package]
publish = false
name = "insta-no-std"
version = "0.1.0"
edition = "2021"

[dev-dependencies]
insta = "1.37.0"

[lib]
name = "insta_no_std"
path = "lib.rs"
```

And now `lib.rs`:

```rust

extern crate std;

fn itworks() {
    insta::assert_snapshot!((-128i8).saturating_abs());
}
```

And now here's what happens when you try to build it:

```
$ cargo t --no-run
   Compiling insta-no-std v0.1.0 (/home/andrew/tmp/scratch/rust/insta-no-std)
error: cannot find macro `format` in this scope
 --> lib.rs:8:5
  |
8 |     insta::assert_snapshot!((-128i8).saturating_abs());
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `insta::assert_snapshot` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this macro
  |
3 + use std::format;
  |

error: could not compile `insta-no-std` (lib test) due to 1 previous error
```

This can be pretty easily worked around by adding `use std::format;` so
that the macro can call it. But I figure it's better to just have the
macros do the right thing by default. Moreover, I suspect this is also
important in other (perhaps strange) contexts where there is a custom
`format!` macro defined and in scope. `insta` would use the custom macro
instead of the one from `std`. But with this change, it will always use
the one from `std`.
@mitsuhiko mitsuhiko merged commit e90e71d into mitsuhiko:master Mar 28, 2024
11 checks passed
@mitsuhiko
Copy link
Owner

Thank you :)

@BurntSushi BurntSushi deleted the ag/fully-qualified-macro-calls branch March 28, 2024 11:19
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