Add clippy to ci and fix warnings

Fixes #82

cloudbuild.yaml

@@ -19,6 +19,9 @@ steps:
   - name: gcr.io/$PROJECT_ID/ci
     args: ["fmt", "--", "--check"]
     id: fmt-check
+  - name: gcr.io/$PROJECT_ID/ci
+    args: ["clippy"]
+    id: clippy
   - name: gcr.io/$PROJECT_ID/ci
     args: ["test"]
     id: test

src/extensions/filter_chain.rs

@@ -61,11 +61,11 @@ impl FilterChain {
 impl Filter for FilterChain {
     fn on_downstream_receive(
         &self,
-        endpoints: &Vec<EndPoint>,
+        endpoints: &[EndPoint],
         from: SocketAddr,
         contents: Vec<u8>,
     ) -> Option<(Vec<EndPoint>, Vec<u8>)> {
-        let mut e = endpoints.clone();
+        let mut e = endpoints.to_vec();
         let mut c = contents;
         for f in &self.filters {
             match f.on_downstream_receive(&e, from, c) {

src/extensions/filter_registry.rs

@@ -31,7 +31,7 @@ pub trait Filter: Send + Sync {
     /// If the packet should be rejected, return None.
     fn on_downstream_receive(
         &self,
-        endpoints: &Vec<EndPoint>,
+        endpoints: &[EndPoint],
         from: SocketAddr,
         contents: Vec<u8>,
     ) -> Option<(Vec<EndPoint>, Vec<u8>)>;
@@ -75,17 +75,12 @@ pub trait FilterFactory: Sync + Send {
 }
 
 /// FilterRegistry is the registry of all Filters that can be applied in the system.
+#[derive(Default)]
 pub struct FilterRegistry {
     registry: HashMap<String, Box<dyn FilterFactory>>,
 }
 
 impl FilterRegistry {
-    pub fn new() -> FilterRegistry {
-        FilterRegistry {
-            registry: Default::default(),
-        }
-    }
-
     /// insert registers a Filter under the provider's given name.
     pub fn insert<P: 'static>(&mut self, provider: P)
     where
@@ -96,13 +91,13 @@ impl FilterRegistry {
 
     /// get returns an instance of a filter for a given Key. Returns Error if not found,
     /// or if there is a configuration issue.
-    pub fn get(&self, key: &String, config: &serde_yaml::Value) -> Result<Box<dyn Filter>, Error> {
+    pub fn get(&self, key: &str, config: &serde_yaml::Value) -> Result<Box<dyn Filter>, Error> {
         match self
             .registry
             .get(key)
             .map(|p| p.create_from_config(&config))
         {
-            None => Err(Error::NotFound(key.clone())),
+            None => Err(Error::NotFound(key.into())),
             Some(filter) => filter,
         }
     }
@@ -121,7 +116,7 @@ mod tests {
     impl Filter for TestFilter {
         fn on_downstream_receive(
             &self,
-            _: &Vec<EndPoint>,
+            _: &[EndPoint],
             _: SocketAddr,
             _: Vec<u8>,
         ) -> Option<(Vec<EndPoint>, Vec<u8>)> {
@@ -141,7 +136,7 @@ mod tests {
 
     #[test]
     fn insert_and_get() {
-        let mut reg = FilterRegistry::new();
+        let mut reg = FilterRegistry::default();
         reg.insert(TestFilterFactory {});
         let config = serde_yaml::Value::Null;
 

src/extensions/filters/debug_filter.rs

@@ -73,7 +73,7 @@ impl DebugFilterFactory {
 
 impl FilterFactory for DebugFilterFactory {
     fn name(&self) -> String {
-        return String::from("quilkin.core.v1alpaha1.debug");
+        "quilkin.core.v1alpaha1.debug".into()
     }
 
     fn create_from_config(&self, config: &Value) -> Result<Box<dyn Filter>, Error> {
@@ -83,7 +83,7 @@ impl FilterFactory for DebugFilterFactory {
             _ => None,
         };
 
-        return match prefix {
+        match prefix {
             // if no config value supplied, then no prefix, which is fine
             None => Ok(Box::new(DebugFilter::new(&self.log, None))),
             // return an Error if the id exists but is not a string.
@@ -97,14 +97,14 @@ impl FilterFactory for DebugFilterFactory {
                     Some(prefix.to_string()),
                 ))),
             },
-        };
+        }
     }
 }
 
 impl Filter for DebugFilter {
     fn on_downstream_receive(
         &self,
-        endpoints: &Vec<EndPoint>,
+        endpoints: &[EndPoint],
         from: SocketAddr,
         contents: Vec<u8>,
     ) -> Option<(Vec<EndPoint>, Vec<u8>)> {

src/extensions/filters/local_rate_limit.rs

@@ -125,11 +125,12 @@ impl Drop for RateLimitFilter {
 impl Filter for RateLimitFilter {
     fn on_downstream_receive(
         &self,
-        endpoints: &Vec<EndPoint>,
+        endpoints: &[EndPoint],
         _from: SocketAddr,
         contents: Vec<u8>,
     ) -> Option<(Vec<EndPoint>, Vec<u8>)> {
-        self.acquire_token().map(|()| (endpoints.clone(), contents))
+        self.acquire_token()
+            .map(|()| (endpoints.to_vec(), contents))
     }
 
     fn on_upstream_receive(

src/extensions/mod.rs

@@ -27,7 +27,7 @@ mod filter_chain;
 /// default_registry returns a FilterRegistry with the default
 /// set of filters that are user configurable registered to it
 pub fn default_registry(base: &Logger) -> FilterRegistry {
-    let mut fr = FilterRegistry::new();
+    let mut fr = FilterRegistry::default();
     fr.insert(filters::DebugFilterFactory::new(base));
     fr
 }

src/lib.rs

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#![deny(warnings)]
+
 pub mod config;
 pub mod extensions;
 mod load_balancer_policy;

src/load_balancer_policy.rs

@@ -30,7 +30,7 @@ impl LoadBalancerPolicy {
             } => (
                 lb_policy,
                 addresses
-                    .into_iter()
+                    .iter()
                     .cloned()
                     .enumerate()
                     .map(|(offset, address)| EndPoint {

src/main.rs

@@ -27,7 +27,7 @@ use quilkin::server::{Metrics, Server};
 use tokio::signal;
 use tokio::sync::oneshot;
 
-const VERSION: &'static str = env!("CARGO_PKG_VERSION");
+const VERSION: &str = env!("CARGO_PKG_VERSION");
 
 #[tokio::main]
 async fn main() {
@@ -75,6 +75,5 @@ fn logger() -> Logger {
         .build()
         .fuse();
     let drain = slog_async::Async::new(drain).build().fuse();
-    let log = slog::Logger::root(drain, o!());
-    return log;
+    slog::Logger::root(drain, o!())
 }

src/metrics.rs

@@ -32,7 +32,7 @@ pub fn histogram_opts(
 ) -> HistogramOpts {
     HistogramOpts {
         common_opts: opts(name, subsystem, description),
-        buckets: buckets.unwrap_or(Vec::from(DEFAULT_BUCKETS as &'static [f64])),
+        buckets: buckets.unwrap_or_else(|| Vec::from(DEFAULT_BUCKETS as &'static [f64])),
     }
 }
 

src/server/metrics.rs

@@ -35,7 +35,7 @@ pub fn start_metrics_server(
                     warn!(log, "failed to convert metrics to utf8: {:?}", err);
                 })
             })
-            .unwrap_or("# failed to gather metrics".to_string())
+            .unwrap_or_else(|_| "# failed to gather metrics".to_string())
     });
 
     let (_, server) = warp::serve(metrics_route).bind_with_graceful_shutdown(addr, async {

src/server/mod.rs

@@ -15,8 +15,8 @@
  */
 
 pub use metrics::Metrics;
-pub use server::Server;
+pub use proxy_server::Server;
 
 mod metrics;
-mod server;
+mod proxy_server;
 mod sessions;

src/server/server.rs → src/server/proxy_server.rs

@@ -49,11 +49,11 @@ impl Server {
     /// new Server. Takes a logger, and the registry of available Filters.
     pub fn new(base: Logger, filter_registry: FilterRegistry, metrics: Metrics) -> Self {
         let log = base.new(o!("source" => "server::Server"));
-        return Server {
+        Server {
             log,
             filter_registry,
             metrics,
-        };
+        }
     }
 
     /// start the async processing of incoming UDP packets. Will block until an
@@ -268,7 +268,7 @@ impl Server {
     /// bind binds the local configured port
     async fn bind(config: &Config) -> Result<UdpSocket> {
         let addr = SocketAddrV4::new(Ipv4Addr::new(0, 0, 0, 0), config.local.port);
-        return UdpSocket::bind(addr).await;
+        UdpSocket::bind(addr).await
     }
 
     /// ensure_session makes sure there is a value session for the name in the sessions map
@@ -300,7 +300,7 @@ impl Server {
             let mut map = sessions.write().await;
             map.insert(s.key(), Mutex::new(s));
         }
-        return Ok(());
+        Ok(())
     }
 
     /// prune_sessions removes expired Sessions from the SessionMap.
@@ -315,8 +315,7 @@ impl Server {
                 let session = v.lock().await;
                 let expiration = session.expiration().await;
                 if expiration.lt(&now) {
-                    let value = k.clone();
-                    remove_keys.push(value);
+                    remove_keys.push(*k);
                 }
             }
         }
@@ -361,7 +360,7 @@ mod tests {
     #[tokio::test]
     async fn run_server() {
         let log = logger();
-        let server = Server::new(log.clone(), FilterRegistry::new(), Metrics::default());
+        let server = Server::new(log.clone(), FilterRegistry::default(), Metrics::default());
 
         let socket1 = ephemeral_socket().await;
         let endpoint1 = socket1.local_addr().unwrap();
@@ -412,7 +411,7 @@ mod tests {
     #[tokio::test]
     async fn run_client() {
         let log = logger();
-        let server = Server::new(log.clone(), FilterRegistry::new(), Metrics::default());
+        let server = Server::new(log.clone(), FilterRegistry::default(), Metrics::default());
         let socket = ephemeral_socket().await;
         let endpoint_addr = socket.local_addr().unwrap();
         let (recv, mut send) = socket.split();
@@ -446,7 +445,7 @@ mod tests {
     #[tokio::test]
     async fn run_with_filter() {
         let log = logger();
-        let mut registry = FilterRegistry::new();
+        let mut registry = FilterRegistry::default();
         registry.insert(TestFilterFactory {});
 
         let server = Server::new(log.clone(), registry, Metrics::default());
@@ -692,7 +691,7 @@ mod tests {
 
     #[tokio::test]
     async fn run_receive_packet() {
-        let server = Server::new(logger(), FilterRegistry::new(), Metrics::default());
+        let server = Server::new(logger(), FilterRegistry::default(), Metrics::default());
         let msg = "hello";
 
         // without a filter