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

Clear clippy warnings from --all-targets #1043

Merged
merged 18 commits into from Jun 8, 2022

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jun 7, 2022

Clear all the clippy warnings (excl. #1042) that are returned by running cargo clippy --all-targets.

I apologize in advance for the review burden :)

Clippy emits:

 warning: you seem to be trying to use `match` for destructuring a
 single pattern. Consider using `if let`

As suggested, use `if let`.
Clippy emits:

  warning: this `if` statement can be collapsed

As suggested, collapse the if statements into a single statement, with
no loss of clarity.
Clippy emits:

  warning: redundant field names in struct initialization

As suggested, remove redundant field names in struct initialization.
Clippy emits:

  warning: digits grouped inconsistently by underscores

Add allow directive for grouping that aims to make explicit 100,000,000
sats/per bitcoin.
Clippy emits:

  warning: used `assert_eq!` with a literal bool

Use `assert!` instead of `assert_eq!(foo, true)`.
Clippy emits:

  warning: casting integer literal to `usize` is unnecessary

Remove the unnecessary cast.
Clippy emits:

  warning: redundant clone

Remove the redundant calls to clone.
Clippy emits:

  warning: `assert!(false)` should probably be replaced

As suggested, replace assert with a call to panic.
Clippy emits:

  warning: manual implementation of an assign operation

Use the more conventional `+=` operator.
Clippy warns about creating a reference that is immediately
de-referenced.

Remove unnecessary explicit `&`, while we are at it remove unnecessary
explicit types that appear on the same lines of code.
Clippy emits:

  warning: using `clone` on type `blockdata::transaction::OutPoint`
  which implements the `Copy` trait

Remove calls to `clone` from types that implement `Copy`.
Clippy emits:

  warning: called `map(..).flatten()` on `Iterator`

As suggested, use `flat_map` instead of chaining `map` with `flatten`.
Clippy emits:

  warning: returning the result of a `let` binding from a block

Remove the local binding, return the `Address` directly.
Clippy emits:

  warning: length comparison to zero

Remove length comparison to zero, use `!is_empty`.
Clippy warns of useless use of `vec!` macro, remove it.
Clippy emits:

  warning: you seem to want to iterate on a map's values

As suggested, iterate using `values`.
Clippy emits:

  warning: calls to `push` immediately after creation

Use `vec` instead of pushing to a mutable vector.
This is a unit test helper function, it is ok to have a whole bunch of
arguments.
@tcharding tcharding marked this pull request as draft June 7, 2022 06:04
@tcharding tcharding marked this pull request as ready for review June 7, 2022 06:11
Copy link
Contributor

@dpc dpc left a comment

Choose a reason for hiding this comment

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

Looked through it and LGTM

Copy link
Member

@elichai elichai left a comment

Choose a reason for hiding this comment

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

ACK 271d0ba

Reviewed all the changes and none of them seem to be behavior changing, and using clippy is great

@tcharding
Copy link
Member Author

Thanks guys! Just got all the feature gate code to go and then hopefully we can get clippy running on CI.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 271d0ba

@apoelstra
Copy link
Member

Yep, lots of commits but all small and non-controversial.

@apoelstra apoelstra merged commit 8fd7008 into rust-bitcoin:master Jun 8, 2022
@tcharding tcharding deleted the 06-07-clippy branch June 18, 2022 02:20
ChallengeDev210 pushed a commit to ChallengeDev210/rust-bitcoin that referenced this pull request Aug 1, 2022
…ll-targets`

271d0ba Allow many arguments in test function (Tobin C. Harding)
c0c88fe Use vec instead of pushing to a mutable vector (Tobin C. Harding)
73066e7 Use values() to iterate map values (Tobin C. Harding)
38ff025 Remove useless use of vec! (Tobin C. Harding)
d8e82d5 Remove length comparison to zero (Tobin C. Harding)
c1f34f5 Return Address directly (Tobin C. Harding)
ff8d585 Use flat_map instead of map().flatten() (Tobin C. Harding)
b24a112 Remove calls to clone from types that implement Copy (Tobin C. Harding)
2b8d93e Remove unnecessary explicit reference (Tobin C. Harding)
ef90e3d Use plus-equals operator (Tobin C. Harding)
922b820 Replace assert!(false) with panic! (Tobin C. Harding)
a8039e1 Remove redundant clone (Tobin C. Harding)
cf8de73 Remove unnecessary cast of integer literal (Tobin C. Harding)
999ac45 Do not use assert_eq with literal bool (Tobin C. Harding)
827fcd8 Allow unusual digit grouping (Tobin C. Harding)
242c640 Remove redundant field names (Tobin C. Harding)
0f8f4c5 Collapse if statements (Tobin C. Harding)
229fcb9 Use if let instead of destructuring pattern (Tobin C. Harding)

Pull request description:

  Clear all the clippy warnings (excl. #1042) that are returned by running `cargo clippy --all-targets`.

  I apologize in advance for the review burden :)

ACKs for top commit:
  elichai:
    ACK 271d0ba
  apoelstra:
    ACK 271d0ba

Tree-SHA512: 71ad2ec3db808e795791b7513f8b2a1c13dc90317f5328602c9ecbc31c09f82471f79c9c31a71a0ded5280554d1019d2bb4899fb9e8fa6421d46a2397cd31242
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

4 participants