e2e465056c Add clippy to pre-commit githook (Tobin C. Harding)
820adc0de0 Add githooks directory (Tobin C. Harding)
668b37af5c Enable clippy on CI (Tobin C. Harding)
a2a54b3982 Remove unnecessary ? operator (Tobin C. Harding)
67ed8f673e Remove unneeded clone (Tobin C. Harding)
eccd401fc4 Remove unneeded reference (Tobin C. Harding)
fd4239f1d2 Use custom digit grouping (Tobin C. Harding)
acd551e644 Remove unnecessary 'static lifetime (Tobin C. Harding)
3102a48d39 Allow clippy::collapsible_else_if (Tobin C. Harding)
63f4ff6406 Remove redundant field names (Tobin C. Harding)

Pull request description:

  We are almost ready to enable clippy on CI!

  First clear a few remaining warnings from feature gated code. Then add a CI job to run clippy using `--all-features` (note no `--all-targets`). Next add githooks directory (this is the patch from https://github.com/rust-bitcoin/rust-bitcoin/pull/1044/commits). Finally add `cargo clippy` to the pre-commit hook.

  EDIT: I just realized the running of clippy could have been done in `test.sh` instead of as a github action. Does that matter?

ACKs for top commit:
  apoelstra:
    ACK e2e465056c
  Kixunil:
    ACK e2e465056c

Tree-SHA512: c78cb78973d3935e5be7908eec7d6ffaa7989724b3e29551b9fa2cb35df1f39574e31e5cc93fdfb32b35039ac9dac5c080ae4287a1e6979dd076bab56e6eda1b
This commit is contained in:
Andrew Poelstra 2022-06-23 20:23:44 +00:00
commit cf5503af74
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
9 changed files with 96 additions and 17 deletions

View File

@ -107,3 +107,18 @@ jobs:
CARGO_TARGET_THUMBV7M_NONE_EABI_RUNNER: "qemu-system-arm -cpu cortex-m3 -machine mps2-an385 -nographic -semihosting-config enable=on,target=native -kernel" CARGO_TARGET_THUMBV7M_NONE_EABI_RUNNER: "qemu-system-arm -cpu cortex-m3 -machine mps2-an385 -nographic -semihosting-config enable=on,target=native -kernel"
run: cd embedded && cargo run --target thumbv7m-none-eabi run: cd embedded && cargo run --target thumbv7m-none-eabi
Clippy:
name: Clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true
- run: rustup component add clippy
- uses: actions-rs/cargo@v1
with:
command: clippy
args: --all-features -- -D warnings

View File

@ -125,6 +125,17 @@ In order to speed up the review process the CI pipeline can be run locally using
skipped when using `act` due to caching being unsupported at this time. We do skipped when using `act` due to caching being unsupported at this time. We do
not *actively* support `act` but will merge PRs fixing `act` issues. not *actively* support `act` but will merge PRs fixing `act` issues.
### Githooks
To assist devs in catching errors _before_ running CI we provide some githooks. If you do not
already have locally configured githooks you can use the ones in this repository by running, in the
root directory of the repository:
```
git config --local core.hooksPath githooks/
```
Alternatively add symlinks in your `.git/hooks` directory to any of the githooks we provide.
## Policy on Altcoins/Altchains ## Policy on Altcoins/Altchains
Patches which add support for non-Bitcoin cryptocurrencies by adding constants Patches which add support for non-Bitcoin cryptocurrencies by adding constants

49
githooks/pre-commit Executable file
View File

@ -0,0 +1,49 @@
#!/bin/sh
#
# Verify what is about to be committed. Called by "git commit" with no
# arguments. The hook should exit with non-zero status after issuing an
# appropriate message if it wants to stop the commit.
if git rev-parse --verify HEAD >/dev/null 2>&1
then
against=HEAD
else
# Initial commit: diff against an empty tree object
against=$(git hash-object -t tree /dev/null)
fi
# If you want to allow non-ASCII filenames set this variable to true.
allownonascii=$(git config --bool hooks.allownonascii)
# Redirect output to stderr.
exec 1>&2
# Cross platform projects tend to avoid non-ASCII filenames; prevent
# them from being added to the repository. We exploit the fact that the
# printable range starts at the space character and ends with tilde.
if [ "$allownonascii" != "true" ] &&
# Note that the use of brackets around a tr range is ok here, (it's
# even required, for portability to Solaris 10's /usr/bin/tr), since
# the square bracket bytes happen to fall in the designated range.
test $(git diff --cached --name-only --diff-filter=A -z $against |
LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0
then
cat <<\EOF
Error: Attempt to add a non-ASCII file name.
This can cause problems if you want to work with people on other platforms.
To be portable it is advisable to rename the file.
If you know what you are doing you can disable this check using:
git config hooks.allownonascii true
EOF
exit 1
fi
# If there are whitespace errors, print the offending file names and fail.
git diff-index --check --cached $against -- || exit 1
# Check that code lints cleanly.
cargo clippy --all-features -- -D warnings || exit 1

View File

@ -1085,7 +1085,7 @@ impl serde::Serialize for Script {
if serializer.is_human_readable() { if serializer.is_human_readable() {
serializer.serialize_str(&format!("{:x}", self)) serializer.serialize_str(&format!("{:x}", self))
} else { } else {
serializer.serialize_bytes(&self.as_bytes()) serializer.serialize_bytes(self.as_bytes())
} }
} }
} }

View File

@ -631,7 +631,7 @@ impl Transaction {
if let Some(output) = spent(&input.previous_output) { if let Some(output) = spent(&input.previous_output) {
output.script_pubkey.verify_with_flags(idx, crate::Amount::from_sat(output.value), tx.as_slice(), flags)?; output.script_pubkey.verify_with_flags(idx, crate::Amount::from_sat(output.value), tx.as_slice(), flags)?;
} else { } else {
return Err(script::Error::UnknownSpentOutput(input.previous_output.clone())); return Err(script::Error::UnknownSpentOutput(input.previous_output));
} }
} }
Ok(()) Ok(())
@ -1662,7 +1662,7 @@ mod benches {
use crate::hashes::hex::FromHex; use crate::hashes::hex::FromHex;
use test::{black_box, Bencher}; use test::{black_box, Bencher};
const SOME_TX: &'static str = "0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000"; const SOME_TX: &str = "0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000";
#[bench] #[bench]
pub fn bench_transaction_size(bh: &mut Bencher) { pub fn bench_transaction_size(bh: &mut Bencher) {

View File

@ -276,7 +276,7 @@ macro_rules! serde_struct_human_string_impl {
)* )*
let ret = $name { let ret = $name {
$($fe: $fe),* $($fe),*
}; };
Ok(ret) Ok(ret)
@ -312,7 +312,7 @@ macro_rules! serde_struct_human_string_impl {
)* )*
let ret = $name { let ret = $name {
$($fe: $fe),* $($fe),*
}; };
Ok(ret) Ok(ret)

View File

@ -1310,7 +1310,7 @@ pub mod serde {
} }
fn des_btc<'d, D: Deserializer<'d>>(d: D) -> Result<Self, D::Error> { fn des_btc<'d, D: Deserializer<'d>>(d: D) -> Result<Self, D::Error> {
use serde::de::Error; use serde::de::Error;
Ok(Amount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)?) Amount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)
} }
} }
@ -1338,7 +1338,7 @@ pub mod serde {
} }
fn des_btc<'d, D: Deserializer<'d>>(d: D) -> Result<Self, D::Error> { fn des_btc<'d, D: Deserializer<'d>>(d: D) -> Result<Self, D::Error> {
use serde::de::Error; use serde::de::Error;
Ok(SignedAmount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)?) SignedAmount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)
} }
} }
@ -1622,9 +1622,9 @@ mod tests {
assert_eq!(p("1.1", btc), Ok(Amount::from_sat(1_100_000_00))); assert_eq!(p("1.1", btc), Ok(Amount::from_sat(1_100_000_00)));
assert_eq!(p("100", sat), Ok(Amount::from_sat(100))); assert_eq!(p("100", sat), Ok(Amount::from_sat(100)));
assert_eq!(p("55", sat), Ok(Amount::from_sat(55))); assert_eq!(p("55", sat), Ok(Amount::from_sat(55)));
assert_eq!(p("5500000000000000000", sat), Ok(Amount::from_sat(5_500_000_000_000_000_000))); assert_eq!(p("5500000000000000000", sat), Ok(Amount::from_sat(55_000_000_000_000_000_00)));
// Should this even pass? // Should this even pass?
assert_eq!(p("5500000000000000000.", sat), Ok(Amount::from_sat(5_500_000_000_000_000_000))); assert_eq!(p("5500000000000000000.", sat), Ok(Amount::from_sat(55_000_000_000_000_000_00)));
assert_eq!( assert_eq!(
p("12345678901.12345678", btc), p("12345678901.12345678", btc),
Ok(Amount::from_sat(12_345_678_901__123_456_78)) Ok(Amount::from_sat(12_345_678_901__123_456_78))
@ -2006,6 +2006,7 @@ mod tests {
#[cfg(feature = "serde")] #[cfg(feature = "serde")]
#[test] #[test]
#[allow(clippy::inconsistent_digit_grouping)] // Group to show 100,000,000 sats per bitcoin.
fn serde_as_btc() { fn serde_as_btc() {
use serde_json; use serde_json;
@ -2041,6 +2042,7 @@ mod tests {
#[cfg(feature = "serde")] #[cfg(feature = "serde")]
#[test] #[test]
#[allow(clippy::inconsistent_digit_grouping)] // Group to show 100,000,000 sats per bitcoin.
fn serde_as_btc_opt() { fn serde_as_btc_opt() {
use serde_json; use serde_json;
@ -2054,8 +2056,8 @@ mod tests {
} }
let with = T { let with = T {
amt: Some(Amount::from_sat(2__500_000_00)), amt: Some(Amount::from_sat(2_500_000_00)),
samt: Some(SignedAmount::from_sat(-2__500_000_00)), samt: Some(SignedAmount::from_sat(-2_500_000_00)),
}; };
let without = T { let without = T {
amt: None, amt: None,
@ -2085,6 +2087,7 @@ mod tests {
#[cfg(feature = "serde")] #[cfg(feature = "serde")]
#[test] #[test]
#[allow(clippy::inconsistent_digit_grouping)] // Group to show 100,000,000 sats per bitcoin.
fn serde_as_sat_opt() { fn serde_as_sat_opt() {
use serde_json; use serde_json;
@ -2098,8 +2101,8 @@ mod tests {
} }
let with = T { let with = T {
amt: Some(Amount::from_sat(2__500_000_00)), amt: Some(Amount::from_sat(2_500_000_00)),
samt: Some(SignedAmount::from_sat(-2__500_000_00)), samt: Some(SignedAmount::from_sat(-2_500_000_00)),
}; };
let without = T { let without = T {
amt: None, amt: None,

View File

@ -411,6 +411,7 @@ impl<'de> ::serde::Deserialize<'de> for PrivateKey {
#[cfg(feature = "serde")] #[cfg(feature = "serde")]
#[cfg_attr(docsrs, doc(cfg(feature = "serde")))] #[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
#[allow(clippy::collapsible_else_if)] // Aids readability.
impl ::serde::Serialize for PublicKey { impl ::serde::Serialize for PublicKey {
fn serialize<S: ::serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> { fn serialize<S: ::serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
if s.is_human_readable() { if s.is_human_readable() {
@ -549,9 +550,9 @@ mod tests {
fn test_key_serde() { fn test_key_serde() {
use serde_test::{Configure, Token, assert_tokens}; use serde_test::{Configure, Token, assert_tokens};
static KEY_WIF: &'static str = "cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy"; static KEY_WIF: &str = "cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy";
static PK_STR: &'static str = "039b6347398505f5ec93826dc61c19f47c66c0283ee9be980e29ce325a0f4679ef"; static PK_STR: &str = "039b6347398505f5ec93826dc61c19f47c66c0283ee9be980e29ce325a0f4679ef";
static PK_STR_U: &'static str = "\ static PK_STR_U: &str = "\
04\ 04\
9b6347398505f5ec93826dc61c19f47c66c0283ee9be980e29ce325a0f4679ef\ 9b6347398505f5ec93826dc61c19f47c66c0283ee9be980e29ce325a0f4679ef\
87288ed73ce47fc4f5c79d19ebfa57da7cff3aff6e819e4ee971d86b5e61875d\ 87288ed73ce47fc4f5c79d19ebfa57da7cff3aff6e819e4ee971d86b5e61875d\

View File

@ -269,7 +269,7 @@ mod display_from_str {
fn from_str(s: &str) -> Result<Self, Self::Err> { fn from_str(s: &str) -> Result<Self, Self::Err> {
let data = ::base64::decode(s).map_err(PsbtParseError::Base64Encoding)?; let data = ::base64::decode(s).map_err(PsbtParseError::Base64Encoding)?;
Ok(encode::deserialize(&data).map_err(PsbtParseError::PsbtEncoding)?) encode::deserialize(&data).map_err(PsbtParseError::PsbtEncoding)
} }
} }
} }