Add clippy to ci and fix warnings

ci/Dockerfile

@@ -12,8 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM rust:1.42.0
+FROM rust:1.45.0
 
-RUN rustup component add rustfmt
+RUN rustup component add rustfmt clippy
 
 ENTRYPOINT ["cargo"]

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>)>;
@@ -85,17 +85,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
@@ -106,13 +101,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,
         }
     }
@@ -131,7 +126,7 @@ mod tests {
     impl Filter for TestFilter {
         fn on_downstream_receive(
             &self,
-            _: &Vec<EndPoint>,
+            _: &[EndPoint],
             _: SocketAddr,
             _: Vec<u8>,
         ) -> Option<(Vec<EndPoint>, Vec<u8>)> {
@@ -151,7 +146,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.extensions.filters.debug_filter.v1alpha1.DebugFilter");
+        "quilkin.extensions.filters.debug_filter.v1alpha1.DebugFilter".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

@@ -61,14 +61,9 @@ struct Config {
 }
 
 /// Creates instances of RateLimitFilter.
+#[derive(Default)]
 pub struct RateLimitFilterFactory;
 
-impl RateLimitFilterFactory {
-    pub fn new() -> Self {
-        RateLimitFilterFactory {}
-    }
-}
-
 /// A filter that implements rate limiting on packets based on
 /// the token-bucket algorithm.
 /// Packets that violate the rate limit are dropped.
@@ -95,7 +90,7 @@ impl FilterFactory for RateLimitFilterFactory {
         match config.period {
             Some(period) if period.lt(&Duration::from_millis(100)) => Err(Error::FieldInvalid {
                 field: "period".into(),
-                reason: format!("value must be at least 100ms"),
+                reason: "value must be at least 100ms".into(),
             }),
             _ => Ok(Box::new(RateLimitFilter::new(config))),
         }
@@ -176,11 +171,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,8 +27,8 @@ 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.insert(filters::RateLimitFilterFactory::new());
+    fr.insert(filters::RateLimitFilterFactory::default());
     fr
 }

src/lib.rs

@@ -14,9 +14,12 @@
  * limitations under the License.
  */
 
+// Fail the build if clippy finds any warnings.
+#![deny(warnings)]
+
 pub mod config;
 pub mod extensions;
 mod load_balancer_policy;
 pub mod metrics;
-pub mod server;
+pub mod proxy;
 pub mod test_utils;

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

@@ -23,11 +23,11 @@ use slog::{info, o, Drain, Logger};
 use prometheus::Registry;
 use quilkin::config::Config;
 use quilkin::extensions::default_registry;
-use quilkin::server::{Metrics, Server};
+use quilkin::proxy::{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 → src/proxy/metrics.rs

@@ -1,4 +1,4 @@
-use crate::server::sessions::metrics::Metrics as SessionMetrics;
+use crate::proxy::sessions::metrics::Metrics as SessionMetrics;
 use prometheus::{Encoder, Registry, Result as MetricsResult, TextEncoder};
 use slog::{info, warn, Logger};
 use std::net::SocketAddr;
@@ -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/server.rs → src/proxy/server.rs

@@ -31,7 +31,7 @@ use tokio::time::{delay_for, Duration, Instant};
 use crate::config::{Config, ConnectionConfig, EndPoint};
 use crate::extensions::{Filter, FilterChain, FilterRegistry};
 use crate::load_balancer_policy::LoadBalancerPolicy;
-use crate::server::sessions::{Packet, Session, SESSION_TIMEOUT_SECONDS};
+use crate::proxy::sessions::{Packet, Session, SESSION_TIMEOUT_SECONDS};
 
 use super::metrics::{start_metrics_server, Metrics};
 
@@ -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);
                 }
             }
         }
@@ -351,7 +350,7 @@ mod tests {
     use crate::config;
     use crate::config::{Config, ConnectionConfig, EndPoint, Local};
     use crate::extensions::default_registry;
-    use crate::server::sessions::{Packet, SESSION_TIMEOUT_SECONDS};
+    use crate::proxy::sessions::{Packet, SESSION_TIMEOUT_SECONDS};
     use crate::test_utils::{
         ephemeral_socket, logger, recv_udp, recv_udp_done, TestFilter, TestFilterFactory,
     };
@@ -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