Clear some clippy warnings #367

tcharding · 2022-04-22T04:17:10Z

Make a start on clearing all the non-contentious clippy warnings.

sanket1729 · 2022-05-09T23:56:55Z

@tcharding, would you prefer to get this in first? This seems like a big change. Almost of the open PRs are from you, let me know what order do you prefer to merge them to avoid least pain for you

tcharding · 2022-05-10T01:46:35Z

This one is painful to rebase so the sooner it goes in the better from my perspective.

Add a clippy configuration file with the MSRV set to the current 1.41.1

Clippy emits: warning: this import is redundant As suggested, remove the redundant import.

Clippy emits: warning: redundant field names in struct initialization As suggested, use the field init shorthand.

Clippy emits: warning: unneeded unit return type As suggested, remove the unneeded unit return type

Clippy emits a bunch of warnings of the form warning: this expression borrows a reference (`&Self`) that is immediately dereferenced by the compiler As suggested, remove the unneeded explicit reference.

Clippy emits a bunch of warnings of the form: warning: unneeded `return` statement As suggested, remove the explicit return statement.

Clippy emits: warning: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead As suggested, use `&` instead of `ref`.

Clippy emits: warning: redundant slicing of the whole range As suggested, remove the redundant slicing of whole range.

Clippy emits: warning: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) As suggested, remove explicit lifetimes and use `'_`,

Clippy emits: warning: use of `ok_or` followed by a function call As suggested, use `ok_or_else` with a closure.

Clippy emits: warning: you don't need to add `&` to all patterns Remove the reference on match arms.

Clippy emits: warning: you don't need to add `&` to both the expression and the patterns Remove reference from expression and patterns

Clippy emits: warning: useless conversion to the same type: `psbt::Error` As suggested, remove the call to `into`.

Clippy emits a bunch of warnings of form: warning: this `.into_iter()` call is equivalent to `.iter()` and will not consume the `slice` As suggested, use `iter` instead of `into_iter`.

Clippy emits a bunch of warnings of form: warning: redundant closure As suggested, remove the redundant closures and just pass in the function.

Clippy emits a couple of warnings of form: warning: length comparison to zero As suggested, use `is_empty` instead of manually comparing to zero.

Clippy emits: warning: manual `!RangeInclusive::contains` implementation As suggested, use contains instead of manual implementation.

We can derive these implementations. Found by clippy.

Clippy emits two warnings of the form: warning: redundant pattern matching, consider using ... As suggested use `is_none` and `is_ok` where appropriate.

Clippy emits: warning: useless conversion to the same type: `std::string::String` As suggested, remove the useless call to `String::from`

Clippy emits a couple of warnings of the form: warning: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` As suggested, use `map` instead of `and_then`

This function consumes self so no need to clone it both times.

Now we have Rust 1.31 we can use `copied`. Clears clippy warning.

Clippy emits: warning: useless use of `format!` As suggested, use `to_string`.

Clippy emits: warning: calling `push_str()` using a single-character string literal As suggested, use `push` with character literal.

Clippy emits: error: this `if` has identical blocks Simplify the code by removing the duplicate blocks.

Clippy emits: warning: returning the result of a `let` binding from a block As suggested, return the block directly.

Clippy emits: warning: this pattern creates a reference to a reference As suggested remove the `ref` and use `v` directly.

Clippy emits a bunch of warnings of type: warning: using `clone` on type <elided> which implements the `Copy` trait As suggested, remove the redundant calls to clone.

Clippy emits: warning: use of `unwrap_or` followed by a function call As suggested use `unwrap_or_else`.

Clippy emits: warning: called `map(..).flatten()` on an `Iterator` As suggested, use `flat_map` instead.

Clippy emits: error: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false The vars `k` and `thresh` are both of type `uszie` and can never be below zero, use `==` instead.

Clippy emits a few warnings of type: warning: used `sort` on primitive type `u32` As suggested, use `sort_unstable` on `u32` and `usize` primitives.

Clippy emits a bunch of warnings of type: warning: called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(..)` instead As suggested, use `find` instead of chaining `filter` and `next`.

Clippy emits: error: the since field must contain a semver-compliant version As suggested, use a semvar-compliant version number.

tcharding · 2022-05-10T02:36:51Z

I did not check that all the warnings of the type being fixed were fixed after rebase. Will do another round of clippy fixes after this one is merged.

sanket1729

ACK 7da5327. Verified that there are only refactoring changes and no logical changes.

sanket1729 · 2022-05-10T03:07:26Z

src/policy/semantic.rs

@@ -188,7 +188,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
    // Witness is currently encoded as policy. Only accepts leaf fragment and
    // a normalized policy
    fn satisfy_constraint(self, witness: &Policy<Pk>, available: bool) -> Policy<Pk> {
-        debug_assert!(self.clone().normalized() == self.clone());
+        debug_assert!(self.clone().normalized() == self);


Interesting, so self is not moved here?

Because we are using self couple of lines below.

sanket1729 · 2022-05-10T03:10:55Z

src/policy/semantic.rs

@@ -489,7 +489,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
    /// which appear in the policy
    pub fn relative_timelocks(&self) -> Vec<u32> {
        let mut ret = self.real_relative_timelocks();
-        ret.sort();
+        ret.sort_unstable();


Interesting. I learned something new today.

sanket1729 · 2022-05-10T03:29:15Z

src/psbt/finalizer.rs

+            // THIS IS A BUG AND *WILL* PRODUCE WRONG SATISFACTIONS FOR UNCOMPRESSED KEYS
+            // Partial sigs loses the compressed flag that is necessary
+            // TODO: See https://github.com/rust-bitcoin/rust-bitcoin/pull/836
+            // The type checker will fail again after we update to 0.28 and this can be removed


Oops, this bug was fixed upstream, this comment should be removed.

Note to self: Instead of writing TODOs, make new issues.

sanket1729 · 2022-05-10T03:31:31Z

src/psbt/finalizer.rs

+            // Create a pubkey hash and check if they are the same.
+            let addr = bitcoin::Address::p2wpkh(&pk, bitcoin::Network::Bitcoin)
+                .expect("Address corresponding to valid pubkey");
+            *script_pubkey == addr.script_pubkey()


I think this might motivate a Script::p2wpkh(bitcoin::PublicKey)

tcharding changed the title ~~Clear clippy warnings~~ Clear some clippy warnings Apr 26, 2022

tcharding force-pushed the clippy branch 5 times, most recently from 0663dae to c1c8ab0 Compare May 3, 2022 00:23

tcharding marked this pull request as ready for review May 3, 2022 00:40

tcharding added 21 commits May 10, 2022 11:48

Add clippy config file

77eb4a3

Add a clippy configuration file with the MSRV set to the current 1.41.1

Remove redundant import statement

e94bf6e

Clippy emits: warning: this import is redundant As suggested, remove the redundant import.

Use struct field shorthand

a1cf394

Clippy emits: warning: redundant field names in struct initialization As suggested, use the field init shorthand.

Remove unneeded unit return type

952af97

Clippy emits: warning: unneeded unit return type As suggested, remove the unneeded unit return type

Remove unneeded explicit reference

03c6587

Clippy emits a bunch of warnings of the form warning: this expression borrows a reference (`&Self`) that is immediately dereferenced by the compiler As suggested, remove the unneeded explicit reference.

Remove explicit return statement

81d5ae6

Clippy emits a bunch of warnings of the form: warning: unneeded `return` statement As suggested, remove the explicit return statement.

Use & instead of ref

52f7dce

Clippy emits: warning: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead As suggested, use `&` instead of `ref`.

Remove redundant slicing of whole range

5647e29

Clippy emits: warning: redundant slicing of the whole range As suggested, remove the redundant slicing of whole range.

Remove explicit lifetimes

8944ca0

Clippy emits: warning: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) As suggested, remove explicit lifetimes and use `'_`,

Use ok_or_else instead of ok_or

be1a4ee

Clippy emits: warning: use of `ok_or` followed by a function call As suggested, use `ok_or_else` with a closure.

Do not use & for pattern match arms

0a1b5e4

Clippy emits: warning: you don't need to add `&` to all patterns Remove the reference on match arms.

Remove reference from expression and patterns

d29d10a

Clippy emits: warning: you don't need to add `&` to both the expression and the patterns Remove reference from expression and patterns

Remove useless conversion call from error return

3fdd0f5

Clippy emits: warning: useless conversion to the same type: `psbt::Error` As suggested, remove the call to `into`.

Use iter instead of into_iter

63dee9e

Clippy emits a bunch of warnings of form: warning: this `.into_iter()` call is equivalent to `.iter()` and will not consume the `slice` As suggested, use `iter` instead of `into_iter`.

Remove redundant closures

738ff15

Clippy emits a bunch of warnings of form: warning: redundant closure As suggested, remove the redundant closures and just pass in the function.

Use is_empty

7b91918

Clippy emits a couple of warnings of form: warning: length comparison to zero As suggested, use `is_empty` instead of manually comparing to zero.

Use contains instead of manual implementation

ea57d6e

Clippy emits: warning: manual `!RangeInclusive::contains` implementation As suggested, use contains instead of manual implementation.

Derive Default

ad51d83

We can derive these implementations. Found by clippy.

Remove redundant pattern matching

35015bf

Clippy emits two warnings of the form: warning: redundant pattern matching, consider using ... As suggested use `is_none` and `is_ok` where appropriate.

Remove unnecessary call to String::from

bc7be32

Clippy emits: warning: useless conversion to the same type: `std::string::String` As suggested, remove the useless call to `String::from`

Use map instead of and_then

230cab9

Clippy emits a couple of warnings of the form: warning: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` As suggested, use `map` instead of `and_then`

tcharding added 14 commits May 10, 2022 12:22

Remove redundant clone

54f2384

This function consumes self so no need to clone it both times.

Use copied

592b479

Now we have Rust 1.31 we can use `copied`. Clears clippy warning.

Use to_string instead of format macro

ebc4dc9

Clippy emits: warning: useless use of `format!` As suggested, use `to_string`.

Use push with character literal

45fd5f7

Clippy emits: warning: calling `push_str()` using a single-character string literal As suggested, use `push` with character literal.

Simplify duplicate code

54412f1

Clippy emits: error: this `if` has identical blocks Simplify the code by removing the duplicate blocks.

Return block directly

af2260f

Clippy emits: warning: returning the result of a `let` binding from a block As suggested, return the block directly.

Remove unneeded ref

98d4a06

Clippy emits: warning: this pattern creates a reference to a reference As suggested remove the `ref` and use `v` directly.

Remove the redundant calls to clone on Copy type

da6e068

Clippy emits a bunch of warnings of type: warning: using `clone` on type <elided> which implements the `Copy` trait As suggested, remove the redundant calls to clone.

Use unwrap_or_else instead of unwrap_or

90ba692

Clippy emits: warning: use of `unwrap_or` followed by a function call As suggested use `unwrap_or_else`.

Use flat_map instead of chaining map and flatten

289a1a8

Clippy emits: warning: called `map(..).flatten()` on an `Iterator` As suggested, use `flat_map` instead.

Do not check for < 0 for usize types

769cd6d

Clippy emits: error: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false The vars `k` and `thresh` are both of type `uszie` and can never be below zero, use `==` instead.

Use sort_unstable on primitive types

b75964c

Clippy emits a few warnings of type: warning: used `sort` on primitive type `u32` As suggested, use `sort_unstable` on `u32` and `usize` primitives.

Use find instead of chaining filter and next

17bd217

Clippy emits a bunch of warnings of type: warning: called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(..)` instead As suggested, use `find` instead of chaining `filter` and `next`.

Use semvar-compliant version number

7da5327

Clippy emits: error: the since field must contain a semver-compliant version As suggested, use a semvar-compliant version number.

tcharding force-pushed the clippy branch from c1c8ab0 to 7da5327 Compare May 10, 2022 02:23

sanket1729 approved these changes May 10, 2022

View reviewed changes

sanket1729 merged commit 9ab6d4d into rust-bitcoin:master May 10, 2022

tcharding deleted the clippy branch May 10, 2022 04:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Clear some clippy warnings #367

Clear some clippy warnings #367

Uh oh!

tcharding commented Apr 22, 2022 •

edited

Loading

Uh oh!

sanket1729 commented May 9, 2022

Uh oh!

tcharding commented May 10, 2022

Uh oh!

tcharding commented May 10, 2022 •

edited

Loading

Uh oh!

sanket1729 left a comment •

edited

Loading

Uh oh!

sanket1729 May 10, 2022

Uh oh!

sanket1729 May 10, 2022

Uh oh!

sanket1729 May 10, 2022

Uh oh!

tcharding May 10, 2022

Uh oh!

sanket1729 May 10, 2022

Uh oh!

Uh oh!

Clear some clippy warnings #367

Clear some clippy warnings #367

Uh oh!

Conversation

tcharding commented Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanket1729 commented May 9, 2022

Uh oh!

tcharding commented May 10, 2022

Uh oh!

tcharding commented May 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanket1729 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanket1729 May 10, 2022

Choose a reason for hiding this comment

Uh oh!

sanket1729 May 10, 2022

Choose a reason for hiding this comment

Uh oh!

sanket1729 May 10, 2022

Choose a reason for hiding this comment

Uh oh!

tcharding May 10, 2022

Choose a reason for hiding this comment

Uh oh!

sanket1729 May 10, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tcharding commented Apr 22, 2022 •

edited

Loading

tcharding commented May 10, 2022 •

edited

Loading

sanket1729 left a comment •

edited

Loading