Fix clippy lints and run clippy in CI

.github/workflows/rust.yml

@@ -14,6 +14,22 @@ 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
+      # TODO: Migrate these options to a config file in the workspace,
+      # once https://github.com/rust-lang/rust-clippy/issues/6625 is resolved.
+      #
+      # - field_reassign_with_default:
+      # Fixed by https://github.com/rust-lang/rust-clippy/issues/6344
+      # Fix exists on nightly, not on stable.
+      run: >
+        cargo clippy -- -D warnings
+        -A clippy::field_reassign_with_default
+
   build-and-test:
     runs-on: ${{ matrix.os }}
     strategy:

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
+keeep 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

api_identity/src/lib.rs

@@ -28,12 +28,9 @@ fn do_api_identity(item: TokenStream) -> Result<TokenStream, syn::Error> {
     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(

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<KeyType, Arc<ValueType>>,
-    pagparams: &'b DataPageParams<'_, KeyType>,
+pub fn collection_page<KeyType, ValueType>(
+    search_tree: &BTreeMap<KeyType, Arc<ValueType>>,
+    pagparams: &DataPageParams<'_, KeyType>,
 ) -> ListResult<ValueType>
 where
     KeyType: std::cmp::Ord,

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<S>(
     limit: NonZeroUsize,
-    pag_params: &'a PaginationParams<S, ApiPageSelector<S, S::MarkerValue>>,
-) -> Result<DataPageParams<'a, S::MarkerValue>, HttpError>
+    pag_params: &PaginationParams<S, ApiPageSelector<S, S::MarkerValue>>,
+) -> Result<DataPageParams<S::MarkerValue>, HttpError>
 where
     S: ScanParams,
 {
@@ -287,7 +287,7 @@ impl ScanParams for ApiScanById {
         PaginationOrder::Ascending
     }
     fn marker_for_item<T: ApiObjectIdentity>(&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<DataPageParams<'a, ApiName>, HttpError> {
+    pag_params: &ApiPaginatedByNameOrId,
+) -> Result<DataPageParams<ApiName>, 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<DataPageParams<'a, Uuid>, HttpError> {
+    pag_params: &ApiPaginatedByNameOrId,
+) -> Result<DataPageParams<Uuid>, HttpError> {
     let data_page = data_page_params_with_limit(limit, pag_params)?;
     let direction = data_page.direction;
     let marker = match data_page.marker {