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

Prevent clippy::ignored_unit_patterns in macro expansions #497

Merged
merged 3 commits into from May 3, 2024

Conversation

mweber15
Copy link
Contributor

@mweber15 mweber15 commented Sep 5, 2023

This is a pedantic lint added in 1.73.0. Because it occurs inside a macro expansion, the lint is triggered from user code. The justification given by the lint definition is:

Matching with () explicitly instead of _ outlines the fact that
the pattern contains no data. Also it would detect a type change that
_ would ignore.

Removing the lint requires a trivial change. It does introduce the possibility for compilation errors in the event the return type of the function currently returning () changes, but that seems like more of a benefit than a drawback. In these cases, it seems unlikely that the return type in question will change in the future.

The user experience can be seen by linting the examples:

% cargo +nightly clippy --examples -- -A clippy::all -W clippy::ignored_unit_patterns -Z macro-backtrace
   Compiling prometheus v0.13.3 (/home/matt/src/rust-prometheus)
...
warning: matching over `()` is more explicit
   --> /home/matt/src/rust-prometheus/src/macros.rs:217:58
    |
214 |   macro_rules! register_counter {
    |   -----------------------------
    |   |
    |   in this expansion of `register_counter!` (#1)
    |   in this expansion of `register_counter!` (#2)
    |   in this expansion of `register_counter!` (#3)
...
217 |           $crate::register(Box::new(counter.clone())).map(|_| counter)
    |                                                            ^
...
221 |           register_counter!(@of_type Counter, $OPTS)
    |           ------------------------------------------ in this macro invocation (#3)
...
225 |           register_counter!(opts!($NAME, $HELP))
    |           -------------------------------------- in this macro invocation (#2)
    |
   ::: examples/example_push.rs:16:40
    |
16  |       static ref PUSH_COUNTER: Counter = register_counter!(
    |  ________________________________________-
17  | |         "example_push_total",
18  | |         "Total number of prometheus client pushed."
19  | |     )
    | |_____- in this macro invocation (#1)
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns
    = note: requested on the command line with `-W clippy::ignored-unit-patterns`
...

This is a pedantic lint added in 1.73.0. Because it occurs inside a
macro expansion, the lint is triggered from user code. The justification
given by the lint definition is:

> Matching with `()` explicitly instead of `_` outlines the fact that
> the pattern contains no data. Also it would detect a type change that
> `_` would ignore.

Removing the lint requires a trivial change. It does introduce the
possibility for compilation errors in the event the return type of the
function currently returning `()` changes, but that seems like more of a
benefit than a drawback. In these cases, it seems unlikely that the
return type in question will change in the future.

The user experience can be seen by linting the examples:

```
% cargo +nightly clippy --examples -- -A clippy::all -W clippy::ignored_unit_patterns -Z macro-backtrace
   Compiling prometheus v0.13.3 (/home/matt/src/rust-prometheus)
...
warning: matching over `()` is more explicit
   --> /home/matt/src/rust-prometheus/src/macros.rs:217:58
    |
214 |   macro_rules! register_counter {
    |   -----------------------------
    |   |
    |   in this expansion of `register_counter!` (tikv#1)
    |   in this expansion of `register_counter!` (tikv#2)
    |   in this expansion of `register_counter!` (tikv#3)
...
217 |           $crate::register(Box::new(counter.clone())).map(|_| counter)
    |                                                            ^
...
221 |           register_counter!(@of_type Counter, $OPTS)
    |           ------------------------------------------ in this macro invocation (tikv#3)
...
225 |           register_counter!(opts!($NAME, $HELP))
    |           -------------------------------------- in this macro invocation (tikv#2)
    |
   ::: examples/example_push.rs:16:40
    |
16  |       static ref PUSH_COUNTER: Counter = register_counter!(
    |  ________________________________________-
17  | |         "example_push_total",
18  | |         "Total number of prometheus client pushed."
19  | |     )
    | |_____- in this macro invocation (tikv#1)
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns
    = note: requested on the command line with `-W clippy::ignored-unit-patterns`
...
```

Signed-off-by: Matt Weber <30441572+mweber15@users.noreply.github.com>
@mweber15 mweber15 force-pushed the clippy-ignored-unit-patterns branch 2 times, most recently from 69f44ff to 38b21bf Compare September 5, 2023 16:45
Copy link
Member

@lucab lucab left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, LGTM!

@lucab lucab merged commit 439e3b8 into tikv:master May 3, 2024
7 checks passed
@lucab lucab mentioned this pull request May 4, 2024
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.

None yet

2 participants