From d254e720b20cae2c8076ac196023a51af0f1fb6c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Sat, 30 Jan 2021 10:17:01 -0500 Subject: [PATCH] Fix clippy lints and run clippy in CI (#29) --- .github/workflows/rust.yml | 8 ++++++++ README.adoc | 4 ++++ api_identity/src/lib.rs | 9 +++------ src/datastore.rs | 6 +++--- src/http_pagination.rs | 24 ++++++++++++------------ src/lib.rs | 4 ++++ 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3d06cf6b60..e63ccb7673 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -14,6 +14,14 @@ jobs: - name: Check style run: cargo fmt -- --check + clippy-lint: + runs-on: ubuntu-18.04 + steps: + # actions/checkout@v2 + - uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b + - name: Run Clippy Lints + run: cargo clippy -- -D warnings + build-and-test: runs-on: ${{ matrix.os }} strategy: diff --git a/README.adoc b/README.adoc index 3f442f2eaf..15b972cb48 100644 --- a/README.adoc +++ b/README.adoc @@ -59,6 +59,10 @@ suite runs cleanly and should remain clean. You can **format the code** using `cargo fmt`. Make sure to run this before pushing changes. The CI checks that the code is correctly formatted. +The https://github.com/rust-lang/rust-clippy[Clippy Linter] is used to help +keep the codebase efficient and idiomatic, and can be executed with +`cargo clippy`. + To **run the system:** there are two executables: the `oxide_controller` (which is a real prototype backed by an in-memory store; this component manages an Oxide fleet), and the `sled_agent` (which is a simulation of the component that diff --git a/api_identity/src/lib.rs b/api_identity/src/lib.rs index 3fc06247c1..697e2bf7d2 100644 --- a/api_identity/src/lib.rs +++ b/api_identity/src/lib.rs @@ -28,12 +28,9 @@ fn do_api_identity(item: TokenStream) -> Result { let name = &ast.ident; if !match ast.fields { - Fields::Named(ref fields) => { - fields.named.iter().any(|field| match &field.ident { - Some(ident) if ident.to_string() == "identity" => true, - _ => false, - }) - } + Fields::Named(ref fields) => fields.named.iter().any( + |field| matches!(&field.ident, Some(ident) if *ident == "identity"), + ), _ => false, } { return Err(syn::Error::new_spanned( diff --git a/src/datastore.rs b/src/datastore.rs index 52878f167c..25b1b08ab2 100644 --- a/src/datastore.rs +++ b/src/datastore.rs @@ -628,9 +628,9 @@ impl ControlDataStore { * TODO-cleanup this is only public because we haven't built Servers into the * datastore yet so the controller needs this interface. */ -pub fn collection_page<'a, 'b, KeyType, ValueType>( - search_tree: &'a BTreeMap>, - pagparams: &'b DataPageParams<'_, KeyType>, +pub fn collection_page( + search_tree: &BTreeMap>, + pagparams: &DataPageParams<'_, KeyType>, ) -> ListResult where KeyType: std::cmp::Ord, diff --git a/src/http_pagination.rs b/src/http_pagination.rs index fb89fc99b1..3ec997798d 100644 --- a/src/http_pagination.rs +++ b/src/http_pagination.rs @@ -156,7 +156,7 @@ where { ApiPageSelector { scan: scan_params.clone(), - last_seen: scan_params.marker_for_item(item).clone(), + last_seen: scan_params.marker_for_item(item), } } @@ -184,10 +184,10 @@ where * test the bulk of the logic without needing to cons up a Dropshot * `RequestContext` just to get the limit. */ -fn data_page_params_with_limit<'a, S>( +fn data_page_params_with_limit( limit: NonZeroUsize, - pag_params: &'a PaginationParams>, -) -> Result, HttpError> + pag_params: &PaginationParams>, +) -> Result, HttpError> where S: ScanParams, { @@ -287,7 +287,7 @@ impl ScanParams for ApiScanById { PaginationOrder::Ascending } fn marker_for_item(&self, item: &T) -> Uuid { - item.identity().id.clone() + item.identity().id } fn from_query(p: &ApiPaginatedById) -> Result<&Self, HttpError> { Ok(match p.page { @@ -384,7 +384,7 @@ impl ScanParams for ApiScanByNameOrId { let identity = item.identity(); match pagination_field_for_scan_params(self) { ApiPagField::Name => ApiNameOrIdMarker::Name(identity.name.clone()), - ApiPagField::Id => ApiNameOrIdMarker::Id(identity.id.clone()), + ApiPagField::Id => ApiNameOrIdMarker::Id(identity.id), } } @@ -437,10 +437,10 @@ pub fn data_page_params_nameid_name<'a>( data_page_params_nameid_name_limit(limit, pag_params) } -fn data_page_params_nameid_name_limit<'a>( +fn data_page_params_nameid_name_limit( limit: NonZeroUsize, - pag_params: &'a ApiPaginatedByNameOrId, -) -> Result, HttpError> { + pag_params: &ApiPaginatedByNameOrId, +) -> Result, HttpError> { let data_page = data_page_params_with_limit(limit, pag_params)?; let direction = data_page.direction; let marker = match data_page.marker { @@ -467,10 +467,10 @@ pub fn data_page_params_nameid_id<'a>( data_page_params_nameid_id_limit(limit, pag_params) } -fn data_page_params_nameid_id_limit<'a>( +fn data_page_params_nameid_id_limit( limit: NonZeroUsize, - pag_params: &'a ApiPaginatedByNameOrId, -) -> Result, HttpError> { + pag_params: &ApiPaginatedByNameOrId, +) -> Result, HttpError> { let data_page = data_page_params_with_limit(limit, pag_params)?; let direction = data_page.direction; let marker = match data_page.marker { diff --git a/src/lib.rs b/src/lib.rs index 2fd8ca5bb3..fff6993576 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,6 +57,10 @@ * it's expected that we'll have links to private items in the docs. */ #![allow(private_intra_doc_links)] +/* + * TODO(#32): Remove this exception once resolved. + */ +#![allow(clippy::field_reassign_with_default)] mod api_error; pub mod api_model;