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

Check docs and run clippy on PRs #326

Merged
merged 9 commits into from
Nov 19, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 119 additions & 21 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,137 @@ name: Rust

on:
push:
branches: [ master ]
# Run jobs when commits are pushed to
# master or release-like branches:
branches:
- master
pull_request:
branches: [ master ]
# Run jobs for any external PR that wants
# to merge to master, too:
branches:
- master

env:
CARGO_TERM_COLOR: always

jobs:
build:
name: Cargo check
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v2

- name: Install Rust stable toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true

- name: Rust Cache
uses: Swatinem/rust-cache@v1.3.0

- name: Build
uses: actions-rs/cargo@v1.0.3
with:
command: check
args: --all-targets --all-features --workspace

fmt:
name: Cargo fmt
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v2

- name: Install Rust nightly toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly
override: true
components: rustfmt

- name: Rust Cache
uses: Swatinem/rust-cache@v1.3.0

- name: Cargo fmt
uses: actions-rs/cargo@v1.0.3
with:
command: fmt
args: --all -- --check

docs:
name: Check documentation
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v2

- name: Install Rust stable toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true

- name: Rust Cache
uses: Swatinem/rust-cache@v1.3.0

- name: Check internal documentation links
run: RUSTDOCFLAGS="--deny broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items

tests:
name: Cargo test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Checkout sources
uses: actions/checkout@v2

- name: Download Substrate
run: |
curl "https://releases.parity.io/substrate/x86_64-debian:stretch/latest/substrate/substrate" --output substrate --location
chmod +x ./substrate
mkdir -p ~/.local/bin
mv substrate ~/.local/bin

- name: setup
uses: actions-rs/toolchain@v1
with:
- name: Install Rust stable toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly
toolchain: stable
override: true
components: rustfmt
target: wasm32-unknown-unknown

- name: download-substrate
run: |
curl "https://releases.parity.io/substrate/x86_64-debian:stretch/latest/substrate/substrate" --output substrate --location
chmod +x ./substrate
mkdir -p ~/.local/bin
mv substrate ~/.local/bin
- name: Rust Cache
uses: Swatinem/rust-cache@v1.3.0

- name: Cargo test
uses: actions-rs/cargo@v1.0.3
with:
command: test
args: --all-targets --workspace

clippy:
name: Cargo clippy
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v2

- name: fmt
run: cargo fmt --all -- --check
- name: Install Rust stable toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
components: clippy
override: true

- name: build
run: cargo build --workspace --verbose
- name: Rust Cache
uses: Swatinem/rust-cache@v1.3.0

- name: test
run: cargo test --workspace --verbose
- name: Run clippy
uses: actions-rs/cargo@v1
with:
command: clippy
args: -- -D warnings
Copy link
Member

Choose a reason for hiding this comment

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

all targets?!

Suggested change
args: -- -D warnings
args: --all-targets -- -D warnings

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ooh good catch! That picked up some more lint issues to fix too :D :'(

Copy link
Member

@niklasad1 niklasad1 Nov 18, 2021

Choose a reason for hiding this comment

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

maybe postpone it to another PR I suspect that it will require plenty of work :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I've just finished fixing them so it's all gravy :)

3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/target
**/*.rs.bk
**/.DS_Store
Cargo.lock
cargo-timing*
cargo-timing*
2 changes: 1 addition & 1 deletion codegen/src/api/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub fn generate_storage(
let (storage_structs, storage_fns): (Vec<_>, Vec<_>) = storage
.entries
.iter()
.map(|entry| generate_storage_entry_fns(&type_gen, &pallet, entry))
.map(|entry| generate_storage_entry_fns(type_gen, pallet, entry))
.unzip();

quote! {
Expand Down
13 changes: 5 additions & 8 deletions codegen/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,9 @@ impl From<syn::Item> for Item {
let meta = attr.parse_meta().unwrap_or_else(|e| {
abort!(attr.span(), "Error parsing attribute: {}", e)
});
let substitute_type_args =
<attrs::Subxt as darling::FromMeta>::from_meta(&meta)
.unwrap_or_else(|e| {
abort!(attr.span(), "Error parsing attribute meta: {}", e)
});
substitute_type_args
<attrs::Subxt as darling::FromMeta>::from_meta(&meta).unwrap_or_else(
|e| abort!(attr.span(), "Error parsing attribute meta: {}", e),
)
})
.collect::<Vec<_>>();
if substitute_attrs.len() > 1 {
Expand All @@ -102,11 +99,11 @@ impl From<syn::Item> for Item {
"Duplicate `substitute_type` attributes"
)
}
if let Some(attr) = substitute_attrs.iter().next() {
if let Some(attr) = substitute_attrs.get(0) {
let use_path = &use_.tree;
let substitute_with: syn::TypePath = syn::parse_quote!( #use_path );
let type_substitute = SubxtItem::TypeSubstitute {
generated_type_path: attr.substitute_type().to_string(),
generated_type_path: attr.substitute_type(),
substitute_with,
};
Self::Subxt(type_substitute)
Expand Down
28 changes: 19 additions & 9 deletions src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ where
let mut event_data = Vec::<u8>::new();
let mut event_errors = Vec::<RuntimeError>::new();
let result = self.decode_raw_event(
&event_metadata,
event_metadata,
input,
&mut event_data,
&mut event_errors,
Expand Down Expand Up @@ -204,9 +204,13 @@ where
TypeDef::Variant(variant) => {
let variant_index = u8::decode(input)?;
variant_index.encode_to(output);
let variant = variant.variants().get(variant_index as usize).ok_or(
Error::Other(format!("Variant {} not found", variant_index)),
)?;
let variant =
variant
.variants()
.get(variant_index as usize)
.ok_or_else(|| {
Error::Other(format!("Variant {} not found", variant_index))
})?;
for field in variant.fields() {
self.decode_type(field.ty().id(), input, output)?;
}
Expand Down Expand Up @@ -299,15 +303,21 @@ where
TypeDef::Composite(composite) => {
match composite.fields() {
[field] => {
let field_ty =
self.metadata.resolve_type(field.ty().id()).ok_or(
MetadataError::TypeNotFound(field.ty().id()),
)?;
let field_ty = self
.metadata
.resolve_type(field.ty().id())
.ok_or_else(|| {
MetadataError::TypeNotFound(field.ty().id())
})?;
if let TypeDef::Primitive(primitive) = field_ty.type_def()
{
decode_compact_primitive(primitive)
} else {
Err(EventsDecodingError::InvalidCompactType("Composite type must have a single primitive field".into()).into())
Err(EventsDecodingError::InvalidCompactType(
"Composite type must have a single primitive field"
.into(),
)
.into())
}
}
_ => {
Expand Down
14 changes: 8 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ pub use crate::{
Signer,
UncheckedExtrinsic,
},
metadata::Metadata,
metadata::{
Metadata,
MetadataError,
PalletMetadata,
},
rpc::{
BlockNumber,
ExtrinsicSuccess,
Expand Down Expand Up @@ -167,10 +171,8 @@ pub enum Phase {

/// A wrapper for any type `T` which implement encode/decode in a way compatible with `Vec<u8>`.
///
/// This type is similar to [`WrapperOpaque`], but it differs in the way it stores the type `T`.
/// While [`WrapperOpaque`] stores the decoded type, the [`WrapperKeepOpaque`] stores the type only
/// in its opaque format, aka as a `Vec<u8>`. To access the real type `T` [`Self::try_decode`] needs
/// to be used.
/// [`WrapperKeepOpaque`] stores the type only in its opaque format, aka as a `Vec<u8>`. To
/// access the real type `T` [`Self::try_decode`] needs to be used.
#[derive(Debug, Eq, PartialEq, Default, Clone, Decode, Encode)]
pub struct WrapperKeepOpaque<T> {
data: Vec<u8>,
Expand All @@ -182,7 +184,7 @@ impl<T: Decode> WrapperKeepOpaque<T> {
///
/// Returns `None` if the decoding failed.
pub fn try_decode(&self) -> Option<T> {
T::decode_all(&mut &self.data[..]).ok()
T::decode_all(&self.data[..]).ok()
}

/// Returns the length of the encoded `T`.
Expand Down
12 changes: 8 additions & 4 deletions src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ pub enum MetadataError {
/// Constant is not in metadata.
#[error("Constant {0} not found")]
ConstantNotFound(&'static str),
/// Type is not in metadata.
#[error("Type {0} missing from type registry")]
TypeNotFound(u32),
}
Expand All @@ -91,7 +92,7 @@ impl Metadata {
pub fn pallet(&self, name: &'static str) -> Result<&PalletMetadata, MetadataError> {
self.pallets
.get(name)
.ok_or(MetadataError::PalletNotFound(name.to_string()))
.ok_or_else(|| MetadataError::PalletNotFound(name.to_string()))
}

/// Returns the metadata for the event at the given pallet and event indices.
Expand Down Expand Up @@ -131,6 +132,7 @@ impl Metadata {
}
}

/// Metadata for a specific pallet.
#[derive(Clone, Debug)]
pub struct PalletMetadata {
index: u8,
Expand All @@ -141,6 +143,7 @@ pub struct PalletMetadata {
}

impl PalletMetadata {
/// Encode a call based on this pallet metadata.
pub fn encode_call<C>(&self, call: &C) -> Result<Encoded, MetadataError>
where
C: Call,
Expand All @@ -154,6 +157,7 @@ impl PalletMetadata {
Ok(Encoded(bytes))
}

/// Return [`StorageEntryMetadata`] given some storage key.
pub fn storage(
&self,
key: &'static str,
Expand All @@ -163,7 +167,7 @@ impl PalletMetadata {
.ok_or(MetadataError::StorageNotFound(key))
}

/// Get a constant's metadata by name
/// Get a constant's metadata by name.
pub fn constant(
&self,
key: &'static str,
Expand Down Expand Up @@ -239,11 +243,11 @@ impl TryFrom<RuntimeMetadataPrefixed> for Metadata {

fn try_from(metadata: RuntimeMetadataPrefixed) -> Result<Self, Self::Error> {
if metadata.0 != META_RESERVED {
return Err(InvalidMetadataError::InvalidPrefix.into())
return Err(InvalidMetadataError::InvalidPrefix)
}
let metadata = match metadata.1 {
RuntimeMetadata::V14(meta) => meta,
_ => return Err(InvalidMetadataError::InvalidVersion.into()),
_ => return Err(InvalidMetadataError::InvalidVersion),
};

let get_type_def_variant = |type_id: u32| {
Expand Down
4 changes: 2 additions & 2 deletions src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,10 +616,10 @@ impl<T: Config> Rpc<T> {
Err(RpcError::Custom("RPC subscription dropped".into()).into())
}

async fn process_block<'a>(
async fn process_block(
&self,
events_sub: EventStorageSubscription<T>,
decoder: &'a EventsDecoder<T>,
decoder: &EventsDecoder<T>,
block_hash: T::Hash,
ext_hash: T::Hash,
) -> Result<ExtrinsicSuccess<T>, Error> {
Expand Down