scylla-cql, examples: fix clippy 1.75 issues #900
Conversation
In some places, we have the following expression:
x.iter().map(|(k, v)| (k, v))
Although the .map call looks like a no-op, it changes the type of the
argument from `&(K, V)` to `(&K, &V)`. This happens due to the so-called
"reference matching ergonomics" feature of Rust which allows binding a
reference to a tuple to the `(k, v)` expression and allows referring to
the tuple's constituents by reference (`k: &K`, `v: &V`).
The issue is already known and reported and will most likely be fixed
in a future release, but for the time being let's reformulate the code
to avoid the lint like this:
x.iter().map(|p| (&p.0, &p.1))
Clippy suggests that .first() is more readable than .get(0).
| CqlValue::Map(m) => serialize_map(m.iter().map(|(k, v)| (k, v)), m.len(), buf), | ||
| CqlValue::Map(m) => serialize_map(m.iter().map(|p| (&p.0, &p.1)), m.len(), buf), |
There was a problem hiding this comment.
What lint does it fail? Can the change be made differently? Imo the old version is much easier to read - you know that the argument is 2-element tuple, in the new version you don't know that
There was a problem hiding this comment.
What lint does it fail?
https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
For example, I get this message:
warning: unnecessary map of the identity function
--> scylla-cql/src/frame/value.rs:944:55
|
944 | CqlValue::Map(m) => serialize_map(m.iter().map(|(k, v)| (k, v)), m.len(), buf),
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
= note: `#[warn(clippy::map_identity)]` on by default
See the commit description for more details.
Imo the old version is much easier to read - you know that the argument is 2-element tuple, in the new version you don't know that
I think it's quite clear here that m.iter() iterates over pairs, so I'm not sure if it's a huge problem. I'd argue that the code is now more clear as it's now more obvious what the lambda in the .map does.
Can the change be made differently?
Perhaps it can, but I'm not sure how would you like me to change it.
| @@ -488,7 +488,7 @@ fn serialize_cql_value<'b>( | |||
| CqlValue::Map(m) => serialize_mapping( | |||
| std::any::type_name::<CqlValue>(), | |||
| m.len(), | |||
| m.iter().map(|(ref k, ref v)| (k, v)), | |||
| m.iter().map(|p| (&p.0, &p.1)), | |||
With the new rust toolchain version comes a new clippy version, and therefore new complaints of the linter to address. This PR fixes the newly reported issues.
Pre-review checklist
I added relevant tests for new features and bug fixes.I have provided docstrings for the public items that I want to introduce.I have adjusted the documentation in./docs/source/.I added appropriateFixes:annotations to PR description.