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

    // 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

    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)

7da532763924c9a0d3b5569745182e552ed14ea0 Use semvar-compliant version number (Tobin C. Harding) 17bd217697035fe183c76625f623e4995ae4bdae Use find instead of chaining filter and next (Tobin C. Harding) b75964cca195bd79c062cc635ba4d67d7dfb6cb7 Use sort_unstable on primitive types (Tobin C. Harding) 769cd6db4cb7c039b4d0ee0c05fb55127d495b58 Do not check for < 0 for usize types (Tobin C. Harding) 289a1a8bda3e5af0bb1b68708c5c811482631fd4 Use flat_map instead of chaining map and flatten (Tobin C. Harding) 90ba69228912def84265883f3edb728f6a0b1c08 Use unwrap_or_else instead of unwrap_or (Tobin C. Harding) da6e068df151867003ec0768f9fc20c5c684a3a3 Remove the redundant calls to clone on Copy type (Tobin C. Harding) 98d4a063a52f2a882ba0ff7f94113a23ef447f99 Remove unneeded `ref` (Tobin C. Harding) af2260fae9989558c513cd6034113f9f51bd227a Return block directly (Tobin C. Harding) 54412f137cb97623fb1f935a6c099a028645d520 Simplify duplicate code (Tobin C. Harding) 45fd5f7fc7676bbcfa10f327482bd58b542536ab Use push with character literal (Tobin C. Harding) ebc4dc9f78ee87bbbc3f8833a4da890c29fdcb9f Use to_string instead of format macro (Tobin C. Harding) 592b4790f3f229b2bcad28238777f6440235d28d Use copied (Tobin C. Harding) 54f23840ae190f4240b3438279320254aa3c4591 Remove redundant clone (Tobin C. Harding) 230cab996721b64a1de8efb04fe6c5c212399d22 Use map instead of and_then (Tobin C. Harding) bc7be32fab6e4ecf7ba9235013def628ba77a8f4 Remove unnecessary call to String::from (Tobin C. Harding) 35015bf7e702de0c619face925971429c69d3e76 Remove redundant pattern matching (Tobin C. Harding) ad51d83a9d481e2b0c9b278f4d27625a76c37ea2 Derive Default (Tobin C. Harding) ea57d6e1bc4b4fa778da347a9d8cf27d039fe32a Use contains instead of manual implementation (Tobin C. Harding) 7b91918a4a37f60348dabd8e7029eeeaef248eef Use is_empty (Tobin C. Harding) 738ff15174f8405c305b843b4118a587018bea0e Remove redundant closures (Tobin C. Harding) 63dee9eb228a08a40a3028ab47647e3adc8c4da4 Use iter instead of into_iter (Tobin C. Harding) 3fdd0f516bd0ff5d26e37c984332036752adeb1a Remove useless conversion call from error return (Tobin C. Harding) d29d10a95b9357a9bff9abfe73cd9fd5974f95a8 Remove reference from expression and patterns (Tobin C. Harding) 0a1b5e497944da82a5fad08fb67bcd1c1e34992e Do not use & for pattern match arms (Tobin C. Harding) be1a4ee0af639ba727c93a8b3221c2e41de4e779 Use ok_or_else instead of ok_or (Tobin C. Harding) 8944ca0d4f6cc54ff712e45448f2230ea2432933 Remove explicit lifetimes (Tobin C. Harding) 5647e29606d9503aa7c8180a70827f2d4f14f877 Remove redundant slicing of whole range (Tobin C. Harding) 52f7dce75808b678d11c1c8a0c9a434a714ba19f Use & instead of `ref` (Tobin C. Harding) 81d5ae61375eddcdae7cd5fc97895dac2a0f4d82 Remove explicit return statement (Tobin C. Harding) 03c65871b50581ba7d203b0a77ea09bfcf4f4f49 Remove unneeded explicit reference (Tobin C. Harding) 952af97e2aa3986f1141f7fd0cace331ac943157 Remove unneeded unit return type (Tobin C. Harding) a1cf3944650e4e3508e78e4bdc0bb80d1e0b1471 Use struct field shorthand (Tobin C. Harding) e94bf6efb776c234800164451e871fc473fbffa4 Remove redundant import statement (Tobin C. Harding) 77eb4a34803ffd13e3bfd6298fe8a249940ab2f8 Add clippy config file (Tobin C. Harding) Pull request description: Make a start on clearing all the non-contentious clippy warnings. ACKs for top commit: sanket1729: ACK 7da532763924c9a0d3b5569745182e552ed14ea0 Tree-SHA512: cb7ad9d8983404a01148a61aa920dfd5657ff2a5fea1ea9d3ff565a1665148e31a693916a253bf8bad274063579d4484dfa738294b0eda4271f0e8fd0a9abbdd

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tcharding commented Apr 22, 2022 •

edited

Loading

tcharding commented May 10, 2022 •

edited

Loading

sanket1729 left a comment •

edited

Loading